validation: make IsInitialBlockDownload() lock-free

`ChainstateManager::IsInitialBlockDownload()` is queried on hot paths and previously acquired `cs_main` internally, contributing to lock contention.

Cache the IBD status in `m_cached_is_ibd`, and introduce `ChainstateManager::UpdateIBDStatus()` to latch it once block loading has finished and the current chain tip has enough work and is recent.
Call the updater after tip updates and after `ImportBlocks()` completes.

Since `IsInitialBlockDownload()` no longer updates the cache, drop `mutable` from `m_cached_is_ibd` and only update it from `UpdateIBDStatus()` under `cs_main`.

Update the new unit test to showcase the new `UpdateIBDStatus()`.

Co-authored-by: Patrick Strateman <patrick.strateman@gmail.com>
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>

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))()) {

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;

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}) {

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

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);