From 6082c84713f42f5fa66f9a76baef17e8ed231633 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 29 May 2024 20:27:27 -0400 Subject: [PATCH] refactor: Add Chainstate::m_target_blockhash member Make Chainstate objects aware of what block they are targeting. This makes Chainstate objects more self contained, so it's possible for validation code to look at one Chainstate object and know what blocks to connect to it without needing to consider global validation state or look at other Chainstate objects. The motivation for this change is to make validation and networking code more readable, so understanding it just requires knowing about chains and blocks, not reasoning about assumeutxo download states. This change also enables simplifications to the ChainstateManager interface in subsequent commits, and could make it easier to implement new features like creating new Chainstate objects to generate UTXO snapshots or index UTXO data. Note that behavior of the MaybeCompleteSnapshotValidation function is not changing here but some checks that were previously impossible to trigger like the BASE_BLOCKHASH_MISMATCH case have been turned into asserts. --- src/init.cpp | 2 +- src/net_processing.cpp | 15 +-- src/node/chainstate.cpp | 4 +- src/test/validation_chainstate_tests.cpp | 11 +-- src/validation.cpp | 116 ++++++++++++++--------- src/validation.h | 53 ++++++++--- 6 files changed, 125 insertions(+), 76 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 09e7523e329..593467e3439 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1880,7 +1880,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } } else { // Prior to setting NODE_NETWORK, check if we can provide historical blocks. - if (!WITH_LOCK(chainman.GetMutex(), return chainman.BackgroundSyncInProgress())) { + if (!WITH_LOCK(chainman.GetMutex(), return chainman.HistoricalChainstate())) { LogInfo("Setting NODE_NETWORK in non-prune mode"); g_local_services = ServiceFlags(g_local_services | NODE_NETWORK); } else { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c8bc4a846df..6e60ce824fe 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5921,18 +5921,19 @@ bool PeerManagerImpl::SendMessages(CNode* pto) return std::max(0, MAX_BLOCKS_IN_TRANSIT_PER_PEER - static_cast(state.vBlocksInFlight.size())); }; - // If a snapshot chainstate is in use, we want to find its next blocks - // before the background chainstate to prioritize getting to network tip. + // If there are multiple chainstates, download blocks for the + // current chainstate first, to prioritize getting to network tip + // before downloading historical blocks. FindNextBlocksToDownload(*peer, get_inflight_budget(), vToDownload, staller); - if (m_chainman.BackgroundSyncInProgress() && !IsLimitedPeer(*peer)) { - // If the background tip is not an ancestor of the snapshot block, + auto historical_blocks{m_chainman.GetHistoricalBlockRange()}; + if (historical_blocks && !IsLimitedPeer(*peer)) { + // If the first needed historical block is not an ancestor of the last, // we need to start requesting blocks from their last common ancestor. - const CBlockIndex *from_tip = LastCommonAncestor(m_chainman.GetBackgroundSyncTip(), m_chainman.GetSnapshotBaseBlock()); + const CBlockIndex* from_tip = LastCommonAncestor(historical_blocks->first, historical_blocks->second); TryDownloadingHistoricalBlocks( *peer, get_inflight_budget(), - vToDownload, from_tip, - Assert(m_chainman.GetSnapshotBaseBlock())); + vToDownload, from_tip, historical_blocks->second); } for (const CBlockIndex *pindex : vToDownload) { uint32_t nFetchFlags = GetFetchFlags(*peer); diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 1f0cf805895..0cebcad336f 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -166,12 +166,14 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize chainman.m_total_coinsdb_cache = cache_sizes.coins_db; // Load the fully validated chainstate. - chainman.InitializeChainstate(options.mempool); + Chainstate& validated_cs{chainman.InitializeChainstate(options.mempool)}; // Load a chain created from a UTXO snapshot, if any exist. bool has_snapshot = chainman.DetectSnapshotChainstate(); if (has_snapshot && options.wipe_chainstate_db) { + // Reset chainstate target to network tip instead of snapshot block. + validated_cs.SetTargetBlock(nullptr); LogInfo("[snapshot] deleting snapshot chainstate due to reindexing"); if (!chainman.DeleteSnapshotChainstate()) { return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Couldn't remove snapshot chainstate.")}; diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 7204f1688dd..b4aeb5b3370 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -107,16 +107,7 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) curr_tip = get_notify_tip(); - BOOST_CHECK_EQUAL(chainman.GetAll().size(), 2); - - Chainstate& background_cs{*Assert([&]() -> Chainstate* { - for (Chainstate* cs : chainman.GetAll()) { - if (cs != &chainman.ActiveChainstate()) { - return cs; - } - } - return nullptr; - }())}; + Chainstate& background_cs{*Assert(WITH_LOCK(::cs_main, return chainman.HistoricalChainstate()))}; // Append the first block to the background chain. BlockValidationState state; diff --git a/src/validation.cpp b/src/validation.cpp index 3464e588456..0831b07ccd9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1891,6 +1891,29 @@ const CBlockIndex* Chainstate::SnapshotBase() const return m_cached_snapshot_base; } +const CBlockIndex* Chainstate::TargetBlock() const +{ + if (!m_target_blockhash) return nullptr; + if (!m_cached_target_block) m_cached_target_block = Assert(m_chainman.m_blockman.LookupBlockIndex(*m_target_blockhash)); + return m_cached_target_block; +} + +void Chainstate::SetTargetBlock(CBlockIndex* block) +{ + if (block) { + m_target_blockhash = block->GetBlockHash(); + } else { + m_target_blockhash.reset(); + } + m_cached_target_block = block; +} + +void Chainstate::SetTargetBlockHash(uint256 block_hash) +{ + m_target_blockhash = block_hash; + m_cached_target_block = nullptr; +} + void Chainstate::InitCoinsDB( size_t cache_size_bytes, bool in_memory, @@ -3144,8 +3167,6 @@ bool Chainstate::ConnectTip( // If we are the background validation chainstate, check to see if we are done // validating the snapshot (i.e. our tip has reached the snapshot's base block). if (this != &m_chainman.ActiveChainstate()) { - // This call may set `m_disabled`, which is referenced immediately afterwards in - // ActivateBestChain, so that we stop connecting blocks past the snapshot base. m_chainman.MaybeCompleteSnapshotValidation(); } @@ -3439,12 +3460,8 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< } } - // This will have been toggled in - // ActivateBestChainStep -> ConnectTip -> MaybeCompleteSnapshotValidation, - // if at all, so we should catch it here. - // - // Break this do-while to ensure we don't advance past the base snapshot. - if (m_disabled) { + // Break this do-while to ensure we don't advance past the target block. + if (ReachedTarget()) { break; } } while (!m_chain.Tip() || (starting_tip && CBlockIndexWorkComparator()(m_chain.Tip(), starting_tip))); @@ -3486,7 +3503,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< } // release cs_main // When we reach this point, we switched to a new tip (stored in pindexNewTip). - bool disabled{false}; + bool reached_target; { LOCK(m_chainman.GetMutex()); if (exited_ibd) { @@ -3500,15 +3517,14 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< return false; } - disabled = m_disabled; + reached_target = ReachedTarget(); } - if (disabled) { - // Background chainstate has reached the snapshot base block, so exit. - - // Restart indexes to resume indexing for all blocks unique to the snapshot - // chain. This resumes indexing "in order" from where the indexing on the - // background validation chain left off. + if (reached_target) { + // Chainstate has reached the target block, so exit. + // + // Restart indexes so indexes can resync and index new blocks after + // the target block. // // This cannot be done while holding cs_main (within // MaybeCompleteSnapshotValidation) or a cs_main deadlock will occur. @@ -3789,17 +3805,15 @@ void Chainstate::TryAddBlockIndexCandidate(CBlockIndex* pindex) return; } - bool is_active_chainstate = this == &m_chainman.ActiveChainstate(); - if (is_active_chainstate) { - // The active chainstate should always add entries that have more + const CBlockIndex* target_block{TargetBlock()}; + if (!target_block) { + // If no specific target block, add all entries that have more // work than the tip. setBlockIndexCandidates.insert(pindex); - } else if (!m_disabled) { - // For the background chainstate, we only consider connecting blocks - // towards the snapshot base (which can't be nullptr or else we'll - // never make progress). - const CBlockIndex* snapshot_base{Assert(m_chainman.GetSnapshotBaseBlock())}; - if (snapshot_base->GetAncestor(pindex->nHeight) == pindex) { + } else { + // If there is a target block, only consider connecting blocks + // towards the target block. + if (target_block->GetAncestor(pindex->nHeight) == pindex) { setBlockIndexCandidates.insert(pindex); } } @@ -4483,7 +4497,7 @@ bool ChainstateManager::ProcessNewBlock(const std::shared_ptr& blo return false; } - Chainstate* bg_chain{WITH_LOCK(cs_main, return BackgroundSyncInProgress() ? m_ibd_chainstate.get() : nullptr)}; + Chainstate* bg_chain{WITH_LOCK(cs_main, return HistoricalChainstate())}; BlockValidationState bg_state; if (bg_chain && !bg_chain->ActivateBestChain(bg_state, block)) { LogError("%s: [background] ActivateBestChain failed (%s)\n", __func__, bg_state.ToString()); @@ -5787,6 +5801,11 @@ util::Result ChainstateManager::ActivateSnapshot( const bool chaintip_loaded = m_snapshot_chainstate->LoadChainTip(); assert(chaintip_loaded); + // Set snapshot block as the target block for the historical chainstate. + assert(m_ibd_chainstate.get()); + assert(!m_ibd_chainstate->m_target_blockhash); + m_ibd_chainstate->SetTargetBlockHash(base_blockhash); + // Transfer possession of the mempool to the snapshot chainstate. // Mempool is empty at this point because we're still in IBD. Assert(m_active_chainstate->m_mempool->size() == 0); @@ -6048,23 +6067,25 @@ util::Result ChainstateManager::PopulateAndValidateSnapshot( SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() { AssertLockHeld(cs_main); + + // If a snapshot does not need to be validated... if (m_ibd_chainstate.get() == &this->ActiveChainstate() || + // Or if either chainstate is unusable... !this->IsUsable(m_snapshot_chainstate.get()) || !this->IsUsable(m_ibd_chainstate.get()) || - !m_ibd_chainstate->m_chain.Tip()) { - // Nothing to do - this function only applies to the background - // validation chainstate. + !m_snapshot_chainstate->m_from_snapshot_blockhash || + !m_ibd_chainstate->m_chain.Tip() || + // Or the validated chainstate is not targeting the snapshot block... + !m_ibd_chainstate->m_target_blockhash || + *m_ibd_chainstate->m_target_blockhash != *m_snapshot_chainstate->m_from_snapshot_blockhash || + // Or the validated chainstate has not reached the snapshot block yet... + !m_ibd_chainstate->ReachedTarget()) { + // Then a snapshot cannot be validated and there is nothing to do. return SnapshotCompletionResult::SKIPPED; } const int snapshot_tip_height = this->ActiveHeight(); const int snapshot_base_height = *Assert(this->GetSnapshotBaseHeight()); const CBlockIndex& index_new = *Assert(m_ibd_chainstate->m_chain.Tip()); - - if (index_new.nHeight < snapshot_base_height) { - // Background IBD not complete yet. - return SnapshotCompletionResult::SKIPPED; - } - assert(SnapshotBlockhash()); uint256 snapshot_blockhash = *Assert(SnapshotBlockhash()); @@ -6087,6 +6108,9 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() LogError("[snapshot] !!! %s\n", user_error.original); LogError("[snapshot] deleting snapshot, reverting to validated chain, and stopping node\n"); + // Reset chainstate target to network tip instead of snapshot block. + m_ibd_chainstate->SetTargetBlock(nullptr); + m_active_chainstate = m_ibd_chainstate.get(); m_snapshot_chainstate->m_disabled = true; assert(!this->IsUsable(m_snapshot_chainstate.get())); @@ -6100,14 +6124,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() GetNotifications().fatalError(user_error); }; - if (index_new.GetBlockHash() != snapshot_blockhash) { - LogWarning("[snapshot] supposed base block %s does not match the " - "snapshot base block %s (height %d). Snapshot is not valid.", - index_new.ToString(), snapshot_blockhash.ToString(), snapshot_base_height); - handle_invalid_snapshot(); - return SnapshotCompletionResult::BASE_BLOCKHASH_MISMATCH; - } - + assert(index_new.GetBlockHash() == snapshot_blockhash); assert(index_new.nHeight == snapshot_base_height); int curr_height = m_ibd_chainstate->m_chain.Height(); @@ -6286,6 +6303,12 @@ Chainstate& ChainstateManager::ActivateExistingSnapshot(uint256 base_blockhash) std::make_unique(nullptr, m_blockman, *this, base_blockhash); LogInfo("[snapshot] switching active chainstate to %s", m_snapshot_chainstate->ToString()); + // Set target block for historical chainstate to snapshot block. + assert(m_ibd_chainstate.get()); + assert(!m_ibd_chainstate->m_target_blockhash); + m_ibd_chainstate->SetTargetBlockHash(base_blockhash); + + // Transfer possession of the mempool to the chainstate. // Mempool is empty at this point because we're still in IBD. Assert(m_active_chainstate->m_mempool->size() == 0); Assert(!m_snapshot_chainstate->m_mempool); @@ -6522,3 +6545,10 @@ std::pair ChainstateManager::GetPruneRange(const Chainstate& chainstat return {prune_start, prune_end}; } + +std::optional> ChainstateManager::GetHistoricalBlockRange() const +{ + const Chainstate* chainstate{HistoricalChainstate()}; + if (!chainstate) return {}; + return std::make_pair(chainstate->m_chain.Tip(), chainstate->TargetBlock()); +} diff --git a/src/validation.h b/src/validation.h index cd448f3ca9e..e547f88c674 100644 --- a/src/validation.h +++ b/src/validation.h @@ -561,6 +561,9 @@ protected: //! Cached result of LookupBlockIndex(*m_from_snapshot_blockhash) mutable const CBlockIndex* m_cached_snapshot_base GUARDED_BY(::cs_main){nullptr}; + //! Cached result of LookupBlockIndex(*m_target_blockhash) + mutable const CBlockIndex* m_cached_target_block GUARDED_BY(::cs_main){nullptr}; + std::optional m_last_script_check_reason_logged GUARDED_BY(::cs_main){}; public: @@ -620,6 +623,12 @@ public: */ const std::optional m_from_snapshot_blockhash; + //! Target block for this chainstate. If this is not set, chainstate will + //! target the most-work, valid block. If this is set, ChainstateManager + //! considers this a "historical" chainstate since it will only contain old + //! blocks up to the target block, not newer blocks. + std::optional m_target_blockhash GUARDED_BY(::cs_main); + /** * The base of the snapshot this chainstate was created from. * @@ -627,6 +636,26 @@ public: */ const CBlockIndex* SnapshotBase() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! Return target block which chainstate tip is expected to reach, if this + //! is a historic chainstate being used to validate a snapshot, or null if + //! chainstate targets the most-work block. + const CBlockIndex* TargetBlock() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! Set target block for this chainstate. If null, chainstate will target + //! the most-work valid block. If non-null chainstate will be a historic + //! chainstate and target the specified block. + void SetTargetBlock(CBlockIndex* block) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! Set target block for this chainstate using just a block hash. Useful + //! when the block database has not been loaded yet. + void SetTargetBlockHash(uint256 block_hash) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + + //! Return true if chainstate reached target block. + bool ReachedTarget() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main) + { + const CBlockIndex* target_block{TargetBlock()}; + assert(!target_block || target_block->GetAncestor(m_chain.Height()) == m_chain.Tip()); + return target_block && target_block == m_chain.Tip(); + } + /** * The set of all CBlockIndex entries that have as much work as our current * tip or more, and transaction data needed to be validated (with @@ -862,10 +891,6 @@ enum class SnapshotCompletionResult { // The UTXO set hash of the background validation chainstate does not match // the one expected by assumeutxo chainparams. HASH_MISMATCH, - - // The blockhash of the current tip of the background validation chainstate does - // not match the one expected by the snapshot chainstate. - BASE_BLOCKHASH_MISMATCH, }; /** @@ -1121,22 +1146,19 @@ public: //! Returns nullptr if no snapshot has been loaded. const CBlockIndex* GetSnapshotBaseBlock() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! Return historical chainstate targeting a specific block, if any. + Chainstate* HistoricalChainstate() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) + { + auto* cs{m_ibd_chainstate.get()}; + return IsUsable(cs) && cs->m_target_blockhash ? cs : nullptr; + } + //! The most-work chain. Chainstate& ActiveChainstate() const; CChain& ActiveChain() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChainstate().m_chain; } int ActiveHeight() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChain().Height(); } CBlockIndex* ActiveTip() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChain().Tip(); } - //! The state of a background sync (for net processing) - bool BackgroundSyncInProgress() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { - return IsUsable(m_snapshot_chainstate.get()) && IsUsable(m_ibd_chainstate.get()); - } - - //! The tip of the background sync chain - const CBlockIndex* GetBackgroundSyncTip() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { - return BackgroundSyncInProgress() ? m_ibd_chainstate->m_chain.Tip() : nullptr; - } - node::BlockMap& BlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); @@ -1337,6 +1359,9 @@ public: //! nullopt. std::optional GetSnapshotBaseHeight() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! Get range of historical blocks to download. + std::optional> GetHistoricalBlockRange() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! If, due to invalidation / reconsideration of blocks, the previous //! best header is no longer valid / guaranteed to be the most-work //! header in our block-index not known to be invalid, recalculate it.