From e6543b9c7c5022e2caeb7554a5a27b7bcfe58161 Mon Sep 17 00:00:00 2001 From: Tim Flynn Date: Fri, 24 Mar 2017 06:34:10 -0400 Subject: [PATCH 1/7] Don't add non-current wd's to seen map (#1417) --- src/governance.cpp | 19 ++++++++++++++----- src/governance.h | 2 +- src/rpcgovernance.cpp | 3 ++- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/governance.cpp b/src/governance.cpp index d107c3dc8..772cc5838 100644 --- a/src/governance.cpp +++ b/src/governance.cpp @@ -219,14 +219,18 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, std::string& strCommand, C govobj.UpdateSentinelVariables(); //this sets local vars in object - if(AddGovernanceObject(govobj, pfrom)) + bool fAddToSeen = true; + if(AddGovernanceObject(govobj, fAddToSeen, pfrom)) { LogPrintf("MNGOVERNANCEOBJECT -- %s new\n", strHash); govobj.Relay(); } - // UPDATE THAT WE'VE SEEN THIS OBJECT - mapSeenGovernanceObjects.insert(std::make_pair(nHash, SEEN_OBJECT_IS_VALID)); + if(fAddToSeen) { + // UPDATE THAT WE'VE SEEN THIS OBJECT + mapSeenGovernanceObjects.insert(std::make_pair(nHash, SEEN_OBJECT_IS_VALID)); + } + masternodeSync.AddedGovernanceItem(); @@ -305,13 +309,15 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, CGovernance fRateChecksEnabled = true; } -bool CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CNode* pfrom) +bool CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, bool& fAddToSeen, CNode* pfrom) { LOCK2(cs_main, cs); std::string strError = ""; DBG( cout << "CGovernanceManager::AddGovernanceObject START" << endl; ); + fAddToSeen = true; + uint256 nHash = govobj.GetHash(); // MAKE SURE THIS OBJECT IS OK @@ -341,6 +347,8 @@ bool CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CNode* p } if(!UpdateCurrentWatchdog(govobj)) { + // Allow wd's which are not current to be reprocessed + fAddToSeen = false; if(pfrom && (nHashWatchdogCurrent != uint256())) { pfrom->PushInventory(CInv(MSG_GOVERNANCE_OBJECT, nHashWatchdogCurrent)); } @@ -1007,7 +1015,8 @@ void CGovernanceManager::CheckMasternodeOrphanObjects() continue; } - if(AddGovernanceObject(govobj)) { + bool fAddToSeen = true; + if(AddGovernanceObject(govobj, fAddToSeen)) { LogPrintf("CGovernanceManager::CheckMasternodeOrphanObjects -- %s new\n", govobj.GetHash().ToString()); govobj.Relay(); mapMasternodeOrphanObjects.erase(it++); diff --git a/src/governance.h b/src/governance.h index 03a9ab663..bca0c3964 100644 --- a/src/governance.h +++ b/src/governance.h @@ -288,7 +288,7 @@ public: std::vector GetAllNewerThan(int64_t nMoreThanTime); bool IsBudgetPaymentBlock(int nBlockHeight); - bool AddGovernanceObject(CGovernanceObject& govobj, CNode* pfrom = NULL); + bool AddGovernanceObject(CGovernanceObject& govobj, bool& fAddToSeen, CNode* pfrom = NULL); std::string GetRequiredPaymentsString(int nBlockHeight); diff --git a/src/rpcgovernance.cpp b/src/rpcgovernance.cpp index 9a6931110..4e4f7c13c 100644 --- a/src/rpcgovernance.cpp +++ b/src/rpcgovernance.cpp @@ -221,7 +221,8 @@ UniValue gobject(const UniValue& params, bool fHelp) governance.AddSeenGovernanceObject(govobj.GetHash(), SEEN_OBJECT_IS_VALID); govobj.Relay(); LogPrintf("gobject(submit) -- Adding locally created governance object - %s\n", strHash); - governance.AddGovernanceObject(govobj); + bool fAddToSeen = true; + governance.AddGovernanceObject(govobj, fAddToSeen); return govobj.GetHash().ToString(); } From 3069e0c81ae94daaa40ee97b02414c9f691d8251 Mon Sep 17 00:00:00 2001 From: Holger Schinzel Date: Fri, 24 Mar 2017 11:47:10 +0100 Subject: [PATCH 2/7] bump to 0.12.1.5 (#1418) --- configure.ac | 2 +- src/clientversion.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 740b473e2..d790e7d22 100644 --- a/configure.ac +++ b/configure.ac @@ -3,7 +3,7 @@ AC_PREREQ([2.60]) define(_CLIENT_VERSION_MAJOR, 0) define(_CLIENT_VERSION_MINOR, 12) define(_CLIENT_VERSION_REVISION, 1) -define(_CLIENT_VERSION_BUILD, 4) +define(_CLIENT_VERSION_BUILD, 5) define(_CLIENT_VERSION_IS_RELEASE, true) define(_COPYRIGHT_YEAR, 2017) AC_INIT([Dash Core],[_CLIENT_VERSION_MAJOR._CLIENT_VERSION_MINOR._CLIENT_VERSION_REVISION],[https://github.com/dashpay/dash/issues],[dashcore]) diff --git a/src/clientversion.h b/src/clientversion.h index 25c4464a2..a31fb47e3 100644 --- a/src/clientversion.h +++ b/src/clientversion.h @@ -17,7 +17,7 @@ #define CLIENT_VERSION_MAJOR 0 #define CLIENT_VERSION_MINOR 12 #define CLIENT_VERSION_REVISION 1 -#define CLIENT_VERSION_BUILD 4 +#define CLIENT_VERSION_BUILD 5 //! Set to true for release, false for prerelease or test build #define CLIENT_VERSION_IS_RELEASE true From d8fd73fcd5fcc3e2c37d1ab4726efd1361c3fc3e Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 1 Apr 2017 20:40:13 +0300 Subject: [PATCH 3/7] Reject payment vote if masternode rank can't be calculated (#1422) --- src/masternode-payments.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/masternode-payments.cpp b/src/masternode-payments.cpp index af879c6d9..6b6c1b6a3 100644 --- a/src/masternode-payments.cpp +++ b/src/masternode-payments.cpp @@ -687,6 +687,12 @@ bool CMasternodePaymentVote::IsValid(CNode* pnode, int nValidationHeight, std::s int nRank = mnodeman.GetMasternodeRank(vinMasternode, nBlockHeight - 101, nMinRequiredProtocol, false); + if(nRank == -1) { + LogPrint("mnpayments", "CMasternodePaymentVote::IsValid -- Can't calculate rank for masternode %s\n", + vinMasternode.prevout.ToStringShort()); + return false; + } + if(nRank > MNPAYMENTS_SIGNATURES_TOTAL) { // It's common to have masternodes mistakenly think they are in the top 10 // We don't want to print all of these messages in normal mode, debug mode should print though From 7f4ff495c9e19f89b734d7e55471f472934ac14e Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 1 Apr 2017 20:40:28 +0300 Subject: [PATCH 4/7] Fix ProcessVerifyBroadcast (#1423) - check if mn rank can be calculated - fix "is in top" condition --- src/masternodeman.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp index 351a2eaa8..159b2cab3 100644 --- a/src/masternodeman.cpp +++ b/src/masternodeman.cpp @@ -1293,9 +1293,16 @@ void CMasternodeMan::ProcessVerifyBroadcast(CNode* pnode, const CMasternodeVerif } int nRank = GetMasternodeRank(mnv.vin2, mnv.nBlockHeight, MIN_POSE_PROTO_VERSION); - if(nRank < MAX_POSE_RANK) { - LogPrint("masternode", "MasternodeMan::ProcessVerifyBroadcast -- Mastrernode is not in top %d, current rank %d, peer=%d\n", - (int)MAX_POSE_RANK, nRank, pnode->id); + + if (nRank == -1) { + LogPrint("masternode", "CMasternodeMan::ProcessVerifyBroadcast -- Can't calculate rank for masternode %s\n", + mnv.vin2.prevout.ToStringShort()); + return; + } + + if(nRank > MAX_POSE_RANK) { + LogPrint("masternode", "CMasternodeMan::ProcessVerifyBroadcast -- Mastrernode %s is not in top %d, current rank %d, peer=%d\n", + mnv.vin2.prevout.ToStringShort(), (int)MAX_POSE_RANK, nRank, pnode->id); return; } From 17a36de02d79ffbc6d82eecf6580bb6b27a634b9 Mon Sep 17 00:00:00 2001 From: Tim Flynn Date: Sun, 2 Apr 2017 15:58:54 -0400 Subject: [PATCH 5/7] Fix potential race condition in vote processing (#1424) --- src/governance-object.cpp | 10 +++++++++- src/masternodeman.cpp | 5 +++-- src/masternodeman.h | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/governance-object.cpp b/src/governance-object.cpp index 7a5407ec1..b33d96bd3 100644 --- a/src/governance-object.cpp +++ b/src/governance-object.cpp @@ -178,9 +178,17 @@ bool CGovernanceObject::ProcessVote(CNode* pfrom, governance.AddInvalidVote(vote); return false; } + if(!mnodeman.AddGovernanceVote(vote.GetVinMasternode(), vote.GetParentHash())) { + std::ostringstream ostr; + ostr << "CGovernanceObject::ProcessVote -- Unable to add governance vote " + << ", MN outpoint = " << vote.GetVinMasternode().prevout.ToStringShort() + << ", governance object hash = " << GetHash().ToString() << "\n"; + LogPrint("gobject", ostr.str().c_str()); + exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_PERMANENT_ERROR); + return false; + } voteInstance = vote_instance_t(vote.GetOutcome(), nVoteTimeUpdate, vote.GetTimestamp()); fileVotes.AddVote(vote); - mnodeman.AddGovernanceVote(vote.GetVinMasternode(), vote.GetParentHash()); fDirtyCache = true; return true; } diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp index 159b2cab3..a67b06965 100644 --- a/src/masternodeman.cpp +++ b/src/masternodeman.cpp @@ -1555,14 +1555,15 @@ bool CMasternodeMan::IsWatchdogActive() return (GetTime() - nLastWatchdogVoteTime) <= MASTERNODE_WATCHDOG_MAX_SECONDS; } -void CMasternodeMan::AddGovernanceVote(const CTxIn& vin, uint256 nGovernanceObjectHash) +bool CMasternodeMan::AddGovernanceVote(const CTxIn& vin, uint256 nGovernanceObjectHash) { LOCK(cs); CMasternode* pMN = Find(vin); if(!pMN) { - return; + return false; } pMN->AddGovernanceVote(nGovernanceObjectHash); + return true; } void CMasternodeMan::RemoveGovernanceObject(uint256 nGovernanceObjectHash) diff --git a/src/masternodeman.h b/src/masternodeman.h index ff7fb9309..05f2d998f 100644 --- a/src/masternodeman.h +++ b/src/masternodeman.h @@ -341,7 +341,7 @@ public: bool IsWatchdogActive(); void UpdateWatchdogVoteTime(const CTxIn& vin); - void AddGovernanceVote(const CTxIn& vin, uint256 nGovernanceObjectHash); + bool AddGovernanceVote(const CTxIn& vin, uint256 nGovernanceObjectHash); void RemoveGovernanceObject(uint256 nGovernanceObjectHash); void CheckMasternode(const CTxIn& vin, bool fForce = false); From d7fbaf907f944672a840be7157b2a80dfcd929bc Mon Sep 17 00:00:00 2001 From: Tim Flynn Date: Mon, 3 Apr 2017 16:06:33 -0400 Subject: [PATCH 6/7] V0.12.1.x multiple vote fix (#1425) * Avoid adding the same vote multiple times to the vote file * Cleanup multiple votes in vote file --- src/governance-object.cpp | 4 +++- src/governance-votedb.cpp | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/governance-object.cpp b/src/governance-object.cpp index b33d96bd3..1b9c63160 100644 --- a/src/governance-object.cpp +++ b/src/governance-object.cpp @@ -188,7 +188,9 @@ bool CGovernanceObject::ProcessVote(CNode* pfrom, return false; } voteInstance = vote_instance_t(vote.GetOutcome(), nVoteTimeUpdate, vote.GetTimestamp()); - fileVotes.AddVote(vote); + if(!fileVotes.HasVote(vote.GetHash())) { + fileVotes.AddVote(vote); + } fDirtyCache = true; return true; } diff --git a/src/governance-votedb.cpp b/src/governance-votedb.cpp index ed62c43fa..0a817f966 100644 --- a/src/governance-votedb.cpp +++ b/src/governance-votedb.cpp @@ -58,6 +58,7 @@ void CGovernanceObjectVoteFile::RemoveVotesFromMasternode(const CTxIn& vinMaster vote_l_it it = listVotes.begin(); while(it != listVotes.end()) { if(it->GetVinMasternode() == vinMasternode) { + --nMemoryVotes; mapVoteIndex.erase(it->GetHash()); listVotes.erase(it++); } @@ -78,8 +79,18 @@ CGovernanceObjectVoteFile& CGovernanceObjectVoteFile::operator=(const CGovernanc void CGovernanceObjectVoteFile::RebuildIndex() { mapVoteIndex.clear(); - for(vote_l_it it = listVotes.begin(); it != listVotes.end(); ++it) { + nMemoryVotes = 0; + vote_l_it it = listVotes.begin(); + while(it != listVotes.end()) { CGovernanceVote& vote = *it; - mapVoteIndex[vote.GetHash()] = it; + uint256 nHash = vote.GetHash(); + if(mapVoteIndex.find(nHash) == mapVoteIndex.end()) { + mapVoteIndex[nHash] = it; + ++nMemoryVotes; + ++it; + } + else { + listVotes.erase(it++); + } } } From 86525601d5915f380976c9d2e686ad7f66db991f Mon Sep 17 00:00:00 2001 From: Tim Flynn Date: Wed, 5 Apr 2017 12:30:08 -0400 Subject: [PATCH 7/7] V0.12.1.x multiple wd rate check (#1426) * Modify MasternodeRateCheck to support updating buffers only on failure * Update rate check buffer only when fAddToSeen is true --- src/governance.cpp | 66 +++++++++++++++++++++++++------------------ src/governance.h | 10 +++++-- src/rpcgovernance.cpp | 2 +- 3 files changed, 48 insertions(+), 30 deletions(-) diff --git a/src/governance.cpp b/src/governance.cpp index 772cc5838..ace46ef17 100644 --- a/src/governance.cpp +++ b/src/governance.cpp @@ -186,7 +186,7 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, std::string& strCommand, C } bool fRateCheckBypassed = false; - if(!MasternodeRateCheck(govobj, true, false, fRateCheckBypassed)) { + if(!MasternodeRateCheck(govobj, UPDATE_FAIL_ONLY, false, fRateCheckBypassed)) { LogPrintf("MNGOVERNANCEOBJECT -- masternode rate check failed - %s - (current block height %d) \n", strHash, nCachedBlockHeight); return; } @@ -209,7 +209,7 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, std::string& strCommand, C } if(fRateCheckBypassed) { - if(!MasternodeRateCheck(govobj, true, true, fRateCheckBypassed)) { + if(!MasternodeRateCheck(govobj, UPDATE_FAIL_ONLY, true, fRateCheckBypassed)) { LogPrintf("MNGOVERNANCEOBJECT -- masternode rate check failed (after signature verification) - %s - (current block height %d) \n", strHash, nCachedBlockHeight); return; } @@ -229,6 +229,8 @@ void CGovernanceManager::ProcessMessage(CNode* pfrom, std::string& strCommand, C if(fAddToSeen) { // UPDATE THAT WE'VE SEEN THIS OBJECT mapSeenGovernanceObjects.insert(std::make_pair(nHash, SEEN_OBJECT_IS_VALID)); + // Update the rate buffer + MasternodeRateCheck(govobj, UPDATE_TRUE, true, fRateCheckBypassed); } masternodeSync.AddedGovernanceItem(); @@ -814,13 +816,13 @@ void CGovernanceManager::Sync(CNode* pfrom, const uint256& nProp, const CBloomFi LogPrintf("CGovernanceManager::Sync -- sent %d objects and %d votes to peer=%d\n", nObjCount, nVoteCount, pfrom->id); } -bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateLast) +bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, update_mode_enum_t eUpdateLast) { bool fRateCheckBypassed = false; - return MasternodeRateCheck(govobj, fUpdateLast, true, fRateCheckBypassed); + return MasternodeRateCheck(govobj, eUpdateLast, true, fRateCheckBypassed); } -bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateLast, bool fForce, bool& fRateCheckBypassed) +bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, update_mode_enum_t eUpdateLast, bool fForce, bool& fRateCheckBypassed) { LOCK(cs); @@ -848,7 +850,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo txout_m_it it = mapLastMasternodeObject.find(vin.prevout); if(it == mapLastMasternodeObject.end()) { - if(fUpdateLast) { + if(eUpdateLast == UPDATE_TRUE) { it = mapLastMasternodeObject.insert(txout_m_t::value_type(vin.prevout, last_object_rec(true))).first; switch(nObjectType) { case GOVERNANCE_OBJECT_TRIGGER: @@ -886,44 +888,54 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo double dMaxRate = 1.1 / nSuperblockCycleSeconds; double dRate = 0.0; CRateCheckBuffer buffer; + CRateCheckBuffer* pBuffer = NULL; switch(nObjectType) { case GOVERNANCE_OBJECT_TRIGGER: // Allow 1 trigger per mn per cycle, with a small fudge factor + pBuffer = &it->second.triggerBuffer; dMaxRate = 2 * 1.1 / double(nSuperblockCycleSeconds); - buffer = it->second.triggerBuffer; - buffer.AddTimestamp(nTimestamp); - dRate = buffer.GetRate(); - if(fUpdateLast) { - it->second.triggerBuffer.AddTimestamp(nTimestamp); - } break; case GOVERNANCE_OBJECT_WATCHDOG: + pBuffer = &it->second.watchdogBuffer; dMaxRate = 2 * 1.1 / 3600.; - buffer = it->second.watchdogBuffer; - buffer.AddTimestamp(nTimestamp); - dRate = buffer.GetRate(); - if(fUpdateLast) { - it->second.watchdogBuffer.AddTimestamp(nTimestamp); - } break; default: break; } - if(dRate < dMaxRate) { - if(fUpdateLast) { - it->second.fStatusOK = true; + if(!pBuffer) { + LogPrintf("CGovernanceManager::MasternodeRateCheck -- Internal Error returning false, NULL ptr found for object %s masternode vin = %s, timestamp = %d, current time = %d\n", + strHash, vin.prevout.ToStringShort(), nTimestamp, nNow); + return false; + } + + buffer = *pBuffer; + buffer.AddTimestamp(nTimestamp); + dRate = buffer.GetRate(); + + bool fRateOK = ( dRate < dMaxRate ); + + switch(eUpdateLast) { + case UPDATE_TRUE: + pBuffer->AddTimestamp(nTimestamp); + it->second.fStatusOK = fRateOK; + break; + case UPDATE_FAIL_ONLY: + if(!fRateOK) { + pBuffer->AddTimestamp(nTimestamp); + it->second.fStatusOK = false; } + default: + return true; + } + + if(fRateOK) { return true; } else { - if(fUpdateLast) { - it->second.fStatusOK = false; - } + LogPrintf("CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = %s, masternode vin = %s, object timestamp = %d, rate = %f, max rate = %f\n", + strHash, vin.prevout.ToStringShort(), nTimestamp, dRate, dMaxRate); } - - LogPrintf("CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = %s, masternode vin = %s, object timestamp = %d, rate = %f, max rate = %f\n", - strHash, vin.prevout.ToStringShort(), nTimestamp, dRate, dMaxRate); return false; } diff --git a/src/governance.h b/src/governance.h index bca0c3964..da2a78b0c 100644 --- a/src/governance.h +++ b/src/governance.h @@ -134,6 +134,12 @@ public: } }; +enum update_mode_enum_t { + UPDATE_FALSE, + UPDATE_TRUE, + UPDATE_FAIL_ONLY +}; + // // Governance Manager : Contains all proposals for the budget // @@ -362,9 +368,9 @@ public: void AddSeenVote(uint256 nHash, int status); - bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateLast = false); + bool MasternodeRateCheck(const CGovernanceObject& govobj, update_mode_enum_t eUpdateLast = UPDATE_FALSE); - bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateLast, bool fForce, bool& fRateCheckBypassed); + bool MasternodeRateCheck(const CGovernanceObject& govobj, update_mode_enum_t eUpdateLast, bool fForce, bool& fRateCheckBypassed); bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception) { bool fOK = ProcessVote(NULL, vote, exception); diff --git a/src/rpcgovernance.cpp b/src/rpcgovernance.cpp index 4e4f7c13c..4bf99751c 100644 --- a/src/rpcgovernance.cpp +++ b/src/rpcgovernance.cpp @@ -214,7 +214,7 @@ UniValue gobject(const UniValue& params, bool fHelp) throw JSONRPCError(RPC_INVALID_PARAMETER, "Object creation rate limit exceeded"); } // This check should always pass, update buffer - if(!governance.MasternodeRateCheck(govobj, true)) { + if(!governance.MasternodeRateCheck(govobj, UPDATE_TRUE)) { LogPrintf("gobject(submit) -- Object submission rejected because of rate check failure (buffer updated) - hash = %s\n", strHash); throw JSONRPCError(RPC_INVALID_PARAMETER, "Object creation rate limit exceeded"); }