From 222e5b4f7f07465acb8baf4bb84b3be7104c3e91 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Thu, 20 Dec 2018 14:27:48 +0100 Subject: [PATCH] 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 | 49 +++++++++++++++++++++++++++++++++++++-- src/governance-object.h | 4 ++++ src/governance-votedb.cpp | 21 +++++++++++++++++ src/governance-votedb.h | 1 + src/governance.cpp | 47 ++++++++++++++++++++++++++++++++++--- src/governance.h | 8 +++++++ src/rpc/governance.cpp | 4 ++-- 7 files changed, 127 insertions(+), 7 deletions(-) diff --git a/src/governance-object.cpp b/src/governance-object.cpp index ac026e268..06608d663 100644 --- a/src/governance-object.cpp +++ b/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 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); diff --git a/src/governance-object.h b/src/governance-object.h index 673491990..ae3f55321 100644 --- a/src/governance-object.h +++ b/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 RemoveInvalidProposalVotes(const COutPoint& mnOutpoint); + void CheckOrphanVotes(CConnman& connman); // TODO can be removed after DIP3 is fully deployed diff --git a/src/governance-votedb.cpp b/src/governance-votedb.cpp index 2e9f5e948..c45586c32 100644 --- a/src/governance-votedb.cpp +++ b/src/governance-votedb.cpp @@ -68,6 +68,27 @@ void CGovernanceObjectVoteFile::RemoveVotesFromMasternode(const COutPoint& outpo } } +std::set CGovernanceObjectVoteFile::RemoveInvalidProposalVotes(const COutPoint& outpointMasternode) +{ + std::set 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 CGovernanceObjectVoteFile::RemoveOldVotes(unsigned int nMinTime) { std::vector removed; diff --git a/src/governance-votedb.h b/src/governance-votedb.h index 3a13d2e81..5d98f826b 100644 --- a/src/governance-votedb.h +++ b/src/governance-votedb.h @@ -73,6 +73,7 @@ public: std::vector GetVotes() const; void RemoveVotesFromMasternode(const COutPoint& outpointMasternode); + std::set RemoveInvalidProposalVotes(const COutPoint& outpointMasternode); // TODO can be removed after full DIP3 deployment std::vector RemoveOldVotes(unsigned int nMinTime); diff --git a/src/governance.cpp b/src/governance.cpp index a3cd27efd..e2917a071 100644 --- a/src/governance.cpp +++ b/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 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); diff --git a/src/governance.h b/src/governance.h index 6790c2b2e..657134779 100644 --- a/src/governance.h +++ b/src/governance.h @@ -19,6 +19,8 @@ #include "timedata.h" #include "util.h" +#include "evo/deterministicmns.h" + #include 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(); diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index d5bdf80bf..441f7453d 100644 --- a/src/rpc/governance.cpp +++ b/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."); }