From 41ab95dbf8baf7bc0ba4784acc5e89eeae5d9045 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 24 May 2024 00:52:32 +0300 Subject: [PATCH 1/4] feat: skip governance checks for blocks below the best chainlock --- src/masternode/payments.cpp | 7 +++++-- src/masternode/payments.h | 4 ++-- src/validation.cpp | 6 ++++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/masternode/payments.cpp b/src/masternode/payments.cpp index 05b907339d..7fd80aeed1 100644 --- a/src/masternode/payments.cpp +++ b/src/masternode/payments.cpp @@ -181,7 +181,7 @@ CAmount PlatformShare(const CAmount reward) * - Other blocks are 10% lower in outgoing value, so in total, no extra coins are created * - When non-superblocks are detected, the normal schedule should be maintained */ -bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlockHeight, const CAmount blockReward, std::string& strErrorRet) +bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlockHeight, const CAmount blockReward, std::string& strErrorRet, const bool check_superblock) { bool isBlockRewardValueMet = (block.vtx[0]->GetValueOut() <= blockReward); @@ -242,6 +242,8 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlo return isBlockRewardValueMet; } + if (!check_superblock) return true; + const auto tip_mn_list = m_dmnman.GetListAtChainTip(); if (!CSuperblockManager::IsSuperblockTriggered(m_govman, tip_mn_list, nBlockHeight)) { @@ -267,7 +269,7 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlo return true; } -bool CMNPaymentsProcessor::IsBlockPayeeValid(const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward) +bool CMNPaymentsProcessor::IsBlockPayeeValid(const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, const bool check_superblock) { const int nBlockHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; @@ -298,6 +300,7 @@ bool CMNPaymentsProcessor::IsBlockPayeeValid(const CTransaction& txNew, const CB // superblocks started if (AreSuperblocksEnabled(m_sporkman)) { + if (!check_superblock) return true; const auto tip_mn_list = m_dmnman.GetListAtChainTip(); if (CSuperblockManager::IsSuperblockTriggered(m_govman, tip_mn_list, nBlockHeight)) { if (CSuperblockManager::IsValid(m_govman, tip_mn_list, txNew, nBlockHeight, blockSubsidy + feeReward)) { diff --git a/src/masternode/payments.h b/src/masternode/payments.h index 14dc6fa083..16e6a34b50 100644 --- a/src/masternode/payments.h +++ b/src/masternode/payments.h @@ -52,8 +52,8 @@ public: const CMasternodeSync& mn_sync, const CSporkManager& sporkman) : m_dmnman{dmnman}, m_govman{govman}, m_consensus_params{consensus_params}, m_mn_sync{mn_sync}, m_sporkman{sporkman} {} - bool IsBlockValueValid(const CBlock& block, const int nBlockHeight, const CAmount blockReward, std::string& strErrorRet); - bool IsBlockPayeeValid(const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward); + bool IsBlockValueValid(const CBlock& block, const int nBlockHeight, const CAmount blockReward, std::string& strErrorRet, const bool check_superblock); + bool IsBlockPayeeValid(const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, const bool check_superblock); void FillBlockPayments(CMutableTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, std::vector& voutMasternodePaymentsRet, std::vector& voutSuperblockPaymentsRet); }; diff --git a/src/validation.cpp b/src/validation.cpp index a5faa07ef4..5f46e90447 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2445,7 +2445,9 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, int64_t nTime5_3 = GetTimeMicros(); nTimeCreditPool += nTime5_3 - nTime5_2; LogPrint(BCLog::BENCHMARK, " - CheckCreditPoolDiffForBlock: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_3 - nTime5_2), nTimeCreditPool * MICRO, nTimeCreditPool * MILLI / nBlocksTotal); - if (!m_chain_helper->mn_payments->IsBlockValueValid(block, pindex->nHeight, blockSubsidy + feeReward, strError)) { + bool check_superblock = m_clhandler->GetBestChainLock().getHeight() < pindex->nHeight; + + if (!m_chain_helper->mn_payments->IsBlockValueValid(block, pindex->nHeight, blockSubsidy + feeReward, strError, check_superblock)) { // NOTE: Do not punish, the node might be missing governance data LogPrintf("ERROR: ConnectBlock(DASH): %s\n", strError); return state.Invalid(BlockValidationResult::BLOCK_RESULT_UNSET, "bad-cb-amount"); @@ -2454,7 +2456,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, int64_t nTime5_4 = GetTimeMicros(); nTimeValueValid += nTime5_4 - nTime5_3; LogPrint(BCLog::BENCHMARK, " - IsBlockValueValid: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_4 - nTime5_3), nTimeValueValid * MICRO, nTimeValueValid * MILLI / nBlocksTotal); - if (!m_chain_helper->mn_payments->IsBlockPayeeValid(*block.vtx[0], pindex->pprev, blockSubsidy, feeReward)) { + if (!m_chain_helper->mn_payments->IsBlockPayeeValid(*block.vtx[0], pindex->pprev, blockSubsidy, feeReward, check_superblock)) { // NOTE: Do not punish, the node might be missing governance data LogPrintf("ERROR: ConnectBlock(DASH): couldn't find masternode or superblock payments\n"); return state.Invalid(BlockValidationResult::BLOCK_RESULT_UNSET, "bad-cb-payee"); From 3c3489d7a1d3ac67f40ba262af070d69e012d611 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 25 May 2024 23:07:38 +0300 Subject: [PATCH 2/4] test: add test --- test/functional/feature_governance_cl.py | 141 +++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 142 insertions(+) create mode 100755 test/functional/feature_governance_cl.py diff --git a/test/functional/feature_governance_cl.py b/test/functional/feature_governance_cl.py new file mode 100755 index 0000000000..114794c370 --- /dev/null +++ b/test/functional/feature_governance_cl.py @@ -0,0 +1,141 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018-2024 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Tests governance checks can be skipped for blocks covered by the best chainlock.""" + +import json + +from test_framework.messages import uint256_to_string +from test_framework.test_framework import DashTestFramework +from test_framework.util import assert_equal, satoshi_round + +class DashGovernanceTest (DashTestFramework): + def set_test_params(self): + self.set_dash_test_params(6, 5, [["-budgetparams=10:10:10"]] * 6, fast_dip3_enforcement=True) + + def prepare_object(self, object_type, parent_hash, creation_time, revision, name, amount, payment_address): + proposal_rev = revision + proposal_time = int(creation_time) + proposal_template = { + "type": object_type, + "name": name, + "start_epoch": proposal_time, + "end_epoch": proposal_time + 24 * 60 * 60, + "payment_amount": float(amount), + "payment_address": payment_address, + "url": "https://dash.org" + } + proposal_hex = ''.join(format(x, '02x') for x in json.dumps(proposal_template).encode()) + collateral_hash = self.nodes[0].gobject("prepare", parent_hash, proposal_rev, proposal_time, proposal_hex) + return { + "parentHash": parent_hash, + "collateralHash": collateral_hash, + "createdAt": proposal_time, + "revision": proposal_rev, + "hex": proposal_hex, + "data": proposal_template, + } + + def have_trigger_for_height(self, sb_block_height, nodes = None): + if nodes is None: + nodes = self.nodes + count = 0 + for node in nodes: + valid_triggers = node.gobject("list", "valid", "triggers") + for trigger in list(valid_triggers.values()): + if json.loads(trigger["DataString"])["event_block_height"] != sb_block_height: + continue + if trigger['AbsoluteYesCount'] > 0: + count = count + 1 + break + return count == len(nodes) + + def run_test(self): + sb_cycle = 20 + + self.log.info("Make sure ChainLocks are active") + + self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) + self.wait_for_sporks_same() + self.activate_v19(expected_activation_height=900) + self.log.info("Activated v19 at height:" + str(self.nodes[0].getblockcount())) + self.move_to_next_cycle() + self.log.info("Cycle H height:" + str(self.nodes[0].getblockcount())) + self.move_to_next_cycle() + self.log.info("Cycle H+C height:" + str(self.nodes[0].getblockcount())) + self.move_to_next_cycle() + self.log.info("Cycle H+2C height:" + str(self.nodes[0].getblockcount())) + + self.mine_cycle_quorum(llmq_type_name='llmq_test_dip0024', llmq_type=103) + + self.sync_blocks() + self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash()) + + self.nodes[0].sporkupdate("SPORK_9_SUPERBLOCKS_ENABLED", 0) + self.wait_for_sporks_same() + + self.log.info("Prepare and submit proposals") + + proposal_time = self.mocktime + self.p0_payout_address = self.nodes[0].getnewaddress() + self.p1_payout_address = self.nodes[0].getnewaddress() + self.p0_amount = satoshi_round("1.1") + self.p1_amount = satoshi_round("3.3") + + p0_collateral_prepare = self.prepare_object(1, uint256_to_string(0), proposal_time, 1, "Proposal_0", self.p0_amount, self.p0_payout_address) + p1_collateral_prepare = self.prepare_object(1, uint256_to_string(0), proposal_time, 1, "Proposal_1", self.p1_amount, self.p1_payout_address) + self.bump_mocktime(60 * 10 + 1) + + self.nodes[0].generate(6) + self.bump_mocktime(6 * 156) + self.sync_blocks() + + assert_equal(len(self.nodes[0].gobject("list-prepared")), 2) + assert_equal(len(self.nodes[0].gobject("list")), 0) + + self.p0_hash = self.nodes[0].gobject("submit", "0", 1, proposal_time, p0_collateral_prepare["hex"], p0_collateral_prepare["collateralHash"]) + self.p1_hash = self.nodes[0].gobject("submit", "0", 1, proposal_time, p1_collateral_prepare["hex"], p1_collateral_prepare["collateralHash"]) + + assert_equal(len(self.nodes[0].gobject("list")), 2) + + self.log.info("Isolate a node so that it would miss votes and triggers") + # Isolate a node + self.isolate_node(5) + + self.log.info("Vote proposals") + + self.nodes[0].gobject("vote-many", self.p0_hash, "funding", "yes") + self.nodes[0].gobject("vote-many", self.p1_hash, "funding", "yes") + assert_equal(self.nodes[0].gobject("get", self.p0_hash)["FundingResult"]["YesCount"], self.mn_count) + assert_equal(self.nodes[0].gobject("get", self.p1_hash)["FundingResult"]["YesCount"], self.mn_count) + + assert_equal(len(self.nodes[0].gobject("list", "valid", "triggers")), 0) + + n = sb_cycle - self.nodes[0].getblockcount() % sb_cycle + assert n > 1 + + # Move remaining n blocks until the next Superblock + for _ in range(n - 1): + self.nodes[0].generate(1) + self.bump_mocktime(156) + self.sync_blocks(self.nodes[0:5]) + + self.log.info("Wait for new trigger and votes on non-isolated nodes") + sb_block_height = self.nodes[0].getblockcount() + 1 + self.wait_until(lambda: self.have_trigger_for_height(sb_block_height, self.nodes[0:5]), timeout=5) + # Mine superblock + self.nodes[0].generate(1) + self.bump_mocktime(156) + self.sync_blocks(self.nodes[0:5]) + self.wait_for_chainlocked_block(self.nodes[0], self.nodes[0].getbestblockhash()) + + self.log.info("Reconnect isolated node and confirm the next ChainLock will let it sync") + self.reconnect_isolated_node(5, 0) + self.nodes[0].generate(1) + self.bump_mocktime(156) + self.sync_blocks() + + +if __name__ == '__main__': + DashGovernanceTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2605b5341b..06a56b65ab 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -280,6 +280,7 @@ BASE_SCRIPTS = [ 'feature_new_quorum_type_activation.py', 'feature_governance_objects.py', 'feature_governance.py --legacy-wallet', + 'feature_governance_cl.py --legacy-wallet', 'rpc_uptime.py', 'wallet_resendwallettransactions.py --legacy-wallet', 'wallet_resendwallettransactions.py --descriptors', From 08331bb95019ccf0eb6f9341c3fe773d9f429d39 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 4 Jun 2024 18:08:35 +0300 Subject: [PATCH 3/4] fix: apply suggestions --- src/validation.cpp | 2 +- test/functional/feature_governance_cl.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 5f46e90447..32d6487d27 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2445,7 +2445,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, int64_t nTime5_3 = GetTimeMicros(); nTimeCreditPool += nTime5_3 - nTime5_2; LogPrint(BCLog::BENCHMARK, " - CheckCreditPoolDiffForBlock: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_3 - nTime5_2), nTimeCreditPool * MICRO, nTimeCreditPool * MILLI / nBlocksTotal); - bool check_superblock = m_clhandler->GetBestChainLock().getHeight() < pindex->nHeight; + const bool check_superblock = m_clhandler->GetBestChainLock().getHeight() < pindex->nHeight; if (!m_chain_helper->mn_payments->IsBlockValueValid(block, pindex->nHeight, blockSubsidy + feeReward, strError, check_superblock)) { // NOTE: Do not punish, the node might be missing governance data diff --git a/test/functional/feature_governance_cl.py b/test/functional/feature_governance_cl.py index 114794c370..5950bc86b4 100755 --- a/test/functional/feature_governance_cl.py +++ b/test/functional/feature_governance_cl.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2018-2024 The Dash Core developers +# Copyright (c) 2024 The Dash Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Tests governance checks can be skipped for blocks covered by the best chainlock.""" From 18328279ec20b8090a13f5948d1a6ed839c1dae3 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 4 Jun 2024 18:16:18 +0300 Subject: [PATCH 4/4] fix: force mnsync to skip gov obj sync on reconnection --- test/functional/feature_governance_cl.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/functional/feature_governance_cl.py b/test/functional/feature_governance_cl.py index 5950bc86b4..2e1f9c31e0 100755 --- a/test/functional/feature_governance_cl.py +++ b/test/functional/feature_governance_cl.py @@ -8,7 +8,7 @@ import json from test_framework.messages import uint256_to_string from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_equal, satoshi_round +from test_framework.util import assert_equal, force_finish_mnsync, satoshi_round class DashGovernanceTest (DashTestFramework): def set_test_params(self): @@ -132,6 +132,9 @@ class DashGovernanceTest (DashTestFramework): self.log.info("Reconnect isolated node and confirm the next ChainLock will let it sync") self.reconnect_isolated_node(5, 0) + # Force isolated node to be fully synced so that it would not request gov objects when reconnected + assert_equal(self.nodes[5].mnsync("status")["IsSynced"], False) + force_finish_mnsync(self.nodes[5]) self.nodes[0].generate(1) self.bump_mocktime(156) self.sync_blocks()