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 that are ancestors of the
snapshotted block and that have more work than the tip as potential candidates.
  • Loading branch information
@sdaftuar
sdaftuar committed Jul 24, 2023
1 parent d43a1f1 commit 768690b
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 64 deletions.
37 changes: 23 additions & 14 deletions src/test/validation_chainstatemanager_tests.cpp
View file
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}
Expand All @@ -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.
Expand Down
57 changes: 7 additions & 50 deletions src/validation.cpp
View file
Expand Up @@ -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)) {
Expand Down

0 comments on commit 768690b

Please sign in to comment.