validation: remove m_failed_blocks

We now mark all blocks that descend from an invalid block
immediately as the invalid block is encountered
(by iterating over the entire block index).
As a result, m_failed_blocks, which was a heuristic to only
mark descendants of failed blocks as failed when necessary,
(i.e., when we have do decide whether to add another descendant or not)
is no longer required.
This commit is contained in:
Martin Zumsande 2024-12-01 00:08:51 -05:00
parent eca8d9182e
commit 4ba2e480ff
2 changed files with 0 additions and 64 deletions

View File

@ -2091,7 +2091,6 @@ void Chainstate::InvalidBlockFound(CBlockIndex* pindex, const BlockValidationSta
AssertLockHeld(cs_main);
if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) {
pindex->nStatus |= BLOCK_FAILED_VALID;
m_chainman.m_failed_blocks.insert(pindex);
m_blockman.m_dirty_blockindex.insert(pindex);
setBlockIndexCandidates.erase(pindex);
InvalidChainFound(pindex);
@ -3817,7 +3816,6 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
to_mark_failed->nStatus |= BLOCK_FAILED_VALID;
m_blockman.m_dirty_blockindex.insert(to_mark_failed);
setBlockIndexCandidates.erase(to_mark_failed);
m_chainman.m_failed_blocks.insert(to_mark_failed);
// If any new blocks somehow arrived while we were disconnecting
// (above), then the pre-calculation of what should go into
@ -3883,7 +3881,6 @@ void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex) {
// Reset invalid block marker if it was pointing to one of those.
m_chainman.m_best_invalid = nullptr;
}
m_chainman.m_failed_blocks.erase(&block_index);
}
}
@ -3892,7 +3889,6 @@ void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex) {
if (pindex->nStatus & BLOCK_FAILED_MASK) {
pindex->nStatus &= ~BLOCK_FAILED_MASK;
m_blockman.m_dirty_blockindex.insert(pindex);
m_chainman.m_failed_blocks.erase(pindex);
}
pindex = pindex->pprev;
}
@ -4396,45 +4392,6 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida
LogDebug(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString());
return false;
}
/* Determine if this block descends from any block which has been found
* invalid (m_failed_blocks), then mark pindexPrev and any blocks between
* them as failed. For example:
*
* D3
* /
* B2 - C2
* / \
* A D2 - E2 - F2
* \
* B1 - C1 - D1 - E1
*
* In the case that we attempted to reorg from E1 to F2, only to find
* C2 to be invalid, we would mark D2, E2, and F2 as BLOCK_FAILED_CHILD
* but NOT D3 (it was not in any of our candidate sets at the time).
*
* In any case D3 will also be marked as BLOCK_FAILED_CHILD at restart
* in LoadBlockIndex.
*/
if (!pindexPrev->IsValid(BLOCK_VALID_SCRIPTS)) {
// The above does not mean "invalid": it checks if the previous block
// hasn't been validated up to BLOCK_VALID_SCRIPTS. This is a performance
// optimization, in the common case of adding a new block to the tip,
// we don't need to iterate over the failed blocks list.
for (const CBlockIndex* failedit : m_failed_blocks) {
if (pindexPrev->GetAncestor(failedit->nHeight) == failedit) {
assert(failedit->nStatus & BLOCK_FAILED_VALID);
CBlockIndex* invalid_walk = pindexPrev;
while (invalid_walk != failedit) {
invalid_walk->nStatus |= BLOCK_FAILED_CHILD;
m_blockman.m_dirty_blockindex.insert(invalid_walk);
invalid_walk = invalid_walk->pprev;
}
LogDebug(BCLog::VALIDATION, "header %s has prev block invalid: %s\n", hash.ToString(), block.hashPrevBlock.ToString());
return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk");
}
}
}
}
if (!min_pow_checked) {
LogDebug(BCLog::VALIDATION, "%s: not adding new block header %s, missing anti-dos proof-of-work validation\n", __func__, hash.ToString());

View File

@ -1038,27 +1038,6 @@ public:
}
/**
* In order to efficiently track invalidity of headers, we keep the set of
* blocks which we tried to connect and found to be invalid here (ie which
* were set to BLOCK_FAILED_VALID since the last restart). We can then
* walk this set and check if a new header is a descendant of something in
* this set, preventing us from having to walk m_block_index when we try
* to connect a bad block and fail.
*
* While this is more complicated than marking everything which descends
* from an invalid block as invalid at the time we discover it to be
* invalid, doing so would require walking all of m_block_index to find all
* descendants. Since this case should be very rare, keeping track of all
* BLOCK_FAILED_VALID blocks in a set should be just fine and work just as
* well.
*
* Because we already walk m_block_index in height-order at startup, we go
* ahead and mark descendants of invalid blocks as FAILED_CHILD at that time,
* instead of putting things in this set.
*/
std::set<CBlockIndex*> m_failed_blocks;
/** Best header we've seen so far (used for getheaders queries' starting points). */
CBlockIndex* m_best_header GUARDED_BY(::cs_main){nullptr};