refactor: use clearer variables in InvalidateBlock()
What changed, and why it matters
This commit is a straightforward code cleanup in Bitcoin Core's block invalidation logic. It renames a variable from 'invalid_walk_tip' to 'disconnected_tip' and introduces a new variable 'new_tip' to make the code easier to read. There are no functional changes that would affect security.
No security action required. This is a non-functional refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors Chainstate::InvalidateBlock() in src/validation.cpp. It moves three local variable declarations later in the function and renames ‘invalid_walk_tip’ to ‘disconnected_tip’, adding ‘new_tip’ as an alias for m_chain.Tip() after disconnection. All assertions and logic are preserved; the changes are purely stylistic/readability improvements.
Changed components
src/validation.cppInspect captured patch +18 / −17
diff --git a/src/validation.cpp b/src/validation.cpp
index ab55e591..93b7dc37 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3565,10 +3565,6 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
assert(pindex);
if (pindex->nHeight == 0) return false;
- CBlockIndex* to_mark_failed = pindex;
- bool pindex_was_in_chain = false;
- int disconnected = 0;
-
// We do not allow ActivateBestChain() to run while InvalidateBlock() is
// running, as that could cause the tip to change while we disconnect
// blocks.
@@ -3600,6 +3596,10 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
}
}
+ CBlockIndex* to_mark_failed = pindex;
+ bool pindex_was_in_chain = false;
+ int disconnected = 0;
+
// Disconnect (descendants of) pindex, and mark them invalid.
while (true) {
if (m_chainman.m_interrupt) break;
@@ -3613,7 +3613,7 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
LOCK(MempoolMutex());
if (!m_chain.Contains(pindex)) break;
pindex_was_in_chain = true;
- CBlockIndex *invalid_walk_tip = m_chain.Tip();
+ CBlockIndex* disconnected_tip{m_chain.Tip()};
// ActivateBestChain considers blocks already in m_chain
// unconditionally valid already, so force disconnect away from it.
@@ -3626,32 +3626,33 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
// keeping the mempool up to date is probably futile anyway).
MaybeUpdateMempoolForReorg(disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret);
if (!ret) return false;
- assert(invalid_walk_tip->pprev == m_chain.Tip());
+ CBlockIndex* new_tip{m_chain.Tip()};
+ assert(disconnected_tip->pprev == new_tip);
// We immediately mark the disconnected blocks as invalid.
// This prevents a case where pruned nodes may fail to invalidateblock
// and be left unable to start as they have no tip candidates (as there
// are no blocks that meet the "have data and are not invalid per
// nStatus" criteria for inclusion in setBlockIndexCandidates).
- invalid_walk_tip->nStatus |= BLOCK_FAILED_VALID;
- m_blockman.m_dirty_blockindex.insert(invalid_walk_tip);
- setBlockIndexCandidates.erase(invalid_walk_tip);
- setBlockIndexCandidates.insert(invalid_walk_tip->pprev);
+ disconnected_tip->nStatus |= BLOCK_FAILED_VALID;
+ m_blockman.m_dirty_blockindex.insert(disconnected_tip);
+ setBlockIndexCandidates.erase(disconnected_tip);
+ setBlockIndexCandidates.insert(new_tip);
// Mark out-of-chain descendants of the invalidated block as invalid
// Add any equal or more work headers that are not invalidated to setBlockIndexCandidates
// Recalculate m_best_header if it became invalid.
- auto candidate_it = highpow_outofchain_headers.lower_bound(invalid_walk_tip->pprev->nChainWork);
+ auto candidate_it = highpow_outofchain_headers.lower_bound(new_tip->nChainWork);
- const bool best_header_needs_update{m_chainman.m_best_header->GetAncestor(invalid_walk_tip->nHeight) == invalid_walk_tip};
+ const bool best_header_needs_update{m_chainman.m_best_header->GetAncestor(disconnected_tip->nHeight) == disconnected_tip};
if (best_header_needs_update) {
- // pprev is definitely still valid at this point, but there may be better ones
- m_chainman.m_best_header = invalid_walk_tip->pprev;
+ // new_tip is definitely still valid at this point, but there may be better ones
+ m_chainman.m_best_header = new_tip;
}
while (candidate_it != highpow_outofchain_headers.end()) {
CBlockIndex* candidate{candidate_it->second};
- if (candidate->GetAncestor(invalid_walk_tip->nHeight) == invalid_walk_tip) {
+ if (candidate->GetAncestor(disconnected_tip->nHeight) == disconnected_tip) {
// Children of failed blocks are marked as BLOCK_FAILED_VALID.
candidate->nStatus |= BLOCK_FAILED_VALID;
m_blockman.m_dirty_blockindex.insert(candidate);
@@ -3660,7 +3661,7 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
candidate_it = highpow_outofchain_headers.erase(candidate_it);
continue;
}
- if (!CBlockIndexWorkComparator()(candidate, invalid_walk_tip->pprev) &&
+ if (!CBlockIndexWorkComparator()(candidate, new_tip) &&
candidate->IsValid(BLOCK_VALID_TRANSACTIONS) &&
candidate->HaveNumChainTxs()) {
setBlockIndexCandidates.insert(candidate);
@@ -3675,7 +3676,7 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
}
// Track the last disconnected block to call InvalidChainFound on it.
- to_mark_failed = invalid_walk_tip;
+ to_mark_failed = disconnected_tip;
}
m_chainman.CheckBlockIndex();
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.