From 34e1375d3697bde863a93ccde84b86ae637361d5 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Thu, 23 Sep 2021 17:12:29 +0200 Subject: [PATCH 1/8] [BUG] Add missing initialization for MNsInfo::total --- src/masternodeman.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/masternodeman.h b/src/masternodeman.h index 00f5a9e1dd8e..9ebfb0642a73 100644 --- a/src/masternodeman.h +++ b/src/masternodeman.h @@ -161,7 +161,7 @@ class CMasternodeMan struct MNsInfo { // All the known MNs - int total; + int total{0}; // enabled MNs eligible for payments. Older than 8000 seconds. int stableSize{0}; // MNs enabled. From bf995eb09fb4513c96853b4e4f5541c68b7a721b Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 20 Sep 2021 15:45:27 +0200 Subject: [PATCH 2/8] [Cleanup] Remove unused active protocol argument in CountEnabled() --- src/budget/budgetmanager.cpp | 6 +++--- src/budget/budgetproposal.cpp | 2 +- src/masternodeman.cpp | 4 ++-- src/masternodeman.h | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/budget/budgetmanager.cpp b/src/budget/budgetmanager.cpp index 21cdf671082a..e4159edbbd3d 100644 --- a/src/budget/budgetmanager.cpp +++ b/src/budget/budgetmanager.cpp @@ -619,7 +619,7 @@ bool CBudgetManager::GetFinalizedBudget(const uint256& nHash, CFinalizedBudget& bool CBudgetManager::IsBudgetPaymentBlock(int nBlockHeight, int& nCountThreshold) const { int nHighestCount = GetHighestVoteCount(nBlockHeight); - int nCountEnabled = mnodeman.CountEnabled(ActiveProtocol()); + int nCountEnabled = mnodeman.CountEnabled(); int nFivePercent = nCountEnabled / 20; // threshold for highest finalized budgets (highest vote count - 10% of active masternodes) nCountThreshold = nHighestCount - (nCountEnabled / 10); @@ -712,7 +712,7 @@ std::vector CBudgetManager::GetBudget() const int nBlocksPerCycle = Params().GetConsensus().nBudgetCycleBlocks; int nBlockStart = nHeight - nHeight % nBlocksPerCycle + nBlocksPerCycle; int nBlockEnd = nBlockStart + nBlocksPerCycle - 1; - int mnCount = mnodeman.CountEnabled(ActiveProtocol()); + int mnCount = mnodeman.CountEnabled(); CAmount nTotalBudget = GetTotalBudget(nBlockStart); for (CBudgetProposal* pbudgetProposal: vBudgetPorposalsSort) { @@ -736,7 +736,7 @@ std::vector CBudgetManager::GetBudget() } else { LogPrint(BCLog::MNBUDGET,"%s: - Check 1 failed: valid=%d | %ld <= %ld | %ld >= %ld | Yeas=%d Nays=%d Count=%d | established=%d\n", __func__, pbudgetProposal->IsValid(), pbudgetProposal->GetBlockStart(), nBlockStart, pbudgetProposal->GetBlockEnd(), - nBlockEnd, pbudgetProposal->GetYeas(), pbudgetProposal->GetNays(), mnodeman.CountEnabled(ActiveProtocol()) / 10, + nBlockEnd, pbudgetProposal->GetYeas(), pbudgetProposal->GetNays(), mnodeman.CountEnabled() / 10, pbudgetProposal->IsEstablished()); } diff --git a/src/budget/budgetproposal.cpp b/src/budget/budgetproposal.cpp index 4c35466bb119..90a27e462f9f 100644 --- a/src/budget/budgetproposal.cpp +++ b/src/budget/budgetproposal.cpp @@ -79,7 +79,7 @@ void CBudgetProposal::SyncVotes(CNode* pfrom, bool fPartial, int& nInvCount) con bool CBudgetProposal::IsHeavilyDownvoted(bool fNewRules) { - if (GetNays() - GetYeas() > (fNewRules ? 3 : 1) * mnodeman.CountEnabled(ActiveProtocol()) / 10) { + if (GetNays() - GetYeas() > (fNewRules ? 3 : 1) * mnodeman.CountEnabled() / 10) { strInvalid = "Heavily Downvoted"; return true; } diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp index aeae284b7f55..da85b7d47cba 100644 --- a/src/masternodeman.cpp +++ b/src/masternodeman.cpp @@ -400,10 +400,10 @@ CMasternodeMan::MNsInfo CMasternodeMan::getMNsInfo() const return info; } -int CMasternodeMan::CountEnabled(int protocolVersion) const +int CMasternodeMan::CountEnabled() const { int i = 0; - protocolVersion = protocolVersion == -1 ? ActiveProtocol() : protocolVersion; + int protocolVersion = ActiveProtocol(); for (const auto& it : mapMasternodes) { const MasternodeRef& mn = it.second; diff --git a/src/masternodeman.h b/src/masternodeman.h index 9ebfb0642a73..92694c6271b8 100644 --- a/src/masternodeman.h +++ b/src/masternodeman.h @@ -126,7 +126,7 @@ class CMasternodeMan void SetBestHeight(int height) { nBestHeight.store(height, std::memory_order_release); }; int GetBestHeight() const { return nBestHeight.load(std::memory_order_acquire); } - int CountEnabled(int protocolVersion = -1) const; + int CountEnabled() const; /// Count the number of nodes with a specific proto version for each network. Return the total. int CountNetworks(int& ipv4, int& ipv6, int& onion) const; From a0830a71919836fc42201fe6c3241351fc376232 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 20 Sep 2021 19:06:44 +0200 Subject: [PATCH 3/8] [BUG] Consider DMN in CountEnabled and CountNetworks except when dealing with mnsync (only legacy list is synced via P2P) --- src/masternode-sync.cpp | 4 +- src/masternodeman.cpp | 84 ++++++++++++++++++++++------------------- src/masternodeman.h | 5 +-- src/qt/clientmodel.cpp | 12 +++--- 4 files changed, 55 insertions(+), 50 deletions(-) diff --git a/src/masternode-sync.cpp b/src/masternode-sync.cpp index e514d45b9410..f83e99ee5e3a 100644 --- a/src/masternode-sync.cpp +++ b/src/masternode-sync.cpp @@ -277,7 +277,7 @@ void CMasternodeSync::Process() /* Resync if we lose all masternodes from sleep/wake or failure to sync originally */ - if (mnodeman.CountEnabled() == 0 && !isRegTestNet) { + if (mnodeman.CountEnabled(true /* only_legacy */) == 0 && !isRegTestNet) { Reset(); } else return; @@ -401,7 +401,7 @@ bool CMasternodeSync::SyncWithNode(CNode* pnode, bool fLegacyMnObsolete) if (pnode->HasFulfilledRequest("mnwsync")) return true; pnode->FulfilledRequest("mnwsync"); - int nMnCount = mnodeman.CountEnabled(); + int nMnCount = mnodeman.CountEnabled(true /* only_legacy */); g_connman->PushMessage(pnode, msgMaker.Make(NetMsgType::GETMNWINNERS, nMnCount)); //sync payees RequestedMasternodeAttempt++; return false; diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp index da85b7d47cba..521e10ce0da1 100644 --- a/src/masternodeman.cpp +++ b/src/masternodeman.cpp @@ -352,11 +352,11 @@ void CMasternodeMan::Clear() nDsqCount = 0; } -static void CountNetwork(const MasternodeRef& mn, int& ipv4, int& ipv6, int& onion) +static void CountNetwork(const CService& addr, int& ipv4, int& ipv6, int& onion) { std::string strHost; int port; - SplitHostPort(mn->addr.ToString(), port, strHost); + SplitHostPort(addr.ToString(), port, strHost); CNetAddr node; LookupHost(strHost, node, false); switch(node.GetNetwork()) { @@ -378,29 +378,44 @@ CMasternodeMan::MNsInfo CMasternodeMan::getMNsInfo() const { MNsInfo info; int nMinProtocol = ActiveProtocol(); + bool spork_8_active = sporkManager.IsSporkActive(SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT); - LOCK(cs); - for (const auto& it : mapMasternodes) { - const MasternodeRef& mn = it.second; - info.total++; - CountNetwork(mn, info.ipv4, info.ipv6, info.onion); - if (mn->protocolVersion < nMinProtocol || !mn->IsEnabled()) { - continue; - } - info.enabledSize++; - // Eligible for payments - if (sporkManager.IsSporkActive (SPORK_8_MASTERNODE_PAYMENT_ENFORCEMENT)) { - if (GetAdjustedTime() - mn->sigTime < MN_WINNER_MINIMUM_AGE) { + // legacy masternodes + { + LOCK(cs); + for (const auto& it : mapMasternodes) { + const MasternodeRef& mn = it.second; + info.total++; + CountNetwork(mn->addr, info.ipv4, info.ipv6, info.onion); + if (mn->protocolVersion < nMinProtocol || !mn->IsEnabled()) { + continue; + } + info.enabledSize++; + // Eligible for payments + if (spork_8_active && (GetAdjustedTime() - mn->sigTime < MN_WINNER_MINIMUM_AGE)) { continue; // Skip masternodes younger than (default) 8000 sec (MUST be > MASTERNODE_REMOVAL_SECONDS) } + info.stableSize++; } - info.stableSize++; + } + + // deterministic masternodes + if (deterministicMNManager->IsDIP3Enforced()) { + auto mnList = deterministicMNManager->GetListAtChainTip(); + mnList.ForEachMN(false, [&](const CDeterministicMNCPtr& dmn) { + info.total++; + CountNetwork(dmn->pdmnState->addr, info.ipv4, info.ipv6, info.onion); + if (mnList.IsMNValid(dmn)) { + info.enabledSize++; + info.stableSize++; + } + }); } return info; } -int CMasternodeMan::CountEnabled() const +int CMasternodeMan::CountEnabled(bool only_legacy) const { int i = 0; int protocolVersion = ActiveProtocol(); @@ -411,17 +426,11 @@ int CMasternodeMan::CountEnabled() const i++; } - return i; -} - -int CMasternodeMan::CountNetworks(int& ipv4, int& ipv6, int& onion) const -{ - LOCK(cs); - for (const auto& it : mapMasternodes) { - const MasternodeRef& mn = it.second; - CountNetwork(mn, ipv4, ipv6, onion); + if (!only_legacy && deterministicMNManager->IsDIP3Enforced()) { + i += deterministicMNManager->GetListAtChainTip().GetValidMNsCount(); } - return (int) mapMasternodes.size(); + + return i; } void CMasternodeMan::DsegUpdate(CNode* pnode) @@ -530,19 +539,13 @@ MasternodeRef CMasternodeMan::GetNextMasternodeInQueueForPayment(int nBlockHeigh MasternodeRef pBestMasternode = nullptr; std::vector > vecMasternodeLastPaid; - CDeterministicMNList mnList; - if (deterministicMNManager->IsDIP3Enforced()) { - mnList = deterministicMNManager->GetListAtChainTip(); - } - /* Make a vector with all of the last paid times */ int minProtocol = ActiveProtocol(); - int nMnCount = mnList.GetValidMNsCount(); + int nMnCount = CountEnabled(); { LOCK(cs); - nMnCount += CountEnabled(); for (const auto& it : mapMasternodes) { if (!it.second->IsEnabled()) continue; if (canScheduleMN(fFilterSigTime, it.second, minProtocol, nMnCount, nBlockHeight)) { @@ -551,12 +554,15 @@ MasternodeRef CMasternodeMan::GetNextMasternodeInQueueForPayment(int nBlockHeigh } } // Add deterministic masternodes to the vector - mnList.ForEachMN(true, [&](const CDeterministicMNCPtr& dmn) { - const MasternodeRef mn = MakeMasternodeRefForDMN(dmn); - if (canScheduleMN(fFilterSigTime, mn, minProtocol, nMnCount, nBlockHeight)) { - vecMasternodeLastPaid.emplace_back(SecondsSincePayment(mn, BlockReading), mn); - } - }); + if (deterministicMNManager->IsDIP3Enforced()) { + CDeterministicMNList mnList = deterministicMNManager->GetListAtChainTip(); + mnList.ForEachMN(true, [&](const CDeterministicMNCPtr& dmn) { + const MasternodeRef mn = MakeMasternodeRefForDMN(dmn); + if (canScheduleMN(fFilterSigTime, mn, minProtocol, nMnCount, nBlockHeight)) { + vecMasternodeLastPaid.emplace_back(SecondsSincePayment(mn, BlockReading), mn); + } + }); + } nCount = (int)vecMasternodeLastPaid.size(); diff --git a/src/masternodeman.h b/src/masternodeman.h index 92694c6271b8..903b49b6bd19 100644 --- a/src/masternodeman.h +++ b/src/masternodeman.h @@ -126,10 +126,7 @@ class CMasternodeMan void SetBestHeight(int height) { nBestHeight.store(height, std::memory_order_release); }; int GetBestHeight() const { return nBestHeight.load(std::memory_order_acquire); } - int CountEnabled() const; - - /// Count the number of nodes with a specific proto version for each network. Return the total. - int CountNetworks(int& ipv4, int& ipv6, int& onion) const; + int CountEnabled(bool only_legacy = false) const; void DsegUpdate(CNode* pnode); diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index 89c4bf2b63e7..c0d473d61220 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -76,11 +76,13 @@ int ClientModel::getNumConnections(unsigned int flags) const QString ClientModel::getMasternodeCountString() const { - int ipv4 = 0, ipv6 = 0, onion = 0; - int total = mnodeman.CountNetworks(ipv4, ipv6, onion); - int nUnknown = total - ipv4 - ipv6 - onion; - if(nUnknown < 0) nUnknown = 0; - return tr("Total: %1 (IPv4: %2 / IPv6: %3 / Tor: %4 / Unknown: %5)").arg(QString::number(total)).arg(QString::number((int)ipv4)).arg(QString::number((int)ipv6)).arg(QString::number((int)onion)).arg(QString::number((int)nUnknown)); + const auto& info = mnodeman.getMNsInfo(); + int unknown = std::max(0, info.total - info.ipv4 - info.ipv6 - info.onion); + return tr("Total: %1 (IPv4: %2 / IPv6: %3 / Tor: %4 / Unknown: %5)").arg(QString::number(info.total)) + .arg(QString::number(info.ipv4)) + .arg(QString::number(info.ipv6)) + .arg(QString::number(info.onion)) + .arg(QString::number(unknown)); } QString ClientModel::getMasternodesCount() From 8cc9735d37a780c1f9ef02b4d9f1a4f0e435ab62 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 20 Sep 2021 18:28:30 +0200 Subject: [PATCH 4/8] [QA] Check enabled masternode count in functional tests --- test/functional/tiertwo_deterministicmns.py | 42 ++++++++++++++++++++- test/functional/tiertwo_mn_compatibility.py | 12 ++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/test/functional/tiertwo_deterministicmns.py b/test/functional/tiertwo_deterministicmns.py index 4b1adb25f7ec..02f7bf396700 100755 --- a/test/functional/tiertwo_deterministicmns.py +++ b/test/functional/tiertwo_deterministicmns.py @@ -48,6 +48,12 @@ def check_mn_list(self, mns): self.check_mn_list_on_node(i, mns) self.log.info("Deterministic list contains %d masternodes for all peers." % len(mns)) + def check_mn_enabled_count(self, enabled, total): + for node in self.nodes: + node_count = node.getmasternodecount() + assert_equal(node_count['enabled'], enabled) + assert_equal(node_count['total'], total) + def get_addr_balance(self, node, addr): rcv = node.listreceivedbyaddress(0, False, False, addr) return rcv[0]['amount'] if len(rcv) > 0 else 0 @@ -131,6 +137,9 @@ def run_test(self): # -- DIP3 enforced and SPORK_21 active here -- self.wait_until_mnsync_completed() + # enabled/total masternodes: 0/0 + self.check_mn_enabled_count(0, 0) + # Create 3 DMNs and init the remote nodes self.log.info("Initializing masternodes...") self.add_new_dmn(mns, "internal") @@ -142,6 +151,9 @@ def run_test(self): miner.generate(1) self.sync_blocks() + # enabled/total masternodes: 3/3 + self.check_mn_enabled_count(3, 3) + # Init the other 3 remote nodes before creating the ProReg tx self.log.info("Initializing more masternodes...") op_keys = [] @@ -162,6 +174,9 @@ def run_test(self): self.sync_blocks() time.sleep(1) self.log.info("Masternodes started.") + + # enabled/total masternodes: 6/6 + self.check_mn_enabled_count(6, 6) self.check_mn_list(mns) # Check status from remote nodes @@ -188,6 +203,9 @@ def run_test(self): miner.generate(1) self.sync_blocks() assert_greater_than(miner.getrawtransaction(spend_txid, True)["confirmations"], 0) + + # enabled/total masternodes: 5/5 + self.check_mn_enabled_count(5, 5) self.check_mn_list(mns) # Register dmn again, with the collateral of dmn2 @@ -200,6 +218,9 @@ def run_test(self): outpoint=dmn2.collateral, op_addr_and_key=dmn_keys)) miner.generate(1) self.sync_blocks() + + # enabled/total masternodes: 5/5 + self.check_mn_enabled_count(5, 5) self.check_mn_list(mns) # Now try to register dmn2 again with an already-used IP @@ -235,6 +256,9 @@ def run_test(self): json_tx = self.nodes[dmn2c.idx].getrawtransaction(dmn2c.proTx, True) assert_greater_than(json_tx['confirmations'], 0) self.check_proreg_payload(dmn2c, json_tx) + + # enabled/total masternodes: 6/6 + self.check_mn_enabled_count(6, 6) self.check_mn_list(mns) # 6 masternodes again # Test payments. @@ -317,9 +341,13 @@ def run_test(self): self.sync_mempools([miner, controller]) miner.generate(1) self.sync_blocks() - # Updating the operator address, clears the IP (and puts the mn in PoSe banned state) + + # enabled/total masternodes: 5/6 + # Updating the operator key, clears the IP (and puts the mn in PoSe banned state) + self.check_mn_enabled_count(5, 6) mns[0].ipport = "[::]:0" self.check_mn_list(mns) + old_mn0_balance = self.get_addr_balance(controller, mns[0].payee) self.log.info("Update operator address (with external key)...") mns[0].operator = self.nodes[mns[0].idx].getnewaddress() @@ -328,6 +356,7 @@ def run_test(self): miner.protx_update_registrar(mns[0].proTx, mns[0].operator, "", "", ownerKey) miner.generate(1) self.sync_blocks() + self.check_mn_enabled_count(5, 6) # stil not valid until new operator sends proUpServ self.check_mn_list(mns) self.log.info("Update voting address...") mns[1].voting = controller.getnewaddress() @@ -335,6 +364,7 @@ def run_test(self): self.sync_mempools([miner, controller]) miner.generate(1) self.sync_blocks() + self.check_mn_enabled_count(5, 6) self.check_mn_list(mns) self.log.info("Update payout address...") old_payee = mns[2].payee @@ -346,6 +376,7 @@ def run_test(self): old_mn2_bal = self.get_addr_balance(controller, old_payee) miner.generate(len(mns)-1) self.sync_blocks() + self.check_mn_enabled_count(5, 6) self.check_mn_list(mns) # Check payment to new address self.log.info("Checking payments...") @@ -367,6 +398,7 @@ def run_test(self): self.sync_mempools([miner, controller]) miner.generate(1) self.sync_blocks() + self.check_mn_enabled_count(4, 6) # mn3 has been revoked self.check_mn_list(mns) old_mn3_bal = self.get_addr_balance(controller, mns[3].payee) self.log.info("Revoke masternode (with external key)...") @@ -378,7 +410,11 @@ def run_test(self): old_mn4_bal = self.get_addr_balance(controller, mns[4].payee) miner.generate(len(mns) + 1) self.sync_blocks() + + # enabled/total masternodes: 3/6 (mn0 banned, mn3 and mn4 revoked) + self.check_mn_enabled_count(3, 6) self.check_mn_list(mns) + # Check (no) payments self.log.info("Checking payments...") assert_equal(self.get_addr_balance(controller, mns[3].payee), old_mn3_bal) @@ -394,7 +430,11 @@ def run_test(self): miner.protx_update_service(mns[3].proTx, mns[3].ipport, "", mns[3].operator_key) miner.generate(len(mns)) self.sync_blocks() + + # enabled/total masternodes: 4/6 (mn3 is back) + self.check_mn_enabled_count(4, 6) self.check_mn_list(mns) + self.log.info("Checking payments...") assert_equal(self.get_addr_balance(controller, mns[3].payee), old_mn3_bal + Decimal('3')) diff --git a/test/functional/tiertwo_mn_compatibility.py b/test/functional/tiertwo_mn_compatibility.py index b7cb5fec4f5b..3a0bf8aed75b 100755 --- a/test/functional/tiertwo_mn_compatibility.py +++ b/test/functional/tiertwo_mn_compatibility.py @@ -60,6 +60,12 @@ def check_mns_status(self, node, txhash): assert_equal(status["dmnstate"]["PoSePenalty"], 0) assert_equal(status["status"], "Ready") + def check_mn_enabled_count(self, enabled, total): + for node in self.nodes: + node_count = node.getmasternodecount() + assert_equal(node_count['enabled'], enabled) + assert_equal(node_count['total'], total) + """ Checks the block at specified height Returns the address of the mn paid (in the coinbase), and the json coinstake tx @@ -92,6 +98,9 @@ def run_test(self): self.enable_mocktime() self.setup_3_masternodes_network() + # start with 3 masternodes (2 legacy + 1 DMN) + self.check_mn_enabled_count(3, 3) + # add two more nodes to the network self.remoteDMN2 = self.nodes[self.remoteDMN2Pos] self.remoteDMN3 = self.nodes[self.remoteDMN3Pos] @@ -124,6 +133,7 @@ def run_test(self): self.remoteDMN2.initmasternode(self.dmn2Privkey, "", True) # check list and status + self.check_mn_enabled_count(4, 4) # 2 legacy + 2 DMN txHashSet.add(self.proRegTx2) self.check_mn_list(self.miner, txHashSet) self.check_mns_status(self.remoteDMN2, self.proRegTx2) @@ -156,6 +166,7 @@ def run_test(self): # The legacy masternode must no longer be in the list # and the DMN must have taken its place + self.check_mn_enabled_count(4, 4) # 1 legacy + 3 DMN txHashSet.remove(self.mnOneCollateral.hash) txHashSet.add(self.proRegTx3) for node in self.nodes: @@ -170,6 +181,7 @@ def run_test(self): self.send_3_pings() # the masternode list hasn't changed + self.check_mn_enabled_count(4, 4) for node in self.nodes: self.check_mn_list(node, txHashSet) self.log.info("Masternode list correctly unchanged in all nodes.") From 5da28cc7d149ec59c569366fe9f63a71ba62d97c Mon Sep 17 00:00:00 2001 From: random-zebra Date: Mon, 20 Sep 2021 21:35:53 +0200 Subject: [PATCH 5/8] [BUG] Fix potential deadlock CountEnabled is being called while holding CMasternodeMan::cs, by SecondsSincePayment, and it locks CDeterministicMNManager::cs to get the deterministic manager's tip height. Pass the enabled masternode count to SecondsSincePayment, directly from the caller, so the two locks are not held at the same time. --- src/masternodeman.cpp | 35 +++++++++++++++++++---------------- src/masternodeman.h | 4 ++-- src/rpc/masternode.cpp | 3 ++- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp index 521e10ce0da1..34ba4a260820 100644 --- a/src/masternodeman.cpp +++ b/src/masternodeman.cpp @@ -420,10 +420,13 @@ int CMasternodeMan::CountEnabled(bool only_legacy) const int i = 0; int protocolVersion = ActiveProtocol(); - for (const auto& it : mapMasternodes) { - const MasternodeRef& mn = it.second; - if (mn->protocolVersion < protocolVersion || !mn->IsEnabled()) continue; - i++; + { + LOCK(cs); + for (const auto& it : mapMasternodes) { + const MasternodeRef& mn = it.second; + if (mn->protocolVersion < protocolVersion || !mn->IsEnabled()) continue; + i++; + } } if (!only_legacy && deterministicMNManager->IsDIP3Enforced()) { @@ -543,13 +546,13 @@ MasternodeRef CMasternodeMan::GetNextMasternodeInQueueForPayment(int nBlockHeigh Make a vector with all of the last paid times */ int minProtocol = ActiveProtocol(); - int nMnCount = CountEnabled(); + int count_enabled = CountEnabled(); { LOCK(cs); for (const auto& it : mapMasternodes) { if (!it.second->IsEnabled()) continue; - if (canScheduleMN(fFilterSigTime, it.second, minProtocol, nMnCount, nBlockHeight)) { - vecMasternodeLastPaid.emplace_back(SecondsSincePayment(it.second, BlockReading), it.second); + if (canScheduleMN(fFilterSigTime, it.second, minProtocol, count_enabled, nBlockHeight)) { + vecMasternodeLastPaid.emplace_back(SecondsSincePayment(it.second, count_enabled, BlockReading), it.second); } } } @@ -558,8 +561,8 @@ MasternodeRef CMasternodeMan::GetNextMasternodeInQueueForPayment(int nBlockHeigh CDeterministicMNList mnList = deterministicMNManager->GetListAtChainTip(); mnList.ForEachMN(true, [&](const CDeterministicMNCPtr& dmn) { const MasternodeRef mn = MakeMasternodeRefForDMN(dmn); - if (canScheduleMN(fFilterSigTime, mn, minProtocol, nMnCount, nBlockHeight)) { - vecMasternodeLastPaid.emplace_back(SecondsSincePayment(mn, BlockReading), mn); + if (canScheduleMN(fFilterSigTime, mn, minProtocol, count_enabled, nBlockHeight)) { + vecMasternodeLastPaid.emplace_back(SecondsSincePayment(mn, count_enabled, BlockReading), mn); } }); } @@ -567,7 +570,7 @@ MasternodeRef CMasternodeMan::GetNextMasternodeInQueueForPayment(int nBlockHeigh nCount = (int)vecMasternodeLastPaid.size(); //when the network is in the process of upgrading, don't penalize nodes that recently restarted - if (fFilterSigTime && nCount < nMnCount / 3) return GetNextMasternodeInQueueForPayment(nBlockHeight, false, nCount, BlockReading); + if (fFilterSigTime && nCount < count_enabled / 3) return GetNextMasternodeInQueueForPayment(nBlockHeight, false, nCount, BlockReading); // Sort them high to low sort(vecMasternodeLastPaid.rbegin(), vecMasternodeLastPaid.rend(), CompareScoreMN()); @@ -576,7 +579,7 @@ MasternodeRef CMasternodeMan::GetNextMasternodeInQueueForPayment(int nBlockHeigh // -- This doesn't look at who is being paid in the +8-10 blocks, allowing for double payments very rarely // -- 1/100 payments should be a double payment on mainnet - (1/(3000/10))*2 // -- (chance per block * chances before IsScheduled will fire) - int nTenthNetwork = nMnCount / 10; + int nTenthNetwork = count_enabled / 10; int nCountTenth = 0; arith_uint256 nHigh = ARITH_UINT256_ZERO; const uint256& hash = GetHashAtHeight(nBlockHeight - 101); @@ -943,9 +946,9 @@ void CMasternodeMan::UpdateMasternodeList(CMasternodeBroadcast& mnb) } } -int64_t CMasternodeMan::SecondsSincePayment(const MasternodeRef& mn, const CBlockIndex* BlockReading) const +int64_t CMasternodeMan::SecondsSincePayment(const MasternodeRef& mn, int count_enabled, const CBlockIndex* BlockReading) const { - int64_t sec = (GetAdjustedTime() - GetLastPaid(mn, BlockReading)); + int64_t sec = (GetAdjustedTime() - GetLastPaid(mn, count_enabled, BlockReading)); int64_t month = 60 * 60 * 24 * 30; if (sec < month) return sec; //if it's less than 30 days, give seconds @@ -958,7 +961,7 @@ int64_t CMasternodeMan::SecondsSincePayment(const MasternodeRef& mn, const CBloc return month + hash.GetCompact(false); } -int64_t CMasternodeMan::GetLastPaid(const MasternodeRef& mn, const CBlockIndex* BlockReading) const +int64_t CMasternodeMan::GetLastPaid(const MasternodeRef& mn, int count_enabled, const CBlockIndex* BlockReading) const { if (BlockReading == nullptr) return false; @@ -972,8 +975,8 @@ int64_t CMasternodeMan::GetLastPaid(const MasternodeRef& mn, const CBlockIndex* // use a deterministic offset to break a tie -- 2.5 minutes int64_t nOffset = UintToArith256(hash).GetCompact(false) % 150; - int nMnCount = CountEnabled() * 1.25; - for (int n = 0; n < nMnCount; n++) { + int max_depth = count_enabled * 1.25; + for (int n = 0; n < max_depth; n++) { const auto& it = masternodePayments.mapMasternodeBlocks.find(BlockReading->nHeight); if (it != masternodePayments.mapMasternodeBlocks.end()) { // Search for this payee, with at least 2 votes. This will aid in consensus diff --git a/src/masternodeman.h b/src/masternodeman.h index 903b49b6bd19..3d0cb44a30db 100644 --- a/src/masternodeman.h +++ b/src/masternodeman.h @@ -181,8 +181,8 @@ class CMasternodeMan void UpdateMasternodeList(CMasternodeBroadcast& mnb); /// Get the time a masternode was last paid - int64_t GetLastPaid(const MasternodeRef& mn, const CBlockIndex* BlockReading) const; - int64_t SecondsSincePayment(const MasternodeRef& mn, const CBlockIndex* BlockReading) const; + int64_t GetLastPaid(const MasternodeRef& mn, int count_enabled, const CBlockIndex* BlockReading) const; + int64_t SecondsSincePayment(const MasternodeRef& mn, int count_enabled, const CBlockIndex* BlockReading) const; // Block hashes cycling vector management void CacheBlockHash(const CBlockIndex* pindex); diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 42e8a4df6949..8843f0fec6ad 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -191,6 +191,7 @@ UniValue listmasternodes(const JSONRPCRequest& request) int nHeight = chainTip->nHeight; auto mnList = deterministicMNManager->GetListAtChainTip(); + int count_enabled = mnodeman.CountEnabled(); std::vector> vMasternodeRanks = mnodeman.GetMasternodeRanks(nHeight); for (int pos=0; pos < (int) vMasternodeRanks.size(); pos++) { const auto& s = vMasternodeRanks[pos]; @@ -244,7 +245,7 @@ UniValue listmasternodes(const JSONRPCRequest& request) obj.pushKV("version", mn.protocolVersion); obj.pushKV("lastseen", (int64_t)mn.lastPing.sigTime); obj.pushKV("activetime", (int64_t)(mn.lastPing.sigTime - mn.sigTime)); - obj.pushKV("lastpaid", (int64_t)mnodeman.GetLastPaid(s.second, chainTip)); + obj.pushKV("lastpaid", (int64_t)mnodeman.GetLastPaid(s.second, count_enabled, chainTip)); ret.push_back(obj); } From 273052124668861e3fc363642d742a25caaaf384 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Tue, 21 Sep 2021 11:50:14 +0200 Subject: [PATCH 6/8] [BUG] Check budget list for CMasternodeSync::Reset after SPORK 21 Since the count must always remain 0 when the legacy system is obsolete --- src/budget/budgetmanager.h | 2 ++ src/masternode-sync.cpp | 15 ++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/budget/budgetmanager.h b/src/budget/budgetmanager.h index 7c35ba3c2770..ddfe0816c747 100644 --- a/src/budget/budgetmanager.h +++ b/src/budget/budgetmanager.h @@ -137,6 +137,8 @@ class CBudgetManager : public CValidationInterface // Only initialized masternodes: sign and submit votes on valid finalized budgets void VoteOnFinalizedBudgets(); + int CountProposals() { LOCK(cs_proposals); return mapProposals.size(); } + void CheckOrphanVotes(); void Clear() { diff --git a/src/masternode-sync.cpp b/src/masternode-sync.cpp index f83e99ee5e3a..87c7bb2a3a67 100644 --- a/src/masternode-sync.cpp +++ b/src/masternode-sync.cpp @@ -274,13 +274,18 @@ void CMasternodeSync::Process() if (tick++ % MASTERNODE_SYNC_TIMEOUT != 0) return; if (IsSynced()) { - /* - Resync if we lose all masternodes from sleep/wake or failure to sync originally - */ - if (mnodeman.CountEnabled(true /* only_legacy */) == 0 && !isRegTestNet) { + if (isRegTestNet) { + return; + } + bool legacy_obsolete = deterministicMNManager->LegacyMNObsolete(); + // Check if we lost all masternodes from sleep/wake or failure to sync originally (after + // spork 21, check if we lost all proposals instead). If we did, resync from scratch. + if ((!legacy_obsolete && mnodeman.CountEnabled(true /* only_legacy */) == 0) || + (legacy_obsolete && g_budgetman.CountProposals() == 0)) { Reset(); - } else + } else { return; + } } //try syncing again From 4e89bb24141092b2efe7d35c82a318346bc8d392 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Thu, 23 Sep 2021 17:03:46 +0200 Subject: [PATCH 7/8] [Refactor] move IsMNValid/IsMNPoSeBanned to CDeterministicMN --- src/activemasternode.cpp | 23 +++++++++-------- src/budget/budgetmanager.cpp | 8 +++--- src/evo/deterministicmns.cpp | 34 ++----------------------- src/evo/deterministicmns.h | 10 +++----- src/masternodeman.cpp | 4 +-- src/rpc/masternode.cpp | 5 ++-- src/test/evo_deterministicmns_tests.cpp | 3 +++ 7 files changed, 28 insertions(+), 59 deletions(-) diff --git a/src/activemasternode.cpp b/src/activemasternode.cpp index 59c712ce8bb9..df187c132d65 100644 --- a/src/activemasternode.cpp +++ b/src/activemasternode.cpp @@ -125,12 +125,8 @@ void CActiveDeterministicMasternodeManager::Init() return; } - if (!mnList.IsMNValid(dmn->proTxHash)) { - if (mnList.IsMNPoSeBanned(dmn->proTxHash)) { - state = MASTERNODE_POSE_BANNED; - } else { - state = MASTERNODE_REMOVED; - } + if (dmn->IsPoSeBanned()) { + state = MASTERNODE_POSE_BANNED; return; } @@ -181,16 +177,21 @@ void CActiveDeterministicMasternodeManager::UpdatedBlockTip(const CBlockIndex* p return; if (state == MASTERNODE_READY) { - auto oldMNList = deterministicMNManager->GetListForBlock(pindexNew->pprev); - auto newMNList = deterministicMNManager->GetListForBlock(pindexNew); - if (!newMNList.IsMNValid(info.proTxHash)) { + auto newDmn = deterministicMNManager->GetListForBlock(pindexNew).GetValidMN(info.proTxHash); + if (newDmn == nullptr) { // MN disappeared from MN list Reset(MASTERNODE_REMOVED); return; } - auto oldDmn = oldMNList.GetMN(info.proTxHash); - auto newDmn = newMNList.GetMN(info.proTxHash); + auto oldDmn = deterministicMNManager->GetListForBlock(pindexNew->pprev).GetMN(info.proTxHash); + if (oldDmn == nullptr) { + // should never happen if state is MASTERNODE_READY + LogPrintf("%s: WARNING: unable to find active mn %s in prev block list %s\n", + __func__, info.proTxHash.ToString(), pindexNew->pprev->GetBlockHash().ToString()); + return; + } + if (newDmn->pdmnState->keyIDOperator != oldDmn->pdmnState->keyIDOperator) { // MN operator key changed or revoked Reset(MASTERNODE_OPERATOR_KEY_CHANGED); diff --git a/src/budget/budgetmanager.cpp b/src/budget/budgetmanager.cpp index e4159edbbd3d..f1cd307f050d 100644 --- a/src/budget/budgetmanager.cpp +++ b/src/budget/budgetmanager.cpp @@ -821,7 +821,7 @@ void CBudgetManager::RemoveStaleVotesOnProposal(CBudgetProposal* prop) auto mnList = deterministicMNManager->GetListAtChainTip(); auto dmn = mnList.GetMNByCollateral(it->first); if (dmn) { - (*it).second.SetValid(mnList.IsMNValid(dmn)); + (*it).second.SetValid(!dmn->IsPoSeBanned()); } else { // -- Legacy System (!TODO: remove after enforcement) -- CMasternode* pmn = mnodeman.Find(it->first); @@ -845,7 +845,7 @@ void CBudgetManager::RemoveStaleVotesOnFinalBudget(CFinalizedBudget* fbud) auto mnList = deterministicMNManager->GetListAtChainTip(); auto dmn = mnList.GetMNByCollateral(it->first); if (dmn) { - (*it).second.SetValid(mnList.IsMNValid(dmn)); + (*it).second.SetValid(!dmn->IsPoSeBanned()); } else { // -- Legacy System (!TODO: remove after enforcement) -- CMasternode* pmn = mnodeman.Find(it->first); @@ -1020,7 +1020,7 @@ bool CBudgetManager::ProcessProposalVote(CBudgetVote& vote, CNode* pfrom, CValid AddSeenProposalVote(vote); - if (!mnList.IsMNValid(dmn)) { + if (dmn->IsPoSeBanned()) { err = strprintf("masternode (%s) not valid or PoSe banned", mn_protx_id); return state.DoS(0, false, REJECT_INVALID, "bad-mvote", false, err); } @@ -1116,7 +1116,7 @@ bool CBudgetManager::ProcessFinalizedBudgetVote(CFinalizedBudgetVote& vote, CNod AddSeenFinalizedBudgetVote(vote); - if (!mnList.IsMNValid(dmn)) { + if (dmn->IsPoSeBanned()) { err = strprintf("masternode (%s) not valid or PoSe banned", mn_protx_id); return state.DoS(0, false, REJECT_INVALID, "bad-fbvote", false, err); } diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index fbed5b7229e7..f646ea6c4dc6 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -105,36 +105,6 @@ void CDeterministicMN::ToJson(UniValue& obj) const obj.pushKV("dmnstate", stateObj); } -bool CDeterministicMNList::IsMNValid(const uint256& proTxHash) const -{ - auto p = mnMap.find(proTxHash); - if (p == nullptr) { - return false; - } - return IsMNValid(*p); -} - -bool CDeterministicMNList::IsMNPoSeBanned(const uint256& proTxHash) const -{ - auto p = mnMap.find(proTxHash); - if (p == nullptr) { - return false; - } - return IsMNPoSeBanned(*p); -} - -bool CDeterministicMNList::IsMNValid(const CDeterministicMNCPtr& dmn) const -{ - return !IsMNPoSeBanned(dmn); -} - -bool CDeterministicMNList::IsMNPoSeBanned(const CDeterministicMNCPtr& dmn) const -{ - assert(dmn); - const CDeterministicMNState& state = *dmn->pdmnState; - return state.nPoSeBanHeight != -1; -} - CDeterministicMNCPtr CDeterministicMNList::GetMN(const uint256& proTxHash) const { auto p = mnMap.find(proTxHash); @@ -147,7 +117,7 @@ CDeterministicMNCPtr CDeterministicMNList::GetMN(const uint256& proTxHash) const CDeterministicMNCPtr CDeterministicMNList::GetValidMN(const uint256& proTxHash) const { auto dmn = GetMN(proTxHash); - if (dmn && !IsMNValid(dmn)) { + if (dmn && dmn->IsPoSeBanned()) { return nullptr; } return dmn; @@ -171,7 +141,7 @@ CDeterministicMNCPtr CDeterministicMNList::GetMNByCollateral(const COutPoint& co CDeterministicMNCPtr CDeterministicMNList::GetValidMNByCollateral(const COutPoint& collateralOutpoint) const { auto dmn = GetMNByCollateral(collateralOutpoint); - if (dmn && !IsMNValid(dmn)) { + if (dmn && dmn->IsPoSeBanned()) { return nullptr; } return dmn; diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index 4ec4a276ac38..a761a6d48f91 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -213,6 +213,7 @@ class CDeterministicMN } uint64_t GetInternalId() const; + bool IsPoSeBanned() const { return pdmnState->nPoSeBanHeight != -1; } std::string ToString() const; void ToJson(UniValue& obj) const; @@ -321,7 +322,7 @@ class CDeterministicMNList { size_t count = 0; for (const auto& p : mnMap) { - if (IsMNValid(p.second)) { + if (!p.second->IsPoSeBanned()) { count++; } } @@ -332,7 +333,7 @@ class CDeterministicMNList void ForEachMN(bool onlyValid, Callback&& cb) const { for (const auto& p : mnMap) { - if (!onlyValid || IsMNValid(p.second)) { + if (!onlyValid || !p.second->IsPoSeBanned()) { cb(p.second); } } @@ -345,11 +346,6 @@ class CDeterministicMNList void SetHeight(int _height) { nHeight = _height; } void SetBlockHash(const uint256& _blockHash) { blockHash = _blockHash; } - bool IsMNValid(const uint256& proTxHash) const; - bool IsMNPoSeBanned(const uint256& proTxHash) const; - bool IsMNValid(const CDeterministicMNCPtr& dmn) const; - bool IsMNPoSeBanned(const CDeterministicMNCPtr& dmn) const; - bool HasMN(const uint256& proTxHash) const { return GetMN(proTxHash) != nullptr; diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp index 34ba4a260820..33c7a8d9e89f 100644 --- a/src/masternodeman.cpp +++ b/src/masternodeman.cpp @@ -405,7 +405,7 @@ CMasternodeMan::MNsInfo CMasternodeMan::getMNsInfo() const mnList.ForEachMN(false, [&](const CDeterministicMNCPtr& dmn) { info.total++; CountNetwork(dmn->pdmnState->addr, info.ipv4, info.ipv6, info.onion); - if (mnList.IsMNValid(dmn)) { + if (!dmn->IsPoSeBanned()) { info.enabledSize++; info.stableSize++; } @@ -722,7 +722,7 @@ std::vector> CMasternodeMan::GetMasternodeRank auto mnList = deterministicMNManager->GetListAtChainTip(); mnList.ForEachMN(false, [&](const CDeterministicMNCPtr& dmn) { const MasternodeRef mn = MakeMasternodeRefForDMN(dmn); - const uint32_t score = mnList.IsMNValid(dmn) ? mn->CalculateScore(hash).GetCompact(false) : 9999; + const uint32_t score = dmn->IsPoSeBanned() ? 9999 : mn->CalculateScore(hash).GetCompact(false); vecMasternodeScores.emplace_back(score, mn); }); diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 8843f0fec6ad..1e8602bf8a9f 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -177,8 +177,7 @@ UniValue listmasternodes(const JSONRPCRequest& request) mnList.ForEachMN(false, [&](const CDeterministicMNCPtr& dmn) { UniValue obj(UniValue::VOBJ); dmn->ToJson(obj); - bool fEnabled = dmn->pdmnState->nPoSeBanHeight == -1; - if (filterMasternode(obj, strFilter, fEnabled)) { + if (filterMasternode(obj, strFilter, !dmn->IsPoSeBanned())) { ret.push_back(obj); } }); @@ -204,7 +203,7 @@ UniValue listmasternodes(const JSONRPCRequest& request) if (dmn) { UniValue obj(UniValue::VOBJ); dmn->ToJson(obj); - bool fEnabled = dmn->pdmnState->nPoSeBanHeight == -1; + bool fEnabled = !dmn->IsPoSeBanned(); if (filterMasternode(obj, strFilter, fEnabled)) { // Added for backward compatibility with legacy masternodes obj.pushKV("type", "deterministic"); diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 8f48eb19ca1f..3628664f7ae7 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -650,6 +650,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) // check that changing the operator key puts the MN in PoSe banned state BOOST_CHECK_MESSAGE(dmn->pdmnState->addr == CService(), "IP address not cleared after changing operator"); BOOST_CHECK_MESSAGE(dmn->pdmnState->scriptOperatorPayout.empty(), "operator payee not empty after changing operator"); + BOOST_CHECK(dmn->IsPoSeBanned()); BOOST_CHECK_EQUAL(dmn->pdmnState->nPoSeBanHeight, nHeight); // revive the MN @@ -663,6 +664,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) // check updated dmn state BOOST_CHECK_EQUAL(dmn->pdmnState->addr.GetPort(), 2000); BOOST_CHECK_EQUAL(dmn->pdmnState->nPoSeBanHeight, -1); + BOOST_CHECK(!dmn->IsPoSeBanned()); BOOST_CHECK_EQUAL(dmn->pdmnState->nPoSeRevivedHeight, nHeight); // Mine 32 blocks, checking MN reward payments @@ -780,6 +782,7 @@ BOOST_FIXTURE_TEST_CASE(dip3_protx, TestChain400Setup) BOOST_CHECK_MESSAGE(dmn->pdmnState->addr == CService(), "mn IP address not removed"); BOOST_CHECK_MESSAGE(dmn->pdmnState->scriptOperatorPayout.empty(), "mn operator payout not removed"); BOOST_CHECK_EQUAL(dmn->pdmnState->nRevocationReason, reason); + BOOST_CHECK(dmn->IsPoSeBanned()); BOOST_CHECK_EQUAL(dmn->pdmnState->nPoSeBanHeight, nHeight); } From 0f5064fcbfc956cb50b6f822b73833d6166e508e Mon Sep 17 00:00:00 2001 From: random-zebra Date: Fri, 24 Sep 2021 10:48:29 +0200 Subject: [PATCH 8/8] [Trivial] rename local variable in CMasternodeMan::CountEnabled --- src/masternodeman.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp index 33c7a8d9e89f..3f4054978490 100644 --- a/src/masternodeman.cpp +++ b/src/masternodeman.cpp @@ -417,7 +417,7 @@ CMasternodeMan::MNsInfo CMasternodeMan::getMNsInfo() const int CMasternodeMan::CountEnabled(bool only_legacy) const { - int i = 0; + int count_enabled = 0; int protocolVersion = ActiveProtocol(); { @@ -425,15 +425,15 @@ int CMasternodeMan::CountEnabled(bool only_legacy) const for (const auto& it : mapMasternodes) { const MasternodeRef& mn = it.second; if (mn->protocolVersion < protocolVersion || !mn->IsEnabled()) continue; - i++; + count_enabled++; } } if (!only_legacy && deterministicMNManager->IsDIP3Enforced()) { - i += deterministicMNManager->GetListAtChainTip().GetValidMNsCount(); + count_enabled += deterministicMNManager->GetListAtChainTip().GetValidMNsCount(); } - return i; + return count_enabled; } void CMasternodeMan::DsegUpdate(CNode* pnode)