Don't delete/add values to the unique property map when it's null (#2538

) * Don't delete/add values to the unique property map when it's null This happens when revoking operator keys, as the revoked MN will then have the address and operator keys set to the null representation. We shouldn't add the null value to the unique property map as otherwise future revokes crash in an assert. * Assert that no null values are passed to Add/DeleteUniqueProperty * Check for null values before calling Add/DeleteUniqueProperty * Apply suggestions from code review Co-Authored-By: codablock <[email protected]> * Add user generated default constructors to BLS primitives Fixes build issues on Mac: In file included from evo/deterministicmns.cpp:5: ./evo/deterministicmns.h:375:24: error: default initialization of an object of const type 'const CBLSPublicKey' without a user-provided default constructor static const T nullValue; ^
dashpay · Dec 10, 2018 · 88f7bf0 · 88f7bf0
1 parent 15414da
commit 88f7bf0
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 7 deletions.
diff --git a/src/bls/bls.h b/src/bls/bls.h
@@ -203,6 +203,8 @@ class CBLSId : public CBLSWrapper<uint256, BLS_CURVE_ID_SIZE, CBLSId>
     using CBLSWrapper::operator==;
     using CBLSWrapper::operator!=;
 
+    CBLSId() {}
+
     void SetInt(int x);
     void SetHash(const uint256& hash);
 
@@ -221,6 +223,8 @@ class CBLSSecretKey : public CBLSWrapper<bls::PrivateKey, BLS_CURVE_SECKEY_SIZE,
     using CBLSWrapper::operator==;
     using CBLSWrapper::operator!=;
 
+    CBLSSecretKey() {}
+
     void AggregateInsecure(const CBLSSecretKey& o);
     static CBLSSecretKey AggregateInsecure(const std::vector<CBLSSecretKey>& sks);
 
@@ -247,6 +251,8 @@ class CBLSPublicKey : public CBLSWrapper<bls::PublicKey, BLS_CURVE_PUBKEY_SIZE,
     using CBLSWrapper::operator==;
     using CBLSWrapper::operator!=;
 
+    CBLSPublicKey() {}
+
     void AggregateInsecure(const CBLSPublicKey& o);
     static CBLSPublicKey AggregateInsecure(const std::vector<CBLSPublicKey>& pks);
 
@@ -265,8 +271,9 @@ class CBLSSignature : public CBLSWrapper<bls::InsecureSignature, BLS_CURVE_SIG_S
 public:
     using CBLSWrapper::operator==;
     using CBLSWrapper::operator!=;
+    using CBLSWrapper::CBLSWrapper;
 
-    CBLSSignature() = default;
+    CBLSSignature() {}
     CBLSSignature(const CBLSSignature&) = default;
     CBLSSignature& operator=(const CBLSSignature&) = default;
 

diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp
@@ -393,9 +393,13 @@ void CDeterministicMNList::AddMN(const CDeterministicMNCPtr& dmn)
     assert(!mnMap.find(dmn->proTxHash));
     mnMap = mnMap.set(dmn->proTxHash, dmn);
     AddUniqueProperty(dmn, dmn->collateralOutpoint);
-    AddUniqueProperty(dmn, dmn->pdmnState->addr);
+    if (dmn->pdmnState->addr != CService()) {
+        AddUniqueProperty(dmn, dmn->pdmnState->addr);
+    }
     AddUniqueProperty(dmn, dmn->pdmnState->keyIDOwner);
-    AddUniqueProperty(dmn, dmn->pdmnState->pubKeyOperator);
+    if (dmn->pdmnState->pubKeyOperator.IsValid()) {
+        AddUniqueProperty(dmn, dmn->pdmnState->pubKeyOperator);
+    }
 }
 
 void CDeterministicMNList::UpdateMN(const uint256& proTxHash, const CDeterministicMNStateCPtr& pdmnState)
@@ -417,9 +421,13 @@ void CDeterministicMNList::RemoveMN(const uint256& proTxHash)
     auto dmn = GetMN(proTxHash);
     assert(dmn != nullptr);
     DeleteUniqueProperty(dmn, dmn->collateralOutpoint);
-    DeleteUniqueProperty(dmn, dmn->pdmnState->addr);
+    if (dmn->pdmnState->addr != CService()) {
+        DeleteUniqueProperty(dmn, dmn->pdmnState->addr);
+    }
     DeleteUniqueProperty(dmn, dmn->pdmnState->keyIDOwner);
-    DeleteUniqueProperty(dmn, dmn->pdmnState->pubKeyOperator);
+    if (dmn->pdmnState->pubKeyOperator.IsValid()) {
+        DeleteUniqueProperty(dmn, dmn->pdmnState->pubKeyOperator);
+    }
     mnMap = mnMap.erase(proTxHash);
 }
 

diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h
@@ -372,6 +372,9 @@ class CDeterministicMNList
     template <typename T>
     void AddUniqueProperty(const CDeterministicMNCPtr& dmn, const T& v)
     {
+        static const T nullValue;
+        assert(v != nullValue);
+
         auto hash = ::SerializeHash(v);
         auto oldEntry = mnUniquePropertyMap.find(hash);
         assert(!oldEntry || oldEntry->first == dmn->proTxHash);
@@ -384,6 +387,9 @@ class CDeterministicMNList
     template <typename T>
     void DeleteUniqueProperty(const CDeterministicMNCPtr& dmn, const T& oldValue)
     {
+        static const T nullValue;
+        assert(oldValue != nullValue);
+
         auto oldHash = ::SerializeHash(oldValue);
         auto p = mnUniquePropertyMap.find(oldHash);
         assert(p && p->first == dmn->proTxHash);
@@ -399,8 +405,15 @@ class CDeterministicMNList
         if (oldValue == newValue) {
             return;
         }
-        DeleteUniqueProperty(dmn, oldValue);
-        AddUniqueProperty(dmn, newValue);
+        static const T nullValue;
+
+        if (oldValue != nullValue) {
+            DeleteUniqueProperty(dmn, oldValue);
+        }
+
+        if (newValue != nullValue) {
+            AddUniqueProperty(dmn, newValue);
+        }
     }
 };