diff options
author | James O'Beirne <james.obeirne@pm.me> | 2021-10-28 16:07:46 -0400 |
---|---|---|
committer | James O'Beirne <james.obeirne@pm.me> | 2021-12-13 13:45:27 -0500 |
commit | 0fd599a51a700c028d6f7ed8067d2d9f6e6a04a5 (patch) | |
tree | b3f7e049a45d00dc3e849aea9c0f5fe7c53862ed /src | |
parent | d0c6e61f5dd3b6af818459d9d03b7ba316c5a3f7 (diff) | |
download | bitcoin-0fd599a51a700c028d6f7ed8067d2d9f6e6a04a5.tar.xz |
validation: have LoadBlockIndex account for snapshot use
Ensure that blocks past the snapshot base block (i.e. the end of the
assumed-valid region of the chain) are not included in
setBlockIndexCandidates for the background validation chainstate. These
blocks, while fully validated and lacking the BLOCK_ASSUMED_VALID flag,
*rely* on blocks which are assumed-valid, and so shouldn't be added to
the IBD chainstate.
Co-authored-by: Russ Yanofsky <russ@yanofsky.org>
Diffstat (limited to 'src')
-rw-r--r-- | src/validation.cpp | 70 | ||||
-rw-r--r-- | src/validation.h | 12 |
2 files changed, 67 insertions, 15 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index 110218de36..bfa7f4bbf1 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -55,6 +55,7 @@ #include <validationinterface.h> #include <warnings.h> +#include <algorithm> #include <numeric> #include <optional> #include <string> @@ -3607,7 +3608,7 @@ CBlockIndex * BlockManager::InsertBlockIndex(const uint256& hash) bool BlockManager::LoadBlockIndex( const Consensus::Params& consensus_params, - std::set<CBlockIndex*, CBlockIndexWorkComparator>& block_index_candidates) + ChainstateManager& chainman) { if (!m_block_tree_db->LoadBlockIndexGuts(consensus_params, [this](const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return this->InsertBlockIndex(hash); })) { return false; @@ -3622,17 +3623,41 @@ bool BlockManager::LoadBlockIndex( vSortedByHeight.push_back(std::make_pair(pindex->nHeight, pindex)); } sort(vSortedByHeight.begin(), vSortedByHeight.end()); + + // Find start of assumed-valid region. + int first_assumed_valid_height = std::numeric_limits<int>::max(); + + for (const auto& [height, block] : vSortedByHeight) { + if (block->IsAssumedValid()) { + auto chainstates = chainman.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 = height; + break; + } + } + for (const std::pair<int, CBlockIndex*>& item : vSortedByHeight) { if (ShutdownRequested()) return false; CBlockIndex* pindex = item.second; pindex->nChainWork = (pindex->pprev ? pindex->pprev->nChainWork : 0) + GetBlockProof(*pindex); pindex->nTimeMax = (pindex->pprev ? std::max(pindex->pprev->nTimeMax, pindex->nTime) : pindex->nTime); - // We can link the chain of blocks for which we've received transactions at some point. + + // We can link the chain of blocks for which we've received transactions at some point, or + // blocks that are assumed-valid on the basis of snapshot load (see + // PopulateAndValidateSnapshot()). // Pruned nodes may have deleted the block. if (pindex->nTx > 0) { if (pindex->pprev) { - if (pindex->pprev->HaveTxsDownloaded()) { + if (pindex->pprev->nChainTx > 0) { pindex->nChainTx = pindex->pprev->nChainTx + pindex->nTx; } else { pindex->nChainTx = 0; @@ -3649,7 +3674,36 @@ bool BlockManager::LoadBlockIndex( if (pindex->IsAssumedValid() || (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr))) { - block_index_candidates.insert(pindex); + + // 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 the background chainstate, but this is ok, + // because they will still be attached to the active chainstate if they + // actually contain more work. + // + // Instad 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 (CChainState* chainstate : chainman.GetAll()) { + if (chainstate->reliesOnAssumedValid() || + pindex->nHeight < first_assumed_valid_height) { + chainstate->setBlockIndexCandidates.insert(pindex); + } + } } if (pindex->nStatus & BLOCK_FAILED_MASK && (!pindexBestInvalid || pindex->nChainWork > pindexBestInvalid->nChainWork)) pindexBestInvalid = pindex; @@ -3673,11 +3727,9 @@ void BlockManager::Unload() { m_block_index.clear(); } -bool BlockManager::LoadBlockIndexDB(std::set<CBlockIndex*, CBlockIndexWorkComparator>& setBlockIndexCandidates) +bool BlockManager::LoadBlockIndexDB(ChainstateManager& chainman) { - if (!LoadBlockIndex( - ::Params().GetConsensus(), - setBlockIndexCandidates)) { + if (!LoadBlockIndex(::Params().GetConsensus(), chainman)) { return false; } @@ -4023,7 +4075,7 @@ bool ChainstateManager::LoadBlockIndex() // Load block index from databases bool needs_init = fReindex; if (!fReindex) { - bool ret = m_blockman.LoadBlockIndexDB(ActiveChainstate().setBlockIndexCandidates); + bool ret = m_blockman.LoadBlockIndexDB(*this); if (!ret) return false; needs_init = m_blockman.m_block_index.empty(); } diff --git a/src/validation.h b/src/validation.h index 4da8ec8d24..7c7ad4bbc6 100644 --- a/src/validation.h +++ b/src/validation.h @@ -427,20 +427,16 @@ public: std::unique_ptr<CBlockTreeDB> m_block_tree_db GUARDED_BY(::cs_main); - bool LoadBlockIndexDB(std::set<CBlockIndex*, CBlockIndexWorkComparator>& setBlockIndexCandidates) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool LoadBlockIndexDB(ChainstateManager& chainman) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** * Load the blocktree off disk and into memory. Populate certain metadata * per index entry (nStatus, nChainWork, nTimeMax, etc.) as well as peripheral * collections like setDirtyBlockIndex. - * - * @param[out] block_index_candidates Fill this set with any valid blocks for - * which we've downloaded all transactions. */ bool LoadBlockIndex( const Consensus::Params& consensus_params, - std::set<CBlockIndex*, CBlockIndexWorkComparator>& block_index_candidates) - EXCLUSIVE_LOCKS_REQUIRED(cs_main); + ChainstateManager& chainman) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Clear all data members. */ void Unload() EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -626,6 +622,10 @@ public: */ const std::optional<uint256> m_from_snapshot_blockhash; + //! Return true if this chainstate relies on blocks that are assumed-valid. In + //! practice this means it was created based on a UTXO snapshot. + bool reliesOnAssumedValid() { return m_from_snapshot_blockhash.has_value(); } + /** * The set of all CBlockIndex entries with either BLOCK_VALID_TRANSACTIONS (for * itself and all ancestors) *or* BLOCK_ASSUMED_VALID (if using background |