Fix initialization of setBlockIndexCandidates when working with multiple chainstates

When using assumeutxo and multiple chainstates are active, the background
chainstate should consider all HAVE_DATA blocks that are ancestors of the
snapshotted block and that have more work than the tip as potential candidates.
This commit is contained in:
Suhas Daftuar 2023-05-24 13:56:37 -04:00
parent d43a1f1a2f
commit 768690b7ce
2 changed files with 30 additions and 64 deletions

View File

@ -412,7 +412,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, SnapshotTestSetup)
//! - Then mark a region of the chain BLOCK_ASSUMED_VALID and introduce a second chainstate
//! that will tolerate assumed-valid blocks. Run LoadBlockIndex() and ensure that the first
//! chainstate only contains fully validated blocks and the other chainstate contains all blocks,
//! even those assumed-valid.
//! except those marked assume-valid, because those entries don't HAVE_DATA.
//!
BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
{
@ -444,16 +444,17 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
WITH_LOCK(::cs_main, chainman.LoadBlockIndex());
};
// Ensure that without any assumed-valid BlockIndex entries, all entries are considered
// tip candidates.
// Ensure that without any assumed-valid BlockIndex entries, only the current tip is
// considered as a candidate.
reload_all_block_indexes();
BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), cs1.m_chain.Height() + 1);
BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), 1);
// Mark some region of the chain assumed-valid.
// Mark some region of the chain assumed-valid, and remove the HAVE_DATA flag.
for (int i = 0; i <= cs1.m_chain.Height(); ++i) {
LOCK(::cs_main);
auto index = cs1.m_chain[i];
// Blocks with heights in range [20, 40) are marked ASSUMED_VALID
if (i < last_assumed_valid_idx && i >= assumed_valid_start_idx) {
index->nStatus = BlockStatus::BLOCK_VALID_TREE | BlockStatus::BLOCK_ASSUMED_VALID;
}
@ -466,33 +467,41 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
validated_tip = index;
BOOST_CHECK(!index->IsAssumedValid());
}
// Note the block after the last assumed valid block as the snapshot base
if (i == last_assumed_valid_idx) {
// Note the last assumed valid block as the snapshot base
if (i == last_assumed_valid_idx - 1) {
assumed_base = index;
BOOST_CHECK(index->IsAssumedValid());
} else if (i == last_assumed_valid_idx) {
BOOST_CHECK(!index->IsAssumedValid());
}
}
BOOST_CHECK_EQUAL(expected_assumed_valid, num_assumed_valid);
// Note: cs2's tip is not set when ActivateExistingSnapshot is called.
Chainstate& cs2 = WITH_LOCK(::cs_main,
return chainman.ActivateExistingSnapshot(&mempool, *assumed_base->phashBlock));
// Set tip of the fully validated chain to be the validated tip
cs1.m_chain.SetTip(*validated_tip);
// Set tip of the assume-valid-based chain to the assume-valid block
cs2.m_chain.SetTip(*assumed_base);
reload_all_block_indexes();
// The fully validated chain only has candidates up to the start of the assumed-valid
// blocks.
// The fully validated chain should have the current validated tip
// and the assumed valid base as candidates.
BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), 2);
BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(validated_tip), 1);
BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(assumed_tip), 0);
BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.size(), assumed_valid_start_idx);
BOOST_CHECK_EQUAL(cs1.setBlockIndexCandidates.count(assumed_base), 1);
// The assumed-valid tolerant chain has all blocks as candidates.
BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.count(validated_tip), 1);
// The assumed-valid tolerant chain has the assumed valid base as a
// candidate, but otherwise has none of the assumed-valid (which do not
// HAVE_DATA) blocks as candidates.
BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.count(validated_tip), 0);
BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.count(assumed_tip), 1);
BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.size(), num_indexes);
BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.size(), num_indexes - last_assumed_valid_idx + 1);
}
//! Ensure that snapshot chainstates initialize properly when found on disk.

View File

@ -4432,62 +4432,19 @@ 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();
// If we encounter an assumed-valid block index entry, ensure that we have
// one chainstate that tolerates assumed-valid entries and another that does
// not (i.e. the background validation chainstate), since assumed-valid
// entries should always be pending validation by a fully-validated chainstate.
auto any_chain = [&](auto fnc) { return std::any_of(chainstates.cbegin(), chainstates.cend(), fnc); };
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;
}
}
for (CBlockIndex* pindex : vSortedByHeight) {
if (m_interrupt) return false;
if (pindex->IsAssumedValid() ||
// If we have an assumeutxo-based chainstate, then the snapshot
// block will be a candidate for the tip, but it may not be
// VALID_TRANSACTIONS (eg if we haven't yet downloaded the block),
// so we special-case the snapshot block as a potential candidate
// here.
if (pindex == GetSnapshotBaseBlock() ||
(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)) {