Remove logic for handling objects and votes orphaned by not-yet-known MNs (#2954)

This should no longer happen now that we use deterministic masternode list.

Author: UdjinM6

src/dsnotificationinterface.cpp

@@ -116,8 +116,6 @@ void CDSNotificationInterface::BlockDisconnected(const std::shared_ptr<const CBl
 void CDSNotificationInterface::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff)
 {
     CMNAuth::NotifyMasternodeListChanged(undo, oldMNList, diff);
-    governance.CheckMasternodeOrphanObjects(connman);
-    governance.CheckMasternodeOrphanVotes(connman);
     governance.UpdateCachesAndClean();
 }
 

src/governance/governance-object.cpp

@@ -42,7 +42,6 @@ CGovernanceObject::CGovernanceObject() :
     fExpired(false),
     fUnparsable(false),
     mapCurrentMNVotes(),
-    cmmapOrphanVotes(),
     fileVotes()
 {
     // PARSE JSON DATA STORAGE (VCHDATA)
@@ -70,7 +69,6 @@ CGovernanceObject::CGovernanceObject(const uint256& nHashParentIn, int nRevision
     fExpired(false),
     fUnparsable(false),
     mapCurrentMNVotes(),
-    cmmapOrphanVotes(),
     fileVotes()
 {
     // PARSE JSON DATA STORAGE (VCHDATA)
@@ -98,7 +96,6 @@ CGovernanceObject::CGovernanceObject(const CGovernanceObject& other) :
     fExpired(other.fExpired),
     fUnparsable(other.fUnparsable),
     mapCurrentMNVotes(other.mapCurrentMNVotes),
-    cmmapOrphanVotes(other.cmmapOrphanVotes),
     fileVotes(other.fileVotes)
 {
 }
@@ -126,12 +123,7 @@ bool CGovernanceObject::ProcessVote(CNode* pfrom,
     if (!dmn) {
         std::ostringstream ostr;
         ostr << "CGovernanceObject::ProcessVote -- Masternode " << vote.GetMasternodeOutpoint().ToStringShort() << " not found";
-        exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_WARNING);
-        if (cmmapOrphanVotes.Insert(vote.GetMasternodeOutpoint(), vote_time_pair_t(vote, GetAdjustedTime() + GOVERNANCE_ORPHAN_EXPIRATION_TIME))) {
-            LogPrintf("%s\n", ostr.str());
-        } else {
-            LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
-        }
+        exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
         return false;
     }
 
@@ -452,15 +444,13 @@ void CGovernanceObject::UpdateLocalValidity()
 
 bool CGovernanceObject::IsValidLocally(std::string& strError, bool fCheckCollateral) const
 {
-    bool fMissingMasternode = false;
     bool fMissingConfirmations = false;
 
-    return IsValidLocally(strError, fMissingMasternode, fMissingConfirmations, fCheckCollateral);
+    return IsValidLocally(strError, fMissingConfirmations, fCheckCollateral);
 }
 
-bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingMasternode, bool& fMissingConfirmations, bool fCheckCollateral) const
+bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const
 {
-    fMissingMasternode = false;
     fMissingConfirmations = false;
 
     if (fUnparsable) {
@@ -718,34 +708,3 @@ void CGovernanceObject::UpdateSentinelVariables()
 
     if (GetAbsoluteNoCount(VOTE_SIGNAL_VALID) >= nAbsVoteReq) fCachedValid = false;
 }
-
-void CGovernanceObject::CheckOrphanVotes(CConnman& connman)
-{
-    int64_t nNow = GetAdjustedTime();
-    auto mnList = deterministicMNManager->GetListAtChainTip();
-    const vote_cmm_t::list_t& listVotes = cmmapOrphanVotes.GetItemList();
-    vote_cmm_t::list_cit it = listVotes.begin();
-    while (it != listVotes.end()) {
-        bool fRemove = false;
-        const COutPoint& key = it->key;
-        const vote_time_pair_t& pairVote = it->value;
-        const CGovernanceVote& vote = pairVote.first;
-        if (pairVote.second < nNow) {
-            fRemove = true;
-        } else if (!mnList.HasValidMNByCollateral(vote.GetMasternodeOutpoint())) {
-            ++it;
-            continue;
-        }
-        CGovernanceException exception;
-        if (!ProcessVote(nullptr, vote, exception, connman)) {
-            LogPrintf("CGovernanceObject::CheckOrphanVotes -- Failed to add orphan vote: %s\n", exception.what());
-        } else {
-            vote.Relay(connman);
-            fRemove = true;
-        }
-        ++it;
-        if (fRemove) {
-            cmmapOrphanVotes.Erase(key, pairVote);
-        }
-    }
-}

src/governance/governance-object.h

@@ -180,9 +180,6 @@ class CGovernanceObject
 
     vote_m_t mapCurrentMNVotes;
 
-    /// Limited map of votes orphaned by MN
-    vote_cmm_t cmmapOrphanVotes;
-
     CGovernanceObjectVoteFile fileVotes;
 
 public:
@@ -267,7 +264,7 @@ class CGovernanceObject
 
     bool IsValidLocally(std::string& strError, bool fCheckCollateral) const;
 
-    bool IsValidLocally(std::string& strError, bool& fMissingMasternode, bool& fMissingConfirmations, bool fCheckCollateral) const;
+    bool IsValidLocally(std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const;
 
     /// Check the collateral transaction for the budget proposal/finalized budget
     bool IsCollateralValid(std::string& strError, bool& fMissingConfirmations) const;
@@ -350,8 +347,6 @@ class CGovernanceObject
     // also for MNs that were removed from the list completely.
     // Returns deleted vote hashes.
     std::set<uint256> RemoveInvalidVotes(const COutPoint& mnOutpoint);
-
-    void CheckOrphanVotes(CConnman& connman);
 };
 
 

src/governance/governance.cpp

@@ -33,7 +33,6 @@ CGovernanceManager::CGovernanceManager() :
     nCachedBlockHeight(0),
     mapObjects(),
     mapErasedGovernanceObjects(),
-    mapMasternodeOrphanObjects(),
     cmapVoteToObject(MAX_CACHE_SIZE),
     cmapInvalidVotes(MAX_CACHE_SIZE),
     cmmapOrphanVotes(MAX_CACHE_SIZE),
@@ -161,8 +160,7 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, const std::string& strComm
 
         LOCK2(cs_main, cs);
 
-        if (mapObjects.count(nHash) || mapPostponedObjects.count(nHash) ||
-            mapErasedGovernanceObjects.count(nHash) || mapMasternodeOrphanObjects.count(nHash)) {
+        if (mapObjects.count(nHash) || mapPostponedObjects.count(nHash) || mapErasedGovernanceObjects.count(nHash)) {
             // TODO - print error code? what if it's GOVOBJ_ERROR_IMMATURE?
             LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- Received already seen object: %s\n", strHash);
             return;
@@ -177,33 +175,18 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, const std::string& strComm
         std::string strError = "";
         // CHECK OBJECT AGAINST LOCAL BLOCKCHAIN
 
-        bool fMasternodeMissing = false;
         bool fMissingConfirmations = false;
-        bool fIsValid = govobj.IsValidLocally(strError, fMasternodeMissing, fMissingConfirmations, true);
+        bool fIsValid = govobj.IsValidLocally(strError, fMissingConfirmations, true);
 
-        if (fRateCheckBypassed && (fIsValid || fMasternodeMissing)) {
+        if (fRateCheckBypassed && fIsValid) {
             if (!MasternodeRateCheck(govobj, true)) {
                 LogPrintf("MNGOVERNANCEOBJECT -- masternode rate check failed (after signature verification) - %s - (current block height %d)\n", strHash, nCachedBlockHeight);
                 return;
             }
         }
 
         if (!fIsValid) {
-            if (fMasternodeMissing) {
-                int& count = mapMasternodeOrphanCounter[govobj.GetMasternodeOutpoint()];
-                if (count >= 10) {
-                    LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- Too many orphan objects, missing masternode=%s\n", govobj.GetMasternodeOutpoint().ToStringShort());
-                    // ask for this object again in 2 minutes
-                    CInv inv(MSG_GOVERNANCE_OBJECT, govobj.GetHash());
-                    pfrom->AskFor(inv);
-                    return;
-                }
-
-                count++;
-                ExpirationInfo info(pfrom->GetId(), GetAdjustedTime() + GOVERNANCE_ORPHAN_EXPIRATION_TIME);
-                mapMasternodeOrphanObjects.insert(std::make_pair(nHash, object_info_pair_t(govobj, info)));
-                LogPrintf("MNGOVERNANCEOBJECT -- Missing masternode for: %s, strError = %s\n", strHash, strError);
-            } else if (fMissingConfirmations) {
+            if (fMissingConfirmations) {
                 AddPostponedObject(govobj);
                 LogPrintf("MNGOVERNANCEOBJECT -- Not enough fee confirmations for: %s, strError = %s\n", strHash, strError);
             } else {
@@ -863,52 +846,6 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote,
     return fOk;
 }
 
-void CGovernanceManager::CheckMasternodeOrphanVotes(CConnman& connman)
-{
-    LOCK2(cs_main, cs);
-
-    ScopedLockBool guard(cs, fRateChecksEnabled, false);
-
-    for (auto& objPair : mapObjects) {
-        objPair.second.CheckOrphanVotes(connman);
-    }
-}
-
-void CGovernanceManager::CheckMasternodeOrphanObjects(CConnman& connman)
-{
-    LOCK2(cs_main, cs);
-    int64_t nNow = GetAdjustedTime();
-    ScopedLockBool guard(cs, fRateChecksEnabled, false);
-    object_info_m_it it = mapMasternodeOrphanObjects.begin();
-    while (it != mapMasternodeOrphanObjects.end()) {
-        object_info_pair_t& pair = it->second;
-        CGovernanceObject& govobj = pair.first;
-
-        if (pair.second.nExpirationTime >= nNow) {
-            std::string strError;
-            bool fMasternodeMissing = false;
-            bool fConfirmationsMissing = false;
-            bool fIsValid = govobj.IsValidLocally(strError, fMasternodeMissing, fConfirmationsMissing, true);
-
-            if (fIsValid) {
-                AddGovernanceObject(govobj, connman);
-            } else if (fMasternodeMissing) {
-                ++it;
-                continue;
-            }
-        } else {
-            // apply node's ban score
-            Misbehaving(pair.second.idFrom, 20);
-        }
-
-        auto it_count = mapMasternodeOrphanCounter.find(govobj.GetMasternodeOutpoint());
-        if (--it_count->second == 0)
-            mapMasternodeOrphanCounter.erase(it_count);
-
-        mapMasternodeOrphanObjects.erase(it++);
-    }
-}
-
 void CGovernanceManager::CheckPostponedObjects(CConnman& connman)
 {
     if (!masternodeSync.IsSynced()) return;

src/governance/governance.h

@@ -241,9 +241,6 @@ class CGovernanceManager
     //   value - expiration time for deleted objects
     hash_time_m_t mapErasedGovernanceObjects;
 
-    object_info_m_t mapMasternodeOrphanObjects;
-    txout_int_m_t mapMasternodeOrphanCounter;
-
     object_m_t mapPostponedObjects;
     hash_s_t setAdditionalRelayObjects;
 
@@ -402,10 +399,6 @@ class CGovernanceManager
         return fOK;
     }
 
-    void CheckMasternodeOrphanVotes(CConnman& connman);
-
-    void CheckMasternodeOrphanObjects(CConnman& connman);
-
     void CheckPostponedObjects(CConnman& connman);
 
     bool AreRateChecksEnabled() const

src/rpc/governance.cpp

@@ -308,11 +308,10 @@ UniValue gobject_submit(const JSONRPCRequest& request)
     std::string strHash = govobj.GetHash().ToString();
 
     std::string strError = "";
-    bool fMissingMasternode;
     bool fMissingConfirmations;
     {
         LOCK(cs_main);
-        if (!govobj.IsValidLocally(strError, fMissingMasternode, fMissingConfirmations, true) && !fMissingConfirmations) {
+        if (!govobj.IsValidLocally(strError, fMissingConfirmations, true) && !fMissingConfirmations) {
             LogPrintf("gobject(submit) -- Object submission rejected because object is not valid - hash = %s, strError = %s\n", strHash, strError);
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Governance object is not valid - " + strHash + " - " + strError);
         }