validation: make `IsInitialBlockDownload()` lock-free
What changed, and why it matters
This commit is a performance optimization, not a security fix. It changes how Bitcoin Core decides whether it is still downloading the historical blockchain (Initial Block Download, or IBD). Previously, every check for IBD status had to grab a global lock, which could slow down busy code paths. Now the status is cached in an atomic variable and only updated at specific safe points. There is no evidence in the commit or supplied references that this fixes a vulnerability or is being exploited.
No security action required. Treat as a normal performance/refactoring change. Reviewers may want to verify that UpdateIBDStatus() is called on all relevant tip-update paths and that the lock-free read cannot observe a stale true value indefinitely, but the commit explicitly documents this trade-off.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors ChainstateManager::IsInitialBlockDownload() to be lock-free by relying on a cached atomic bool (m_cached_is_ibd) and introduces a new UpdateIBDStatus() helper that latches the cache from true to false under cs_main. UpdateIBDStatus() is called after tip updates (ConnectTip, DisconnectTip, LoadChainTip) and after ImportBlocks() completes. The mutable qualifier on m_cached_is_ibd is removed because only UpdateIBDStatus() writes it. Unit tests are updated to call UpdateIBDStatus() explicitly. No security defect is described or patched.
Changed components
src/validation.cppsrc/validation.hsrc/init.cppsrc/kernel/bitcoinkernel.cppsrc/test/validation_chainstatemanager_tests.cppInspect captured patch +38 / −16
diff --git a/src/init.cpp b/src/init.cpp
index f2af858e..a245ebfb 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 b061a185..a6a9c325 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 c576b89f..41dfdcd1 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 1b3b09cb..09675b2a 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 2c518bad..438aab3e 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);
Why this scored 14/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.