From 48a0357ef3345f1c737ae4ebccb6d45dcc02da1d Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 20 Jun 2025 16:33:01 +0700 Subject: [PATCH 1/8] refactor: only code move for CalcCbTxMerkleRootMNList from cbtx.h to simplifiedmns.h --- src/evo/cbtx.cpp | 44 --------------------------------------- src/evo/cbtx.h | 1 - src/evo/simplifiedmns.cpp | 44 +++++++++++++++++++++++++++++++++++++++ src/evo/simplifiedmns.h | 2 ++ 4 files changed, 46 insertions(+), 45 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 9eb014a814b7..42ed3124aea3 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -88,50 +88,6 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CCbTx& cbTx, const CBlockIn return true; } -bool CalcCbTxMerkleRootMNList(uint256& merkleRootRet, CSimplifiedMNList&& sml, BlockValidationState& state) -{ - try { - static std::atomic nTimeMerkle = 0; - - int64_t nTime1 = GetTimeMicros(); - - static Mutex cached_mutex; - static CSimplifiedMNList smlCached GUARDED_BY(cached_mutex); - static uint256 merkleRootCached GUARDED_BY(cached_mutex); - static bool mutatedCached GUARDED_BY(cached_mutex) {false}; - - LOCK(cached_mutex); - if (sml == smlCached) { - merkleRootRet = merkleRootCached; - if (mutatedCached) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "mutated-cached-calc-cb-mnmerkleroot"); - } - return true; - } - - bool mutated = false; - merkleRootRet = sml.CalcMerkleRoot(&mutated); - - int64_t nTime2 = GetTimeMicros(); - nTimeMerkle += nTime2 - nTime1; - LogPrint(BCLog::BENCHMARK, " - CalcMerkleRoot: %.2fms [%.2fs]\n", 0.001 * (nTime2 - nTime1), - nTimeMerkle * 0.000001); - - smlCached = std::move(sml); - merkleRootCached = merkleRootRet; - mutatedCached = mutated; - - if (mutated) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "mutated-calc-cb-mnmerkleroot"); - } - - return true; - } catch (const std::exception& e) { - LogPrintf("%s -- failed: %s\n", __func__, e.what()); - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "failed-calc-cb-mnmerkleroot"); - } -} - using QcHashMap = std::map>; using QcIndexedHashMap = std::map>; diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index 7db9c2409b8d..efa6dd49ae3e 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -69,7 +69,6 @@ bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationSta bool CheckCbTxMerkleRoots(const CBlock& block, const CCbTx& cbTx, const CBlockIndex* pindex, const llmq::CQuorumBlockProcessor& quorum_block_processor, CSimplifiedMNList&& sml, BlockValidationState& state); -bool CalcCbTxMerkleRootMNList(uint256& merkleRootRet, CSimplifiedMNList&& sml, BlockValidationState& state); bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet, BlockValidationState& state); diff --git a/src/evo/simplifiedmns.cpp b/src/evo/simplifiedmns.cpp index 45b101af0594..d6eda245d3b4 100644 --- a/src/evo/simplifiedmns.cpp +++ b/src/evo/simplifiedmns.cpp @@ -404,3 +404,47 @@ bool BuildSimplifiedMNListDiff(CDeterministicMNManager& dmnman, const Chainstate return true; } + +bool CalcCbTxMerkleRootMNList(uint256& merkleRootRet, CSimplifiedMNList&& sml, BlockValidationState& state) +{ + try { + static std::atomic nTimeMerkle = 0; + + int64_t nTime1 = GetTimeMicros(); + + static Mutex cached_mutex; + static CSimplifiedMNList smlCached GUARDED_BY(cached_mutex); + static uint256 merkleRootCached GUARDED_BY(cached_mutex); + static bool mutatedCached GUARDED_BY(cached_mutex){false}; + + LOCK(cached_mutex); + if (sml == smlCached) { + merkleRootRet = merkleRootCached; + if (mutatedCached) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "mutated-cached-calc-cb-mnmerkleroot"); + } + return true; + } + + bool mutated = false; + merkleRootRet = sml.CalcMerkleRoot(&mutated); + + int64_t nTime2 = GetTimeMicros(); + nTimeMerkle += nTime2 - nTime1; + LogPrint(BCLog::BENCHMARK, " - CalcMerkleRoot: %.2fms [%.2fs]\n", 0.001 * (nTime2 - nTime1), + nTimeMerkle * 0.000001); + + smlCached = std::move(sml); + merkleRootCached = merkleRootRet; + mutatedCached = mutated; + + if (mutated) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "mutated-calc-cb-mnmerkleroot"); + } + + return true; + } catch (const std::exception& e) { + LogPrintf("%s -- failed: %s\n", __func__, e.what()); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "failed-calc-cb-mnmerkleroot"); + } +} diff --git a/src/evo/simplifiedmns.h b/src/evo/simplifiedmns.h index e2c17cccb242..14882d70878b 100644 --- a/src/evo/simplifiedmns.h +++ b/src/evo/simplifiedmns.h @@ -181,4 +181,6 @@ bool BuildSimplifiedMNListDiff(CDeterministicMNManager& dmnman, const Chainstate const uint256& baseBlockHash, const uint256& blockHash, CSimplifiedMNListDiff& mnListDiffRet, std::string& errorRet, bool extended = false) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); +bool CalcCbTxMerkleRootMNList(uint256& merkleRootRet, CSimplifiedMNList&& sml, BlockValidationState& state); + #endif // BITCOIN_EVO_SIMPLIFIEDMNS_H From 1011f6b8baa4724a47677cc787371083441070f9 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 20 Jun 2025 17:08:22 +0700 Subject: [PATCH 2/8] refactor: drop dependency of CbTx on CSimplifiedMNList --- src/evo/cbtx.cpp | 37 ++++------------------- src/evo/cbtx.h | 6 ++-- src/evo/specialtxman.cpp | 39 ++++++++++++++++++++----- test/lint/lint-circular-dependencies.py | 4 +-- 4 files changed, 40 insertions(+), 46 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 42ed3124aea3..bcc23a755fb2 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -4,7 +4,6 @@ #include #include -#include #include #include #include @@ -45,44 +44,18 @@ bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationSta return true; } -// This can only be done after the block has been fully processed, as otherwise we won't have the finished MN list bool CheckCbTxMerkleRoots(const CBlock& block, const CCbTx& cbTx, const CBlockIndex* pindex, - const llmq::CQuorumBlockProcessor& quorum_block_processor, CSimplifiedMNList&& sml, - BlockValidationState& state) + const llmq::CQuorumBlockProcessor& quorum_block_processor, BlockValidationState& state) { - if (pindex) { - static int64_t nTimeMerkleMNL = 0; - static int64_t nTimeMerkleQuorum = 0; - - int64_t nTime1 = GetTimeMicros(); + if (pindex && cbTx.nVersion >= CCbTx::Version::MERKLE_ROOT_QUORUMS) { uint256 calculatedMerkleRoot; - if (!CalcCbTxMerkleRootMNList(calculatedMerkleRoot, std::move(sml), state)) { + if (!CalcCbTxMerkleRootQuorums(block, pindex->pprev, quorum_block_processor, calculatedMerkleRoot, state)) { // pass the state returned by the function above return false; } - if (calculatedMerkleRoot != cbTx.merkleRootMNList) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-mnmerkleroot"); + if (calculatedMerkleRoot != cbTx.merkleRootQuorums) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-quorummerkleroot"); } - - int64_t nTime2 = GetTimeMicros(); - nTimeMerkleMNL += nTime2 - nTime1; - LogPrint(BCLog::BENCHMARK, " - CalcCbTxMerkleRootMNList: %.2fms [%.2fs]\n", 0.001 * (nTime2 - nTime1), - nTimeMerkleMNL * 0.000001); - - if (cbTx.nVersion >= CCbTx::Version::MERKLE_ROOT_QUORUMS) { - if (!CalcCbTxMerkleRootQuorums(block, pindex->pprev, quorum_block_processor, calculatedMerkleRoot, state)) { - // pass the state returned by the function above - return false; - } - if (calculatedMerkleRoot != cbTx.merkleRootQuorums) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-quorummerkleroot"); - } - } - - int64_t nTime3 = GetTimeMicros(); - nTimeMerkleQuorum += nTime3 - nTime2; - LogPrint(BCLog::BENCHMARK, " - CalcCbTxMerkleRootQuorums: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2), - nTimeMerkleQuorum * 0.000001); } return true; diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index efa6dd49ae3e..9a80fef2ddf2 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -15,8 +15,6 @@ class BlockValidationState; class CBlock; class CBlockIndex; class TxValidationState; -class CSimplifiedMNList; - namespace llmq { class CChainLocksHandler; class CQuorumBlockProcessor; @@ -66,9 +64,9 @@ template<> struct is_serializable_enum : std::true_type {}; bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationState& state); +// This can only be done after the block has been fully processed, as otherwise we won't have the finished MN list bool CheckCbTxMerkleRoots(const CBlock& block, const CCbTx& cbTx, const CBlockIndex* pindex, - const llmq::CQuorumBlockProcessor& quorum_block_processor, CSimplifiedMNList&& sml, - BlockValidationState& state); + const llmq::CQuorumBlockProcessor& quorum_block_processor, BlockValidationState& state); bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet, BlockValidationState& state); diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index f4de428ffb1a..6fbafe5c3495 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -90,6 +90,7 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB static int64_t nTimeLoop = 0; static int64_t nTimeQuorum = 0; static int64_t nTimeDMN = 0; + static int64_t nTimeMerkleMNL = 0; static int64_t nTimeMerkle = 0; static int64_t nTimeCbTxCL = 0; static int64_t nTimeMnehf = 0; @@ -117,6 +118,12 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload"); } } + if (fCheckCbTxMerkleRoots) { + // To ensure that opt_cbTx is not missing when it's supposed to be + if (DeploymentActiveAt(*pindex, m_consensus_params, Consensus::DEPLOYMENT_DIP0003) && !opt_cbTx.has_value()) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-version"); + } + } int64_t nTime2 = GetTimeMicros(); nTimePayload += nTime2 - nTime1; @@ -150,7 +157,7 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB nTimeLoop += nTime3 - nTime2; LogPrint(BCLog::BENCHMARK, " - Loop: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2), nTimeLoop * 0.000001); - if (fCheckCbTxMerkleRoots && block.vtx[0]->nType == TRANSACTION_COINBASE) { + if (opt_cbTx.has_value()) { if (!CheckCreditPoolDiffForBlock(block, pindex, *opt_cbTx, state)) { return error("CSpecialTxProcessor: CheckCreditPoolDiffForBlock for block %s failed with %s", pindex->GetBlockHash().ToString(), state.ToString()); @@ -173,8 +180,8 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB nTimeQuorum * 0.000001); - CDeterministicMNList mn_list; - if (DeploymentActiveAt(*pindex, m_consensus_params, Consensus::DEPLOYMENT_DIP0003)) { + if (opt_cbTx.has_value()) { + CDeterministicMNList mn_list; if (!m_dmnman.BuildNewListFromBlock(block, pindex->pprev, state, view, mn_list, m_qsnapman, true)) { // pass the state returned by the function above return false; @@ -185,15 +192,31 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB // pass the state returned by the function above return false; } - } - int64_t nTime6 = GetTimeMicros(); - nTimeDMN += nTime6 - nTime5; + int64_t nTime5_1 = GetTimeMicros(); + nTimeDMN += nTime5_1 - nTime5; + + LogPrint(BCLog::BENCHMARK, " - m_dmnman: %.2fms [%.2fs]\n", 0.001 * (nTime5_1 - nTime5), + nTimeDMN * 0.000001); - LogPrint(BCLog::BENCHMARK, " - m_dmnman: %.2fms [%.2fs]\n", 0.001 * (nTime6 - nTime5), nTimeDMN * 0.000001); + uint256 calculatedMerkleRoot; + if (!CalcCbTxMerkleRootMNList(calculatedMerkleRoot, CSimplifiedMNList{std::move(mn_list)}, state)) { + // pass the state returned by the function above + return false; + } + if (calculatedMerkleRoot != opt_cbTx->merkleRootMNList) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-mnmerkleroot"); + } + int64_t nTime5_2 = GetTimeMicros(); + nTimeMerkleMNL += nTime5_2 - nTime5_1; + LogPrint(BCLog::BENCHMARK, " - CalcCbTxMerkleRootMNList: %.2fms [%.2fs]\n", + 0.001 * (nTime5_2 - nTime5_1), nTimeMerkleMNL * 0.000001); + } + + int64_t nTime6 = GetTimeMicros(); if (opt_cbTx.has_value()) { - if (!CheckCbTxMerkleRoots(block, *opt_cbTx, pindex, m_qblockman, CSimplifiedMNList(mn_list), state)) { + if (!CheckCbTxMerkleRoots(block, *opt_cbTx, pindex, m_qblockman, state)) { // pass the state returned by the function above return false; } diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 145dd7095031..2a385a1d6343 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -34,9 +34,7 @@ "common/bloom -> evo/assetlocktx -> llmq/signing -> net_processing -> merkleblock -> common/bloom", "consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> consensus/tx_verify", "consensus/tx_verify -> evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> consensus/tx_verify", - "core_io -> evo/cbtx -> evo/simplifiedmns -> core_io", "evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> evo/assetlocktx", - "evo/cbtx -> evo/simplifiedmns -> evo/cbtx", "evo/chainhelper -> evo/specialtxman -> validation -> evo/chainhelper", "evo/deterministicmns -> index/txindex -> validation -> evo/deterministicmns", "evo/deterministicmns -> index/txindex -> validation -> txmempool -> evo/deterministicmns", @@ -48,6 +46,8 @@ "evo/deterministicmns -> validationinterface -> governance/vote -> evo/deterministicmns", "evo/netinfo -> evo/providertx -> evo/netinfo", "evo/simplifiedmns -> llmq/blockprocessor -> llmq/utils -> llmq/snapshot -> evo/simplifiedmns", + "core_io -> evo/assetlocktx -> llmq/signing -> net_processing -> evo/simplifiedmns -> core_io", + "evo/cbtx -> llmq/chainlocks -> llmq/instantsend -> net_processing -> evo/simplifiedmns -> evo/cbtx", "evo/specialtxman -> validation -> evo/specialtxman", "governance/governance -> governance/object -> governance/governance", "governance/governance -> masternode/sync -> governance/governance", From c873e55c988dbe2e866e71ad4b80f74b9ec1724a Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 20 Jun 2025 20:24:55 +0700 Subject: [PATCH 3/8] refactor: move CL code validation to break circular dependencies over CbTx --- src/evo/cbtx.cpp | 124 ------------------------ src/evo/cbtx.h | 6 -- src/evo/specialtxman.cpp | 70 +++++++++++++ src/node/miner.cpp | 60 ++++++++++++ test/lint/lint-circular-dependencies.py | 1 - 5 files changed, 130 insertions(+), 131 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index bcc23a755fb2..0ea054809917 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include #include @@ -217,129 +216,6 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre return true; } -bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, - const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state) -{ - if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) { - return true; - } - - static Mutex cached_mutex; - static const CBlockIndex* cached_pindex GUARDED_BY(cached_mutex){nullptr}; - static std::optional> cached_chainlock GUARDED_BY(cached_mutex){std::nullopt}; - - auto best_clsig = chainlock_handler.GetBestChainLock(); - if (best_clsig.getHeight() == pindex->nHeight - 1 && cbTx.bestCLHeightDiff == 0 && cbTx.bestCLSignature == best_clsig.getSig()) { - // matches our best clsig which still hold values for the previous block - LOCK(cached_mutex); - cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff); - cached_pindex = pindex; - return true; - } - - std::optional> prevBlockCoinbaseChainlock{std::nullopt}; - if (LOCK(cached_mutex); cached_pindex == pindex->pprev) { - prevBlockCoinbaseChainlock = cached_chainlock; - } - if (!prevBlockCoinbaseChainlock.has_value()) { - prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex->pprev); - } - // If std::optional prevBlockCoinbaseChainlock is empty, then up to the previous block, coinbase Chainlock is null. - if (prevBlockCoinbaseChainlock.has_value()) { - // Previous block Coinbase has a non-null Chainlock: current block's Chainlock must be non-null and at least as new as the previous one - if (!cbTx.bestCLSignature.IsValid()) { - // IsNull() doesn't exist for CBLSSignature: we assume that a non valid BLS sig is null - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-null-clsig"); - } - if (cbTx.bestCLHeightDiff > prevBlockCoinbaseChainlock.value().second + 1) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig"); - } - } - - // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null - if (cbTx.bestCLSignature.IsValid()) { - int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; - if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == cbTx.bestCLSignature) { - // matches our best (but outdated) clsig, no need to verify it again - LOCK(cached_mutex); - cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff); - cached_pindex = pindex; - return true; - } - uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash(); - if (chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature)) != llmq::VerifyRecSigStatus::Valid) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); - } - LOCK(cached_mutex); - cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff); - cached_pindex = pindex; - } else if (cbTx.bestCLHeightDiff != 0) { - // Null bestCLSignature is allowed only with bestCLHeightDiff = 0 - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); - } - - return true; -} - -bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev, - uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature) -{ - auto best_clsig = chainlock_handler.GetBestChainLock(); - if (best_clsig.getHeight() < Params().GetConsensus().DeploymentHeight(Consensus::DEPLOYMENT_V19)) { - // We don't want legacy BLS ChainLocks in CbTx (can happen on regtest/devenets) - best_clsig = llmq::CChainLockSig{}; - } - if (best_clsig.getHeight() == pindexPrev->nHeight) { - // Our best CL is the newest one possible - bestCLHeightDiff = 0; - bestCLSignature = best_clsig.getSig(); - return true; - } - - auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindexPrev); - if (prevBlockCoinbaseChainlock.has_value()) { - // Previous block Coinbase contains a non-null CL: We must insert the same sig or a better (newest) one - if (best_clsig.IsNull()) { - // We don't know any CL, therefore inserting the CL of the previous block - bestCLHeightDiff = prevBlockCoinbaseChainlock->second + 1; - bestCLSignature = prevBlockCoinbaseChainlock->first; - return true; - } - - // We check if our best CL is newer than the one from previous block Coinbase - int curCLHeight = best_clsig.getHeight(); - int prevCLHeight = pindexPrev->nHeight - static_cast(prevBlockCoinbaseChainlock->second) - 1; - if (curCLHeight < prevCLHeight) { - // Our best CL isn't newer: inserting CL from previous block - bestCLHeightDiff = prevBlockCoinbaseChainlock->second + 1; - bestCLSignature = prevBlockCoinbaseChainlock->first; - } - else { - // Our best CL is newer - bestCLHeightDiff = pindexPrev->nHeight - best_clsig.getHeight(); - bestCLSignature = best_clsig.getSig(); - } - - return true; - } - else { - // Previous block Coinbase has no CL. We can either insert null or any valid CL - if (best_clsig.IsNull()) { - // We don't know any CL, therefore inserting a null CL - bestCLHeightDiff = 0; - bestCLSignature.Reset(); - return false; - } - - // Inserting our best CL - bestCLHeightDiff = pindexPrev->nHeight - best_clsig.getHeight(); - bestCLSignature = chainlock_handler.GetBestChainLock().getSig(); - - return true; - } -} - - std::string CCbTx::ToString() const { return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s, creditPoolBalance=%d.%08d)", diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index 9a80fef2ddf2..32913eb89780 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -16,7 +16,6 @@ class CBlock; class CBlockIndex; class TxValidationState; namespace llmq { -class CChainLocksHandler; class CQuorumBlockProcessor; }// namespace llmq @@ -71,11 +70,6 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet, BlockValidationState& state); -bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindexPrev, - const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state); -bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev, - uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature); - std::optional> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex); #endif // BITCOIN_EVO_CBTX_H diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 6fbafe5c3495..721462572a22 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -17,10 +17,80 @@ #include #include #include +#include +#include #include +#include #include #include +static bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, + const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state) +{ + if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) { + return true; + } + + static Mutex cached_mutex; + static const CBlockIndex* cached_pindex GUARDED_BY(cached_mutex){nullptr}; + static std::optional> cached_chainlock GUARDED_BY(cached_mutex){std::nullopt}; + + auto best_clsig = chainlock_handler.GetBestChainLock(); + if (best_clsig.getHeight() == pindex->nHeight - 1 && cbTx.bestCLHeightDiff == 0 && + cbTx.bestCLSignature == best_clsig.getSig()) { + // matches our best clsig which still hold values for the previous block + LOCK(cached_mutex); + cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff); + cached_pindex = pindex; + return true; + } + + std::optional> prevBlockCoinbaseChainlock{std::nullopt}; + if (LOCK(cached_mutex); cached_pindex == pindex->pprev) { + prevBlockCoinbaseChainlock = cached_chainlock; + } + if (!prevBlockCoinbaseChainlock.has_value()) { + prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex->pprev); + } + // If std::optional prevBlockCoinbaseChainlock is empty, then up to the previous block, coinbase Chainlock is null. + if (prevBlockCoinbaseChainlock.has_value()) { + // Previous block Coinbase has a non-null Chainlock: current block's Chainlock must be non-null and at least as new as the previous one + if (!cbTx.bestCLSignature.IsValid()) { + // IsNull() doesn't exist for CBLSSignature: we assume that a non valid BLS sig is null + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-null-clsig"); + } + if (cbTx.bestCLHeightDiff > prevBlockCoinbaseChainlock.value().second + 1) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig"); + } + } + + // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null + if (cbTx.bestCLSignature.IsValid()) { + int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; + if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == cbTx.bestCLSignature) { + // matches our best (but outdated) clsig, no need to verify it again + LOCK(cached_mutex); + cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff); + cached_pindex = pindex; + return true; + } + uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash(); + if (chainlock_handler.VerifyChainLock( + llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature)) != + llmq::VerifyRecSigStatus::Valid) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); + } + LOCK(cached_mutex); + cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff); + cached_pindex = pindex; + } else if (cbTx.bestCLHeightDiff != 0) { + // Null bestCLSignature is allowed only with bestCLHeightDiff = 0 + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); + } + + return true; +} + static bool CheckSpecialTxInner(CDeterministicMNManager& dmnman, llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman, const llmq::CQuorumManager& qman, const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, diff --git a/src/node/miner.cpp b/src/node/miner.cpp index ad210a3d4c8d..754e94b72486 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -121,6 +121,66 @@ void BlockAssembler::resetBlock() nFees = 0; } +// Helper to calculate best chainlock +static bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev, + uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature) +{ + auto best_clsig = chainlock_handler.GetBestChainLock(); + if (best_clsig.getHeight() < Params().GetConsensus().DeploymentHeight(Consensus::DEPLOYMENT_V19)) { + // We don't want legacy BLS ChainLocks in CbTx (can happen on regtest/devenets) + best_clsig = llmq::CChainLockSig{}; + } + if (best_clsig.getHeight() == pindexPrev->nHeight) { + // Our best CL is the newest one possible + bestCLHeightDiff = 0; + bestCLSignature = best_clsig.getSig(); + return true; + } + + auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindexPrev); + if (prevBlockCoinbaseChainlock.has_value()) { + // Previous block Coinbase contains a non-null CL: We must insert the same sig or a better (newest) one + if (best_clsig.IsNull()) { + // We don't know any CL, therefore inserting the CL of the previous block + bestCLHeightDiff = prevBlockCoinbaseChainlock->second + 1; + bestCLSignature = prevBlockCoinbaseChainlock->first; + return true; + } + + // We check if our best CL is newer than the one from previous block Coinbase + int curCLHeight = best_clsig.getHeight(); + int prevCLHeight = pindexPrev->nHeight - static_cast(prevBlockCoinbaseChainlock->second) - 1; + if (curCLHeight < prevCLHeight) { + // Our best CL isn't newer: inserting CL from previous block + bestCLHeightDiff = prevBlockCoinbaseChainlock->second + 1; + bestCLSignature = prevBlockCoinbaseChainlock->first; + } + else { + // Our best CL is newer + bestCLHeightDiff = pindexPrev->nHeight - best_clsig.getHeight(); + bestCLSignature = best_clsig.getSig(); + } + + return true; + } + else { + // Previous block Coinbase has no CL. We can either insert null or any valid CL + if (best_clsig.IsNull()) { + // We don't know any CL, therefore inserting a null CL + bestCLHeightDiff = 0; + bestCLSignature.Reset(); + return false; + } + + // Inserting our best CL + bestCLHeightDiff = pindexPrev->nHeight - best_clsig.getHeight(); + bestCLSignature = chainlock_handler.GetBestChainLock().getSig(); + + return true; + } +} + + std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) { int64_t nTimeStart = GetTimeMicros(); diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 2a385a1d6343..329c57c99749 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -47,7 +47,6 @@ "evo/netinfo -> evo/providertx -> evo/netinfo", "evo/simplifiedmns -> llmq/blockprocessor -> llmq/utils -> llmq/snapshot -> evo/simplifiedmns", "core_io -> evo/assetlocktx -> llmq/signing -> net_processing -> evo/simplifiedmns -> core_io", - "evo/cbtx -> llmq/chainlocks -> llmq/instantsend -> net_processing -> evo/simplifiedmns -> evo/cbtx", "evo/specialtxman -> validation -> evo/specialtxman", "governance/governance -> governance/object -> governance/governance", "governance/governance -> masternode/sync -> governance/governance", From a20e1a064bf04094f07200ad8efef7d0dc257fb4 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 21 Jun 2025 02:47:19 +0700 Subject: [PATCH 4/8] refactor: drop unused arguments from CDeterministicMNManager::ProcessBlock --- src/evo/deterministicmns.cpp | 3 +-- src/evo/deterministicmns.h | 1 - src/evo/specialtxman.cpp | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 7eecfd6fbbb2..4270310d69c8 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -590,8 +590,7 @@ void CDeterministicMNList::RemoveMN(const uint256& proTxHash) } bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null pindex, - BlockValidationState& state, const CCoinsViewCache& view, - llmq::CQuorumSnapshotManager& qsnapman, const CDeterministicMNList& newList, + BlockValidationState& state, const CDeterministicMNList& newList, std::optional& updatesRet) { AssertLockHeld(::cs_main); diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index 3b0eee1bfbba..1721813b913f 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -563,7 +563,6 @@ class CDeterministicMNManager ~CDeterministicMNManager() = default; bool ProcessBlock(const CBlock& block, gsl::not_null pindex, BlockValidationState& state, - const CCoinsViewCache& view, llmq::CQuorumSnapshotManager& qsnapman, const CDeterministicMNList& newList, std::optional& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(!cs, ::cs_main); bool UndoBlock(gsl::not_null pindex, std::optional& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(!cs); diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 721462572a22..853fdd9dc7b5 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -258,7 +258,7 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB } mn_list.SetBlockHash(pindex->GetBlockHash()); - if (!fJustCheck && !m_dmnman.ProcessBlock(block, pindex, state, view, m_qsnapman, mn_list, updatesRet)) { + if (!fJustCheck && !m_dmnman.ProcessBlock(block, pindex, state, mn_list, updatesRet)) { // pass the state returned by the function above return false; } From 8e8d6ebd782a5352706b86f926834c692cfb0ac6 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 21 Jun 2025 13:04:44 +0700 Subject: [PATCH 5/8] refactor: remove dependency of evo/deterministicmns on llmq/utils and llmq/commitment --- src/evo/deterministicmns.cpp | 334 ----------------------- src/evo/deterministicmns.h | 12 - src/evo/specialtxman.cpp | 339 +++++++++++++++++++++++- src/evo/specialtxman.h | 8 +- src/node/miner.cpp | 5 +- src/node/miner.h | 4 - src/test/util/setup_common.cpp | 4 +- test/lint/lint-circular-dependencies.py | 4 - 8 files changed, 349 insertions(+), 361 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 4270310d69c8..508a302896db 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -9,8 +9,6 @@ #include #include #include -#include -#include #include #include @@ -696,338 +694,6 @@ void CDeterministicMNManager::UpdatedBlockTip(gsl::not_null tipIndex = pindex; } -bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, gsl::not_null pindexPrev, - BlockValidationState& state, const CCoinsViewCache& view, - CDeterministicMNList& mnListRet, - llmq::CQuorumSnapshotManager& qsnapman, bool debugLogs) -{ - int nHeight = pindexPrev->nHeight + 1; - - CDeterministicMNList oldList = GetListForBlock(pindexPrev); - CDeterministicMNList newList = oldList; - newList.SetBlockHash(uint256()); // we can't know the final block hash, so better not return a (invalid) block hash - newList.SetHeight(nHeight); - - auto payee = oldList.GetMNPayee(pindexPrev); - - // we iterate the oldList here and update the newList - // this is only valid as long these have not diverged at this point, which is the case as long as we don't add - // code above this loop that modifies newList - oldList.ForEachMN(false, [&pindexPrev, &newList](auto& dmn) { - if (!dmn.pdmnState->confirmedHash.IsNull()) { - // already confirmed - return; - } - // this works on the previous block, so confirmation will happen one block after nMasternodeMinimumConfirmations - // has been reached, but the block hash will then point to the block at nMasternodeMinimumConfirmations - int nConfirmations = pindexPrev->nHeight - dmn.pdmnState->nRegisteredHeight; - if (nConfirmations >= Params().GetConsensus().nMasternodeMinimumConfirmations) { - auto newState = std::make_shared(*dmn.pdmnState); - newState->UpdateConfirmedHash(dmn.proTxHash, pindexPrev->GetBlockHash()); - newList.UpdateMN(dmn.proTxHash, newState); - } - }); - - newList.DecreaseScores(); - - const bool isMNRewardReallocation{DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR)}; - const bool is_v23_deployed{DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V23)}; - - // we skip the coinbase - for (int i = 1; i < (int)block.vtx.size(); i++) { - const CTransaction& tx = *block.vtx[i]; - - if (!tx.IsSpecialTxVersion()) { - // only interested in special TXs - continue; - } - - if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); - } - auto& proTx = *opt_proTx; - - auto dmn = std::make_shared(newList.GetTotalRegisteredCount(), proTx.nType); - dmn->proTxHash = tx.GetHash(); - - // collateralOutpoint is either pointing to an external collateral or to the ProRegTx itself - if (proTx.collateralOutpoint.hash.IsNull()) { - dmn->collateralOutpoint = COutPoint(tx.GetHash(), proTx.collateralOutpoint.n); - } else { - dmn->collateralOutpoint = proTx.collateralOutpoint; - } - - Coin coin; - CAmount expectedCollateral = GetMnType(proTx.nType).collat_amount; - if (!proTx.collateralOutpoint.hash.IsNull() && (!view.GetCoin(dmn->collateralOutpoint, coin) || coin.IsSpent() || coin.out.nValue != expectedCollateral)) { - // should actually never get to this point as CheckProRegTx should have handled this case. - // We do this additional check nevertheless to be 100% sure - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-collateral"); - } - - auto replacedDmn = newList.GetMNByCollateral(dmn->collateralOutpoint); - if (replacedDmn != nullptr) { - // This might only happen with a ProRegTx that refers an external collateral - // In that case the new ProRegTx will replace the old one. This means the old one is removed - // and the new one is added like a completely fresh one, which is also at the bottom of the payment list - newList.RemoveMN(replacedDmn->proTxHash); - if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s removed from list because collateral was used for a new ProRegTx. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", - __func__, replacedDmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), nHeight, newList.GetAllMNsCount()); - } - } - - for (const NetInfoEntry& entry : proTx.netInfo->GetEntries()) { - if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { - const CService& service{service_opt.value()}; - if (newList.HasUniqueProperty(service)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); - } - } else { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); - } - } - if (newList.HasUniqueProperty(proTx.keyIDOwner) || newList.HasUniqueProperty(proTx.pubKeyOperator)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); - } - - dmn->nOperatorReward = proTx.nOperatorReward; - - auto dmnState = std::make_shared(proTx); - dmnState->nRegisteredHeight = nHeight; - if (proTx.netInfo->IsEmpty()) { - // start in banned pdmnState as we need to wait for a ProUpServTx - dmnState->BanIfNotBanned(nHeight); - } - dmn->pdmnState = dmnState; - - newList.AddMN(dmn); - - if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s added at height %d: %s\n", - __func__, tx.GetHash().ToString(), nHeight, proTx.ToString()); - } - } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); - } - - for (const NetInfoEntry& entry : opt_proTx->netInfo->GetEntries()) { - if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { - const CService& service{service_opt.value()}; - if (newList.HasUniqueProperty(service) && - newList.GetUniquePropertyMN(service)->proTxHash != opt_proTx->proTxHash) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); - } - } else { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); - } - } - - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); - } - if (opt_proTx->nType != dmn->nType) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type-mismatch"); - } - if (!IsValidMnType(opt_proTx->nType)) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type"); - } - - auto newState = std::make_shared(*dmn->pdmnState); - if (is_v23_deployed) { - // Extended addresses support in v23 means that the version can be updated - newState->nVersion = opt_proTx->nVersion; - } - newState->netInfo = opt_proTx->netInfo; - newState->scriptOperatorPayout = opt_proTx->scriptOperatorPayout; - if (opt_proTx->nType == MnType::Evo) { - newState->platformNodeID = opt_proTx->platformNodeID; - newState->platformP2PPort = opt_proTx->platformP2PPort; - newState->platformHTTPPort = opt_proTx->platformHTTPPort; - } - if (newState->IsBanned()) { - // only revive when all keys are set - if (newState->pubKeyOperator != CBLSLazyPublicKey() && !newState->keyIDVoting.IsNull() && - !newState->keyIDOwner.IsNull()) { - newState->Revive(nHeight); - if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s revived at height %d\n", - __func__, opt_proTx->proTxHash.ToString(), nHeight); - } - } - } - - newList.UpdateMN(opt_proTx->proTxHash, newState); - if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s updated at height %d: %s\n", - __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); - } - } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); - } - - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); - } - auto newState = std::make_shared(*dmn->pdmnState); - if (newState->pubKeyOperator != opt_proTx->pubKeyOperator) { - // reset all operator related fields and put MN into PoSe-banned state in case the operator key changes - newState->ResetOperatorFields(); - newState->BanIfNotBanned(nHeight); - // we update pubKeyOperator here, make sure state version matches - // Make sure we don't accidentally downgrade the state version if using version after basic BLS - newState->nVersion = newState->nVersion > ProTxVersion::BasicBLS ? newState->nVersion : opt_proTx->nVersion; - newState->netInfo = NetInfoInterface::MakeNetInfo(newState->nVersion); - newState->pubKeyOperator = opt_proTx->pubKeyOperator; - } - newState->keyIDVoting = opt_proTx->keyIDVoting; - newState->scriptPayout = opt_proTx->scriptPayout; - - newList.UpdateMN(opt_proTx->proTxHash, newState); - - if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s updated at height %d: %s\n", - __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); - } - } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { - const auto opt_proTx = GetTxPayload(tx); - if (!opt_proTx) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); - } - - auto dmn = newList.GetMN(opt_proTx->proTxHash); - if (!dmn) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); - } - auto newState = std::make_shared(*dmn->pdmnState); - newState->ResetOperatorFields(); - newState->BanIfNotBanned(nHeight); - newState->nRevocationReason = opt_proTx->nReason; - - newList.UpdateMN(opt_proTx->proTxHash, newState); - - if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s revoked operator key at height %d: %s\n", - __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); - } - } else if (tx.nType == TRANSACTION_QUORUM_COMMITMENT) { - const auto opt_qc = GetTxPayload(tx); - if (!opt_qc) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-payload"); - } - if (!opt_qc->commitment.IsNull()) { - const auto& llmq_params_opt = Params().GetLLMQ(opt_qc->commitment.llmqType); - if (!llmq_params_opt.has_value()) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-commitment-type"); - } - int qcnHeight = int(opt_qc->nHeight); - int quorumHeight = qcnHeight - (qcnHeight % llmq_params_opt->dkgInterval) + int(opt_qc->commitment.quorumIndex); - auto pQuorumBaseBlockIndex = pindexPrev->GetAncestor(quorumHeight); - if (!pQuorumBaseBlockIndex || pQuorumBaseBlockIndex->GetBlockHash() != opt_qc->commitment.quorumHash) { - // we should actually never get into this case as validation should have caught it...but let's be sure - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-quorum-hash"); - } - - HandleQuorumCommitment(opt_qc->commitment, pQuorumBaseBlockIndex, newList, qsnapman, debugLogs); - } - } - } - - // we skip the coinbase - for (int i = 1; i < (int)block.vtx.size(); i++) { - const CTransaction& tx = *block.vtx[i]; - - // check if any existing MN collateral is spent by this transaction - for (const auto& in : tx.vin) { - auto dmn = newList.GetMNByCollateral(in.prevout); - if (dmn && dmn->collateralOutpoint == in.prevout) { - newList.RemoveMN(dmn->proTxHash); - - if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s removed from list because collateral was spent. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", - __func__, dmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), nHeight, newList.GetAllMNsCount()); - } - } - } - } - - // The payee for the current block was determined by the previous block's list, but it might have disappeared in the - // current block. We still pay that MN one last time, however. - if (auto dmn = payee ? newList.GetMN(payee->proTxHash) : nullptr) { - auto newState = std::make_shared(*dmn->pdmnState); - newState->nLastPaidHeight = nHeight; - // Starting from v19 and until MNRewardReallocation, EvoNodes will be paid 4 blocks in a row - // No need to check if v19 is active, since EvoNode ProRegTxes are allowed only after v19 activation - // Note: If the payee wasn't found in the current block that's fine - if (dmn->nType == MnType::Evo && !isMNRewardReallocation) { - ++newState->nConsecutivePayments; - if (debugLogs) { - LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s is an EvoNode, bumping nConsecutivePayments to %d\n", - __func__, dmn->proTxHash.ToString(), newState->nConsecutivePayments); - } - } - newList.UpdateMN(payee->proTxHash, newState); - if (debugLogs) { - dmn = newList.GetMN(payee->proTxHash); - // Since the previous GetMN query returned a value, after an update, querying the same - // hash *must* give us a result. If it doesn't, that would be a potential logic bug. - assert(dmn); - LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s, nConsecutivePayments=%d\n", - __func__, dmn->proTxHash.ToString(), dmn->pdmnState->nConsecutivePayments); - } - } - - // reset nConsecutivePayments on non-paid EvoNodes - auto newList2 = newList; - newList2.ForEachMN(false, [&](auto& dmn) { - if (dmn.nType != MnType::Evo) return; - if (payee != nullptr && dmn.proTxHash == payee->proTxHash && !isMNRewardReallocation) return; - if (dmn.pdmnState->nConsecutivePayments == 0) return; - if (debugLogs) { - LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s, reset nConsecutivePayments %d->0\n", - __func__, dmn.proTxHash.ToString(), dmn.pdmnState->nConsecutivePayments); - } - auto newState = std::make_shared(*dmn.pdmnState); - newState->nConsecutivePayments = 0; - newList.UpdateMN(dmn.proTxHash, newState); - }); - - mnListRet = std::move(newList); - - return true; -} - -void CDeterministicMNManager::HandleQuorumCommitment(const llmq::CFinalCommitment& qc, - gsl::not_null pQuorumBaseBlockIndex, - CDeterministicMNList& mnList, - llmq::CQuorumSnapshotManager& qsnapman, bool debugLogs) -{ - // The commitment has already been validated at this point, so it's safe to use members of it - - auto members = llmq::utils::GetAllQuorumMembers(qc.llmqType, *this, qsnapman, pQuorumBaseBlockIndex); - - for (size_t i = 0; i < members.size(); i++) { - if (!mnList.HasMN(members[i]->proTxHash)) { - continue; - } - if (!qc.validMembers[i]) { - // punish MN for failed DKG participation - // The idea is to immediately ban a MN when it fails 2 DKG sessions with only a few blocks in-between - // If there were enough blocks between failures, the MN has a chance to recover as he reduces his penalty by 1 for every block - // If it however fails 3 times in the timespan of a single payment cycle, it should definitely get banned - mnList.PoSePunish(members[i]->proTxHash, mnList.CalcPenalty(66), debugLogs); - } - } -} - CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_null pindex) { CDeterministicMNList snapshot; diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index 1721813b913f..6987d8448b8d 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -34,11 +34,6 @@ class TxValidationState; extern RecursiveMutex cs_main; -namespace llmq { -class CFinalCommitment; -class CQuorumSnapshotManager; -} // namespace llmq - class CDeterministicMN { private: @@ -569,13 +564,6 @@ class CDeterministicMNManager void UpdatedBlockTip(gsl::not_null pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs); - // the returned list will not contain the correct block hash (we can't know it yet as the coinbase TX is not updated yet) - bool BuildNewListFromBlock(const CBlock& block, gsl::not_null pindexPrev, - BlockValidationState& state, const CCoinsViewCache& view, CDeterministicMNList& mnListRet, - llmq::CQuorumSnapshotManager& qsnapman, bool debugLogs) EXCLUSIVE_LOCKS_REQUIRED(!cs); - void HandleQuorumCommitment(const llmq::CFinalCommitment& qc, gsl::not_null pQuorumBaseBlockIndex, - CDeterministicMNList& mnList, llmq::CQuorumSnapshotManager& qsnapman, bool debugLogs); - CDeterministicMNList GetListForBlock(gsl::not_null pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); return GetListForBlockInternal(pindex); diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 853fdd9dc7b5..c0b954b682b6 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -21,7 +21,9 @@ #include #include #include +#include #include +#include #include static bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, @@ -151,6 +153,339 @@ bool CSpecialTxProcessor::CheckSpecialTx(const CTransaction& tx, const CBlockInd state); } +static void HandleQuorumCommitment(const llmq::CFinalCommitment& qc, + const std::vector& members, + bool debugLogs, CDeterministicMNList& mnList) +{ + for (size_t i = 0; i < members.size(); i++) { + if (!mnList.HasMN(members[i]->proTxHash)) { + continue; + } + if (!qc.validMembers[i]) { + // punish MN for failed DKG participation + // The idea is to immediately ban a MN when it fails 2 DKG sessions with only a few blocks in-between + // If there were enough blocks between failures, the MN has a chance to recover as he reduces his penalty by 1 for every block + // If it however fails 3 times in the timespan of a single payment cycle, it should definitely get banned + mnList.PoSePunish(members[i]->proTxHash, mnList.CalcPenalty(66), debugLogs); + } + } +} + +bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_null pindexPrev, + const CCoinsViewCache& view, + bool debugLogs, + BlockValidationState& state, + CDeterministicMNList& mnListRet) +{ + AssertLockHeld(cs_main); + + int nHeight = pindexPrev->nHeight + 1; + + CDeterministicMNList oldList = m_dmnman.GetListForBlock(pindexPrev); + CDeterministicMNList newList = oldList; + newList.SetBlockHash(uint256()); // we can't know the final block hash, so better not return a (invalid) block hash + newList.SetHeight(nHeight); + + auto payee = oldList.GetMNPayee(pindexPrev); + + // we iterate the oldList here and update the newList + // this is only valid as long these have not diverged at this point, which is the case as long as we don't add + // code above this loop that modifies newList + oldList.ForEachMN(false, [&pindexPrev, &newList, this](auto& dmn) { + if (!dmn.pdmnState->confirmedHash.IsNull()) { + // already confirmed + return; + } + // this works on the previous block, so confirmation will happen one block after nMasternodeMinimumConfirmations + // has been reached, but the block hash will then point to the block at nMasternodeMinimumConfirmations + int nConfirmations = pindexPrev->nHeight - dmn.pdmnState->nRegisteredHeight; + if (nConfirmations >= this->m_consensus_params.nMasternodeMinimumConfirmations) { + auto newState = std::make_shared(*dmn.pdmnState); + newState->UpdateConfirmedHash(dmn.proTxHash, pindexPrev->GetBlockHash()); + newList.UpdateMN(dmn.proTxHash, newState); + } + }); + + newList.DecreaseScores(); + + const bool isMNRewardReallocation{DeploymentActiveAfter(pindexPrev, m_consensus_params, Consensus::DEPLOYMENT_MN_RR)}; + const bool is_v23_deployed{DeploymentActiveAfter(pindexPrev, m_consensus_params, Consensus::DEPLOYMENT_V23)}; + + // we skip the coinbase + for (int i = 1; i < (int)block.vtx.size(); i++) { + const CTransaction& tx = *block.vtx[i]; + + if (!tx.IsSpecialTxVersion()) { + // only interested in special TXs + continue; + } + + if (tx.nType == TRANSACTION_PROVIDER_REGISTER) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + } + auto& proTx = *opt_proTx; + + auto dmn = std::make_shared(newList.GetTotalRegisteredCount(), proTx.nType); + dmn->proTxHash = tx.GetHash(); + + // collateralOutpoint is either pointing to an external collateral or to the ProRegTx itself + if (proTx.collateralOutpoint.hash.IsNull()) { + dmn->collateralOutpoint = COutPoint(tx.GetHash(), proTx.collateralOutpoint.n); + } else { + dmn->collateralOutpoint = proTx.collateralOutpoint; + } + + Coin coin; + CAmount expectedCollateral = GetMnType(proTx.nType).collat_amount; + if (!proTx.collateralOutpoint.hash.IsNull() && (!view.GetCoin(dmn->collateralOutpoint, coin) || coin.IsSpent() || coin.out.nValue != expectedCollateral)) { + // should actually never get to this point as CheckProRegTx should have handled this case. + // We do this additional check nevertheless to be 100% sure + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-collateral"); + } + + auto replacedDmn = newList.GetMNByCollateral(dmn->collateralOutpoint); + if (replacedDmn != nullptr) { + // This might only happen with a ProRegTx that refers an external collateral + // In that case the new ProRegTx will replace the old one. This means the old one is removed + // and the new one is added like a completely fresh one, which is also at the bottom of the payment list + newList.RemoveMN(replacedDmn->proTxHash); + if (debugLogs) { + LogPrintf("CDeterministicMNManager::%s -- MN %s removed from list because collateral was used for a new ProRegTx. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", + __func__, replacedDmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), nHeight, newList.GetAllMNsCount()); + } + } + + for (const NetInfoEntry& entry : proTx.netInfo->GetEntries()) { + if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { + const CService& service{service_opt.value()}; + if (newList.HasUniqueProperty(service)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); + } + } else { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); + } + } + if (newList.HasUniqueProperty(proTx.keyIDOwner) || newList.HasUniqueProperty(proTx.pubKeyOperator)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-key"); + } + + dmn->nOperatorReward = proTx.nOperatorReward; + + auto dmnState = std::make_shared(proTx); + dmnState->nRegisteredHeight = nHeight; + if (proTx.netInfo->IsEmpty()) { + // start in banned pdmnState as we need to wait for a ProUpServTx + dmnState->BanIfNotBanned(nHeight); + } + dmn->pdmnState = dmnState; + + newList.AddMN(dmn); + + if (debugLogs) { + LogPrintf("CDeterministicMNManager::%s -- MN %s added at height %d: %s\n", + __func__, tx.GetHash().ToString(), nHeight, proTx.ToString()); + } + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + } + + for (const NetInfoEntry& entry : opt_proTx->netInfo->GetEntries()) { + if (const auto& service_opt{entry.GetAddrPort()}; service_opt.has_value()) { + const CService& service{service_opt.value()}; + if (newList.HasUniqueProperty(service) && + newList.GetUniquePropertyMN(service)->proTxHash != opt_proTx->proTxHash) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-dup-netinfo-entry"); + } + } else { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-netinfo-entry"); + } + } + + auto dmn = newList.GetMN(opt_proTx->proTxHash); + if (!dmn) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); + } + if (opt_proTx->nType != dmn->nType) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type-mismatch"); + } + if (!IsValidMnType(opt_proTx->nType)) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-type"); + } + + auto newState = std::make_shared(*dmn->pdmnState); + if (is_v23_deployed) { + // Extended addresses support in v23 means that the version can be updated + newState->nVersion = opt_proTx->nVersion; + } + newState->netInfo = opt_proTx->netInfo; + newState->scriptOperatorPayout = opt_proTx->scriptOperatorPayout; + if (opt_proTx->nType == MnType::Evo) { + newState->platformNodeID = opt_proTx->platformNodeID; + newState->platformP2PPort = opt_proTx->platformP2PPort; + newState->platformHTTPPort = opt_proTx->platformHTTPPort; + } + if (newState->IsBanned()) { + // only revive when all keys are set + if (newState->pubKeyOperator != CBLSLazyPublicKey() && !newState->keyIDVoting.IsNull() && + !newState->keyIDOwner.IsNull()) { + newState->Revive(nHeight); + if (debugLogs) { + LogPrintf("CDeterministicMNManager::%s -- MN %s revived at height %d\n", + __func__, opt_proTx->proTxHash.ToString(), nHeight); + } + } + } + + newList.UpdateMN(opt_proTx->proTxHash, newState); + if (debugLogs) { + LogPrintf("CDeterministicMNManager::%s -- MN %s updated at height %d: %s\n", + __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); + } + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + } + + auto dmn = newList.GetMN(opt_proTx->proTxHash); + if (!dmn) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); + } + auto newState = std::make_shared(*dmn->pdmnState); + if (newState->pubKeyOperator != opt_proTx->pubKeyOperator) { + // reset all operator related fields and put MN into PoSe-banned state in case the operator key changes + newState->ResetOperatorFields(); + newState->BanIfNotBanned(nHeight); + // we update pubKeyOperator here, make sure state version matches + // Make sure we don't accidentally downgrade the state version if using version after basic BLS + newState->nVersion = newState->nVersion > ProTxVersion::BasicBLS ? newState->nVersion : opt_proTx->nVersion; + newState->netInfo = NetInfoInterface::MakeNetInfo(newState->nVersion); + newState->pubKeyOperator = opt_proTx->pubKeyOperator; + } + newState->keyIDVoting = opt_proTx->keyIDVoting; + newState->scriptPayout = opt_proTx->scriptPayout; + + newList.UpdateMN(opt_proTx->proTxHash, newState); + + if (debugLogs) { + LogPrintf("CDeterministicMNManager::%s -- MN %s updated at height %d: %s\n", + __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); + } + } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { + const auto opt_proTx = GetTxPayload(tx); + if (!opt_proTx) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-payload"); + } + + auto dmn = newList.GetMN(opt_proTx->proTxHash); + if (!dmn) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-hash"); + } + auto newState = std::make_shared(*dmn->pdmnState); + newState->ResetOperatorFields(); + newState->BanIfNotBanned(nHeight); + newState->nRevocationReason = opt_proTx->nReason; + + newList.UpdateMN(opt_proTx->proTxHash, newState); + + if (debugLogs) { + LogPrintf("CDeterministicMNManager::%s -- MN %s revoked operator key at height %d: %s\n", + __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); + } + } else if (tx.nType == TRANSACTION_QUORUM_COMMITMENT) { + const auto opt_qc = GetTxPayload(tx); + if (!opt_qc) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-payload"); + } + if (!opt_qc->commitment.IsNull()) { + const auto& llmq_params_opt = Params().GetLLMQ(opt_qc->commitment.llmqType); + if (!llmq_params_opt.has_value()) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-commitment-type"); + } + int qcnHeight = int(opt_qc->nHeight); + int quorumHeight = qcnHeight - (qcnHeight % llmq_params_opt->dkgInterval) + int(opt_qc->commitment.quorumIndex); + auto pQuorumBaseBlockIndex = pindexPrev->GetAncestor(quorumHeight); + if (!pQuorumBaseBlockIndex || pQuorumBaseBlockIndex->GetBlockHash() != opt_qc->commitment.quorumHash) { + // we should actually never get into this case as validation should have caught it...but let's be sure + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-quorum-hash"); + } + + // The commitment has already been validated at this point, so it's safe to use members of it + + const auto members = llmq::utils::GetAllQuorumMembers(opt_qc->commitment.llmqType, m_dmnman, m_qsnapman, pQuorumBaseBlockIndex); + HandleQuorumCommitment(opt_qc->commitment, members, debugLogs, newList); + } + } + } + + // we skip the coinbase + for (int i = 1; i < (int)block.vtx.size(); i++) { + const CTransaction& tx = *block.vtx[i]; + + // check if any existing MN collateral is spent by this transaction + for (const auto& in : tx.vin) { + auto dmn = newList.GetMNByCollateral(in.prevout); + if (dmn && dmn->collateralOutpoint == in.prevout) { + newList.RemoveMN(dmn->proTxHash); + + if (debugLogs) { + LogPrintf("CDeterministicMNManager::%s -- MN %s removed from list because collateral was spent. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", + __func__, dmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), nHeight, newList.GetAllMNsCount()); + } + } + } + } + + // The payee for the current block was determined by the previous block's list, but it might have disappeared in the + // current block. We still pay that MN one last time, however. + if (auto dmn = payee ? newList.GetMN(payee->proTxHash) : nullptr) { + auto newState = std::make_shared(*dmn->pdmnState); + newState->nLastPaidHeight = nHeight; + // Starting from v19 and until MNRewardReallocation, EvoNodes will be paid 4 blocks in a row + // No need to check if v19 is active, since EvoNode ProRegTxes are allowed only after v19 activation + // Note: If the payee wasn't found in the current block that's fine + if (dmn->nType == MnType::Evo && !isMNRewardReallocation) { + ++newState->nConsecutivePayments; + if (debugLogs) { + LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s is an EvoNode, bumping nConsecutivePayments to %d\n", + __func__, dmn->proTxHash.ToString(), newState->nConsecutivePayments); + } + } + newList.UpdateMN(payee->proTxHash, newState); + if (debugLogs) { + dmn = newList.GetMN(payee->proTxHash); + // Since the previous GetMN query returned a value, after an update, querying the same + // hash *must* give us a result. If it doesn't, that would be a potential logic bug. + assert(dmn); + LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s, nConsecutivePayments=%d\n", + __func__, dmn->proTxHash.ToString(), dmn->pdmnState->nConsecutivePayments); + } + } + + // reset nConsecutivePayments on non-paid EvoNodes + auto newList2 = newList; + newList2.ForEachMN(false, [&](auto& dmn) { + if (dmn.nType != MnType::Evo) return; + if (payee != nullptr && dmn.proTxHash == payee->proTxHash && !isMNRewardReallocation) return; + if (dmn.pdmnState->nConsecutivePayments == 0) return; + if (debugLogs) { + LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s, reset nConsecutivePayments %d->0\n", + __func__, dmn.proTxHash.ToString(), dmn.pdmnState->nConsecutivePayments); + } + auto newState = std::make_shared(*dmn.pdmnState); + newState->nConsecutivePayments = 0; + newList.UpdateMN(dmn.proTxHash, newState); + }); + + mnListRet = std::move(newList); + + return true; +} + bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerkleRoots, BlockValidationState& state, std::optional& updatesRet) { @@ -250,9 +585,9 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB nTimeQuorum * 0.000001); - if (opt_cbTx.has_value()) { + if (opt_cbTx.has_value() && pindex->pprev) { CDeterministicMNList mn_list; - if (!m_dmnman.BuildNewListFromBlock(block, pindex->pprev, state, view, mn_list, m_qsnapman, true)) { + if (!BuildNewListFromBlock(block, pindex->pprev, view, true, state, mn_list)) { // pass the state returned by the function above return false; } diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index ac497f667432..933df4395181 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -7,7 +7,7 @@ #include #include - +#include #include class BlockValidationState; @@ -16,6 +16,7 @@ class CBlockIndex; class CCbTx; class CCoinsViewCache; class CCreditPoolManager; +class CDeterministicMNList; class CDeterministicMNManager; class CTransaction; class ChainstateManager; @@ -71,6 +72,11 @@ class CSpecialTxProcessor bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, std::optional& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + + // the returned list will not contain the correct block hash (we can't know it yet as the coinbase TX is not updated yet) + bool BuildNewListFromBlock(const CBlock& block, gsl::not_null pindexPrev, + const CCoinsViewCache& view, bool debugLogs, + BlockValidationState& state, CDeterministicMNList& mnListRet) EXCLUSIVE_LOCKS_REQUIRED(cs_main); private: bool CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const CCbTx& cbTx, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 754e94b72486..0d35e2cbd638 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -73,12 +74,10 @@ BlockAssembler::BlockAssembler(CChainState& chainstate, const NodeContext& node, m_cpoolman(*Assert(node.cpoolman)), m_chain_helper(chainstate.ChainHelper()), m_chainstate(chainstate), - m_dmnman(*Assert(node.dmnman)), m_evoDb(*Assert(node.evodb)), m_mnhfman(*Assert(node.mnhf_manager)), m_clhandler(*Assert(Assert(node.llmq_ctx)->clhandler)), m_isman(*Assert(Assert(node.llmq_ctx)->isman)), - m_qsnapman(*Assert(Assert(node.llmq_ctx)->qsnapman)), chainparams(params), m_mempool(mempool), m_quorum_block_processor(*Assert(Assert(node.llmq_ctx)->quorum_block_processor)), @@ -289,7 +288,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc BlockValidationState state; CDeterministicMNList mn_list; - if (!m_dmnman.BuildNewListFromBlock(*pblock, pindexPrev, state, m_chainstate.CoinsTip(), mn_list, m_qsnapman, true)) { + if (!m_chain_helper.special_tx->BuildNewListFromBlock(*pblock, pindexPrev, m_chainstate.CoinsTip(), true, state, mn_list)) { throw std::runtime_error(strprintf("%s: BuildNewListFromBlock failed: %s", __func__, state.ToString())); } if (!CalcCbTxMerkleRootMNList(cbTx.merkleRootMNList, CSimplifiedMNList(mn_list), state)) { diff --git a/src/node/miner.h b/src/node/miner.h index 96116c0345f1..188416d3ff44 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -24,7 +24,6 @@ class CChainParams; class CChainstateHelper; class CConnman; class CCreditPoolManager; -class CDeterministicMNManager; class CEvoDB; class CMNHFManager; class CScript; @@ -36,7 +35,6 @@ class CChainLocksHandler; class CInstantSendManager; class CQuorumBlockProcessor; class CQuorumManager; -class CQuorumSnapshotManager; } // namespace llmq namespace node { @@ -172,12 +170,10 @@ class BlockAssembler CCreditPoolManager& m_cpoolman; CChainstateHelper& m_chain_helper; CChainState& m_chainstate; - CDeterministicMNManager& m_dmnman; CEvoDB& m_evoDb; CMNHFManager& m_mnhfman; llmq::CChainLocksHandler& m_clhandler; llmq::CInstantSendManager& m_isman; - llmq::CQuorumSnapshotManager& m_qsnapman; const CChainParams& chainparams; const CTxMemPool* const m_mempool; const llmq::CQuorumBlockProcessor& m_quorum_block_processor; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 8e4b054caf3c..eeee3fe3844d 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -48,12 +48,14 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -503,7 +505,7 @@ CBlock TestChainSetup::CreateBlock( Assert(cbTx.has_value()); BlockValidationState state; CDeterministicMNList mn_list; - if (!m_node.dmnman->BuildNewListFromBlock(block, chainstate.m_chain.Tip(), state, chainstate.CoinsTip(), mn_list, *m_node.llmq_ctx->qsnapman, true)) { + if (!chainstate.ChainHelper().special_tx->BuildNewListFromBlock(block, chainstate.m_chain.Tip(), chainstate.CoinsTip(), true, state, mn_list)) { Assert(false); } if (!CalcCbTxMerkleRootMNList(cbTx->merkleRootMNList, CSimplifiedMNList(mn_list), state)) { diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 329c57c99749..c4481fae16bc 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -38,10 +38,6 @@ "evo/chainhelper -> evo/specialtxman -> validation -> evo/chainhelper", "evo/deterministicmns -> index/txindex -> validation -> evo/deterministicmns", "evo/deterministicmns -> index/txindex -> validation -> txmempool -> evo/deterministicmns", - "evo/deterministicmns -> llmq/commitment -> evo/deterministicmns", - "evo/deterministicmns -> llmq/utils -> evo/deterministicmns", - "evo/deterministicmns -> llmq/utils -> llmq/snapshot -> evo/simplifiedmns -> evo/deterministicmns", - "evo/deterministicmns -> llmq/utils -> net -> evo/deterministicmns", "evo/deterministicmns -> validationinterface -> evo/deterministicmns", "evo/deterministicmns -> validationinterface -> governance/vote -> evo/deterministicmns", "evo/netinfo -> evo/providertx -> evo/netinfo", From 51a83e7803005df0aca640c238ad74b399d7c1a2 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 21 Jun 2025 16:35:35 +0700 Subject: [PATCH 6/8] fix: clang formatting and adjusting logs for BuildNewListFromBlock --- src/evo/specialtxman.cpp | 61 +++++++++++++++++++++------------------- src/evo/specialtxman.h | 9 +++--- 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index c0b954b682b6..9efa6b5b62e3 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -153,8 +153,7 @@ bool CSpecialTxProcessor::CheckSpecialTx(const CTransaction& tx, const CBlockInd state); } -static void HandleQuorumCommitment(const llmq::CFinalCommitment& qc, - const std::vector& members, +static void HandleQuorumCommitment(const llmq::CFinalCommitment& qc, const std::vector& members, bool debugLogs, CDeterministicMNList& mnList) { for (size_t i = 0; i < members.size(); i++) { @@ -172,10 +171,8 @@ static void HandleQuorumCommitment(const llmq::CFinalCommitment& qc, } bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_null pindexPrev, - const CCoinsViewCache& view, - bool debugLogs, - BlockValidationState& state, - CDeterministicMNList& mnListRet) + const CCoinsViewCache& view, bool debugLogs, + BlockValidationState& state, CDeterministicMNList& mnListRet) { AssertLockHeld(cs_main); @@ -239,7 +236,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu Coin coin; CAmount expectedCollateral = GetMnType(proTx.nType).collat_amount; - if (!proTx.collateralOutpoint.hash.IsNull() && (!view.GetCoin(dmn->collateralOutpoint, coin) || coin.IsSpent() || coin.out.nValue != expectedCollateral)) { + if (!proTx.collateralOutpoint.hash.IsNull() && (!view.GetCoin(dmn->collateralOutpoint, coin) || + coin.IsSpent() || coin.out.nValue != expectedCollateral)) { // should actually never get to this point as CheckProRegTx should have handled this case. // We do this additional check nevertheless to be 100% sure return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-protx-collateral"); @@ -252,8 +250,10 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu // and the new one is added like a completely fresh one, which is also at the bottom of the payment list newList.RemoveMN(replacedDmn->proTxHash); if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s removed from list because collateral was used for a new ProRegTx. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", - __func__, replacedDmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), nHeight, newList.GetAllMNsCount()); + LogPrintf("%s -- MN %s removed from list because collateral was used for " /* Continued */ + "a new ProRegTx. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", + __func__, replacedDmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), + nHeight, newList.GetAllMNsCount()); } } @@ -284,8 +284,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu newList.AddMN(dmn); if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s added at height %d: %s\n", - __func__, tx.GetHash().ToString(), nHeight, proTx.ToString()); + LogPrintf("%s -- MN %s added at height %d: %s\n", __func__, tx.GetHash().ToString(), nHeight, + proTx.ToString()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) { const auto opt_proTx = GetTxPayload(tx); @@ -334,16 +334,15 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu !newState->keyIDOwner.IsNull()) { newState->Revive(nHeight); if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s revived at height %d\n", - __func__, opt_proTx->proTxHash.ToString(), nHeight); + LogPrintf("%s -- MN %s revived at height %d\n", __func__, opt_proTx->proTxHash.ToString(), nHeight); } } } newList.UpdateMN(opt_proTx->proTxHash, newState); if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s updated at height %d: %s\n", - __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); + LogPrintf("%s -- MN %s updated at height %d: %s\n", __func__, opt_proTx->proTxHash.ToString(), nHeight, + opt_proTx->ToString()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) { const auto opt_proTx = GetTxPayload(tx); @@ -372,8 +371,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu newList.UpdateMN(opt_proTx->proTxHash, newState); if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s updated at height %d: %s\n", - __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); + LogPrintf("%s -- MN %s updated at height %d: %s\n", __func__, opt_proTx->proTxHash.ToString(), nHeight, + opt_proTx->ToString()); } } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REVOKE) { const auto opt_proTx = GetTxPayload(tx); @@ -393,8 +392,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu newList.UpdateMN(opt_proTx->proTxHash, newState); if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s revoked operator key at height %d: %s\n", - __func__, opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); + LogPrintf("%s -- MN %s revoked operator key at height %d: %s\n", __func__, + opt_proTx->proTxHash.ToString(), nHeight, opt_proTx->ToString()); } } else if (tx.nType == TRANSACTION_QUORUM_COMMITMENT) { const auto opt_qc = GetTxPayload(tx); @@ -407,7 +406,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-commitment-type"); } int qcnHeight = int(opt_qc->nHeight); - int quorumHeight = qcnHeight - (qcnHeight % llmq_params_opt->dkgInterval) + int(opt_qc->commitment.quorumIndex); + int quorumHeight = qcnHeight - (qcnHeight % llmq_params_opt->dkgInterval) + + int(opt_qc->commitment.quorumIndex); auto pQuorumBaseBlockIndex = pindexPrev->GetAncestor(quorumHeight); if (!pQuorumBaseBlockIndex || pQuorumBaseBlockIndex->GetBlockHash() != opt_qc->commitment.quorumHash) { // we should actually never get into this case as validation should have caught it...but let's be sure @@ -416,7 +416,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu // The commitment has already been validated at this point, so it's safe to use members of it - const auto members = llmq::utils::GetAllQuorumMembers(opt_qc->commitment.llmqType, m_dmnman, m_qsnapman, pQuorumBaseBlockIndex); + const auto members = llmq::utils::GetAllQuorumMembers(opt_qc->commitment.llmqType, m_dmnman, m_qsnapman, + pQuorumBaseBlockIndex); HandleQuorumCommitment(opt_qc->commitment, members, debugLogs, newList); } } @@ -433,8 +434,10 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu newList.RemoveMN(dmn->proTxHash); if (debugLogs) { - LogPrintf("CDeterministicMNManager::%s -- MN %s removed from list because collateral was spent. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", - __func__, dmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), nHeight, newList.GetAllMNsCount()); + LogPrintf("%s -- MN %s removed from list because collateral was spent. " /* Continued */ + "collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", + __func__, dmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), nHeight, + newList.GetAllMNsCount()); } } } @@ -451,8 +454,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu if (dmn->nType == MnType::Evo && !isMNRewardReallocation) { ++newState->nConsecutivePayments; if (debugLogs) { - LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s is an EvoNode, bumping nConsecutivePayments to %d\n", - __func__, dmn->proTxHash.ToString(), newState->nConsecutivePayments); + LogPrint(BCLog::MNPAYMENTS, "%s -- MN %s is an EvoNode, bumping nConsecutivePayments to %d\n", __func__, + dmn->proTxHash.ToString(), newState->nConsecutivePayments); } } newList.UpdateMN(payee->proTxHash, newState); @@ -461,8 +464,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu // Since the previous GetMN query returned a value, after an update, querying the same // hash *must* give us a result. If it doesn't, that would be a potential logic bug. assert(dmn); - LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s, nConsecutivePayments=%d\n", - __func__, dmn->proTxHash.ToString(), dmn->pdmnState->nConsecutivePayments); + LogPrint(BCLog::MNPAYMENTS, "%s -- MN %s, nConsecutivePayments=%d\n", __func__, dmn->proTxHash.ToString(), + dmn->pdmnState->nConsecutivePayments); } } @@ -473,8 +476,8 @@ bool CSpecialTxProcessor::BuildNewListFromBlock(const CBlock& block, gsl::not_nu if (payee != nullptr && dmn.proTxHash == payee->proTxHash && !isMNRewardReallocation) return; if (dmn.pdmnState->nConsecutivePayments == 0) return; if (debugLogs) { - LogPrint(BCLog::MNPAYMENTS, "CDeterministicMNManager::%s -- MN %s, reset nConsecutivePayments %d->0\n", - __func__, dmn.proTxHash.ToString(), dmn.pdmnState->nConsecutivePayments); + LogPrint(BCLog::MNPAYMENTS, "%s -- MN %s, reset nConsecutivePayments %d->0\n", __func__, + dmn.proTxHash.ToString(), dmn.pdmnState->nConsecutivePayments); } auto newState = std::make_shared(*dmn.pdmnState); newState->nConsecutivePayments = 0; diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index 933df4395181..9adfb26ba748 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -5,10 +5,10 @@ #ifndef BITCOIN_EVO_SPECIALTXMAN_H #define BITCOIN_EVO_SPECIALTXMAN_H -#include -#include #include #include +#include +#include class BlockValidationState; class CBlock; @@ -75,8 +75,9 @@ class CSpecialTxProcessor // the returned list will not contain the correct block hash (we can't know it yet as the coinbase TX is not updated yet) bool BuildNewListFromBlock(const CBlock& block, gsl::not_null pindexPrev, - const CCoinsViewCache& view, bool debugLogs, - BlockValidationState& state, CDeterministicMNList& mnListRet) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + const CCoinsViewCache& view, bool debugLogs, BlockValidationState& state, + CDeterministicMNList& mnListRet) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + private: bool CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const CCbTx& cbTx, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); From 8f40f402f6ff3251955140efcd00a4c3b7e117ae Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 21 Jun 2025 14:37:15 +0700 Subject: [PATCH 7/8] refactor: remove un-needed header validationinterface.h from evo/{dmnstate,deterministicmns} --- src/evo/deterministicmns.cpp | 2 +- src/evo/dmnstate.cpp | 1 - test/lint/lint-circular-dependencies.py | 3 +-- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 508a302896db..25e744cbe8d7 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -20,8 +20,8 @@ #include #include #include +#include #include -#include #include #include diff --git a/src/evo/dmnstate.cpp b/src/evo/dmnstate.cpp index dd95a8b5ee4d..70a1c139c9c9 100644 --- a/src/evo/dmnstate.cpp +++ b/src/evo/dmnstate.cpp @@ -8,7 +8,6 @@ #include #include #include