diff --git a/src/chain.h b/src/chain.h index eb4ff71935d..7b65c76d7b4 100644 --- a/src/chain.h +++ b/src/chain.h @@ -428,6 +428,15 @@ public: return int(vChain.size()) - 1; } + /** Check whether this chain's tip exists, has enough work, and is recent. */ + bool IsTipRecent(const arith_uint256& min_chain_work, std::chrono::seconds max_tip_age) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main) + { + const auto tip{Tip()}; + return tip && + tip->nChainWork >= min_chain_work && + tip->Time() >= Now() - max_tip_age; + } + /** Set/initialize a chain with a given tip. */ void SetTip(CBlockIndex& block); diff --git a/src/init.cpp b/src/init.cpp index 00718a338c7..841fdec5b2c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1978,6 +1978,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) ScheduleBatchPriority(); // Import blocks and ActivateBestChain() ImportBlocks(chainman, vImportFiles); + WITH_LOCK(::cs_main, chainman.UpdateIBDStatus()); if (args.GetBoolArg("-stopafterblockimport", DEFAULT_STOPAFTERBLOCKIMPORT)) { LogInfo("Stopping after block import"); if (!(Assert(node.shutdown_request))()) { diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp index 03698e72b92..c8189b629b6 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -1076,7 +1076,9 @@ int btck_chainstate_manager_import_blocks(btck_ChainstateManager* chainman, cons import_files.emplace_back(std::string{block_file_paths_data[i], block_file_paths_lens[i]}.c_str()); } } - node::ImportBlocks(*btck_ChainstateManager::get(chainman).m_chainman, import_files); + auto& chainman_ref{*btck_ChainstateManager::get(chainman).m_chainman}; + node::ImportBlocks(chainman_ref, import_files); + WITH_LOCK(::cs_main, chainman_ref.UpdateIBDStatus()); } catch (const std::exception& e) { LogError("Failed to import blocks: %s", e.what()); return -1; diff --git a/src/test/fuzz/block_index_tree.cpp b/src/test/fuzz/block_index_tree.cpp index 131ec93533f..a4bbe958400 100644 --- a/src/test/fuzz/block_index_tree.cpp +++ b/src/test/fuzz/block_index_tree.cpp @@ -207,7 +207,7 @@ FUZZ_TARGET(block_index_tree, .init = initialize_block_index_tree) chainman.nBlockSequenceId = 2; chainman.ActiveChain().SetTip(*genesis); chainman.ActiveChainstate().setBlockIndexCandidates.clear(); - chainman.m_cached_finished_ibd = false; + chainman.m_cached_is_ibd = true; blockman.m_blocks_unlinked.clear(); blockman.m_have_pruned = false; blockman.CleanupForFuzzing(); diff --git a/src/test/util/validation.cpp b/src/test/util/validation.cpp index bb9c6db2255..7e09597ea34 100644 --- a/src/test/util/validation.cpp +++ b/src/test/util/validation.cpp @@ -32,14 +32,14 @@ void TestChainstateManager::DisableNextWrite() void TestChainstateManager::ResetIbd() { - m_cached_finished_ibd = false; + m_cached_is_ibd = true; assert(IsInitialBlockDownload()); } void TestChainstateManager::JumpOutOfIbd() { Assert(IsInitialBlockDownload()); - m_cached_finished_ibd = true; + m_cached_is_ibd = false; Assert(!IsInitialBlockDownload()); } diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 24122bd3157..41dfdcd1620 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -143,11 +143,11 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); // Reset IBD state so IsInitialBlockDownload() returns true and causes - // MaybeRebalancesCaches() to prioritize the snapshot chainstate, giving it + // MaybeRebalanceCaches() to prioritize the snapshot chainstate, giving it // more cache space than the snapshot chainstate. Calling ResetIbd() is - // necessary because m_cached_finished_ibd is already latched to true before - // the test starts due to the test setup. After ResetIbd() is called. - // IsInitialBlockDownload will return true because at this point the active + // necessary because m_cached_is_ibd is already latched to false before + // the test starts due to the test setup. After ResetIbd() is called, + // IsInitialBlockDownload() will return true because at this point the active // chainstate has a null chain tip. static_cast(manager).ResetIbd(); @@ -163,6 +163,44 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) BOOST_CHECK_CLOSE(double(c2.m_coinsdb_cache_size_bytes), max_cache * 0.95, 1); } +BOOST_FIXTURE_TEST_CASE(chainstatemanager_ibd_exit_after_loading_blocks, ChainTestingSetup) +{ + CBlockIndex tip; + ChainstateManager& chainman{*Assert(m_node.chainman)}; + auto apply{[&](bool cached_is_ibd, bool loading_blocks, bool tip_exists, bool enough_work, bool tip_recent) { + LOCK(::cs_main); + chainman.ResetChainstates(); + chainman.InitializeChainstate(m_node.mempool.get()); + + const auto recent_time{Now() - chainman.m_options.max_tip_age}; + + chainman.m_cached_is_ibd.store(cached_is_ibd, std::memory_order_relaxed); + chainman.m_blockman.m_importing = loading_blocks; + if (tip_exists) { + tip.nChainWork = chainman.MinimumChainWork() - (enough_work ? 0 : 1); + tip.nTime = (recent_time - (tip_recent ? 0h : 100h)).time_since_epoch().count(); + chainman.ActiveChain().SetTip(tip); + } else { + assert(!chainman.ActiveChain().Tip()); + } + chainman.UpdateIBDStatus(); + }}; + + for (const bool cached_is_ibd : {false, true}) { + for (const bool loading_blocks : {false, true}) { + for (const bool tip_exists : {false, true}) { + for (const bool enough_work : {false, true}) { + for (const bool tip_recent : {false, true}) { + apply(cached_is_ibd, loading_blocks, tip_exists, enough_work, tip_recent); + const bool expected_ibd = cached_is_ibd && (loading_blocks || !tip_exists || !enough_work || !tip_recent); + BOOST_CHECK_EQUAL(chainman.IsInitialBlockDownload(), expected_ibd); + } + } + } + } + } +} + struct SnapshotTestSetup : TestChain100Setup { // Run with coinsdb on the filesystem to support, e.g., moving invalidated // chainstate dirs to "*_invalid". diff --git a/src/validation.cpp b/src/validation.cpp index bb92e51e6a0..3330d261eb9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1934,36 +1934,15 @@ void Chainstate::InitCoinsCache(size_t cache_size_bytes) m_coins_views->InitCache(); } -// Note that though this is marked const, we may end up modifying `m_cached_finished_ibd`, which -// is a performance-related implementation detail. This function must be marked -// `const` so that `CValidationInterface` clients (which are given a `const Chainstate*`) -// can call it. +// This function must be marked `const` so that `CValidationInterface` clients +// (which are given a `const Chainstate*`) can call it. +// +// It is lock-free and depends on `m_cached_is_ibd`, which is latched by +// `UpdateIBDStatus()`. // bool ChainstateManager::IsInitialBlockDownload() const { - // Optimization: pre-test latch before taking the lock. - if (m_cached_finished_ibd.load(std::memory_order_relaxed)) - return false; - - LOCK(cs_main); - if (m_cached_finished_ibd.load(std::memory_order_relaxed)) - return false; - if (m_blockman.LoadingBlocks()) { - return true; - } - CChain& chain{ActiveChain()}; - if (chain.Tip() == nullptr) { - return true; - } - if (chain.Tip()->nChainWork < MinimumChainWork()) { - return true; - } - if (chain.Tip()->Time() < Now() - m_options.max_tip_age) { - return true; - } - LogInfo("Leaving InitialBlockDownload (latching to false)"); - m_cached_finished_ibd.store(true, std::memory_order_relaxed); - return false; + return m_cached_is_ibd.load(std::memory_order_relaxed); } void Chainstate::CheckForkWarningConditions() @@ -3007,6 +2986,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra } m_chain.SetTip(*pindexDelete->pprev); + m_chainman.UpdateIBDStatus(); UpdateTip(pindexDelete->pprev); // Let wallets know transactions went from 1-confirmed to @@ -3136,6 +3116,7 @@ bool Chainstate::ConnectTip( } // Update m_chain & related variables. m_chain.SetTip(*pindexNew); + m_chainman.UpdateIBDStatus(); UpdateTip(pindexNew); const auto time_6{SteadyClock::now()}; @@ -3339,6 +3320,15 @@ static SynchronizationState GetSynchronizationState(bool init, bool blockfiles_i return SynchronizationState::INIT_DOWNLOAD; } +void ChainstateManager::UpdateIBDStatus() +{ + if (!m_cached_is_ibd.load(std::memory_order_relaxed)) return; + if (m_blockman.LoadingBlocks()) return; + if (!CurrentChainstate().m_chain.IsTipRecent(MinimumChainWork(), m_options.max_tip_age)) return; + LogInfo("Leaving InitialBlockDownload (latching to false)"); + m_cached_is_ibd.store(false, std::memory_order_relaxed); +} + bool ChainstateManager::NotifyHeaderTip() { bool fNotify = false; @@ -4621,6 +4611,7 @@ bool Chainstate::LoadChainTip() return false; } m_chain.SetTip(*pindex); + m_chainman.UpdateIBDStatus(); tip = m_chain.Tip(); // Make sure our chain tip before shutting down scores better than any other candidate diff --git a/src/validation.h b/src/validation.h index 28ae96a6f45..ee7257b3ba5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1030,13 +1030,13 @@ public: ValidationCache m_validation_cache; /** - * Whether initial block download has ended and IsInitialBlockDownload - * should return false from now on. + * Whether initial block download (IBD) is ongoing. * - * Mutable because we need to be able to mark IsInitialBlockDownload() - * const, which latches this for caching purposes. + * This value is used for lock-free IBD checks, and latches from true to + * false once block loading has finished and the current chain tip has + * enough work and is recent. */ - mutable std::atomic m_cached_finished_ibd{false}; + std::atomic_bool m_cached_is_ibd{true}; /** * Every received block is assigned a unique and increasing identifier, so we @@ -1157,6 +1157,19 @@ public: CBlockIndex* ActiveTip() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChain().Tip(); } //! @} + /** + * Update and possibly latch the IBD status. + * + * If block loading has finished and the current chain tip has enough work + * and is recent, set `m_cached_is_ibd` to false. This function never sets + * the flag back to true. + * + * This should be called after operations that may affect IBD exit + * conditions (e.g. after updating the active chain tip, or after + * `ImportBlocks()` finishes). + */ + void UpdateIBDStatus() EXCLUSIVE_LOCKS_REQUIRED(cs_main); + node::BlockMap& BlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main);