Skip to content

Commit

Permalink
Fix initialization of setBlockIndexCandidates when working with multi…
Browse files Browse the repository at this point in the history
…ple chainstates

When using assumeutxo and multiple chainstates are active, the background
chainstate should consider all HAVE_DATA blocks on the snapshotted chain that
have more work than the tip as potential candidates.
  • Loading branch information
sdaftuar committed Jun 10, 2023
1 parent df3655c commit 0c44733
Showing 1 changed file with 1 addition and 33 deletions.
34 changes: 1 addition & 33 deletions src/validation.cpp
Expand Up @@ -4462,9 +4462,6 @@ bool ChainstateManager::LoadBlockIndex()
std::sort(vSortedByHeight.begin(), vSortedByHeight.end(),
CBlockIndexHeightOnlyComparator());

// Find start of assumed-valid region.
int first_assumed_valid_height = std::numeric_limits<int>::max();

for (const CBlockIndex* block : vSortedByHeight) {
if (block->IsAssumedValid()) {
auto chainstates = GetAll();
Expand All @@ -4477,9 +4474,6 @@ bool ChainstateManager::LoadBlockIndex()
assert(any_chain([](auto chainstate) { return chainstate->reliesOnAssumedValid(); }));
assert(any_chain([](auto chainstate) { return !chainstate->reliesOnAssumedValid(); }));

first_assumed_valid_height = block->nHeight;
LogPrintf("Saw first assumedvalid block at height %d (%s)\n",
first_assumed_valid_height, block->ToString());
break;
}
}
Expand All @@ -4490,34 +4484,8 @@ bool ChainstateManager::LoadBlockIndex()
(pindex->IsValid(BLOCK_VALID_TRANSACTIONS) &&
(pindex->HaveTxsDownloaded() || pindex->pprev == nullptr))) {

// Fill each chainstate's block candidate set. Only add assumed-valid
// blocks to the tip candidate set if the chainstate is allowed to rely on
// assumed-valid blocks.
//
// If all setBlockIndexCandidates contained the assumed-valid blocks, the
// background chainstate's ActivateBestChain() call would add assumed-valid
// blocks to the chain (based on how FindMostWorkChain() works). Obviously
// we don't want this since the purpose of the background validation chain
// is to validate assued-valid blocks.
//
// Note: This is considering all blocks whose height is greater or equal to
// the first assumed-valid block to be assumed-valid blocks, and excluding
// them from the background chainstate's setBlockIndexCandidates set. This
// does mean that some blocks which are not technically assumed-valid
// (later blocks on a fork beginning before the first assumed-valid block)
// might not get added to the background chainstate, but this is ok,
// because they will still be attached to the active chainstate if they
// actually contain more work.
//
// Instead of this height-based approach, an earlier attempt was made at
// detecting "holistically" whether the block index under consideration
// relied on an assumed-valid ancestor, but this proved to be too slow to
// be practical.
for (Chainstate* chainstate : GetAll()) {
if (chainstate->reliesOnAssumedValid() ||
pindex->nHeight < first_assumed_valid_height) {
chainstate->setBlockIndexCandidates.insert(pindex);
}
chainstate->TryAddBlockIndexCandidate(pindex);
}
}
if (pindex->nStatus & BLOCK_FAILED_MASK && (!m_best_invalid || pindex->nChainWork > m_best_invalid->nChainWork)) {
Expand Down

0 comments on commit 0c44733

Please sign in to comment.