Merge bitcoin/bitcoin#34253: validation: cache tip recency for lock-free IsInitialBlockDownload()

557b41a38ccf2929ca1e5271db1701e5fbe781af validation: make `IsInitialBlockDownload()` lock-free (Lőrinc)
b9c0ab3b75a19d7a1f7c01762374ce85f2d0d7be chain: add `CChain::IsTipRecent` helper (Lőrinc)
8d531c6210eb05bc424c971f621bb0b688ff70e6 validation: invert `m_cached_finished_ibd` to `m_cached_is_ibd` (Lőrinc)
8be54e3b19677b02e19d054a4a5b2f1968bb1c46 test: cover IBD exit conditions (Lőrinc)

Pull request description:

  This PR is a follow-up to the stale #32885.

  ### Problem
  `ChainstateManager::IsInitialBlockDownload()` currently acquires `cs_main` internally, even though most existing call sites already hold the lock. This becomes relevant for proposals like #34054, which would call `IsInitialBlockDownload()` from the scheduler thread without holding `cs_main`, potentially introducing lock contention.

  ### Fix
  Make `ChainstateManager::IsInitialBlockDownload()` lock-free by caching its result in a single atomic `m_cached_is_ibd` (true while in IBD, latched to false on exit).
  Move the IBD exit checks out of `IsInitialBlockDownload()` (reader-side) into a new `ChainstateManager::UpdateIBDStatus()` (writer-side, called under cs_main).

  Call UpdateIBDStatus() at strategic points where IBD exit conditions may change, after active chain tip updates in `ConnectTip()`, `DisconnectTip()`, and `LoadChainTip()`, and after `ImportBlocks()` returns.

  With this, `IsInitialBlockDownload()` becomes a lock-free atomic read, avoiding internal `cs_main` acquisition on hot paths.

  ### Testing and Benchmarks
  This isn't strictly an optimization (though some usecases might benefit from it), so rather as a sanity check I ran a reindex-chainstate and an `AssumeUTXO` load (without background validation).

  <details>
  <summary>assumeutxo load | 910000 blocks | dbcache 4500 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | xfs | SSD</summary>

  ```
  COMMITS="595504a43209bead162da54a204df7d140a25f0e 63e822b637f67242e3689adedc0155b34100e651"; \
  CC=gcc; CXX=g++; \
  BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/ShallowBitcoinData"; LOG_DIR="$BASE_DIR/logs"; UTXO_SNAPSHOT_PATH="$BASE_DIR/utxo-910000.dat"; \
  (echo ""; for c in $COMMITS; do git fetch -q origin $c && git log -1 --pretty='%h %s' $c || exit 1; done; echo "") && \
  for DBCACHE in 4500; do \
    (echo "assumeutxo load | 910000 blocks | dbcache ${DBCACHE} | $(hostname) | $(uname -m) | $(lscpu | grep 'Model name' | head -1 | cut -d: -f2 | xargs) | $(nproc) cores | $(free -h | awk '/^Mem:/{print $2}') RAM | $(df -T $BASE_DIR | awk 'NR==2{print $2}') | $(lsblk -no ROTA $(df --output=source $BASE_DIR | tail -1) | grep -q 0 && echo SSD || echo HDD)";) &&\
    hyperfine \
    --sort command \
    --runs 3 \
    --export-json "$BASE_DIR/assumeutxo-$(sed -E 's/(\w{8})\w+ ?/\1-/g;s/-$//'<<<"$COMMITS")-$DBCACHE-$CC-$(date +%s).json" \
    --parameter-list COMMIT ${COMMITS// /,} \
    --prepare "killall -9 bitcoind 2>/dev/null; rm -rf $DATA_DIR/blocks $DATA_DIR/chainstate $DATA_DIR/chainstate_snapshot $DATA_DIR/debug.log; git clean -fxd; git reset --hard {COMMIT} && \
               cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo && ninja -C build bitcoind bitcoin-cli -j2 && \
               ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=1 -printtoconsole=0; sleep 20 && \
               ./build/bin/bitcoind -datadir=$DATA_DIR -daemon -blocksonly -connect=0 -dbcache=$DBCACHE -printtoconsole=0; sleep 20" \
     --conclude "build/bin/bitcoin-cli -datadir=$DATA_DIR stop || true; killall bitcoind || true; sleep 10; \
                 echo '{COMMIT} | dbcache=$DBCACHE | chainstate: $(find $DATA_DIR/chainstate_snapshot -type f 2>/dev/null | wc -l) files, $(du -sb $DATA_DIR/chainstate_snapshot 2>/dev/null | cut -f1) bytes' >> $DATA_DIR/debug.log; \
                 cp $DATA_DIR/debug.log $LOG_DIR/debug-assumeutxo-{COMMIT}-dbcache-$DBCACHE-$(date +%s).log" \
      "COMPILER=$CC DBCACHE=$DBCACHE ./build/bin/bitcoin-cli -datadir=$DATA_DIR -rpcclienttimeout=0 loadtxoutset $UTXO_SNAPSHOT_PATH"; \
  done

  595504a432 Merge bitcoin/bitcoin#34236: Add sedited to trusted-keys
  63e822b637 validation: make `IsInitialBlockDownload()` lock-free

  assumeutxo load | 910000 blocks | dbcache 4500 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | xfs | SSD

  Benchmark 1: COMPILER=gcc DBCACHE=4500 ./build/bin/bitcoin-cli -datadir=/mnt/my_storage/ShallowBitcoinData -rpcclienttimeout=0 loadtxoutset /mnt/my_storage/utxo-910000.dat (COMMIT = 595504a43209bead162da54a204df7d140a25f0e)
    Time (mean ± σ):     418.452 s ±  0.461 s    [User: 0.001 s, System: 0.001 s]
    Range (min … max):   418.070 s … 418.964 s    3 runs

  Benchmark 2: COMPILER=gcc DBCACHE=4500 ./build/bin/bitcoin-cli -datadir=/mnt/my_storage/ShallowBitcoinData -rpcclienttimeout=0 loadtxoutset /mnt/my_storage/utxo-910000.dat (COMMIT = 63e822b637f67242e3689adedc0155b34100e651)
    Time (mean ± σ):     415.994 s ±  0.294 s    [User: 0.001 s, System: 0.001 s]
    Range (min … max):   415.788 s … 416.330 s    3 runs

  Relative speed comparison
          1.01 ±  0.00  COMPILER=gcc DBCACHE=4500 ./build/bin/bitcoin-cli -datadir=/mnt/my_storage/ShallowBitcoinData -rpcclienttimeout=0 loadtxoutset /mnt/my_storage/utxo-910000.dat (COMMIT = 595504a43209bead162da54a204df7d140a25f0e)
          1.00          COMPILER=gcc DBCACHE=4500 ./build/bin/bitcoin-cli -datadir=/mnt/my_storage/ShallowBitcoinData -rpcclienttimeout=0 loadtxoutset /mnt/my_storage/utxo-910000.dat (COMMIT = 63e822b637f67242e3689adedc0155b34100e651)
  ```

  </details>

  <details>
  <summary>2026-01-12 | reindex-chainstate | 931139 blocks | dbcache 4500 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | SSD</summary>

  ```
  for DBCACHE in 4500; do \
    COMMITS="595504a43209bead162da54a204df7d140a25f0e 63e822b637f67242e3689adedc0155b34100e651"; \
    STOP=931139; CC=gcc; CXX=g++; \
    BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/BitcoinData"; LOG_DIR="$BASE_DIR/logs"; \
    (echo ""; for c in $COMMITS; do git fetch -q origin $c && git log -1 --pretty='%h %s' $c || exit 1; done) && \
    (echo "" && echo "$(date -I) | reindex-chainstate | ${STOP} blocks | dbcache ${DBCACHE} | $(hostname) | $(uname -m) | $(lscpu | grep 'Model name' | head -1 | cut -d: -f2 | xargs) | $(nproc) cores | $(free -h | awk '/^Mem:/{print $2}') RAM | SSD"; echo "") &&\
    hyperfine \
      --sort command \
      --runs 1 \
      --export-json "$BASE_DIR/rdx-$(sed -E 's/(\w{8})\w+ ?/\1-/g;s/-$//'<<<"$COMMITS")-$STOP-$DBCACHE-$CC.json" \
      --parameter-list COMMIT ${COMMITS// /,} \
      --prepare "killall -9 bitcoind 2>/dev/null; rm -f $DATA_DIR/debug.log; git clean -fxd; git reset --hard {COMMIT} && \
        cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DENABLE_IPC=OFF && ninja -C build bitcoind -j1 && \
        ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -dbcache=1000 -printtoconsole=0; sleep 20; rm -f $DATA_DIR/debug.log" \
      --conclude "killall bitcoind || true; sleep 5; grep -q 'height=0' $DATA_DIR/debug.log && grep -q 'Disabling script verification at block #1' $DATA_DIR/debug.log && grep -q 'height=$STOP' $DATA_DIR/debug.log; \
                  cp $DATA_DIR/debug.log $LOG_DIR/debug-{COMMIT}-$(date +%s).log" \
      "COMPILER=$CC ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -dbcache=$DBCACHE -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0";
  done

  595504a432 Merge bitcoin/bitcoin#34236: Add sedited to trusted-keys
  63e822b637 validation: make `IsInitialBlockDownload()` lock-free

  2026-01-12 | reindex-chainstate | 931139 blocks | dbcache 4500 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | SSD

  Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 595504a43209bead162da54a204df7d140a25f0e)
    Time (abs ≡):        17187.310 s               [User: 33104.415 s, System: 937.548 s]

  Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 63e822b637f67242e3689adedc0155b34100e651)
    Time (abs ≡):        17240.300 s               [User: 33164.803 s, System: 976.485 s]

  Relative speed comparison
          1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 595504a43209bead162da54a204df7d140a25f0e)
          1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 63e822b637f67242e3689adedc0155b34100e651)
  ```

  </details>

ACKs for top commit:
  sedited:
    ACK 557b41a38ccf2929ca1e5271db1701e5fbe781af
  sipa:
    utACK 557b41a38ccf2929ca1e5271db1701e5fbe781af
  mzumsande:
    Code Review ACK 557b41a38ccf2929ca1e5271db1701e5fbe781af

Tree-SHA512: 174015b9785846fc3375bd9d6e4ef91de47ffb659a94d645c49d333a33a32986d5c3cb2eb5a0a7245f96580ed6fea4ba5b7f93cac7e42e3b225f0a01538a2e5c
This commit is contained in:
merge-script 2026-01-29 11:51:41 +01:00
commit 1c2f164d34
No known key found for this signature in database
GPG Key ID: 9B79B45691DB4173
8 changed files with 94 additions and 40 deletions

View File

@ -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<NodeSeconds>() - max_tip_age;
}
/** Set/initialize a chain with a given tip. */
void SetTip(CBlockIndex& block);

View File

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

View File

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

View File

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

View File

@ -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());
}

View File

@ -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<TestChainstateManager&>(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<NodeSeconds>() - 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".

View File

@ -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<NodeSeconds>() - 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

View File

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