From b83de7f28e71f0b10a999327395198fbcb02f44b Mon Sep 17 00:00:00 2001 From: stickies-v Date: Tue, 3 Mar 2026 15:17:00 +0800 Subject: [PATCH 1/2] validation: remove sentinel block from ConnectTrace The sentinel pattern was necessary to collect conflicted transactions before their associated block was recorded, but that tracking was removed in 5613f9842b4000fed088b8cf7b99674c328d15e1. --- src/validation.cpp | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 67130a31cfd..b462b7f0144 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2990,39 +2990,24 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra struct PerBlockConnectTrace { CBlockIndex* pindex = nullptr; std::shared_ptr pblock; - PerBlockConnectTrace() = default; }; /** * Used to track blocks whose transactions were applied to the UTXO state as a * part of a single ActivateBestChainStep call. - * - * This class is single-use, once you call GetBlocksConnected() you have to throw - * it away and make a new one. */ class ConnectTrace { private: std::vector blocksConnected; public: - explicit ConnectTrace() : blocksConnected(1) {} - void BlockConnected(CBlockIndex* pindex, std::shared_ptr pblock) { - assert(!blocksConnected.back().pindex); assert(pindex); assert(pblock); - blocksConnected.back().pindex = pindex; - blocksConnected.back().pblock = std::move(pblock); - blocksConnected.emplace_back(); + blocksConnected.emplace_back(pindex, std::move(pblock)); } - std::vector& GetBlocksConnected() { - // We always keep one extra block at the end of our list because - // blocks are added after all the conflicted transactions have - // been filled in. Thus, the last entry should always be an empty - // one waiting for the transactions from the next block. We pop - // the last entry here to make sure the list we return is sane. - assert(!blocksConnected.back().pindex); - blocksConnected.pop_back(); + const std::vector& GetBlocksConnected() const + { return blocksConnected; } }; From 2f8f2e900118c98699db91075bd1b72e6460c6b1 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Tue, 3 Mar 2026 15:30:20 +0800 Subject: [PATCH 2/2] validation: remove ConnectTrace wrapper class Replace ConnectTrace with a plain std::vector, and rename PerBlockConnectTrace to ConnectedBlock and connectTrace to connected_blocks. --- src/validation.cpp | 45 ++++++++++++--------------------------------- src/validation.h | 6 +++--- 2 files changed, 15 insertions(+), 36 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index b462b7f0144..83c16867fe6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2987,42 +2987,22 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra return true; } -struct PerBlockConnectTrace { - CBlockIndex* pindex = nullptr; +struct ConnectedBlock { + const CBlockIndex* pindex; std::shared_ptr pblock; }; -/** - * Used to track blocks whose transactions were applied to the UTXO state as a - * part of a single ActivateBestChainStep call. - */ -class ConnectTrace { -private: - std::vector blocksConnected; - -public: - void BlockConnected(CBlockIndex* pindex, std::shared_ptr pblock) { - assert(pindex); - assert(pblock); - blocksConnected.emplace_back(pindex, std::move(pblock)); - } - - const std::vector& GetBlocksConnected() const - { - return blocksConnected; - } -}; /** * Connect a new block to m_chain. block_to_connect is either nullptr or a pointer to a CBlock * corresponding to pindexNew, to bypass loading it again from disk. * - * The block is added to connectTrace if connection succeeds. + * The block is added to connected_blocks if connection succeeds. */ bool Chainstate::ConnectTip( BlockValidationState& state, CBlockIndex* pindexNew, std::shared_ptr block_to_connect, - ConnectTrace& connectTrace, + std::vector& connected_blocks, DisconnectedBlockTransactions& disconnectpool) { AssertLockHeld(cs_main); @@ -3119,7 +3099,7 @@ bool Chainstate::ConnectTip( Chainstate& current_cs{m_chainman.CurrentChainstate()}; m_chainman.MaybeValidateSnapshot(*this, current_cs); - connectTrace.BlockConnected(pindexNew, std::move(block_to_connect)); + connected_blocks.emplace_back(pindexNew, std::move(block_to_connect)); return true; } @@ -3204,7 +3184,7 @@ void Chainstate::PruneBlockIndexCandidates() { * * @returns true unless a system error occurred */ -bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) +bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, std::vector& connected_blocks) { AssertLockHeld(cs_main); if (m_mempool) AssertLockHeld(m_mempool->cs); @@ -3249,7 +3229,7 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* // Connect new blocks. for (CBlockIndex* pindexConnect : vpindexToConnect | std::views::reverse) { - if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connectTrace, disconnectpool)) { + if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connected_blocks, disconnectpool)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { @@ -3374,7 +3354,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< { LOCK(cs_main); { - // Lock transaction pool for at least as long as it takes for connectTrace to be consumed + // Lock transaction pool for at least as long as it takes for connected_blocks to be consumed LOCK(MempoolMutex()); const bool was_in_ibd = m_chainman.IsInitialBlockDownload(); CBlockIndex* starting_tip = m_chain.Tip(); @@ -3382,7 +3362,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< do { // We absolutely may not unlock cs_main until we've made forward progress // (with the exception of shutdown due to hardware issues, low disk space, etc). - ConnectTrace connectTrace; // Destructed before cs_main is unlocked + std::vector connected_blocks; // Destructed before cs_main is unlocked if (pindexMostWork == nullptr) { pindexMostWork = FindMostWorkChain(); @@ -3399,7 +3379,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< // in case snapshot validation is completed during ActivateBestChainStep, the // result of GetRole() changes from BACKGROUND to NORMAL. const ChainstateRole chainstate_role{this->GetRole()}; - if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) { + if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connected_blocks)) { // A system error occurred return false; } @@ -3411,10 +3391,9 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< } pindexNewTip = m_chain.Tip(); - for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) { - assert(trace.pblock && trace.pindex); + for (const auto& [index, block] : connected_blocks) { if (m_chainman.m_options.signals) { - m_chainman.m_options.signals->BlockConnected(chainstate_role, trace.pblock, trace.pindex); + m_chainman.m_options.signals->BlockConnected(chainstate_role, Assert(block), Assert(index)); } } diff --git a/src/validation.h b/src/validation.h index 482772c0d6f..d65d61ce917 100644 --- a/src/validation.h +++ b/src/validation.h @@ -455,7 +455,7 @@ enum DisconnectResult DISCONNECT_FAILED // Something else went wrong. }; -class ConnectTrace; +struct ConnectedBlock; /** @see Chainstate::FlushStateToDisk */ inline constexpr std::array FlushStateModeNames{"NONE", "IF_NEEDED", "PERIODIC", "FORCE_FLUSH", "FORCE_SYNC"}; @@ -847,12 +847,12 @@ public: std::pair GetPruneRange(int last_height_can_prune) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); protected: - bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); + bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, std::vector& connected_blocks) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); bool ConnectTip( BlockValidationState& state, CBlockIndex* pindexNew, std::shared_ptr block_to_connect, - ConnectTrace& connectTrace, + std::vector& connected_blocks, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs); void InvalidBlockFound(CBlockIndex* pindex, const BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);