validation: invert `m_cached_finished_ibd` to `m_cached_is_ibd`
What changed, and why it matters
This commit is a simple rename and logic inversion inside Bitcoin Core. It changes an internal cached flag from 'finished IBD' to 'is IBD' so the code avoids double negatives. The actual behavior of the program is unchanged.
No security action needed; this is a behavior-preserving refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch renames ChainstateManager’s mutable atomic latch from m_cached_finished_ibd to m_cached_is_ibd and flips its default from false to true. All reads and writes are inverted consistently, and the surrounding IsInitialBlockDownload() logic remains semantically identical. Tests and fuzz harnesses are updated to match the new name and polarity.
Changed components
src/validation.cppsrc/validation.hsrc/test/fuzz/block_index_tree.cppsrc/test/util/validation.cppsrc/test/validation_chainstatemanager_tests.cppInspect captured patch +16 / −18
diff --git a/src/test/fuzz/block_index_tree.cpp b/src/test/fuzz/block_index_tree.cpp
index 131ec935..a4bbe958 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 bb9c6db2..7e09597e 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 d0b731b3..c576b89f 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -145,7 +145,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup)
// Reset IBD state so IsInitialBlockDownload() returns true and causes
// 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
+ // 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.
@@ -167,14 +167,14 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_ibd_exit_after_loading_blocks, ChainTe
{
CBlockIndex tip;
ChainstateManager& chainman{*Assert(m_node.chainman)};
- auto apply{[&](bool cached_finished_ibd, bool loading_blocks, bool tip_exists, bool enough_work, bool tip_recent) {
+ 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<NodeSeconds>() - chainman.m_options.max_tip_age};
- chainman.m_cached_finished_ibd.store(cached_finished_ibd, std::memory_order_relaxed);
+ 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);
@@ -185,13 +185,13 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_ibd_exit_after_loading_blocks, ChainTe
}
}};
- for (const bool cached_finished_ibd : {false, true}) {
+ 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_finished_ibd, loading_blocks, tip_exists, enough_work, tip_recent);
- const bool expected_ibd = !cached_finished_ibd && (loading_blocks || !tip_exists || !enough_work || !tip_recent);
+ 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);
}
}
diff --git a/src/validation.cpp b/src/validation.cpp
index b6da6d2d..3b10237e 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1939,7 +1939,7 @@ 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
+// 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.
@@ -1947,11 +1947,11 @@ void Chainstate::InitCoinsCache(size_t cache_size_bytes)
bool ChainstateManager::IsInitialBlockDownload() const
{
// Optimization: pre-test latch before taking the lock.
- if (m_cached_finished_ibd.load(std::memory_order_relaxed))
+ if (!m_cached_is_ibd.load(std::memory_order_relaxed))
return false;
LOCK(cs_main);
- if (m_cached_finished_ibd.load(std::memory_order_relaxed))
+ if (!m_cached_is_ibd.load(std::memory_order_relaxed))
return false;
if (m_blockman.LoadingBlocks()) {
return true;
@@ -1967,7 +1967,7 @@ bool ChainstateManager::IsInitialBlockDownload() const
return true;
}
LogInfo("Leaving InitialBlockDownload (latching to false)");
- m_cached_finished_ibd.store(true, std::memory_order_relaxed);
+ m_cached_is_ibd.store(false, std::memory_order_relaxed);
return false;
}
diff --git a/src/validation.h b/src/validation.h
index e4b1e555..2c518bad 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -1030,13 +1030,11 @@ 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.
+ * Once set to false, IsInitialBlockDownload() will keep returning false.
*/
- mutable std::atomic<bool> m_cached_finished_ibd{false};
+ mutable std::atomic_bool m_cached_is_ibd{true};
/**
* Every received block is assigned a unique and increasing identifier, so we
Why this scored 15/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.