Remove proposal/funding votes from MNs that changed the voting key (#2570)

* Rename onlyOwnerAllowed boolean variables to onlyVotingKeyAllowed

This was misleading.

* Remove invalid proposal votes when MN voting keys change

src/governance-object.cpp

@@ -177,10 +177,10 @@ bool CGovernanceObject::ProcessVote(CNode* pfrom,
         }
     }
 
-    bool onlyOwnerAllowed = nObjectType == GOVERNANCE_OBJECT_PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING;
+    bool onlyVotingKeyAllowed = nObjectType == GOVERNANCE_OBJECT_PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING;
 
     // Finally check that the vote is actually valid (done last because of cost of signature verification)
-    if (!vote.IsValid(onlyOwnerAllowed)) {
+    if (!vote.IsValid(onlyVotingKeyAllowed)) {
         std::ostringstream ostr;
         ostr << "CGovernanceObject::ProcessVote -- Invalid vote"
              << ", MN outpoint = " << vote.GetMasternodeOutpoint().ToStringShort()
@@ -223,6 +223,51 @@ void CGovernanceObject::ClearMasternodeVotes()
     }
 }
 
+std::set<uint256> CGovernanceObject::RemoveInvalidProposalVotes(const COutPoint& mnOutpoint)
+{
+    LOCK(cs);
+
+    if (nObjectType != GOVERNANCE_OBJECT_PROPOSAL) {
+        return {};
+    }
+
+    auto it = mapCurrentMNVotes.find(mnOutpoint);
+    if (it == mapCurrentMNVotes.end()) {
+        // don't even try as we don't have any votes from this MN
+        return {};
+    }
+
+    auto removedVotes = fileVotes.RemoveInvalidProposalVotes(mnOutpoint);
+    if (removedVotes.empty()) {
+        return {};
+    }
+
+    auto nParentHash = GetHash();
+    for (auto jt = it->second.mapInstances.begin(); jt != it->second.mapInstances.end(); ) {
+        CGovernanceVote tmpVote(mnOutpoint, nParentHash, (vote_signal_enum_t)jt->first, jt->second.eOutcome);
+        tmpVote.SetTime(jt->second.nCreationTime);
+        if (removedVotes.count(tmpVote.GetHash())) {
+            jt = it->second.mapInstances.erase(jt);
+        } else {
+            ++jt;
+        }
+    }
+    if (it->second.mapInstances.empty()) {
+        mapCurrentMNVotes.erase(it);
+    }
+
+    if (!removedVotes.empty()) {
+        std::string removedStr;
+        for (auto& h : removedVotes) {
+            removedStr += strprintf("  %s\n", h.ToString());
+        }
+        LogPrintf("CGovernanceObject::%s -- Removed %d invalid votes for %s from MN %s:\n%s\n", __func__, removedVotes.size(), nParentHash.ToString(), mnOutpoint.ToString(), removedStr);
+        fDirtyCache = true;
+    }
+
+    return removedVotes;
+}
+
 std::string CGovernanceObject::GetSignatureMessage() const
 {
     LOCK(cs);

src/governance-object.h

@@ -349,6 +349,10 @@ private:
     /// Called when MN's which have voted on this object have been removed
     void ClearMasternodeVotes();
 
+    // Revalidate all votes from this MN and delete them if validation fails
+    // This is the case for DIP3 MNs that change voting keys. Returns deleted vote hashes
+    std::set<uint256> RemoveInvalidProposalVotes(const COutPoint& mnOutpoint);
+
     void CheckOrphanVotes(CConnman& connman);
 
     // TODO can be removed after DIP3 is fully deployed

src/governance-votedb.cpp

@@ -68,6 +68,27 @@ void CGovernanceObjectVoteFile::RemoveVotesFromMasternode(const COutPoint& outpo
     }
 }
 
+std::set<uint256> CGovernanceObjectVoteFile::RemoveInvalidProposalVotes(const COutPoint& outpointMasternode)
+{
+    std::set<uint256> removedVotes;
+
+    vote_l_it it = listVotes.begin();
+    while (it != listVotes.end()) {
+        if (it->GetSignal() == VOTE_SIGNAL_FUNDING && it->GetMasternodeOutpoint() == outpointMasternode) {
+            if (!it->IsValid(true)) {
+                removedVotes.emplace(it->GetHash());
+                --nMemoryVotes;
+                mapVoteIndex.erase(it->GetHash());
+                listVotes.erase(it++);
+                continue;
+            }
+        }
+        ++it;
+    }
+
+    return removedVotes;
+}
+
 std::vector<uint256> CGovernanceObjectVoteFile::RemoveOldVotes(unsigned int nMinTime)
 {
     std::vector<uint256> removed;

src/governance-votedb.h

@@ -73,6 +73,7 @@ public:
     std::vector<CGovernanceVote> GetVotes() const;
 
     void RemoveVotesFromMasternode(const COutPoint& outpointMasternode);
+    std::set<uint256> RemoveInvalidProposalVotes(const COutPoint& outpointMasternode);
 
     // TODO can be removed after full DIP3 deployment
     std::vector<uint256> RemoveOldVotes(unsigned int nMinTime);

src/governance.cpp

@@ -23,7 +23,7 @@ CGovernanceManager governance;
 
 int nSubmittedFinalBudget;
 
-const std::string CGovernanceManager::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-13";
+const std::string CGovernanceManager::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-14";
 const int CGovernanceManager::MAX_TIME_FUTURE_DEVIATION = 60 * 60;
 const int CGovernanceManager::RELIABLE_PROPAGATION_TIME = 60;
 
@@ -581,6 +581,7 @@ void CGovernanceManager::DoMaintenance(CConnman& connman)
 
     if (deterministicMNManager->IsDeterministicMNsSporkActive()) {
         ClearPreDIP3Votes();
+        RemoveInvalidProposalVotes();
     }
 
     // CHECK OBJECTS WE'VE ASKED FOR, REMOVE OLD ENTRIES
@@ -686,9 +687,9 @@ void CGovernanceManager::SyncSingleObjAndItsVotes(CNode* pnode, const uint256& n
     for (const auto& vote : fileVotes.GetVotes()) {
         uint256 nVoteHash = vote.GetHash();
 
-        bool onlyOwnerAllowed = govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING;
+        bool onlyVotingKeyAllowed = govobj.GetObjectType() == GOVERNANCE_OBJECT_PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING;
 
-        if (filter.contains(nVoteHash) || !vote.IsValid(onlyOwnerAllowed)) {
+        if (filter.contains(nVoteHash) || !vote.IsValid(onlyVotingKeyAllowed)) {
             continue;
         }
         pnode->PushInventory(CInv(MSG_GOVERNANCE_OBJECT_VOTE, nVoteHash));
@@ -1303,6 +1304,7 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, CConnman& co
 
     if (deterministicMNManager->IsDeterministicMNsSporkActive(pindex->nHeight)) {
         ClearPreDIP3Votes();
+        RemoveInvalidProposalVotes();
     }
 
     CheckPostponedObjects(connman);
@@ -1357,6 +1359,45 @@ void CGovernanceManager::CleanOrphanObjects()
     }
 }
 
+void CGovernanceManager::RemoveInvalidProposalVotes()
+{
+    auto curMNList = deterministicMNManager->GetListAtChainTip();
+    auto diff = lastMNListForVotingKeys.BuildDiff(curMNList);
+
+    LOCK(cs);
+
+    std::vector<COutPoint> changedKeyMNs;
+    for (const auto& p : diff.updatedMNs) {
+        auto oldDmn = lastMNListForVotingKeys.GetMN(p.first);
+        if (p.second->keyIDVoting != oldDmn->pdmnState->keyIDVoting) {
+            changedKeyMNs.emplace_back(oldDmn->collateralOutpoint);
+        }
+    }
+    for (const auto& proTxHash : diff.removedMns) {
+        auto oldDmn = lastMNListForVotingKeys.GetMN(proTxHash);
+        changedKeyMNs.emplace_back(oldDmn->collateralOutpoint);
+    }
+
+    for (const auto& outpoint : changedKeyMNs) {
+        for (auto& p : mapObjects) {
+            auto removed = p.second.RemoveInvalidProposalVotes(outpoint);
+            if (removed.empty()) {
+                continue;
+            }
+            for (auto& voteHash : removed) {
+                cmapVoteToObject.Erase(voteHash);
+                cmapInvalidVotes.Erase(voteHash);
+                cmmapOrphanVotes.Erase(voteHash);
+                setRequestedVotes.erase(voteHash);
+            }
+        }
+    }
+
+    // store current MN list for the next run so that we can determine which keys changed
+    lastMNListForVotingKeys = curMNList;
+}
+
+
 unsigned int CGovernanceManager::GetMinVoteTime()
 {
     LOCK(cs_main);

src/governance.h

@@ -19,6 +19,8 @@
 #include "timedata.h"
 #include "util.h"
 
+#include "evo/deterministicmns.h"
+
 #include <univalue.h>
 
 class CGovernanceManager;
@@ -261,6 +263,9 @@ private:
 
     bool fRateChecksEnabled;
 
+    // used to check for changed voting keys
+    CDeterministicMNList lastMNListForVotingKeys;
+
     class ScopedLockBool
     {
         bool& ref;
@@ -351,6 +356,7 @@ public:
         READWRITE(cmmapOrphanVotes);
         READWRITE(mapObjects);
         READWRITE(mapLastMasternodeObject);
+        READWRITE(lastMNListForVotingKeys);
         if (ser_action.ForRead() && (strVersion != SERIALIZATION_VERSION_STRING)) {
             Clear();
             return;
@@ -449,6 +455,8 @@ private:
 
     void CleanOrphanObjects();
 
+    void RemoveInvalidProposalVotes();
+
     // TODO can be removed after full DIP3 deployment
     unsigned int GetMinVoteTime();
     void ClearPreDIP3Votes();

src/rpc/governance.cpp

@@ -1144,9 +1144,9 @@ UniValue voteraw(const JSONRPCRequest& request)
     vote.SetTime(nTime);
     vote.SetSignature(vchSig);
 
-    bool onlyOwnerAllowed = govObjType == GOVERNANCE_OBJECT_PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING;
+    bool onlyVotingKeyAllowed = govObjType == GOVERNANCE_OBJECT_PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING;
 
-    if (!vote.IsValid(onlyOwnerAllowed)) {
+    if (!vote.IsValid(onlyVotingKeyAllowed)) {
         throw JSONRPCError(RPC_INTERNAL_ERROR, "Failure to verify vote.");
     }