From d6359937bfa41e166f098b53252dd14262fd8de5 Mon Sep 17 00:00:00 2001 From: stratospher <44024636+stratospher@users.noreply.github.com> Date: Sat, 30 May 2026 01:01:40 +0530 Subject: [PATCH] validation: check invariants when inserting into m_blocks_unlinked For an entry A -> B in m_blocks_unlinked, the entry B was added into m_blocks_unlinked either because: - some ancestor of B was never received (or) - some ancestor of B was pruned away. Every insert must satisfy two invariants: 1. B has BLOCK_HAVE_DATA set. 2. No duplicate A -> B entries in m_blocks_unlinked (this is UB zone if this entry gets popped twice in ReceivedBlockTransactions and happens to be in setBlockIndexCandidates) 2 bugs (#35070 and #35168) discovered recently stemmed from the m_blocks_unlinked insertion sites not enforcing these invariants. So add a helper which wraps around insertion sites of m_blocks_unlinked with these invariants. Co-authored-by: Martin Zumsande --- src/node/blockstorage.cpp | 14 +++++++++++++- src/node/blockstorage.h | 1 + src/validation.cpp | 7 ++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index b060108c186..8726c741fa5 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -255,6 +255,18 @@ CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block, CBlockInde return pindexNew; } +void BlockManager::AddUnlinkedBlock(CBlockIndex* block) +{ + AssertLockHeld(cs_main); + Assume(block != nullptr); + Assume(block->nStatus & BLOCK_HAVE_DATA); + auto range = m_blocks_unlinked.equal_range(block->pprev); + for (auto it = range.first; it != range.second; ++it) { + if (it->second == block) return; // don't insert duplicates + } + m_blocks_unlinked.emplace(block->pprev, block); +} + void BlockManager::PruneOneBlockFile(const int fileNumber) { AssertLockHeld(cs_main); @@ -487,7 +499,7 @@ bool BlockManager::LoadBlockIndex(const std::optional& snapshot_blockha } else { pindex->m_chain_tx_count = 0; if (pindex->nStatus & BLOCK_HAVE_DATA) { - m_blocks_unlinked.insert(std::make_pair(pindex->pprev, pindex)); + AddUnlinkedBlock(pindex); } } } else { diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index f9c5753ba5b..0ab595ac851 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -354,6 +354,7 @@ public: * All pairs A->B, where A (or one of its ancestors) misses transactions, but B has transactions. */ std::multimap m_blocks_unlinked; + void AddUnlinkedBlock(CBlockIndex* block) EXCLUSIVE_LOCKS_REQUIRED(cs_main); std::unique_ptr m_block_tree_db GUARDED_BY(::cs_main); diff --git a/src/validation.cpp b/src/validation.cpp index c1fe70da45d..30074bc8113 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3149,10 +3149,7 @@ CBlockIndex* Chainstate::FindMostWorkChain() // processed twice in ReceivedBlockTransactions(), it may be re-added to // setBlockIndexCandidates with a modified nSequenceId, breaking ordering // guarantees and leading to undefined behavior. - auto range = m_blockman.m_blocks_unlinked.equal_range(pindexFailed->pprev); - if (!std::any_of(range.first, range.second, [&](const auto& p) { return p.second == pindexFailed; })) { - m_blockman.m_blocks_unlinked.emplace(pindexFailed->pprev, pindexFailed); - } + m_blockman.AddUnlinkedBlock(pindexFailed); } setBlockIndexCandidates.erase(pindexFailed); } @@ -3816,7 +3813,7 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd } } else { if (pindexNew->pprev && pindexNew->pprev->IsValid(BLOCK_VALID_TREE)) { - m_blockman.m_blocks_unlinked.insert(std::make_pair(pindexNew->pprev, pindexNew)); + m_blockman.AddUnlinkedBlock(pindexNew); } } }