Page MenuHomePhabricator
Files F13711393

D17535.diff
No OneTemporary
Actions

D17535.diff
View Options

diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -441,7 +441,8 @@
//! - 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.
+//! and the other chainstate contains all blocks, except those marked
+//! assume-valid, because those entries don't HAVE_DATA.
//!
BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup) {
ChainstateManager &chainman = *Assert(m_node.chainman);
@@ -462,14 +463,12 @@
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();
Chainstate &cs1 = chainman.ActiveChainstate();
- 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.
int num_indexes{0};
int num_assumed_valid{0};
const int expected_assumed_valid{20};
@@ -479,10 +478,13 @@
CBlockIndex *validated_tip{nullptr};
CBlockIndex *assumed_base{nullptr};
+ // 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()
.withValidity(BlockValidity::TREE)
@@ -499,16 +501,18 @@
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(
&*m_node.mempool, assumed_base->GetBlockHash()));
@@ -516,19 +520,24 @@
// 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.
diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5516,73 +5516,20 @@
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 (ShutdownRequested()) {
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(BlockValidity::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 assumed-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);
}
}

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 26, 11:51 (2 h, 28 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5573462
Default Alt Text
D17535.diff (9 KB)

Event Timeline