From 8e39f2d20d09592035ae048d0cfe955c733310d9 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Sat, 30 Nov 2024 15:39:55 -0500 Subject: [PATCH] validation: in invalidateblock, mark children as invalid right away Before, they would be marked as invalid only after disconnecting multiple blocks, letting go of cs_main in the meantime. This is in preparation for adding a check to CheckBlockIndex() requiring that descendants of invalid block indexes are always marked as invalid. Entries from highpow_outofchain_headers are now only removed if made invalid, no longer after inserting into setBlockIndexCandidates, because they might still become invalid later in the second case. This means that blocks could be inserted multiple times now into setBlockIndexCandidates - this won't have any effect, so behavior isn't changed. --- src/validation.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 067e9b1695f..941e433f008 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3753,18 +3753,30 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde m_blockman.m_dirty_blockindex.insert(to_mark_failed); } - // Add any equal or more work headers to setBlockIndexCandidates + // Mark out-of-chain descendants of the invalidated block as invalid + // (possibly replacing a pre-existing BLOCK_FAILED_VALID with BLOCK_FAILED_CHILD) + // Add any equal or more work headers that are not invalidated to setBlockIndexCandidates auto candidate_it = highpow_outofchain_headers.lower_bound(invalid_walk_tip->pprev->nChainWork); while (candidate_it != highpow_outofchain_headers.end()) { CBlockIndex* candidate{candidate_it->second}; + if (candidate->GetAncestor(invalid_walk_tip->nHeight) == invalid_walk_tip) { + // Children of failed blocks should be marked as BLOCK_FAILED_CHILD instead. + candidate->nStatus &= ~BLOCK_FAILED_VALID; + candidate->nStatus |= BLOCK_FAILED_CHILD; + m_blockman.m_dirty_blockindex.insert(candidate); + // If invalidated, the block is irrelevant for setBlockIndexCandidates + // and can be removed from the cache. + candidate_it = highpow_outofchain_headers.erase(candidate_it); + continue; + } if (!CBlockIndexWorkComparator()(candidate, invalid_walk_tip->pprev) && candidate->IsValid(BLOCK_VALID_TRANSACTIONS) && candidate->HaveNumChainTxs()) { setBlockIndexCandidates.insert(candidate); - candidate_it = highpow_outofchain_headers.erase(candidate_it); - } else { - ++candidate_it; + // Do not remove candidate from the highpow_outofchain_headers cache, because it might be a descendant of the block being invalidated + // which needs to be marked failed later. } + ++candidate_it; } // Track the last disconnected block, so we can correct its BLOCK_FAILED_CHILD status in future