diff --git a/src/init.cpp b/src/init.cpp index f2af858eb46..a245ebfbd76 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1966,6 +1966,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 b061a185483..a6a9c325dd1 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -1056,6 +1056,7 @@ int btck_chainstate_manager_import_blocks(btck_ChainstateManager* chainman, cons } 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/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index c576b89fb33..41dfdcd1620 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -183,6 +183,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_ibd_exit_after_loading_blocks, ChainTe } else { assert(!chainman.ActiveChain().Tip()); } + chainman.UpdateIBDStatus(); }}; for (const bool cached_is_ibd : {false, true}) { diff --git a/src/validation.cpp b/src/validation.cpp index 1b3b09cbabe..09675b2a3d6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1939,23 +1939,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_is_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_is_ibd.load(std::memory_order_relaxed)) return false; - - LOCK(cs_main); - if (!m_cached_is_ibd.load(std::memory_order_relaxed)) return false; - if (m_blockman.LoadingBlocks()) return true; - if (!ActiveChain().IsTipRecent(MinimumChainWork(), m_options.max_tip_age)) return true; - LogInfo("Leaving InitialBlockDownload (latching to false)"); - m_cached_is_ibd.store(false, std::memory_order_relaxed); - return false; + return m_cached_is_ibd.load(std::memory_order_relaxed); } void Chainstate::CheckForkWarningConditions() @@ -2999,6 +2991,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 @@ -3128,6 +3121,7 @@ bool Chainstate::ConnectTip( } // Update m_chain & related variables. m_chain.SetTip(*pindexNew); + m_chainman.UpdateIBDStatus(); UpdateTip(pindexNew); const auto time_6{SteadyClock::now()}; @@ -3331,6 +3325,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; @@ -4614,6 +4617,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 2c518bad151..438aab3e784 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1032,9 +1032,11 @@ public: /** * Whether initial block download (IBD) is ongoing. * - * Once set to false, IsInitialBlockDownload() will keep returning false. + * 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_bool m_cached_is_ibd{true}; + std::atomic_bool m_cached_is_ibd{true}; /** * Every received block is assigned a unique and increasing identifier, so we @@ -1155,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);