validation: cache tip recency for lock-free `IsInitialBlockDownload()` #34253

l0rinc · 2026-01-11T15:15:32Z

Context

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

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)

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)

DrahtBot · 2026-01-11T15:15:40Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34253.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	mzumsande

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33854 (fix assumevalid is ignored during reindex by Eunovo)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

snapshot chainstate, giving it more cache space than the snapshot chainstate. -> snapshot chainstate, giving it more cache space than the non-snapshot chainstate. [The original repeats "snapshot chainstate" twice, making the comparison unclear; replacing the second occurrence clarifies the intended contrast.]

^2026-01-12

mzumsande

Concept ACK (I didn't review the latest approach of #32885, just this one), it would help to be able to call IsIBD() without having to worry about cs_main locking.

mzumsande · 2026-01-11T21:26:12Z

src/validation.h

    mutable std::atomic_bool m_cached_finished_ibd{false};

+    /** Latch that becomes true once the active tip has met IsTipRecent(). */
+    mutable std::atomic_bool m_cached_tip_recent{false};


I think having both m_cached_tip_recent and m_cached_finished_ibd may be more complicated than necessary. Probably the reason for it is that IsIBD() should latch to true also after ImportBlocks() without having to wait for a new block.
Would it work to instead have just one atomic called m_cached_finished_ibd that is only set when LoadingBlocks() is false, ans set it in a function ChainstateManager::MaybeExitIBD() that would be called both from ChainstateManager::SetTip (within the current approach, see also my other comment about that) and at the end of ImportBlocks() when ImportingNow went out of scope`?

Done, thanks

mzumsande · 2026-01-11T21:43:08Z

src/validation.cpp

    }

-    m_chain.SetTip(*pindexDelete->pprev);
+    m_chainman.SetTip(m_chain, *pindexDelete->pprev);


I find it a bit weird to have a function SetTip on the level of ChainstateManager, that would naturally live on a Chainstate/Chain level.
I saw the justification of coupling the two close to make it harder to "miss related state updates", but I am not really convinced by it: It can still be easily forgotten by just using Chain::SetTip instead, like many tests do, and there are only 3 affected call sites outside of tests.
Maybe it would be simpler to not couple the two and call a separate function ChainstateManager::MaybeExitIBD() at the 3 call sites (DisconnectTip, ConnectTip, LoadChainTip) right after the CChain::SetTip() call.

Done, thanks

l0rinc · 2026-01-12T10:04:19Z

Thanks to @mzumsande for the hint to simplify this, added you as coauthor.

The new push makes ChainstateManager::IsInitialBlockDownload() lock-free by caching its result in a single atomic m_cached_is_ibd (inverted to match the method) and moving the exit checks into a new ChainstateManager::UpdateIBDStatus() (named it differently than you suggested, don't like maybes - it's called under cs_main after tip updates and after ImportBlocks() finishes).
IBD exit criteria (enough work + recent tip) are encapsulated in a new CChain::IsTipRecent() helper, and a unit test covers the post-loading IBD exit/latching behavior (added early to show current behavior and changed throughout the PR to demonstrate the changes).

Edit: pushed a more comprehensive test to check the whole matrix of conditions to be sure the behavior doesn't change.

Add a unit test that exercises the `IsInitialBlockDownload()` decision matrix by varying the cached latch, `BlockManager::LoadingBlocks()`, and tip work/recency inputs. This documents the current latching behavior and provides a baseline for later refactors.

Rename and invert the internal IBD latch so the cached value directly matches `IsInitialBlockDownload()` (true while in IBD, then latched to false). This is a behavior-preserving refactor to avoid double negatives.

Factor the chain tip work/recency check out of `ChainstateManager::IsInitialBlockDownload()` into a reusable `CChain::IsTipRecent()` helper, and annotate it as requiring `cs_main` since it's reading mutable state. Also introduce a local `chainman_ref` in the kernel import-blocks wrapper to reduce repetition and keep follow-up diffs small. `IsInitialBlockDownload` returns were also unified to make the followup move clean. Co-authored-by: Patrick Strateman <[email protected]> Co-authored-by: Martin Zumsande <[email protected]>

`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 <[email protected]> Co-authored-by: Martin Zumsande <[email protected]>

DrahtBot added the Validation label Jan 11, 2026

l0rinc force-pushed the l0rinc/cache-ibd-status branch from 3547920 to a4041df Compare January 11, 2026 15:16

DrahtBot added the CI failed label Jan 11, 2026

l0rinc mentioned this pull request Jan 11, 2026

Cache m_cached_finished_ibd where SetTip is called. #32885

Closed

DrahtBot removed the CI failed label Jan 11, 2026

l0rinc marked this pull request as ready for review January 11, 2026 16:37

mzumsande reviewed Jan 11, 2026

View reviewed changes

l0rinc force-pushed the l0rinc/cache-ibd-status branch from a4041df to cac06c4 Compare January 12, 2026 10:00

l0rinc force-pushed the l0rinc/cache-ibd-status branch from cac06c4 to 63e822b Compare January 12, 2026 10:11

DrahtBot added CI failed and removed CI failed labels Jan 12, 2026

l0rinc and others added 4 commits January 12, 2026 16:48

validation: invert m_cached_finished_ibd to m_cached_is_ibd

8d531c6

Rename and invert the internal IBD latch so the cached value directly matches `IsInitialBlockDownload()` (true while in IBD, then latched to false). This is a behavior-preserving refactor to avoid double negatives.

l0rinc force-pushed the l0rinc/cache-ibd-status branch from 63e822b to 557b41a Compare January 12, 2026 16:00

DrahtBot mentioned this pull request Jan 13, 2026

fix assumevalid is ignored during reindex #33854

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

validation: cache tip recency for lock-free `IsInitialBlockDownload()` #34253

validation: cache tip recency for lock-free `IsInitialBlockDownload()` #34253

Uh oh!

l0rinc commented Jan 11, 2026 •

edited

Loading

Uh oh!

DrahtBot commented Jan 11, 2026 •

edited

Loading

Uh oh!

mzumsande left a comment •

edited

Loading

Uh oh!

mzumsande Jan 11, 2026

Uh oh!

l0rinc Jan 12, 2026

Uh oh!

mzumsande Jan 11, 2026

Uh oh!

l0rinc Jan 12, 2026

Uh oh!

l0rinc commented Jan 12, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

l0rinc commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Problem

Fix

Testing and Benchmarks

Uh oh!

DrahtBot commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

mzumsande left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mzumsande Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

l0rinc Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

mzumsande Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

l0rinc Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

l0rinc commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

validation: cache tip recency for lock-free `IsInitialBlockDownload()` #34253

validation: cache tip recency for lock-free `IsInitialBlockDownload()` #34253

l0rinc commented Jan 11, 2026 •

edited

Loading

DrahtBot commented Jan 11, 2026 •

edited

Loading

mzumsande left a comment •

edited

Loading

l0rinc commented Jan 12, 2026 •

edited

Loading