From 2943376da4c9885af8cc69260860b68d75e31131 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 19 Sep 2026 04:36:04 +0700 Subject: [PATCH 01/28] refactor: drop unused active_chain from superblock validation Removed unused parameter cs_main and related code with cs_main --- src/governance/signing.cpp | 2 +- src/governance/superblock.cpp | 12 ++++++------ src/governance/superblock.h | 9 ++++----- src/masternode/payments.cpp | 10 +++++----- src/masternode/payments.h | 5 ++--- src/node/interfaces.cpp | 4 ++-- src/rpc/governance.cpp | 5 ++--- src/test/governance_superblock_tests.cpp | 8 +++----- src/test/masternode_payments_tests.cpp | 6 ++---- src/validation.cpp | 4 ++-- 10 files changed, 29 insertions(+), 36 deletions(-) diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 1eea9b60dac6..da2b5752085e 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -57,7 +57,7 @@ std::optional GovernanceSigner::CreateSuperblockCandidate(int CSuperblock::GetNearestSuperblocksHeights(nHeight, nLastSuperblock, nNextSuperblock); auto SBEpochTime = static_cast(GetTime().count() + (nNextSuperblock - nHeight) * 2.62 * 60); - auto governanceBudget = WITH_LOCK(::cs_main, return CSuperblock::GetPaymentsLimit(m_chainman.ActiveChain(), nNextSuperblock)); + auto governanceBudget = CSuperblock::GetPaymentsLimit(nNextSuperblock); CAmount budgetAllocated{}; for (const auto& proposal : approvedProposals) { diff --git a/src/governance/superblock.cpp b/src/governance/superblock.cpp index 54213f62579b..bb22f1e84dc9 100644 --- a/src/governance/superblock.cpp +++ b/src/governance/superblock.cpp @@ -142,7 +142,7 @@ void CSuperblock::GetNearestSuperblocksHeights(int nBlockHeight, int& nLastSuper } } -CAmount CSuperblock::GetPaymentsLimit(const CChain& active_chain, int nBlockHeight) +CAmount CSuperblock::GetPaymentsLimit(int nBlockHeight) { const Consensus::Params& consensusParams = Params().GetConsensus(); @@ -245,7 +245,7 @@ CAmount CSuperblock::GetPaymentsTotalAmount() * - Does this transaction match the superblock? */ -bool CSuperblock::IsValid(const CChain& active_chain, const CTransaction& txNew, int block_height, CAmount blockReward, bool is_v24) +bool CSuperblock::IsValid(const CTransaction& txNew, int block_height, CAmount blockReward, bool is_v24) { // TODO : LOCK(cs); // No reason for a lock here now since this method only accesses data @@ -279,7 +279,7 @@ bool CSuperblock::IsValid(const CChain& active_chain, const CTransaction& txNew, // payments should not exceed limit CAmount nPaymentsTotalAmount = GetPaymentsTotalAmount(); - CAmount nPaymentsLimit = GetPaymentsLimit(active_chain, block_height); + CAmount nPaymentsLimit = GetPaymentsLimit(block_height); if (nPaymentsTotalAmount > nPaymentsLimit) { LogPrintf("CSuperblock::IsValid -- ERROR: Block invalid, payments limit exceeded: payments %lld, limit %lld\n", nPaymentsTotalAmount, nPaymentsLimit); return false; @@ -619,13 +619,13 @@ bool SuperblockManager::IsSuperblockTriggered(const CDeterministicMNList& tip_mn return false; } -bool SuperblockManager::IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, - const CTransaction& txNew, int nBlockHeight, CAmount blockReward, bool is_v24) const +bool SuperblockManager::IsValidSuperblock(const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, + int nBlockHeight, CAmount blockReward, bool is_v24) const { LOCK(cs_sb); CSuperblock_sptr pSuperblock; if (GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) { - return pSuperblock->IsValid(active_chain, txNew, nBlockHeight, blockReward, is_v24); + return pSuperblock->IsValid(txNew, nBlockHeight, blockReward, is_v24); } return false; } diff --git a/src/governance/superblock.h b/src/governance/superblock.h index 9fb43dc7ce5a..fdb9bfa07763 100644 --- a/src/governance/superblock.h +++ b/src/governance/superblock.h @@ -18,7 +18,6 @@ #include #include -class CChain; class CDeterministicMNList; class CTransaction; class CTxOut; @@ -86,7 +85,7 @@ class CSuperblock : public CGovernanceObject static bool IsValidBlockHeight(int nBlockHeight); static void GetNearestSuperblocksHeights(int nBlockHeight, int& nLastSuperblockRet, int& nNextSuperblockRet); - static CAmount GetPaymentsLimit(const CChain& active_chain, int nBlockHeight); + static CAmount GetPaymentsLimit(int nBlockHeight); SeenObjectStatus GetStatus() const { return nStatus; } void SetStatus(SeenObjectStatus nStatusIn) { nStatus = nStatusIn; } @@ -107,7 +106,7 @@ class CSuperblock : public CGovernanceObject bool GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet); CAmount GetPaymentsTotalAmount(); - bool IsValid(const CChain& active_chain, const CTransaction& txNew, int block_height, CAmount blockReward, bool is_v24); + bool IsValid(const CTransaction& txNew, int block_height, CAmount blockReward, bool is_v24); bool IsExpired(int heightToTest) const; std::vector GetProposalHashes() const; @@ -157,8 +156,8 @@ class SuperblockManager bool IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(!cs_sb); - bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, - int nBlockHeight, CAmount blockReward, bool is_v24) const EXCLUSIVE_LOCKS_REQUIRED(!cs_sb); + bool IsValidSuperblock(const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, + CAmount blockReward, bool is_v24) const EXCLUSIVE_LOCKS_REQUIRED(!cs_sb); bool GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, std::vector& voutSuperblockRet) const EXCLUSIVE_LOCKS_REQUIRED(!cs_sb); diff --git a/src/masternode/payments.cpp b/src/masternode/payments.cpp index b2eebcf2aea2..7d0ba8c4d360 100644 --- a/src/masternode/payments.cpp +++ b/src/masternode/payments.cpp @@ -310,7 +310,7 @@ CAmount GetMasternodePayment(int nHeight, CAmount blockValue, const Consensus::P * - 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 CChain& active_chain, const CBlock& block, const CBlockIndex* pindexPrev, const CAmount blockReward, std::string& strErrorRet, SuperBlockCheckType check_superblock) +bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const CBlockIndex* pindexPrev, const CAmount blockReward, std::string& strErrorRet, SuperBlockCheckType check_superblock) { const int nBlockHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; bool isBlockRewardValueMet = (block.vtx[0]->GetValueOut() <= blockReward); @@ -331,7 +331,7 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CChain& active_chain, const C LogPrint(BCLog::MNPAYMENTS, "block.vtx[0]->GetValueOut() %lld <= blockReward %lld\n", block.vtx[0]->GetValueOut(), blockReward); - CAmount nSuperblockMaxValue = blockReward + CSuperblock::GetPaymentsLimit(active_chain, nBlockHeight); + CAmount nSuperblockMaxValue = blockReward + CSuperblock::GetPaymentsLimit(nBlockHeight); bool isSuperblockMaxValueMet = (block.vtx[0]->GetValueOut() <= nSuperblockMaxValue); LogPrint(BCLog::GOBJECT, "block.vtx[0]->GetValueOut() %lld <= nSuperblockMaxValue %lld\n", block.vtx[0]->GetValueOut(), nSuperblockMaxValue); @@ -377,7 +377,7 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CChain& active_chain, const C // this actually also checks for correct payees and not only amount const bool is_v24{check_superblock == SuperBlockCheckType::DisallowDuplicates}; - if (!m_superblocks.IsValidSuperblock(active_chain, tip_mn_list, *block.vtx[0], nBlockHeight, blockReward, is_v24)) { + if (!m_superblocks.IsValidSuperblock(tip_mn_list, *block.vtx[0], nBlockHeight, blockReward, is_v24)) { // triggered but invalid? that's weird LogPrintf("CMNPaymentsProcessor::%s -- ERROR! Invalid superblock detected at height %d: %s", __func__, nBlockHeight, block.vtx[0]->ToString()); /* Continued */ // should NOT allow invalid superblocks, when superblocks are enabled @@ -389,7 +389,7 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CChain& active_chain, const C return true; } -bool CMNPaymentsProcessor::IsBlockPayeeValid(const CChain& active_chain, const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, MnRewardEra era, bool strict_multiplicity, SuperBlockCheckType check_superblock) +bool CMNPaymentsProcessor::IsBlockPayeeValid(const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, MnRewardEra era, bool strict_multiplicity, SuperBlockCheckType check_superblock) { const int nBlockHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; @@ -423,7 +423,7 @@ bool CMNPaymentsProcessor::IsBlockPayeeValid(const CChain& active_chain, const C const auto tip_mn_list = m_dmnman.GetListAtChainTip(); const bool is_v24{check_superblock == SuperBlockCheckType::DisallowDuplicates}; if (m_superblocks.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) { - if (m_superblocks.IsValidSuperblock(active_chain, tip_mn_list, txNew, nBlockHeight, + if (m_superblocks.IsValidSuperblock(tip_mn_list, txNew, nBlockHeight, blockSubsidy + feeReward, is_v24)) { LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Valid superblock at height %d: %s", /* Continued */ __func__, nBlockHeight, txNew.ToString()); diff --git a/src/masternode/payments.h b/src/masternode/payments.h index a24cc4808f07..f99db57baf93 100644 --- a/src/masternode/payments.h +++ b/src/masternode/payments.h @@ -12,7 +12,6 @@ class CBlock; class CBlockIndex; -class CChain; class CDeterministicMNManager; class CTransaction; class CTxOut; @@ -96,8 +95,8 @@ class CMNPaymentsProcessor { } - bool IsBlockValueValid(const CChain& active_chain, const CBlock& block, const CBlockIndex* pindexPrev, const CAmount blockReward, std::string& strErrorRet, SuperBlockCheckType check_superblock); - bool IsBlockPayeeValid(const CChain& active_chain, const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, MnRewardEra era, bool strict_multiplicity, SuperBlockCheckType check_superblock); + bool IsBlockValueValid(const CBlock& block, const CBlockIndex* pindexPrev, const CAmount blockReward, std::string& strErrorRet, SuperBlockCheckType check_superblock); + bool IsBlockPayeeValid(const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, MnRewardEra era, bool strict_multiplicity, SuperBlockCheckType check_superblock); void FillBlockPayments(CMutableTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, MnRewardEra era, std::vector& voutMasternodePaymentsRet, std::vector& voutSuperblockPaymentsRet); }; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index fce39a312367..019a212715c4 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -430,7 +430,7 @@ class GOVImpl : public GOV const Consensus::Params& consensusParams = context().chainman->GetConsensus(); LOCK(::cs_main); CSuperblock::GetNearestSuperblocksHeights(context().chainman->ActiveHeight(), info.lastsuperblock, info.nextsuperblock); - info.governancebudget = CSuperblock::GetPaymentsLimit(context().chainman->ActiveChain(), info.nextsuperblock); + info.governancebudget = CSuperblock::GetPaymentsLimit(info.nextsuperblock); if (context().dmnman) { info.fundingthreshold = static_cast(context().dmnman->GetListAtChainTip().GetCounts().m_valid_weighted / 10); } @@ -469,7 +469,7 @@ class GOVImpl : public GOV { LOCK(::cs_main); CSuperblock::GetNearestSuperblocksHeights(context().chainman->ActiveHeight(), last_sb, next_sb); - budget = CSuperblock::GetPaymentsLimit(context().chainman->ActiveChain(), next_sb); + budget = CSuperblock::GetPaymentsLimit(next_sb); } for (const auto& proposal : proposals) { UniValue json = proposal->GetJSONObject(); diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 7f3ed99c2010..17145823471b 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -983,7 +983,7 @@ static RPCHelpMan getgovernanceinfo() obj.pushKV("lastsuperblock", nLastSuperblock); obj.pushKV("nextsuperblock", nNextSuperblock); obj.pushKV("fundingthreshold", CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetCounts().m_valid_weighted / 10); - obj.pushKV("governancebudget", ValueFromAmount(WITH_LOCK(::cs_main, return CSuperblock::GetPaymentsLimit(chainman.ActiveChain(), nNextSuperblock)))); + obj.pushKV("governancebudget", ValueFromAmount(CSuperblock::GetPaymentsLimit(nNextSuperblock))); return obj; }, @@ -1011,8 +1011,7 @@ static RPCHelpMan getsuperblockbudget() throw JSONRPCError(RPC_INVALID_PARAMETER, "Block height out of range"); } - const ChainstateManager& chainman = EnsureAnyChainman(request.context); - return ValueFromAmount(WITH_LOCK(::cs_main, return CSuperblock::GetPaymentsLimit(chainman.ActiveChain(), nBlockHeight))); + return ValueFromAmount(CSuperblock::GetPaymentsLimit(nBlockHeight)); }, }; } diff --git a/src/test/governance_superblock_tests.cpp b/src/test/governance_superblock_tests.cpp index 759bc6e9485c..460964a5e863 100644 --- a/src/test/governance_superblock_tests.cpp +++ b/src/test/governance_superblock_tests.cpp @@ -2,7 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include #include #include #include @@ -54,7 +53,6 @@ BOOST_AUTO_TEST_CASE(isvalid_duplicate_payments_require_distinct_outputs) const CScript scriptMinerOrMN = CScript() << OP_RETURN; const CAmount blockReward = 500 * COIN; - CChain dummy_chain; // Case 1 (regression, V24): coinbase carries only ONE output matching the // duplicate expected payment. With the buggy forward scan that restarted @@ -66,7 +64,7 @@ BOOST_AUTO_TEST_CASE(isvalid_duplicate_payments_require_distinct_outputs) CMutableTransaction txNew; txNew.vout.emplace_back(blockReward - nPayAmount, scriptMinerOrMN); txNew.vout.emplace_back(nPayAmount, scriptPayee); // single matching output - BOOST_CHECK(!sb.IsValid(dummy_chain, CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/true)); + BOOST_CHECK(!sb.IsValid(CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/true)); } // Case 2 (V24): coinbase carries TWO outputs matching the duplicate expected @@ -76,7 +74,7 @@ BOOST_AUTO_TEST_CASE(isvalid_duplicate_payments_require_distinct_outputs) txNew.vout.emplace_back(blockReward - 2 * nPayAmount, scriptMinerOrMN); txNew.vout.emplace_back(nPayAmount, scriptPayee); txNew.vout.emplace_back(nPayAmount, scriptPayee); - BOOST_CHECK(sb.IsValid(dummy_chain, CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/true)); + BOOST_CHECK(sb.IsValid(CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/true)); } // Case 3 (pre-V24): the stricter distinct-output rule is gated behind V24. @@ -87,7 +85,7 @@ BOOST_AUTO_TEST_CASE(isvalid_duplicate_payments_require_distinct_outputs) CMutableTransaction txNew; txNew.vout.emplace_back(blockReward - nPayAmount, scriptMinerOrMN); txNew.vout.emplace_back(nPayAmount, scriptPayee); // single matching output - BOOST_CHECK(sb.IsValid(dummy_chain, CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/false)); + BOOST_CHECK(sb.IsValid(CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/false)); } } diff --git a/src/test/masternode_payments_tests.cpp b/src/test/masternode_payments_tests.cpp index cfaaa983a74c..e356dee23e5c 100644 --- a/src/test/masternode_payments_tests.cpp +++ b/src/test/masternode_payments_tests.cpp @@ -116,17 +116,15 @@ BOOST_FIXTURE_TEST_CASE(old_budget_window_accepted_while_unsynced, TestingSetup) block.vtx.push_back(MakeTransactionRef(coinbase)); auto& mn_payments{*Assert(m_node.chain_helper)->mn_payments}; - LOCK(cs_main); - const CChain& active_chain{m_node.chainman->ActiveChain()}; std::string strError; - BOOST_CHECK(mn_payments.IsBlockValueValid(active_chain, block, &pindexPrev, blockReward, strError, + BOOST_CHECK(mn_payments.IsBlockValueValid(block, &pindexPrev, blockReward, strError, SuperBlockCheckType::NoCheck)); BOOST_CHECK(strError.empty()); // The enforcing branch is only reached by a node that is synced and has no // chainlock at that height, and it still rejects the over-reward block. - BOOST_CHECK(!mn_payments.IsBlockValueValid(active_chain, block, &pindexPrev, blockReward, strError, + BOOST_CHECK(!mn_payments.IsBlockValueValid(block, &pindexPrev, blockReward, strError, SuperBlockCheckType::AllowDuplicates)); BOOST_CHECK_EQUAL(strError, "coinbase pays too much at height 332320 (actual=118108031847 vs limit=508031847), " "exceeded block reward, old budgets are disabled"); diff --git a/src/validation.cpp b/src/validation.cpp index 3e20f8617c8b..ce807bc69425 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2843,7 +2843,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, : special_tx_rules.v24 ? SuperBlockCheckType::DisallowDuplicates : SuperBlockCheckType::AllowDuplicates; - if (!m_chain_helper->mn_payments->IsBlockValueValid(m_chain, block, pindex->pprev, blockSubsidy + feeReward, strError, check_superblock)) { + if (!m_chain_helper->mn_payments->IsBlockValueValid(block, pindex->pprev, 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"); @@ -2857,7 +2857,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, Ticks(time_value_valid) / num_blocks_total); const MnRewardEra mn_reward_era{GetMnRewardEraAfter(pindex->pprev, m_chainman)}; - if (!m_chain_helper->mn_payments->IsBlockPayeeValid(m_chain, *block.vtx[0], pindex->pprev, blockSubsidy, feeReward, + if (!m_chain_helper->mn_payments->IsBlockPayeeValid(*block.vtx[0], pindex->pprev, blockSubsidy, feeReward, mn_reward_era, special_tx_rules.v24, 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"); From f7e381603ff4fe817c80d8fc0c630ebb9e6062f4 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 19 Sep 2026 04:42:23 +0700 Subject: [PATCH 02/28] refactor: build LLMQContext and CChainstateHelper in init, not LoadChainstate It helps to remove multiple members of node::ChainstateLoadOptions: - const CMasternodeSync* mn_sync - int8_t bls_threads - int16_t worker_count - int64_t max_recsigs_age Helpers DashChainstateSetup and DashChainstateSetupClose are replaced by MakeDashChainContexts to reduce code duplication Reference to unique_ptr for llmq_ctx is also disappeared --- src/bitcoin-chainstate.cpp | 13 ++++-- src/init.cpp | 18 ++++++-- src/node/chainstate.cpp | 29 +++--------- src/node/chainstate.h | 13 +----- src/test/util/setup_common.cpp | 26 +++++++++-- src/test/util/setup_common.h | 5 +++ .../validation_chainstatemanager_tests.cpp | 44 +++++-------------- 7 files changed, 68 insertions(+), 80 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 204c4701f08d..4ad43d1e3b23 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -104,8 +105,13 @@ int main(int argc, char* argv[]) // TODO: remove isman from bitcoin-chainstate and make it nullable for node::ChainstateLoadOptions same as mempool llmq::CInstantSendManager isman{sporkman, util::DbWrapperParams{.path = gArgs.GetDataDirNet(), .memory = false, .wipe = false}}; - std::unique_ptr llmq_ctx; - std::unique_ptr chain_helper; + auto llmq_ctx = WITH_LOCK(::cs_main, return std::make_unique(dmnman, evodb, chainman, + util::DbWrapperParams{.path = gArgs.GetDataDirNet(), .memory = false, .wipe = false}, + llmq::DEFAULT_BLSCHECK_THREADS, llmq::DEFAULT_WORKER_COUNT, llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE)); + auto chain_helper = std::make_unique(evodb, dmnman, mn_sync, isman, + *llmq_ctx->quorum_block_processor, *llmq_ctx->qsnapman, + chainman, chainman.m_blockman, chainman.GetConsensus(), + chainlocks, *llmq_ctx->qman); node::CacheSizes cache_sizes; cache_sizes.block_tree_db = 2 << 20; @@ -114,11 +120,10 @@ int main(int argc, char* argv[]) node::ChainstateLoadOptions options; options.isman = &isman; options.chainlocks = &chainlocks; - options.mn_sync = &mn_sync; options.data_dir = gArgs.GetDataDirNet(); options.check_interrupt = [] { return false; }; options.coins_error_cb = [] {}; - auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options, evodb, dmnman, llmq_ctx, chain_helper); + auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options, evodb, dmnman, chain_helper); if (status != node::ChainstateLoadStatus::SUCCESS) { std::cerr << "Failed to load Chain state from your datadir." << std::endl; goto epilogue; diff --git a/src/init.cpp b/src/init.cpp index aa4d77b639ed..5bbbcd0fb127 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2026,6 +2026,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // On a retry iteration the previous instances still hold the on-disk // database locks, so release them before opening the databases again. node.mempool.reset(); + node.chain_helper.reset(); + node.llmq_ctx.reset(); node.isman.reset(); node.dmnman.reset(); node.evodb.reset(); @@ -2063,12 +2065,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node::ChainstateLoadOptions options; options.chainlocks = Assert(node.chainlocks.get()); - options.mn_sync = Assert(node.mn_sync.get()); options.data_dir = args.GetDataDirNet(); options.reindex = node::fReindex; options.reindex_chainstate = fReindexChainState; options.prune = chainman.m_blockman.IsPruneMode(); - options.bls_threads = [&args]() -> int8_t { + const int8_t bls_threads = [&args]() -> int8_t { int64_t threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS); if (threads <= 0) { // -parbls=0 means autodetect (number of cores - 1 validator threads) @@ -2079,7 +2080,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const int64_t adjusted_threads = std::clamp(threads, 1, int64_t{llmq::MAX_BLSCHECK_THREADS} + 1) - 1; return static_cast(adjusted_threads); }(); - options.max_recsigs_age = args.GetIntArg("-maxrecsigsage", llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE); + const int64_t max_recsigs_age = args.GetIntArg("-maxrecsigsage", llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE); options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS); options.check_level = args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL); options.require_full_verification = args.IsArgSet("-checkblocks") || args.IsArgSet("-checklevel"); @@ -2095,7 +2096,16 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) if (status == node::ChainstateLoadStatus::SUCCESS) { options.mempool = Assert(node.mempool.get()); options.isman = Assert(node.isman.get()); - std::tie(status, error) = catch_exceptions([&]{ return LoadChainstate(chainman, cache_sizes, options, *node.evodb, *node.dmnman, node.llmq_ctx, node.chain_helper); }); + std::tie(status, error) = catch_exceptions([&]() -> node::ChainstateLoadResult { + WITH_LOCK(::cs_main, node.llmq_ctx = std::make_unique(*node.dmnman, *node.evodb, chainman, + util::DbWrapperParams{.path = args.GetDataDirNet(), .memory = false, .wipe = node::fReindex || fReindexChainState}, + bls_threads, llmq::DEFAULT_WORKER_COUNT, max_recsigs_age)); + node.chain_helper = std::make_unique(*node.evodb, *node.dmnman, *node.mn_sync, *node.isman, + *node.llmq_ctx->quorum_block_processor, *node.llmq_ctx->qsnapman, + chainman, chainman.m_blockman, chainman.GetConsensus(), + *node.chainlocks, *node.llmq_ctx->qman); + return LoadChainstate(chainman, cache_sizes, options, *node.evodb, *node.dmnman, node.chain_helper); + }); } if (status == node::ChainstateLoadStatus::SUCCESS) { uiInterface.InitMessage(_("Verifying blocks…").translated); diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 35c676617b1c..9f1acdec4a2a 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include #include @@ -142,30 +141,15 @@ static bool RecoverSnapshotCleanup(CEvoDB& evodb, const fs::path& data_dir, bili static ChainstateLoadResult CompleteChainstateInitialization(ChainstateManager& chainman, const CacheSizes& cache_sizes, const ChainstateLoadOptions& options, CEvoDB& evodb, CDeterministicMNManager& dmnman, - std::unique_ptr& llmq_ctx, - std::unique_ptr& chain_helper) + const std::unique_ptr& chain_helper) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { - const bool to_wipe_data = options.reindex || options.reindex_chainstate; - auto& pblocktree{chainman.m_blockman.m_block_tree_db}; // new CBlockTreeDB tries to delete the existing file, which // fails if it's still open from the previous loop. Close it first: pblocktree.reset(); pblocktree.reset(new CBlockTreeDB(cache_sizes.block_tree_db, options.block_tree_db_in_memory, options.reindex)); - // Initialize llmq_ctx - llmq_ctx.reset(); - llmq_ctx = std::make_unique(dmnman, evodb, chainman, - util::DbWrapperParams{.path = options.data_dir, .memory = options.dash_dbs_in_memory, .wipe = to_wipe_data}, - options.bls_threads, options.worker_count, options.max_recsigs_age); - - // Initialize chain_helper - chain_helper.reset(); - chain_helper = std::make_unique(evodb, dmnman, *options.mn_sync, *options.isman, *(llmq_ctx->quorum_block_processor), - *(llmq_ctx->qsnapman), chainman, chainman.m_blockman, chainman.GetConsensus(), - *options.chainlocks, *(llmq_ctx->qman)); - if (options.reindex) { pblocktree->WriteReindexing(true); //If we're reindexing in prune mode, wipe away unusable block files and all undo data files @@ -297,12 +281,11 @@ static ChainstateLoadResult CompleteChainstateInitialization(ChainstateManager& ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes, const ChainstateLoadOptions& options, CEvoDB& evodb, - CDeterministicMNManager& dmnman, std::unique_ptr& llmq_ctx, - std::unique_ptr& chain_helper) + CDeterministicMNManager& dmnman, const std::unique_ptr& chain_helper) { assert(options.isman); assert(options.chainlocks); - assert(options.mn_sync); + assert(chain_helper); if (!chainman.AssumedValidBlock().IsNull()) { LogPrintf("Assuming ancestors of block %s have valid signatures.\n", chainman.AssumedValidBlock().GetHex()); @@ -350,7 +333,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize } auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options, evodb, dmnman, - llmq_ctx, chain_helper); + chain_helper); if (init_status != ChainstateLoadStatus::SUCCESS) { return {init_status, init_error}; } @@ -369,8 +352,6 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize // Do nothing; expected case. } else if (snapshot_completion == SnapshotCompletionResult::SUCCESS) { LogPrintf("[snapshot] cleaning up unneeded background chainstate, then reinitializing\n"); - chain_helper.reset(); - llmq_ctx.reset(); if (!chainman.ValidatedSnapshotCleanup()) { return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Background chainstate cleanup failed unexpectedly.")}; } @@ -389,7 +370,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize chainman.ActiveChainstate().ClearBlockIndexCandidates(); std::tie(init_status, init_error) = CompleteChainstateInitialization(chainman, cache_sizes, options, evodb, - dmnman, llmq_ctx, chain_helper); + dmnman, chain_helper); if (init_status != ChainstateLoadStatus::SUCCESS) { return {init_status, init_error}; } diff --git a/src/node/chainstate.h b/src/node/chainstate.h index 82909aca6237..c0d9e0623eae 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -5,7 +5,6 @@ #ifndef BITCOIN_NODE_CHAINSTATE_H #define BITCOIN_NODE_CHAINSTATE_H -#include #include #include @@ -20,9 +19,7 @@ class CChainstateHelper; class CDeterministicMNManager; class CEvoDB; class ChainstateManager; -class CMasternodeSync; class CTxMemPool; -struct LLMQContext; namespace chainlock { class Chainlocks; } namespace llmq { class CInstantSendManager; } @@ -35,7 +32,6 @@ struct ChainstateLoadOptions { CTxMemPool* mempool{nullptr}; llmq::CInstantSendManager* isman{nullptr}; chainlock::Chainlocks* chainlocks{nullptr}; - const CMasternodeSync* mn_sync{nullptr}; fs::path data_dir; bool block_tree_db_in_memory{false}; @@ -44,9 +40,6 @@ struct ChainstateLoadOptions { bool reindex{false}; bool reindex_chainstate{false}; bool prune{false}; - int8_t bls_threads{llmq::DEFAULT_BLSCHECK_THREADS}; - int16_t worker_count{llmq::DEFAULT_WORKER_COUNT}; - int64_t max_recsigs_age{llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE}; //! Setting require_full_verification to true will require all checks at //! check_level (below) to succeed for loading to succeed. Setting it to //! false will skip checks if cache is not big enough to run them, so may be @@ -86,14 +79,10 @@ using ChainstateLoadResult = std::tuple; * - a failure that definitively cannot be recovered from with a reindex * * LoadChainstate returns a (status code, error string) tuple. - * - * The llmq_ctx and chain_helper arguments are outputs: any instance they hold - * is destroyed and replaced with a freshly constructed one. */ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes, const ChainstateLoadOptions& options, CEvoDB& evodb, - CDeterministicMNManager& dmnman, std::unique_ptr& llmq_ctx, - std::unique_ptr& chain_helper); + CDeterministicMNManager& dmnman, const std::unique_ptr& chain_helper); ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options, CEvoDB& evodb, std::function notify_bls_state = nullptr); } // namespace node diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 5fa799d2291e..84c738137b55 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -81,6 +81,7 @@ #include #include #include +#include #include #include #include @@ -298,7 +299,6 @@ node::ChainstateLoadOptions ChainTestingSetup::ChainstateLoadOptionsForTest() options.mempool = Assert(m_node.mempool.get()); options.isman = Assert(m_node.isman.get()); options.chainlocks = Assert(m_node.chainlocks.get()); - options.mn_sync = Assert(m_node.mn_sync.get()); options.data_dir = Assert(m_node.args)->GetDataDirNet(); options.block_tree_db_in_memory = m_block_tree_db_in_memory; options.coins_db_in_memory = m_coins_db_in_memory; @@ -314,6 +314,22 @@ node::ChainstateLoadOptions ChainTestingSetup::ChainstateLoadOptionsForTest() return options; } +void ChainTestingSetup::MakeDashChainContexts(const bool llmq_dbs_wipe) +{ + auto& chainman{*Assert(m_node.chainman)}; + + LOCK(::cs_main); + m_node.chain_helper.reset(); + m_node.llmq_ctx.reset(); + m_node.llmq_ctx = std::make_unique(*m_node.dmnman, *m_node.evodb, chainman, + util::DbWrapperParams{.path = m_node.args->GetDataDirNet(), .memory = m_dash_dbs_in_memory, .wipe = llmq_dbs_wipe}, + llmq::DEFAULT_BLSCHECK_THREADS, llmq::DEFAULT_WORKER_COUNT, llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE); + m_node.chain_helper = std::make_unique(*m_node.evodb, *m_node.dmnman, *Assert(m_node.mn_sync), *Assert(m_node.isman), + *m_node.llmq_ctx->quorum_block_processor, *m_node.llmq_ctx->qsnapman, + chainman, chainman.m_blockman, chainman.GetConsensus(), + *Assert(m_node.chainlocks), *m_node.llmq_ctx->qman); +} + void ChainTestingSetup::LoadVerifyActivateChainstate() { auto& chainman{*Assert(m_node.chainman)}; @@ -342,6 +358,8 @@ void ChainTestingSetup::LoadVerifyActivateChainstate() // here, including the chainlock handler that references the mempool. m_node.clhandler.reset(); m_node.mempool.reset(); + m_node.chain_helper.reset(); + m_node.llmq_ctx.reset(); m_node.isman.reset(); m_node.dmnman.reset(); m_node.evodb.reset(); @@ -350,11 +368,13 @@ void ChainTestingSetup::LoadVerifyActivateChainstate() m_node.isman = std::make_unique(*m_node.sporkman, util::DbWrapperParams{.path = m_node.args->GetDataDirNet(), .memory = m_dash_dbs_in_memory, .wipe = true}); m_node.mempool = std::make_unique(MemPoolOptionsForTest(m_node)); m_node.clhandler = std::make_unique(*m_node.chainlocks, chainman, *m_node.mempool, *m_node.mn_sync); + MakeDashChainContexts(/*llmq_dbs_wipe=*/true); options = ChainstateLoadOptionsForTest(); + } else { + MakeDashChainContexts(/*llmq_dbs_wipe=*/false); } - auto [status, error] = LoadChainstate(chainman, m_cache_sizes, options, *m_node.evodb, *m_node.dmnman, m_node.llmq_ctx, - m_node.chain_helper); + auto [status, error] = LoadChainstate(chainman, m_cache_sizes, options, *m_node.evodb, *m_node.dmnman, m_node.chain_helper); assert(status == node::ChainstateLoadStatus::SUCCESS); std::tie(status, error) = VerifyLoadedChainstate(chainman, options, *Assert(m_node.evodb), [](bool bls_state) { diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index 73652650a83d..3121e548a6f1 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -93,6 +93,11 @@ struct ChainTestingSetup : public BasicTestingSetup { // The options LoadVerifyActivateChainstate() feeds to LoadChainstate(); // exposed for tests that drive LoadChainstate() to an expected failure. node::ChainstateLoadOptions ChainstateLoadOptionsForTest(); + + // (Re)creates m_node.llmq_ctx and m_node.chain_helper, which must exist + // before Chainstate objects can be created; exposed for tests that call + // InitializeChainstate() or LoadChainstate() themselves. + void MakeDashChainContexts(bool llmq_dbs_wipe); }; /** Testing setup that configures a complete environment. diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index e8c97241fd57..8815f8b79bb5 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -7,8 +7,6 @@ #include #include #include -#include -#include #include #include #include @@ -26,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -54,29 +53,6 @@ void SeedSnapshotMarker(CEvoDB& evodb, const uint256& hash) BOOST_FIXTURE_TEST_SUITE(validation_chainstatemanager_tests, ChainTestingSetup) -static void DashChainstateSetup(ChainstateManager& chainman, - node::NodeContext& node, - bool llmq_dbs_in_memory, - bool llmq_dbs_wipe) -{ - LOCK(::cs_main); - node.llmq_ctx.reset(); - node.llmq_ctx = std::make_unique(*node.dmnman, *node.evodb, chainman, - util::DbWrapperParams{.path = node.args->GetDataDirNet(), .memory = llmq_dbs_in_memory, .wipe = llmq_dbs_wipe}, - llmq::DEFAULT_BLSCHECK_THREADS, llmq::DEFAULT_WORKER_COUNT, llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE); - // Initialize chain_helper - node.chain_helper.reset(); - node.chain_helper = std::make_unique(*node.evodb, *node.dmnman, *Assert(node.mn_sync), *Assert(node.isman), *(node.llmq_ctx->quorum_block_processor), - *(node.llmq_ctx->qsnapman), chainman, chainman.m_blockman, chainman.GetConsensus(), - *Assert(node.chainlocks), *(node.llmq_ctx->qman)); -} - -static void DashChainstateSetupClose(node::NodeContext& node) -{ - node.chain_helper.reset(); - node.llmq_ctx.reset(); -} - //! Basic tests for ChainstateManager. //! //! First create a legacy (IBD) chainstate, then create a snapshot chainstate. @@ -91,14 +67,13 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Create a legacy (IBD) chainstate. // + MakeDashChainContexts(/*llmq_dbs_wipe=*/false); Chainstate& c1 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(&mempool, evodb, m_node.chain_helper)); chainstates.push_back(&c1); c1.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23)); - DashChainstateSetup(manager, m_node, /*llmq_dbs_in_memory=*/true, /*llmq_dbs_wipe=*/false); - BOOST_REQUIRE(c1.LoadGenesisBlock()); BlockValidationState val_state; BOOST_CHECK(c1.ActivateBestChain(val_state, nullptr)); @@ -121,7 +96,8 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) BOOST_CHECK(!manager.SnapshotBlockhash().has_value()); - DashChainstateSetupClose(m_node); + m_node.chain_helper.reset(); + m_node.llmq_ctx.reset(); // Create a snapshot-based chainstate. // @@ -137,7 +113,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) BOOST_CHECK_EQUAL(c2.GetMempool(), &mempool); BOOST_CHECK(!c1.GetMempool()); - DashChainstateSetup(manager, m_node, /*llmq_dbs_in_memory=*/true, /*llmq_dbs_wipe=*/false); + MakeDashChainContexts(/*llmq_dbs_wipe=*/false); BOOST_CHECK_EQUAL(manager.SnapshotBlockhash().value(), snapshot_blockhash); @@ -171,7 +147,8 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Let scheduler events finish running to avoid accessing memory that is going to be unloaded SyncWithValidationInterfaceQueue(); - DashChainstateSetupClose(m_node); + m_node.chain_helper.reset(); + m_node.llmq_ctx.reset(); // dmnman holds a reference to m_node.evodb, it mustn't outlive it m_node.dmnman.reset(); } @@ -457,7 +434,8 @@ struct SnapshotTestSetup : TestChain100Setup { cs->ForceFlushStateToDisk(); } } - DashChainstateSetupClose(m_node); + m_node.chain_helper.reset(); + m_node.llmq_ctx.reset(); chainman.ResetChainstates(); BOOST_CHECK_EQUAL(chainman.GetAll().size(), 0); const ChainstateManager::Options chainman_opts{ @@ -1219,13 +1197,13 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_missing_base_fails_load, Snap fs::remove_all(gArgs.GetDataDirNet() / "blocks" / "index"); ChainstateManager& chainman = *Assert(m_node.chainman); + MakeDashChainContexts(/*llmq_dbs_wipe=*/false); node::ChainstateLoadStatus status; bilingual_str error; { ASSERT_DEBUG_LOG("missing from the block index"); std::tie(status, error) = node::LoadChainstate(chainman, m_cache_sizes, ChainstateLoadOptionsForTest(), - *m_node.evodb, *m_node.dmnman, m_node.llmq_ctx, - m_node.chain_helper); + *m_node.evodb, *m_node.dmnman, m_node.chain_helper); } BOOST_CHECK(status == node::ChainstateLoadStatus::FAILURE); } From be3d76e6aab53d0a54c6c73aaeed3e6e13fae2cf Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 19 Sep 2026 04:50:05 +0700 Subject: [PATCH 03/28] refactor: move SuperblockManager out of CChainstateHelper into NodeContext The superblock manager is node-level governance state that outlives every chainstate reload: govman, the masternode RPCs, the node interface and the active-masternode context all r each it, and none of them cares about the chainstate helper that happened to own it. Superblock manager's ownership is moved to the NodeContext and constructed with the other node-level managers before the chainstate loading loop --- src/bitcoin-chainstate.cpp | 4 +++- src/evo/chainhelper.cpp | 6 +++--- src/evo/chainhelper.h | 4 ++-- src/governance/governance.cpp | 2 +- src/init.cpp | 8 +++++--- src/node/context.cpp | 1 + src/node/context.h | 5 +++++ src/node/interfaces.cpp | 2 +- src/rpc/masternode.cpp | 4 ++-- src/test/governance_inv_tests.cpp | 5 +---- src/test/governance_vote_processing_tests.cpp | 5 ++--- src/test/util/setup_common.cpp | 10 ++++++---- 12 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 4ad43d1e3b23..cf039d205d76 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -102,6 +103,7 @@ int main(int argc, char* argv[]) CMasternodeSync mn_sync{std::make_unique()}; CSporkManager sporkman; chainlock::Chainlocks chainlocks(sporkman); + governance::SuperblockManager sbman; // TODO: remove isman from bitcoin-chainstate and make it nullable for node::ChainstateLoadOptions same as mempool llmq::CInstantSendManager isman{sporkman, util::DbWrapperParams{.path = gArgs.GetDataDirNet(), .memory = false, .wipe = false}}; @@ -111,7 +113,7 @@ int main(int argc, char* argv[]) auto chain_helper = std::make_unique(evodb, dmnman, mn_sync, isman, *llmq_ctx->quorum_block_processor, *llmq_ctx->qsnapman, chainman, chainman.m_blockman, chainman.GetConsensus(), - chainlocks, *llmq_ctx->qman); + chainlocks, *llmq_ctx->qman, sbman); node::CacheSizes cache_sizes; cache_sizes.block_tree_db = 2 << 20; diff --git a/src/evo/chainhelper.cpp b/src/evo/chainhelper.cpp index 8e92000851e1..2007fa42874e 100644 --- a/src/evo/chainhelper.cpp +++ b/src/evo/chainhelper.cpp @@ -23,15 +23,15 @@ CChainstateHelper::CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmn llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman, const node::BlockManager& blockman, const Consensus::Params& consensus_params, - const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman) : + const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman, + governance::SuperblockManager& sbman) : isman{isman}, mn_sync{mn_sync}, m_dmnman{dmnman}, credit_pool_manager{std::make_unique(evodb, chainman)}, m_chainlocks{chainlocks}, ehf_manager{std::make_unique(evodb, consensus_params)}, - superblocks{std::make_unique()}, - mn_payments{std::make_unique(dmnman, *superblocks, consensus_params)}, + mn_payments{std::make_unique(dmnman, sbman, consensus_params)}, special_tx{std::make_unique(*credit_pool_manager, dmnman, *ehf_manager, qblockman, qsnapman, chainman, blockman, consensus_params, chainlocks, qman)} {} diff --git a/src/evo/chainhelper.h b/src/evo/chainhelper.h index a501dd82ceaf..9f72821eb0e9 100644 --- a/src/evo/chainhelper.h +++ b/src/evo/chainhelper.h @@ -51,7 +51,6 @@ class CChainstateHelper const std::unique_ptr credit_pool_manager; const chainlock::Chainlocks& m_chainlocks; const std::unique_ptr ehf_manager; - const std::unique_ptr superblocks; const std::unique_ptr mn_payments; const std::unique_ptr special_tx; @@ -63,7 +62,8 @@ class CChainstateHelper llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman, const node::BlockManager& blockman, const Consensus::Params& consensus_params, - const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman); + const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman, + governance::SuperblockManager& sbman); ~CChainstateHelper(); bool IsSuperblockValidationRequired(const CBlockIndex* const pindex); diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 6e34c5c56e95..78adb6e72af3 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -88,7 +88,7 @@ CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, const Cha CGovernanceManager::~CGovernanceManager() { // Reset the paired SuperblockManager so its loaded-flag and trigger map don't - // outlive us. chain_helper (its owner) must outlive govman -- see PrepareShutdown + // outlive us. node.sbman (its owner) must outlive govman -- see PrepareShutdown // in init.cpp and the test teardown ordering. m_superblocks.Clear(); if (!is_loaded) return; diff --git a/src/init.cpp b/src/init.cpp index 5bbbcd0fb127..e5bb204e5c6e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -108,6 +108,7 @@ #include #include #include +#include #include #include #include @@ -1742,6 +1743,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.sporkman); node.sporkman = std::make_unique(); node.chainlocks = std::make_unique(*node.sporkman); + node.sbman = std::make_unique(); const std::string spork_address{args.GetArg("-sporkaddr", chainparams.SporkAddress())}; if (args.GetArgs("-sporkaddr").size() > 1) { @@ -2103,7 +2105,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.chain_helper = std::make_unique(*node.evodb, *node.dmnman, *node.mn_sync, *node.isman, *node.llmq_ctx->quorum_block_processor, *node.llmq_ctx->qsnapman, chainman, chainman.m_blockman, chainman.GetConsensus(), - *node.chainlocks, *node.llmq_ctx->qman); + *node.chainlocks, *node.llmq_ctx->qman, *node.sbman); return LoadChainstate(chainman, cache_sizes, options, *node.evodb, *node.dmnman, node.chain_helper); }); } @@ -2163,7 +2165,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) RegisterValidationInterface(node.clhandler.get()); assert(!node.govman); - node.govman = std::make_unique(*node.mn_metaman, *node.chainman, *node.chain_helper->superblocks, *node.dmnman, *node.mn_sync); + node.govman = std::make_unique(*node.mn_metaman, *node.chainman, *node.sbman, *node.dmnman, *node.mn_sync); // ********************************************************* Step 7c: Setup masternode mode or watch-only mode assert(!node.active_ctx); @@ -2180,7 +2182,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } // Will init later in ThreadImport node.active_ctx = std::make_unique(*node.llmq_ctx->bls_worker, chainman, *node.connman, *node.dmnman, - *node.govman, *node.chain_helper->superblocks, + *node.govman, *node.sbman, *node.sporkman, *node.chainlocks, *node.mempool, *node.clhandler, *node.isman, *node.llmq_ctx->qman, *node.llmq_ctx->qsnapman, *node.llmq_ctx->sigman, *node.mn_sync, operator_sk, dash_db_params, quorums_watch); diff --git a/src/node/context.cpp b/src/node/context.cpp index e5e07563dcdc..4ac3ffe12728 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include diff --git a/src/node/context.h b/src/node/context.h index 56f80d825161..a3e084783e69 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -47,6 +47,10 @@ class Chainlocks; class ChainlockHandler; } // namespace chainlock +namespace governance { +class SuperblockManager; +} // namespace governance + namespace interfaces { class Chain; class ChainClient; @@ -108,6 +112,7 @@ struct NodeContext { std::unique_ptr mn_metaman; std::unique_ptr mn_sync; std::unique_ptr netfulfilledman; + std::unique_ptr sbman; std::unique_ptr sporkman; std::unique_ptr chainlocks; std::unique_ptr clhandler; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 019a212715c4..eb44feddb57a 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -447,7 +447,7 @@ class GOVImpl : public GOV { if (context().chain_helper != nullptr && context().chainman != nullptr) { const int32_t nTipHeight = WITH_LOCK(::cs_main, return context().chainman->ActiveHeight()); - for (const auto& trigger : context().chain_helper->superblocks->GetActiveTriggers()) { + for (const auto& trigger : context().sbman->GetActiveTriggers()) { if (!trigger || trigger->GetBlockHeight() > nTipHeight) continue; for (const auto& hash : trigger->GetProposalHashes()) { if (hash == proposal_hash) { diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index dce33a47300a..e0c455e056d7 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -330,7 +330,7 @@ static RPCHelpMan masternode_winners() const CBlockIndex* pIndex = pindexTip->GetAncestor(h - 1); auto payee = node.dmnman->GetListForBlock(pIndex).GetMNPayee(pIndex); if (payee) { - std::string strPayments = GetRequiredPaymentsString(*CHECK_NONFATAL(node.chain_helper)->superblocks, + std::string strPayments = GetRequiredPaymentsString(*CHECK_NONFATAL(node.sbman), tip_mn_list, h, payee); if (!strFilter.empty() && strPayments.find(strFilter) == std::string::npos) continue; obj.pushKV(strprintf("%d", h), strPayments); @@ -340,7 +340,7 @@ static RPCHelpMan masternode_winners() auto projection = node.dmnman->GetListForBlock(pindexTip).GetProjectedMNPayees(pindexTip, /*nCount=*/20); for (size_t i = 0; i < projection.size(); i++) { int h = nChainTipHeight + 1 + i; - std::string strPayments = GetRequiredPaymentsString(*node.chain_helper->superblocks, tip_mn_list, h, projection[i]); + std::string strPayments = GetRequiredPaymentsString(*node.sbman, tip_mn_list, h, projection[i]); if (!strFilter.empty() && strPayments.find(strFilter) == std::string::npos) continue; obj.pushKV(strprintf("%d", h), strPayments); } diff --git a/src/test/governance_inv_tests.cpp b/src/test/governance_inv_tests.cpp index 880670bf862d..2d4caef5e95b 100644 --- a/src/test/governance_inv_tests.cpp +++ b/src/test/governance_inv_tests.cpp @@ -3,7 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#include #include #include #include @@ -47,7 +46,7 @@ struct GovernanceInvSetup : public TestingSetup { // Note: mn_metaman is left unloaded. Reaching CGovernanceObject::ProcessVote asserts // metaman.IsValid(), so a test that delivers a vote whose parent object exists must // load it first (see invalid_vote_is_scored_alike_with_and_without_a_parent_object). - m_node.govman = std::make_unique(*m_node.mn_metaman, *m_node.chainman, *m_node.chain_helper->superblocks, *m_node.dmnman, *m_node.mn_sync); + m_node.govman = std::make_unique(*m_node.mn_metaman, *m_node.chainman, *m_node.sbman, *m_node.dmnman, *m_node.mn_sync); // Match runtime preconditions: NetGovernance::AlreadyHave claims we // already have the inv when governance isn't loaded (e.g. // -disablegovernance), so ConfirmInventoryRequest would never run. @@ -76,8 +75,6 @@ struct GovernanceInvSetup : public TestingSetup { SetMockTime(1'700'000'000s); } ~GovernanceInvSetup() { - // govman holds a reference to chain_helper->superblocks, so it must be reset before chain_helper (matches PrepareShutdown - // ordering in init.cpp). m_node.peerman->RemoveHandlers(); m_node.govman.reset(); } diff --git a/src/test/governance_vote_processing_tests.cpp b/src/test/governance_vote_processing_tests.cpp index 3317312d477a..bbae436392c2 100644 --- a/src/test/governance_vote_processing_tests.cpp +++ b/src/test/governance_vote_processing_tests.cpp @@ -3,7 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#include #include #include #include @@ -118,7 +117,7 @@ struct GovernanceVoteSetup : public TestChainSetup { BOOST_REQUIRE(m_node.mn_sync->IsSynced()); m_node.govman = std::make_unique(*m_node.mn_metaman, *m_node.chainman, - *m_node.chain_helper->superblocks, *m_node.dmnman, + *m_node.sbman, *m_node.dmnman, *m_node.mn_sync); BOOST_REQUIRE(m_node.govman->LoadCache(/*load_cache=*/false)); @@ -130,7 +129,7 @@ struct GovernanceVoteSetup : public TestChainSetup { void TearDown() { - // govman holds a reference into chain_helper, so it must go first (matches PrepareShutdown + // govman holds a reference to node.sbman, so it must go first (matches PrepareShutdown // ordering in init.cpp). m_node.govman.reset(); if (g_txindex) { diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 84c738137b55..f718fd2ed866 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -79,6 +79,7 @@ #include #include #include +#include #include #include #include @@ -213,6 +214,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve m_node.netfulfilledman = std::make_unique(); m_node.sporkman = std::make_unique(); m_node.chainlocks = std::make_unique(*m_node.sporkman); + m_node.sbman = std::make_unique(); m_node.evodb = std::make_unique(util::DbWrapperParams{.path = m_node.args->GetDataDirNet(), .memory = m_dash_dbs_in_memory, .wipe = true}); m_node.dmnman = std::make_unique(*m_node.evodb, *m_node.mn_metaman); m_node.isman = std::make_unique(*m_node.sporkman, util::DbWrapperParams{.path = m_node.args->GetDataDirNet(), .memory = m_dash_dbs_in_memory, .wipe = true}); @@ -327,7 +329,7 @@ void ChainTestingSetup::MakeDashChainContexts(const bool llmq_dbs_wipe) m_node.chain_helper = std::make_unique(*m_node.evodb, *m_node.dmnman, *Assert(m_node.mn_sync), *Assert(m_node.isman), *m_node.llmq_ctx->quorum_block_processor, *m_node.llmq_ctx->qsnapman, chainman, chainman.m_blockman, chainman.GetConsensus(), - *Assert(m_node.chainlocks), *m_node.llmq_ctx->qman); + *Assert(m_node.chainlocks), *m_node.llmq_ctx->qman, *Assert(m_node.sbman)); } void ChainTestingSetup::LoadVerifyActivateChainstate() @@ -454,9 +456,9 @@ TestingSetup::~TestingSetup() m_node.connman->Stop(); } - // govman holds a reference to chain_helper->superblocks, so it must be - // reset before chain_helper is destroyed (matches PrepareShutdown ordering - // in init.cpp). Keep this defensive for fixtures that construct govman. + // govman holds a reference to node.sbman, which NodeContext destroys + // last, but reset it here anyway to match PrepareShutdown ordering in + // init.cpp for fixtures that construct govman. m_node.govman.reset(); m_node.chain_helper.reset(); From 17ef1342109866d8c9e0d9fd68fa920b93871322 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 19 Sep 2026 04:51:43 +0700 Subject: [PATCH 04/28] refactor: construct CMasternodeSync before the chainstate loading loop CMasternodeSync is not need to be recreated on every reindex because there is nothing synced when a retry happens Its inputs (connman, netfulfilledman) already exist before the loop. --- src/init.cpp | 22 +++++++++++----------- src/test/util/setup_common.cpp | 5 ++--- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index e5bb204e5c6e..ea5da8177d98 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1745,6 +1745,17 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.chainlocks = std::make_unique(*node.sporkman); node.sbman = std::make_unique(); + /** + * The manager needs to be constructed regardless of whether governance + * validation is needed or not. + * + * Instead, we decide whether to initialize its database based on whether we + * need it or not further down and then query if the database is initialized + * to check if validation is enabled. + */ + assert(!node.mn_sync); + node.mn_sync = std::make_unique(std::make_unique(*node.connman, *node.netfulfilledman)); + const std::string spork_address{args.GetArg("-sporkaddr", chainparams.SporkAddress())}; if (args.GetArgs("-sporkaddr").size() > 1) { InitWarning(strprintf(_("More than one spork address is provided. Using %s."), spork_address)); @@ -2007,7 +2018,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.mempool); assert(!node.chainman); - assert(!node.mn_sync); CTxMemPool::Options mempool_opts{ .estimator = node.fee_estimator.get(), @@ -2055,16 +2065,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.chainman = std::make_unique(chainman_opts); ChainstateManager& chainman = *node.chainman; - /** - * The manager needs to be constructed regardless of whether governance - * validation is needed or not. - * - * Instead, we decide whether to initialize its database based on whether we - * need it or not further down and then query if the database is initialized - * to check if validation is enabled. - */ - node.mn_sync = std::make_unique(std::make_unique(*node.connman, *node.netfulfilledman)); - node::ChainstateLoadOptions options; options.chainlocks = Assert(node.chainlocks.get()); options.data_dir = args.GetDataDirNet(); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index f718fd2ed866..d1ca552e46de 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -215,6 +215,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve m_node.sporkman = std::make_unique(); m_node.chainlocks = std::make_unique(*m_node.sporkman); m_node.sbman = std::make_unique(); + m_node.mn_sync = std::make_unique(std::make_unique(*m_node.connman, *m_node.netfulfilledman)); m_node.evodb = std::make_unique(util::DbWrapperParams{.path = m_node.args->GetDataDirNet(), .memory = m_dash_dbs_in_memory, .wipe = true}); m_node.dmnman = std::make_unique(*m_node.evodb, *m_node.mn_metaman); m_node.isman = std::make_unique(*m_node.sporkman, util::DbWrapperParams{.path = m_node.args->GetDataDirNet(), .memory = m_dash_dbs_in_memory, .wipe = true}); @@ -273,8 +274,6 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve m_node.chainman = std::make_unique(chainman_opts); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(m_cache_sizes.block_tree_db, true); - m_node.mn_sync = std::make_unique(std::make_unique(*m_node.connman, *m_node.netfulfilledman)); - m_node.clhandler = std::make_unique(*m_node.chainlocks, *m_node.chainman, *m_node.mempool, *m_node.mn_sync); constexpr int script_check_threads = 2; @@ -287,8 +286,8 @@ ChainTestingSetup::~ChainTestingSetup() StopScriptCheckWorkerThreads(); GetMainSignals().FlushBackgroundCallbacks(); GetMainSignals().UnregisterBackgroundSignalScheduler(); - m_node.mn_sync.reset(); m_node.chainman.reset(); + m_node.mn_sync.reset(); m_node.mempool.reset(); m_node.fee_estimator.reset(); m_node.scheduler.reset(); From 0289991303c525ac3470f6d171b014df3c998ce9 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 1 Oct 2026 23:10:40 +0700 Subject: [PATCH 05/28] refactor: extract the superblock height and payments-limit rules CSuperblock::IsValidBlockHeight and GetPaymentsLimit are consensus-parameter math that block validation will need without going through governance. Give them homes that depend on nothing but the parameters: IsSuperblockHeight in masternode/payments and GetSuperblockPaymentsLimit next to the subsidy math it is derived from. CSuperblock keeps its statics as forwarders. --- src/governance/superblock.cpp | 22 +++------------------- src/masternode/payments.cpp | 7 +++++++ src/masternode/payments.h | 3 +++ src/validation.cpp | 18 ++++++++++++++++++ src/validation.h | 3 +++ 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/governance/superblock.cpp b/src/governance/superblock.cpp index bb22f1e84dc9..61577c19ce5b 100644 --- a/src/governance/superblock.cpp +++ b/src/governance/superblock.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -118,9 +119,7 @@ CSuperblock::CSuperblock(int nBlockHeight, std::vector vecPa bool CSuperblock::IsValidBlockHeight(int nBlockHeight) { - // SUPERBLOCKS CAN HAPPEN ONLY after hardfork and only ONCE PER CYCLE - return nBlockHeight >= Params().GetConsensus().nSuperblockStartBlock && - ((nBlockHeight % Params().GetConsensus().nSuperblockCycle) == 0); + return IsSuperblockHeight(nBlockHeight, Params().GetConsensus()); } void CSuperblock::GetNearestSuperblocksHeights(int nBlockHeight, int& nLastSuperblockRet, int& nNextSuperblockRet) @@ -144,22 +143,7 @@ void CSuperblock::GetNearestSuperblocksHeights(int nBlockHeight, int& nLastSuper CAmount CSuperblock::GetPaymentsLimit(int nBlockHeight) { - const Consensus::Params& consensusParams = Params().GetConsensus(); - - if (!IsValidBlockHeight(nBlockHeight)) { - return 0; - } - - const bool fV20Active{nBlockHeight >= consensusParams.V20Height}; - - // min subsidy for high diff networks and vice versa - int nBits = consensusParams.fPowAllowMinDifficultyBlocks ? UintToArith256(consensusParams.powLimit).GetCompact() : 1; - // some part of all blocks issued during the cycle goes to superblock, see GetBlockSubsidy - CAmount nSuperblockPartOfSubsidy = GetSuperblockSubsidyInner(nBits, nBlockHeight - 1, consensusParams, fV20Active); - CAmount nPaymentsLimit = nSuperblockPartOfSubsidy * consensusParams.nSuperblockCycle; - LogPrint(BCLog::GOBJECT, "CSuperblock::GetPaymentsLimit -- Valid superblock height %d, payments max %lld\n", nBlockHeight, nPaymentsLimit); - - return nPaymentsLimit; + return GetSuperblockPaymentsLimit(nBlockHeight, Params().GetConsensus()); } void CSuperblock::ParsePaymentSchedule(const std::string& strPaymentAddresses, const std::string& strPaymentAmounts, const std::string& strProposalHashes) diff --git a/src/masternode/payments.cpp b/src/masternode/payments.cpp index 7d0ba8c4d360..8989b0ec9222 100644 --- a/src/masternode/payments.cpp +++ b/src/masternode/payments.cpp @@ -128,6 +128,13 @@ CAmount GetMasternodePayment(int nHeight, CAmount blockValue, const Consensus::P return static_cast(blockValue * vecPeriods[nCurrentPeriod] / 1000); } +bool IsSuperblockHeight(const int nBlockHeight, const Consensus::Params& consensus_params) +{ + // SUPERBLOCKS CAN HAPPEN ONLY after hardfork and only ONCE PER CYCLE + return nBlockHeight >= consensus_params.nSuperblockStartBlock && + ((nBlockHeight % consensus_params.nSuperblockCycle) == 0); +} + [[nodiscard]] bool CMNPaymentsProcessor::GetBlockTxOuts(const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, MnRewardEra era, std::vector& voutMasternodePaymentsRet) { diff --git a/src/masternode/payments.h b/src/masternode/payments.h index f99db57baf93..1ac11ed6eb78 100644 --- a/src/masternode/payments.h +++ b/src/masternode/payments.h @@ -70,6 +70,9 @@ enum class SuperBlockCheckType { CAmount GetMasternodePayment(int nHeight, CAmount blockValue, const Consensus::Params& consensus_params, MnRewardEra era); +/** Superblocks happen once per cycle after the superblock hardfork. */ +bool IsSuperblockHeight(int nBlockHeight, const Consensus::Params& consensus_params); + class CMNPaymentsProcessor { private: diff --git a/src/validation.cpp b/src/validation.cpp index ce807bc69425..7b069cd89a80 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1888,6 +1888,24 @@ CAmount GetBlockSubsidy(const CBlockIndex* const pindex, const Consensus::Params return GetBlockSubsidyInner(pindex->pprev->nBits, pindex->pprev->nHeight, consensusParams, isV20Active); } +CAmount GetSuperblockPaymentsLimit(int nBlockHeight, const Consensus::Params& consensusParams) +{ + if (!IsSuperblockHeight(nBlockHeight, consensusParams)) { + return 0; + } + + const bool fV20Active{nBlockHeight >= consensusParams.V20Height}; + + // min subsidy for high diff networks and vice versa + int nBits = consensusParams.fPowAllowMinDifficultyBlocks ? UintToArith256(consensusParams.powLimit).GetCompact() : 1; + // some part of all blocks issued during the cycle goes to superblock, see GetBlockSubsidy + CAmount nSuperblockPartOfSubsidy = GetSuperblockSubsidyInner(nBits, nBlockHeight - 1, consensusParams, fV20Active); + CAmount nPaymentsLimit = nSuperblockPartOfSubsidy * consensusParams.nSuperblockCycle; + LogPrint(BCLog::GOBJECT, "%s -- Valid superblock height %d, payments max %lld\n", __func__, nBlockHeight, nPaymentsLimit); + + return nPaymentsLimit; +} + CoinsViews::CoinsViews( fs::path ldb_name, size_t cache_size_bytes, diff --git a/src/validation.h b/src/validation.h index 2b6e8e6ce9f9..ad490f74f1de 100644 --- a/src/validation.h +++ b/src/validation.h @@ -131,6 +131,9 @@ CAmount GetBlockSubsidyInner(int nPrevBits, int nPrevHeight, const Consensus::Pa CAmount GetSuperblockSubsidyInner(int nPrevBits, int nPrevHeight, const Consensus::Params& consensusParams, bool fV20Active); CAmount GetBlockSubsidy(const CBlockIndex* const pindex, const Consensus::Params& consensusParams); +/** The superblock share of all subsidies issued during the cycle ending at a superblock height. */ +CAmount GetSuperblockPaymentsLimit(int nBlockHeight, const Consensus::Params& consensusParams); + bool AbortNode(BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage = bilingual_str{}); /** Guess verification progress (as a fraction between 0.0=genesis and 1.0=current tip). */ From 93d76417444af5fe4fe0820b8d79aeb75a2c3cc7 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 1 Oct 2026 23:12:07 +0700 Subject: [PATCH 06/28] refactor: validate a superblock against a plain list of outputs CSuperblock::IsValid read the expected payments out of the trigger object itself, so checking a coinbase required the governance object. The rule only needs the expected outputs and the payments limit: make it a static check over a std::vector, with the limit passed in, and have SuperblockManager::IsValidSuperblock hand it the winning trigger's outputs. --- src/governance/superblock.cpp | 51 ++++++++---------------- src/governance/superblock.h | 8 +++- src/test/governance_superblock_tests.cpp | 17 +++----- 3 files changed, 28 insertions(+), 48 deletions(-) diff --git a/src/governance/superblock.cpp b/src/governance/superblock.cpp index 61577c19ce5b..f85d8fafe4af 100644 --- a/src/governance/superblock.cpp +++ b/src/governance/superblock.cpp @@ -218,24 +218,8 @@ bool CSuperblock::GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet) return true; } -CAmount CSuperblock::GetPaymentsTotalAmount() +bool CSuperblock::IsValid(const std::vector& payments, const CTransaction& txNew, int block_height, CAmount blockReward, bool is_v24, CAmount nPaymentsLimit) { - return std23::ranges::fold_left(vecPayments, CAmount{0}, [](CAmount s, const auto& p) { return s + p.nAmount; }); -} - -/** -* Is Transaction Valid -* -* - Does this transaction match the superblock? -*/ - -bool CSuperblock::IsValid(const CTransaction& txNew, int block_height, CAmount blockReward, bool is_v24) -{ - // TODO : LOCK(cs); - // No reason for a lock here now since this method only accesses data - // internal to *this and since CSuperblock's are accessed only through - // shared pointers there's no way our object can get deleted while this - // code is running. if (!IsValidBlockHeight(block_height)) { LogPrintf("CSuperblock::IsValid -- ERROR: Block invalid, incorrect block height\n"); return false; @@ -244,11 +228,10 @@ bool CSuperblock::IsValid(const CTransaction& txNew, int block_height, CAmount b // CONFIGURE SUPERBLOCK OUTPUTS int nOutputs = txNew.vout.size(); - int nPayments = CountPayments(); + int nPayments = payments.size(); int nMinerAndMasternodePayments = nOutputs - nPayments; - LogPrint(BCLog::GOBJECT, "CSuperblock::IsValid -- nOutputs = %d, nPayments = %d, hash = %s\n", nOutputs, nPayments, - nGovObjHash.ToString()); + LogPrint(BCLog::GOBJECT, "CSuperblock::IsValid -- nOutputs = %d, nPayments = %d\n", nOutputs, nPayments); // We require an exact match (including order) between the expected // superblock payments and the payments actually in the block. @@ -262,8 +245,7 @@ bool CSuperblock::IsValid(const CTransaction& txNew, int block_height, CAmount b } // payments should not exceed limit - CAmount nPaymentsTotalAmount = GetPaymentsTotalAmount(); - CAmount nPaymentsLimit = GetPaymentsLimit(block_height); + CAmount nPaymentsTotalAmount = std23::ranges::fold_left(payments, CAmount{0}, [](CAmount s, const auto& p) { return s + p.nValue; }); if (nPaymentsTotalAmount > nPaymentsLimit) { LogPrintf("CSuperblock::IsValid -- ERROR: Block invalid, payments limit exceeded: payments %lld, limit %lld\n", nPaymentsTotalAmount, nPaymentsLimit); return false; @@ -278,13 +260,7 @@ bool CSuperblock::IsValid(const CTransaction& txNew, int block_height, CAmount b int nVoutIndex = -1; for (int i = 0; i < nPayments; i++) { - CGovernancePayment payment; - if (!GetPayment(i, payment)) { - // This shouldn't happen so log a warning - LogPrintf("CSuperblock::IsValid -- WARNING: Failed to find payment: %d of %d total payments\n", i, nPayments); - continue; - } - + const CTxOut& payment = payments[i]; bool fPaymentMatch = false; // From V24 on, start past the previously matched output so each expected @@ -297,8 +273,8 @@ bool CSuperblock::IsValid(const CTransaction& txNew, int block_height, CAmount b const int nVoutStart = is_v24 ? nVoutIndex + 1 : std::max(nVoutIndex, 0); for (int j = nVoutStart; j < nOutputs; j++) { // Find superblock payment - fPaymentMatch = ((payment.script == txNew.vout[j].scriptPubKey) && - (payment.nAmount == txNew.vout[j].nValue)); + fPaymentMatch = ((payment.scriptPubKey == txNew.vout[j].scriptPubKey) && + (payment.nValue == txNew.vout[j].nValue)); if (fPaymentMatch) { nVoutIndex = j; @@ -310,8 +286,8 @@ bool CSuperblock::IsValid(const CTransaction& txNew, int block_height, CAmount b // Superblock payment not found! CTxDestination dest; - ExtractDestination(payment.script, dest); - LogPrintf("CSuperblock::IsValid -- ERROR: Block invalid: %d payment %d to %s not found\n", i, payment.nAmount, EncodeDestination(dest)); + ExtractDestination(payment.scriptPubKey, dest); + LogPrintf("CSuperblock::IsValid -- ERROR: Block invalid: %d payment %d to %s not found\n", i, payment.nValue, EncodeDestination(dest)); return false; } @@ -609,7 +585,14 @@ bool SuperblockManager::IsValidSuperblock(const CDeterministicMNList& tip_mn_lis LOCK(cs_sb); CSuperblock_sptr pSuperblock; if (GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) { - return pSuperblock->IsValid(txNew, nBlockHeight, blockReward, is_v24); + std::vector payments; + for (int i = 0; i < pSuperblock->CountPayments(); i++) { + CGovernancePayment payment; + if (pSuperblock->GetPayment(i, payment)) { + payments.emplace_back(payment.nAmount, payment.script); + } + } + return CSuperblock::IsValid(payments, txNew, nBlockHeight, blockReward, is_v24, CSuperblock::GetPaymentsLimit(nBlockHeight)); } return false; } diff --git a/src/governance/superblock.h b/src/governance/superblock.h index fdb9bfa07763..96c238e1af1a 100644 --- a/src/governance/superblock.h +++ b/src/governance/superblock.h @@ -104,9 +104,13 @@ class CSuperblock : public CGovernanceObject int CountPayments() const { return static_cast(vecPayments.size()); } bool GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet); - CAmount GetPaymentsTotalAmount(); - bool IsValid(const CTransaction& txNew, int block_height, CAmount blockReward, bool is_v24); + /** + * Whether the coinbase carries every expected superblock payment, in order, + * within the superblock payments limit, and without paying the miner and + * masternodes more than the block reward. + */ + static bool IsValid(const std::vector& payments, const CTransaction& txNew, int block_height, CAmount blockReward, bool is_v24, CAmount nPaymentsLimit); bool IsExpired(int heightToTest) const; std::vector GetProposalHashes() const; diff --git a/src/test/governance_superblock_tests.cpp b/src/test/governance_superblock_tests.cpp index 460964a5e863..3f31b3e126c4 100644 --- a/src/test/governance_superblock_tests.cpp +++ b/src/test/governance_superblock_tests.cpp @@ -10,7 +10,6 @@ #include #include