validation: do not add the snapshot block to candidates of bg chainstate
What the developer wrote
Authored by Martin Zumsande
validation: do not add the snapshot block to candidates of bg chainstate
The snapshot block needs to be added to the candidates set of the
assumed-valid chain because it will be the tip of that chainstate
right after snapshot activation.
However, adding it also to the background chainstate is not necessary
for anything. Before, the index would linger in the set without being
connectable. It will be eventually added to the set as part of the
normal block download - no extra logic is necessary here.
What changed, and why it matters
This commit fixes a minor bookkeeping error in Bitcoin Core's block validation engine. During a special one-time process called assumeutxo snapshot activation, the snapshot block was being incorrectly added to the candidate set of both the new lightweight chain and the older background chain. The background chain does not need or want this block as a candidate, because it cannot connect it. The patch removes the extra entry so the candidate set matches the intended design. There is no direct evidence this causes a security exploit, but it removes a confusing inconsistency that could theoretically affect validation decisions.
Treat as a correctness fix with low security urgency. Reviewers should confirm that removing the snapshot block from the background candidate set does not alter tip-selection behavior in any observable way beyond the intended cleanup. No immediate user action is required.
Security signals we found
Logic error in chainstate candidate selection
Assumeutxo snapshot activation edge case
Background chainstate received an unconnectable candidate block
No explicit security claim in commit message
Evidence from the diff
In Chainstate::PopulateBlockIndexCandidates(), the condition that adds the snapshot block as a candidate was over-broad. It checked pindex == SnapshotBase() || pindex == TargetBlock(), where TargetBlock() returns the snapshot block for the background chainstate. That meant the snapshot block was added to the background chainstate’s setBlockIndexCandidates even though the background chainstate will validate from genesis and cannot use the snapshot as a tip. The patch removes the pindex == TargetBlock() branch so only SnapshotBase() (the assume-valid base for the snapshot chainstate) gets the special-case treatment. The unit test is updated to expect one candidate instead of two for the background chainstate.
Changed components
src/validation.cpp Chainstate::PopulateBlockIndexCandidates()src/test/validation_chainstatemanager_tests.cpp chainstatemanager_loadblockindex testInspect captured patch +6 / −13
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 28471169..fe064990 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -551,8 +551,8 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
// check contents below.
reload_all_block_indexes();
- // The fully validated chain should only have the current validated tip and
- // the assumed valid base as candidates, blocks 90 and 110. Specifically:
+ // The fully validated chain should only have the current validated tip
+ // as a candidate (block 90). Specifically:
//
// - It does not have blocks 0-89 because they contain less work than the
// chain tip.
@@ -560,20 +560,13 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
// - It has block 90 because it has data and equal work to the chain tip,
// (since it is the chain tip).
//
- // - It does not have blocks 91-109 because they do not contain data.
- //
- // - It has block 110 even though it does not have data, because
- // LoadBlockIndex has a special case to always add the snapshot block as a
- // candidate. The special case is only actually intended to apply to the
- // snapshot chainstate cs2, not the background chainstate cs1, but it is
- // written broadly and applies to both.
+ // - It does not have blocks 91-110 because they do not contain data.
//
// - It does not have any blocks after height 110 because cs1 is a background
- // chainstate, and only blocks where are ancestors of the snapshot block
+ // chainstate, and only blocks that are ancestors of the snapshot block
// are added as candidates for the background chainstate.
- BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), 2);
+ BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), 1);
BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(validated_tip), 1);
- BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(assumed_base), 1);
// The assumed-valid tolerant chain has the assumed valid base as a
// candidate, but otherwise has none of the assumed-valid (which do not
diff --git a/src/validation.cpp b/src/validation.cpp
index 5c02ec23..afbd809b 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4921,7 +4921,7 @@ void Chainstate::PopulateBlockIndexCandidates()
// With assumeutxo, the snapshot block is a candidate for the tip, but it
// may not have BLOCK_VALID_TRANSACTIONS (e.g. if we haven't yet downloaded
// the block), so we special-case it here.
- if (pindex == SnapshotBase() || pindex == TargetBlock() ||
+ if (pindex == SnapshotBase() ||
(pindex->IsValid(BLOCK_VALID_TRANSACTIONS) &&
(pindex->HaveNumChainTxs() || pindex->pprev == nullptr))) {
TryAddBlockIndexCandidate(pindex);
Why this scored 23/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.