validation: remove redundant marking in FindMostWorkChain
What the developer wrote
Authored by stratospher
validation: remove redundant marking in FindMostWorkChain
since ed764ea, all descendants in m_block_index are required to be
marked BLOCK_FAILED_VALID when an invalid block is encountered,
as enforced by a CheckBlockIndex assert.
remove the now-redundant marking in FindMostWorkChain's inner loop,
so it is only responsible for cleaning up setBlockIndexCandidates,
not for modifying block validity state
What changed, and why it matters
This commit removes a redundant piece of code in Bitcoin Core's chain-selection logic. Since an earlier change, invalid blocks are already marked as invalid elsewhere, so this inner loop no longer needs to do that marking itself. It is a cleanup/refactoring change, not a security fix, and does not introduce a vulnerability.
No action required. Treat as routine refactoring/cleanup. Reviewers may optionally verify that ed764ea's CheckBlockIndex invariant indeed covers all paths reaching this loop, but the commit message asserts it does.
Security signals we found
No security-relevant behavior change: the removed marking is redundant because an earlier invariant guarantees the same state.
No new attack surface introduced; no network input, consensus rule, or cryptographic change.
Commit message frames the change as code cleanup, not as a vulnerability fix.
Evidence from the diff
In Chainstate::FindMostWorkChain(), the loop previously set BLOCK_FAILED_VALID on descendants of a failed chain. Commit ed764ea made it a CheckBlockIndex invariant that all descendants of an invalid block are already marked BLOCK_FAILED_VALID when the invalid block is encountered. Therefore the marking inside FindMostWorkChain is redundant and is removed. The remaining branch handles missing-data blocks by re-adding them to m_blocks_unlinked. The change narrows the conditional from if (fFailedChain) / else if (fMissingData) to if (fMissingData && !fFailedChain).
Changed components
src/validation.cppChainstate::FindMostWorkChain()Inspect captured patch +4 / −7
diff --git a/src/validation.cpp b/src/validation.cpp
index 00c1bab5..e0a8c1b3 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3142,13 +3142,10 @@ CBlockIndex* Chainstate::FindMostWorkChain()
CBlockIndex *pindexFailed = pindexNew;
// Remove the entire chain from the set.
while (pindexTest != pindexFailed) {
- if (fFailedChain) {
- pindexFailed->nStatus |= BLOCK_FAILED_VALID;
- m_blockman.m_dirty_blockindex.insert(pindexFailed);
- } else if (fMissingData) {
- // If we're missing data, then add back to m_blocks_unlinked,
- // so that if the block arrives in the future we can try adding
- // to setBlockIndexCandidates again.
+ if (fMissingData && !fFailedChain) {
+ // If we're missing data and not a descendant of an invalid block,
+ // then add back to m_blocks_unlinked, so that if the block arrives in the future
+ // we can try adding to setBlockIndexCandidates again.
m_blockman.m_blocks_unlinked.insert(
std::make_pair(pindexFailed->pprev, pindexFailed));
}
Why this scored 15/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.