Skip to content

Commit ee673b9

Browse files
committed
validation: remove m_failed_blocks
After changes in previous commits, we now mark all blocks that descend from an invalid block immediately as the block is found invalid. This happens both in the AcceptBlock and ConnectBlock stages of block validation. As a result, the pindexPrev->nStatus check in AcceptBlockHeader is now sufficient to detect invalid blocks and checking m_failed_blocks there is no longer necessary.
1 parent ed764ea commit ee673b9

File tree

2 files changed

+0
-64
lines changed

2 files changed

+0
-64
lines changed

src/validation.cpp

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2091,7 +2091,6 @@ void Chainstate::InvalidBlockFound(CBlockIndex* pindex, const BlockValidationSta
20912091
AssertLockHeld(cs_main);
20922092
if (state.GetResult() != BlockValidationResult::BLOCK_MUTATED) {
20932093
pindex->nStatus |= BLOCK_FAILED_VALID;
2094-
m_chainman.m_failed_blocks.insert(pindex);
20952094
m_blockman.m_dirty_blockindex.insert(pindex);
20962095
setBlockIndexCandidates.erase(pindex);
20972096
InvalidChainFound(pindex);
@@ -3810,7 +3809,6 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
38103809
pindex->nStatus |= BLOCK_FAILED_VALID;
38113810
m_blockman.m_dirty_blockindex.insert(pindex);
38123811
setBlockIndexCandidates.erase(pindex);
3813-
m_chainman.m_failed_blocks.insert(pindex);
38143812
}
38153813

38163814
// If any new blocks somehow arrived while we were disconnecting
@@ -3878,7 +3876,6 @@ void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex) {
38783876
// Reset invalid block marker if it was pointing to one of those.
38793877
m_chainman.m_best_invalid = nullptr;
38803878
}
3881-
m_chainman.m_failed_blocks.erase(&block_index);
38823879
}
38833880
}
38843881

@@ -3887,7 +3884,6 @@ void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex) {
38873884
if (pindex->nStatus & BLOCK_FAILED_MASK) {
38883885
pindex->nStatus &= ~BLOCK_FAILED_MASK;
38893886
m_blockman.m_dirty_blockindex.insert(pindex);
3890-
m_chainman.m_failed_blocks.erase(pindex);
38913887
}
38923888
pindex = pindex->pprev;
38933889
}
@@ -4383,45 +4379,6 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida
43834379
LogDebug(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString());
43844380
return false;
43854381
}
4386-
4387-
/* Determine if this block descends from any block which has been found
4388-
* invalid (m_failed_blocks), then mark pindexPrev and any blocks between
4389-
* them as failed. For example:
4390-
*
4391-
* D3
4392-
* /
4393-
* B2 - C2
4394-
* / \
4395-
* A D2 - E2 - F2
4396-
* \
4397-
* B1 - C1 - D1 - E1
4398-
*
4399-
* In the case that we attempted to reorg from E1 to F2, only to find
4400-
* C2 to be invalid, we would mark D2, E2, and F2 as BLOCK_FAILED_CHILD
4401-
* but NOT D3 (it was not in any of our candidate sets at the time).
4402-
*
4403-
* In any case D3 will also be marked as BLOCK_FAILED_CHILD at restart
4404-
* in LoadBlockIndex.
4405-
*/
4406-
if (!pindexPrev->IsValid(BLOCK_VALID_SCRIPTS)) {
4407-
// The above does not mean "invalid": it checks if the previous block
4408-
// hasn't been validated up to BLOCK_VALID_SCRIPTS. This is a performance
4409-
// optimization, in the common case of adding a new block to the tip,
4410-
// we don't need to iterate over the failed blocks list.
4411-
for (const CBlockIndex* failedit : m_failed_blocks) {
4412-
if (pindexPrev->GetAncestor(failedit->nHeight) == failedit) {
4413-
assert(failedit->nStatus & BLOCK_FAILED_VALID);
4414-
CBlockIndex* invalid_walk = pindexPrev;
4415-
while (invalid_walk != failedit) {
4416-
invalid_walk->nStatus |= BLOCK_FAILED_CHILD;
4417-
m_blockman.m_dirty_blockindex.insert(invalid_walk);
4418-
invalid_walk = invalid_walk->pprev;
4419-
}
4420-
LogDebug(BCLog::VALIDATION, "header %s has prev block invalid: %s\n", hash.ToString(), block.hashPrevBlock.ToString());
4421-
return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk");
4422-
}
4423-
}
4424-
}
44254382
}
44264383
if (!min_pow_checked) {
44274384
LogDebug(BCLog::VALIDATION, "%s: not adding new block header %s, missing anti-dos proof-of-work validation\n", __func__, hash.ToString());

src/validation.h

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,27 +1038,6 @@ class ChainstateManager
10381038
}
10391039

10401040

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

0 commit comments

Comments
 (0)