net processing: Add ibd check before processing block for txdownloadman #34054

pull sedited wants to merge 2 commits into bitcoin:master from sedited:txdownloadman_ibd_check changing 2 files +19 −5
  1. sedited commented at 9:06 am on December 12, 2025: contributor

    Calculating the rolling bloom filters for the txorphanage takes some CPU time from the scheduler thread. This can be observed for example in this flamegraph, where handling the filter takes about 2.6% of total time (and most of the scheduler thread’s time).

    During ibd the entries in the tx download bloom filter are just continuously rolled over and aren’t consumed, since no mempool entries are created by incoming transactions from peers during ibd. The mempool does accept transactions via RPC, or the wallet at the time, however these don’t interact with the orphanage and the txdownloadman, because adding anything to those is guarded by IsInitialBlockDownload() checks as well.

    We’re usually latching ibd to false a few blocks before catching up to the tip, so this should also not significantly degrade the performance of the filter once fully caught up.

  2. DrahtBot added the label P2P on Dec 12, 2025
  3. DrahtBot commented at 9:06 am on December 12, 2025: contributor

    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/34054.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK l0rinc, instagibbs, fjahr
    Concept ACK mzumsande

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

    Conflicts

    No conflicts as of last run.

  4. maflcko commented at 10:17 am on December 12, 2025: member

    During ibd the entries in the tx download bloom filter are just continuously rolled over, and aren’t consumed, since no mempool is maintained at the time.

    I think this could be clarified a bit, why this is safe. My understanding is that a mempool does exist and is maintained (albeit it will normally be empty). The mempool will also happily accept transactions via RPC, or the wallet. However, this is fine, because they don’t interact with the orphanage and the txdownloadman, because adding anything to those is guarded by IsInitialBlockDownload() checks as well?

    Ref:

    https://github.com/bitcoin/bitcoin/blob/8da8ff30a57f0fe7f627d121c5e40466f1ecb0b2/src/net_processing.cpp#L4258-L4268

    and

    https://github.com/bitcoin/bitcoin/blob/8da8ff30a57f0fe7f627d121c5e40466f1ecb0b2/src/net_processing.cpp#L3987-L3990

  5. sedited commented at 8:21 pm on December 15, 2025: contributor

    Re #34054 (comment)

    I think this could be clarified a bit, why this is safe.

    Thanks, that was not precise. Slightly reworded your explanation and added it to the pull request description. I was curious if the ibd check might cause a performance regression, since we’re probably triggering fewer de-allocs on the scheduler thread. I tried to get some results in benchcoin. The results were kind of mixed, but there is clearly no more work being done through the txdownloadman during ibd.

  6. maflcko commented at 12:32 pm on December 16, 2025: member

    fewer de-allocs on the scheduler thread.

    I think this patch does not change whether the event is run on the scheduler thread, or not, but rather if the event runs faster in the scheduler thread with the early return. So I think this should not move destructor calls between threads. Or am I missing something?

  7. sedited commented at 12:52 pm on December 16, 2025: contributor

    Re #34054 (comment)

    but rather if the event runs faster in the scheduler thread with the early return.

    Yes, and I was curious if this might change where the final decrement for the block’s shared pointer happens.

  8. maflcko commented at 1:31 pm on December 16, 2025: member
    Oh, I see. I guess if the behavior should be enforced at compile-time, the blocks could be std::moved into the event
  9. DrahtBot added the label Needs rebase on Dec 16, 2025
  10. sedited force-pushed on Dec 16, 2025
  11. sedited commented at 3:26 pm on December 16, 2025: contributor

    Rebased 2233420fcff7c9c3d7d5727880a8ae1e6173eb89 -> 42dc1725b978329e0808db5e2c3a64c038c35de3 (txdownloadman_ibd_check_0 -> txdownloadman_ibd_check_1, compare)

  12. DrahtBot removed the label Needs rebase on Dec 16, 2025
  13. in src/net_processing.cpp:1969 in 42dc1725b9 outdated
    1966@@ -1967,7 +1967,7 @@ void PeerManagerImpl::BlockConnected(
    1967 
    1968     // The following task can be skipped since we don't maintain a mempool for
    1969     // the historical chainstate.
    


    mzumsande commented at 8:46 pm on December 23, 2025:
    comment could be adjusted for why we skip it during IBD.
  14. mzumsande commented at 9:16 pm on December 23, 2025: contributor

    Concept ACK

    In addition to RPC/wallet mentioned above, the mempool could also contain transactions from disk - downloaded at a time when we weren’t in IBD. This is very common for nodes that aren’t online 24/7 , so that they will go back into IBD at each subsequent start. But those shouldn’t really be a problem either I think.

    Also, this introduces a cs_main lock in the scheduler thread, which could cause some lock contention with msghand - not sure how important this it, but it might go well together with #32885.

  15. sedited commented at 9:32 am on December 24, 2025: contributor

    Also, this introduces a cs_main lock in the scheduler thread, which could cause some lock contention with msghand - not sure how important this it, but it might go well together with #32885.

    Yes, the original PR explicitly worked around this by introducing a variable to track the ibd state: https://github.com/bitcoin/bitcoin/pull/32730/files#diff-b1e19192258d83199d8adaa5ac31f067af98f63554bfdd679bd8e8073815e69dL1911-L1912 . A prior approach of that PR passes the ibd state directly in the BlockConnected callback. I think blocking here might be fine (which is why it opened this), but it’s not great either. What do you think of drafting this for now, and we try to make progress with #32885, or do you prefer one of the other approaches?

  16. fjahr commented at 9:28 pm on December 25, 2025: contributor
    Concept ACK
  17. mzumsande commented at 6:51 pm on December 26, 2025: contributor

    What do you think of drafting this for now, and we try to make progress with #32885, or do you prefer one of the other approaches?

    I think I like the approach from #32885 because it addresses the issue at its root (so that many IBD checks would benefit, not just this one). As to whether this should be drafted I’m unsure - it’s probably an improvement even with the added locking due to the saved work, but given the mixed results from benchcoin maybe we’d need more testing?

  18. maflcko commented at 10:46 am on January 12, 2026: member

    given the mixed results from benchcoin maybe we’d need more testing?

    The latest runs shows a speedup in both scenarios, but the earlier run showed a slowdown in mainnet-default-uninstrumented, but the only difference was a rebase, so this seems to indicate that the benchcoin run itself is not stable?

    (Commit 8da8ff30a57f0fe7f627d121c5e40466f1ecb0b2 was deleted by GitHub, so I am just going after the comment #34054 (comment))

  19. l0rinc commented at 2:44 pm on January 16, 2026: contributor

    The latest runs shows a speedup in both scenarios, but the earlier run showed a slowdown in mainnet-default-uninstrumented, but the only difference was a rebase, so this seems to indicate that the benchcoin run itself is not stable?

    We’re not really using benchcoin anymore - at least I haven’t seen anyone take decisions based on it lately. It was an experiment we tried, I don’t think it’s viable anymore - as you also noticed. I’m using the script below to check these and they’re a lot more reliable and stable on the infrastructure I’m running them on. I ran a few pruned IBDs over real nodes, these were stable and show no change in IBD speed:

     0COMMITS="13891a8a685d255cb13dd5018e3d5ccc18b07c34 42dc1725b978329e0808db5e2c3a64c038c35de3"; \
     1STOP=931139; DBCACHE=4500; PRUNE=550; \
     2CC=gcc; CXX=g++; \
     3BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/ShallowBitcoinData"; LOG_DIR="$BASE_DIR/logs"; \
     4(echo ""; for c in $COMMITS; do git fetch -q origin $c && git log -1 --pretty='%h %s' $c || exit 1; done) && \
     5(echo "" && echo "$(date -I) | pruned IBD | ${STOP} blocks | dbcache ${DBCACHE} | pruning ${PRUNE} | $(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)"; echo "") &&\
     6hyperfine \
     7  --sort command \
     8  --runs 2 \
     9  --export-json "$BASE_DIR/ibd-$(sed -E 's/(\w{8})\w+ ?/\1-/g;s/-$//'<<<"$COMMITS")-$STOP-$DBCACHE-$CC.json" \
    10  --parameter-list COMMIT ${COMMITS// /,} \
    11  --prepare "killall -9 bitcoind 2>/dev/null; rm -rf $DATA_DIR/*; git clean -fxd; git reset --hard {COMMIT} && \
    12    cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release && ninja -C build bitcoind -j2 && \
    13    ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=1 -prune=$PRUNE -printtoconsole=0; sleep 20" \
    14  --conclude "killall bitcoind || true; sleep 5; grep -q 'height=0' $DATA_DIR/debug.log && grep -q 'Disabling script verification at block [#1](/bitcoin-bitcoin/1/)' $DATA_DIR/debug.log && grep -q 'height=$STOP' $DATA_DIR/debug.log && grep 'Bitcoin Core version' $DATA_DIR/debug.log | grep -q "$(printf %.12s {COMMIT})"; \
    15              cp $DATA_DIR/debug.log $LOG_DIR/debug-{COMMIT}-$(date +%s).log" \
    16  "COMPILER=$CC ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -dbcache=$DBCACHE -blocksonly -prune=$PRUNE -printtoconsole=0"
    17
    1813891a8a68 Merge bitcoin/bitcoin#34050: fuzz: exercise `ComputeMerkleRoot` without `mutated` parameter
    1942dc1725b9 net processing: Check if we are in ibd before processing block for txdownloadman
    20
    212026-01-14 | pruned IBD | 931139 blocks | dbcache 4500 | pruning 550 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | xfs | SSD
    22
    23Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    24  Time (mean ± σ):     32486.491 s ± 552.685 s    [User: 68289.029 s, System: 4354.310 s]
    25  Range (min  max):   32095.684 s  32877.298 s    2 runs
    26 
    27Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c3
    28  Time (mean ± σ):     32647.522 s ± 1124.203 s    [User: 67219.308 s, System: 4265.439 s]
    29  Range (min  max):   31852.591 s  33442.454 s    2 runs
    30 
    31Relative speed comparison
    32        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    33        1.00 ±  0.04  COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=931139 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)
    

    Similarly for -reindex-chainstate - which was probably unaffected anyway since no download happened, but it’s a good sanity-check anyway:

     0for DBCACHE in 450 4500; do   COMMITS="13891a8a685d255cb13dd5018e3d5ccc18b07c34 42dc1725b978329e0808db5e2c3a64c038c35de3";   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} && \
     1      cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DENABLE_IPC=OFF && ninja -C build bitcoind -j1 && \
     2      ./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](/bitcoin-bitcoin/1/)' $DATA_DIR/debug.log && grep -q 'height=$STOP' $DATA_DIR/debug.log; \
     3                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
     4
     513891a8a68 Merge bitcoin/bitcoin#34050: fuzz: exercise `ComputeMerkleRoot` without `mutated` parameter
     642dc1725b9 net processing: Check if we are in ibd before processing block for txdownloadman
     7
     82026-01-13 | reindex-chainstate | 931139 blocks | dbcache 450 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | SSD
     9
    10Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    11  Time (abs ):        21626.602 s               [User: 49157.755 s, System: 3039.249 s]
    12
    13Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)
    14  Time (abs ):        21755.087 s               [User: 47668.251 s, System: 2952.652 s]
    15
    16Relative speed comparison
    17        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    18        1.01          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=450 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)
    19
    2013891a8a68 Merge bitcoin/bitcoin#34050: fuzz: exercise `ComputeMerkleRoot` without `mutated` parameter
    2142dc1725b9 net processing: Check if we are in ibd before processing block for txdownloadman
    22
    232026-01-14 | reindex-chainstate | 931139 blocks | dbcache 4500 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | SSD
    24
    25Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    26  Time (abs ):        17263.550 s               [User: 33148.005 s, System: 945.859 s]
    27
    28Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)
    29  Time (abs ):        17480.331 s               [User: 31675.951 s, System: 739.137 s]
    30
    31Relative speed comparison
    32        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 13891a8a685d255cb13dd5018e3d5ccc18b07c34)
    33        1.01          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=931139 -dbcache=4500 -reindex-chainstate -blocksonly -connect=0 -printtoconsole=0 (COMMIT = 42dc1725b978329e0808db5e2c3a64c038c35de3)
    
  20. maflcko commented at 2:52 pm on January 16, 2026: member

    Nice. Thanks for running those. I guess it is expected that there is no difference, unless one is running on a single-core-single-thread system, because the scheduler thread runs in the background.

    I guess the flamegraphs from benchcoin are still useful and indicate that the schedulder now eats 50% less background CPU?

  21. maflcko commented at 2:54 pm on January 16, 2026: member
    I am still thinking if the std::move patch (https://github.com/bitcoin/bitcoin/pull/34054#issuecomment-3660534822) makes sense to include here.
  22. l0rinc commented at 2:59 pm on January 16, 2026: contributor

    unless one is running on a single-core-single-thread system

    I don’t think we support those. The lousiest machine I have is an rpi4 with 2 GB memory (even this has nproc = 4) and we can’t even compile on that machine without OOMing - unless we do some tricks with ccache). I’m only at 50% of the blocks after almost 9 days.

    0sudo cat ../BitcoinData/debug.log | egrep 'height=0|height=690000'                                 
    12026-01-07T19:49:56Z UpdateTip: new best=000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f height=0 version=0x00000001 log2_work=32.000022 tx=1 date='2009-01-03T18:15:05Z' progress=0.000000 cache=0.3MiB(0txo)
    22026-01-16T14:45:18Z UpdateTip: new best=00000000000000000002a23d6df20eecec15b21d32c75833cce28f113de888b7 height=690000 version=0x20000004 log2_work=92.968172 tx=653953301 date='2021-07-07T07:43:07Z' progress=0.503054 cache=80.5MiB(612179txo)
    

    The PR might still make sense, I haven’t investigated it in detail yet.

  23. sedited referenced this in commit 1c2f164d34 on Jan 29, 2026
  24. sedited force-pushed on Jan 29, 2026
  25. sedited commented at 11:04 am on January 29, 2026: contributor

    Rebased 42dc1725b978329e0808db5e2c3a64c038c35de3 -> 62383b79498a6e6293f22790674deecfa3516d19 (txdownloadman_ibd_check_1 -> txdownloadman_ibd_check_2, compare)

    • Rebased to get changes from #34253 .
  26. sedited force-pushed on Jan 30, 2026
  27. sedited commented at 10:14 am on January 30, 2026: contributor

    Updated 62383b79498a6e6293f22790674deecfa3516d19 -> c11090886c91af61bb399980c6e7caeaa0bfc3af (txdownloadman_ibd_check_2 -> txdownloadman_ibd_check_3, compare)

  28. l0rinc commented at 12:15 pm on January 31, 2026: contributor

    Rechecked after the rebase (before the comment change), this time a pruned IBD only on the strong i9 server. It doesn’t seem to introduce any regression, it’s even a bit faster for both runs. I will review the effects of the change in detail a bit later.

     0COMMITS="f7e0c3d3d370a4e5b4060fefcb9e8d83e2533bbc 62383b79498a6e6293f22790674deecfa3516d19"; \
     1STOP=933339; DBCACHE=4500; PRUNE=550; \
     2CC=gcc; CXX=g++; \
     3BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/ShallowBitcoinData"; LOG_DIR="$BASE_DIR/logs"; \
     4(echo ""; for c in $COMMITS; do git fetch -q origin $c && git log -1 --pretty='%h %s' $c || exit 1; done) && \
     5(echo "" && echo "$(date -I) | pruned IBD | ${STOP} blocks | dbcache ${DBCACHE} | pruning ${PRUNE} | $(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)"; echo "") &&\
     6hyperfine \
     7  --sort command \
     8  --runs 2 \
     9  --export-json "$BASE_DIR/ibd-$(sed -E 's/(\w{8})\w+ ?/\1-/g;s/-$//'<<<"$COMMITS")-$STOP-$DBCACHE-$CC.json" \
    10  --parameter-list COMMIT ${COMMITS// /,} \
    11  --prepare "killall -9 bitcoind 2>/dev/null; rm -rf $DATA_DIR/*; git clean -fxd; git reset --hard {COMMIT} && \
    12    cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release && ninja -C build bitcoind -j2 && \
    13    ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=1 -prune=$PRUNE -printtoconsole=0; sleep 20" \
    14  --conclude "killall bitcoind || true; sleep 5; grep -q 'height=0' $DATA_DIR/debug.log && grep -q 'Disabling script verification at block [#1](/bitcoin-bitcoin/1/)' $DATA_DIR/debug.log && grep -q 'height=$STOP' $DATA_DIR/debug.log && grep 'Bitcoin Core version' $DATA_DIR/debug.log | grep -q "$(printf %.12s {COMMIT})"; \
    15              cp $DATA_DIR/debug.log $LOG_DIR/debug-{COMMIT}-$(date +%s).log" \
    16  "COMPILER=$CC ./build/bin/bitcoind -datadir=$DATA_DIR -stopatheight=$STOP -dbcache=$DBCACHE -blocksonly -prune=$PRUNE -printtoconsole=0"
    17
    18f7e0c3d3d3 Merge bitcoin/bitcoin#34346: test: use IP_PORTRANGE_HIGH on FreeBSD for dynamic port allocation
    1962383b7949 net processing: Check if we are in ibd before processing block for txdownloadman
    20
    212026-01-29 | pruned IBD | 933339 blocks | dbcache 4500 | pruning 550 | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM | xfs | SSD
    22
    23Benchmark 1: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=933339 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = f7e0c3d3d370a4e5b4060fefcb9e8d83e2533bbc)
    24  Time (mean ± σ):     32583.370 s ± 81.871 s    [User: 70301.113 s, System: 4405.505 s]
    25  Range (min … max):   32525.479 s … 32641.261 s    2 runs
    26
    27Benchmark 2: COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=933339 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 62383b79498a6e6293f22790674deecfa3516d19)
    28  Time (mean ± σ):     32294.825 s ± 200.602 s    [User: 69078.534 s, System: 4274.122 s]
    29  Range (min … max):   32152.977 s … 32436.672 s    2 runs
    30
    31Relative speed comparison
    32        1.01 ±  0.01  COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=933339 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = f7e0c3d3d370a4e5b4060fefcb9e8d83e2533bbc)
    33        1.00          COMPILER=gcc ./build/bin/bitcoind -datadir=/mnt/my_storage/ShallowBitcoinData -stopatheight=933339 -dbcache=4500 -blocksonly -prune=550 -printtoconsole=0 (COMMIT = 62383b79498a6e6293f22790674deecfa3516d19)
    
  29. sedited requested review from l0rinc on Feb 17, 2026
  30. sedited requested review from l0rinc on Feb 17, 2026
  31. sedited requested review from mzumsande on Feb 17, 2026
  32. sedited requested review from theuni on Feb 18, 2026
  33. l0rinc commented at 3:23 pm on February 19, 2026: contributor

    RecentConfirmedTransactionsFilter is a probabilistic set that tracks recently confirmed (w)txids so the node can skip re-requesting them. This is especially useful for checking dangling txs without confirmed parents.

    It’s updated in BlockConnected (adjusted here), reset in BlockDisconnected (we don’t have reorgs during IBD, assuming headers-first sync, so we don’t really care - and even in the worst case this just resets the set), and queried via AlreadyHaveTx from four call sites:

    MempoolRejectedTx and GetRequestsToSend aren’t directly IBD-gated, but they’re transitively unreachable, so the orphanage stays empty and m_txrequest is never populated.

    See the graph below for a visual representation of the call sites and guards.

     0graph TD
     1    classDef guard fill:#4CAF50,color:white
     2    classDef note fill:#FF9800,color:white
     3    classDef filter fill:#2196F3,color:white
     4
     5    RCF["RecentConfirmedTransactionsFilter"]:::filter
     6
     7    INS["insert() in BlockConnected"]
     8    RST["reset() in BlockDisconnected"]
     9    CNT["contains() in AlreadyHaveTx"]
    10
    11    RCF --> INS
    12    RCF --> RST
    13    RCF --> CNT
    14
    15    INS --> PBC["PeerManagerImpl::BlockConnected (pre-PR)"]
    16    PBC --> PRG["PR [#34054](/bitcoin-bitcoin/34054/): add IsInitialBlockDownload to guard"]:::guard
    17
    18    RST --> PBD["PeerManagerImpl::BlockDisconnected (unconditional call)"]:::note
    19
    20    CNT --> ATA["AddTxAnnouncement"]
    21    CNT --> GRS["GetRequestsToSend"]
    22    CNT --> RT["ReceivedTx"]
    23    CNT --> MRT["MempoolRejectedTx"]
    24
    25    ATA --> INVG["INV handler: AddTxAnnouncement only if !IBD"]:::guard
    26
    27    GRS --> SM["SendMessages -> GetRequestsToSend"]
    28    SM --> TXRQ1["m_txrequest producer: AddTxAnnouncement::ReceivedInv"]
    29    SM --> TXRQ2["m_txrequest producer: MaybeAddOrphanResolutionCandidate::ReceivedInv"]
    30    TXRQ1 --> INVG
    31    TXRQ2 --> TXG["TX handler returns early in IBD"]:::guard
    32
    33    RT --> RTX["TX handler -> ReceivedTx"]
    34    RTX --> TXG
    35
    36    MRT --> PIT["ProcessInvalidTx"]
    37    PIT --> PIT1["caller: TX handler"]
    38    PIT --> PIT2["caller: ProcessPackageResult (TX path)"]
    39    PIT --> PIT3["caller: ProcessOrphanTx"]
    40    PIT1 --> TXG
    41    PIT2 --> TXG
    42    PIT3 --> TXG
    43
    44    click INS "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/node/txdownloadman_impl.cpp#L103-L105" "insert txid/wtxid" _blank
    45    click RST "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/node/txdownloadman_impl.cpp#L122" "reset filter" _blank
    46    click CNT "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/node/txdownloadman_impl.cpp#L144" "contains check" _blank
    47
    48    click PBC "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L2059-L2065" "pre-PR BlockConnected guard" _blank
    49    click PRG "https://github.com/bitcoin/bitcoin/blob/c11090886c91af61bb399980c6e7caeaa0bfc3af/src/net_processing.cpp#L2059-L2064" "PR [#34054](/bitcoin-bitcoin/34054/) guard change" _blank
    50    click PBD "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L2068-L2071" "BlockDisconnected unconditional call" _blank
    51
    52    click ATA "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/node/txdownloadman_impl.cpp#L170-L224" "AddTxAnnouncement" _blank
    53    click GRS "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/node/txdownloadman_impl.cpp#L264-L286" "GetRequestsToSend" _blank
    54    click RT "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/node/txdownloadman_impl.cpp#L505-L527" "ReceivedTx -> AlreadyHaveTx" _blank
    55    click MRT "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/node/txdownloadman_impl.cpp#L350-L396" "MempoolRejectedTx -> AlreadyHaveTx" _blank
    56
    57    click INVG "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L4149-L4151" "INV IBD guard + AddTxAnnouncement call" _blank
    58    click SM "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L6178-L6180" "SendMessages calls GetRequestsToSend" _blank
    59    click TXRQ1 "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/node/txdownloadman_impl.cpp#L221" "AddTxAnnouncement producer" _blank
    60    click TXRQ2 "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/node/txdownloadman_impl.cpp#L258" "Orphan-resolution producer" _blank
    61    click TXG "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L4453-L4456" "TX handler IBD early return" _blank
    62    click RTX "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L4478-L4481" "ReceivedTx call site" _blank
    63
    64    click PIT "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L3095-L3111" "ProcessInvalidTx" _blank
    65    click PIT1 "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L4515-L4517" "TX handler caller" _blank
    66    click PIT2 "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L3182-L3184" "ProcessPackageResult caller" _blank
    67    click PIT3 "https://github.com/bitcoin/bitcoin/blob/9e4567b17a285a266fa540b4141e0db5781cc228/src/net_processing.cpp#L3208-L3230" "ProcessOrphanTx caller" _blank
    

    So after understanding that this change seems perfectly fine during IBD, the only request I have is to cover it with tests:

     0diff --git a/test/functional/p2p_ibd_txrelay.py b/test/functional/p2p_ibd_txrelay.py
     1--- a/test/functional/p2p_ibd_txrelay.py	(revision 3c143ec9f61a03b38129d290e84c8d9c465eb629)
     2+++ b/test/functional/p2p_ibd_txrelay.py	(date 1771511513133)
     3@@ -11,6 +11,7 @@
     4 from decimal import Decimal
     5 import time
     6
     7+from test_framework.blocktools import create_block, create_coinbase
     8 from test_framework.messages import (
     9         CInv,
    10         COIN,
    11@@ -48,6 +49,12 @@
    12             self.wait_until(lambda: all(peer['minfeefilter'] == MAX_FEE_FILTER for peer in node.getpeerinfo()))
    13
    14         self.nodes[0].setmocktime(int(time.time()))
    15+        self.log.info("Mine one old block so we stay in IBD, then remember its coinbase wtxid")
    16+        block = create_block(int(self.nodes[0].getbestblockhash(), 16), create_coinbase(1), int(time.time()) - 2 * 24 * 60 * 60)
    17+        block.solve()
    18+        self.nodes[0].submitblock(block.serialize().hex())
    19+        assert self.nodes[0].getblockchaininfo()['initialblockdownload']
    20+        ibd_wtxid = int(self.nodes[0].getblock(f"{block.hash_int:064x}", 2)["tx"][0]["hash"], 16)
    21
    22         self.log.info("Check that nodes don't send getdatas for transactions while still in IBD")
    23         peer_inver = self.nodes[0].add_p2p_connection(P2PDataStore())
    24@@ -82,6 +89,13 @@
    25             assert not node.getblockchaininfo()['initialblockdownload']
    26             self.wait_until(lambda: all(peer['minfeefilter'] == NORMAL_FEE_FILTER for peer in node.getpeerinfo()))
    27
    28+        self.log.info("Check that txs confirmed during IBD are not in the recently-confirmed filter")
    29+        peer_inver = self.nodes[0].add_p2p_connection(P2PDataStore())
    30+        peer_inver.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=ibd_wtxid)]))
    31+        self.nodes[0].bumpmocktime(NONPREF_PEER_TX_DELAY)
    32+        peer_inver.wait_for_getdata([ibd_wtxid])
    33+        self.nodes[0].disconnect_p2ps()
    34+
    35         self.log.info("Check that nodes process the same transaction, even when unsolicited, when no longer in IBD")
    36         peer_txer = self.nodes[0].add_p2p_connection(P2PInterface())
    37         with self.nodes[0].assert_debug_log(expected_msgs=["was not accepted"]):
    

    which passes with the fix and fails on master with a timeout on:

    0peer_inver.wait_for_getdata([ibd_wtxid])
    

    Ideally - to document the slight behavior change - I would prefer adding a version of this test which documents the exact current behavior, and in the next commit (where we add the fix) to adjust it slightly to document how it changes the behavior of the application.

    Nit: unless the goal of the break is to skip whatever follows next, please consider inverting the condition to avoid the jump and negation:

    0    if (!role.historical && !m_chainman.IsInitialBlockDownload()) {
    1        LOCK(m_tx_download_mutex);
    2        m_txdownloadman.BlockConnected(pblock);
    3    }
    
  34. sedited requested review from instagibbs on Feb 19, 2026
  35. instagibbs commented at 3:53 pm on February 19, 2026: member

    To recap there are three things this would change in ibd:

    1. We would not evict orphans based on input conflicts
    2. If a peer offered historical transactions on inv, we would download them
    3. If we had queued requests for historical transactions, we would not clear them

    (1) is clearly fine; we have DoS limits and it would eventually be trimmed when required (2) Depending on “ibd” is defined, this would simply mean misconfigured nodes would waste more of our bandwidth, potentially (3) Again, we could waste more bandwidth, but I don’t recall how we typically do things on idb

    I don’t see anything obviously breaking, but I’m also not an expert on how/when ibd latches/resets.

  36. sedited commented at 5:19 pm on February 19, 2026: contributor
    1. If a peer offered historical transactions on inv, we would download them

    This is only true though for transasctions present in the current bloom filter window and for a brief amount of time after ibd given the check here, right?

  37. sedited commented at 11:05 am on February 21, 2026: contributor

    Thanks for the review @l0rinc!

    Updated c11090886c91af61bb399980c6e7caeaa0bfc3af -> ae19284024329640a174e13079e4cc9bd0c31905 (txdownloadman_ibd_check_3 -> txdownloadman_ibd_check_4, compare)

    • Applied @l0rinc’s suggested functional test, creating a commit first document current behaviour, then tweaking it for this patch.
    • Took @l0rinc’s suggestion, gating m_txdownloadman.BlockConnected with the IBD check.
  38. sedited force-pushed on Feb 21, 2026
  39. in test/functional/p2p_ibd_txrelay.py:98 in d641c5a66a
    93+        peer_inver = self.nodes[0].add_p2p_connection(P2PDataStore())
    94+        peer_inver.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=ibd_wtxid)]))
    95+        self.nodes[0].bumpmocktime(NONPREF_PEER_TX_DELAY)
    96+        peer_inver.sync_with_ping()
    97+        with p2p_lock:
    98+            assert txid not in peer_inver.getdata_requests
    


    l0rinc commented at 1:03 pm on February 21, 2026:

    d641c5a: Add functional test exercising tx downloadman recently confirmed filter:

    This passes with the fix, which means we’re not fully reproducing the error:

    0            assert ibd_wtxid not in peer_inver.getdata_requests
    

    sedited commented at 11:56 am on February 22, 2026:
    Thanks, I think I pasted another test case by mistake.
  40. l0rinc commented at 1:17 pm on February 21, 2026: contributor

    ACK ae19284024329640a174e13079e4cc9bd0c31905

    • Checked out and rebased locally, all test are passing
    • Test fails correctly with only reverting the fix on the latest commit
    • Test doesn’t fail (incorrectly) with the first commit + the fix applied - but it’s not a blocker for me, but if you decide to fix it, I’m happy to reack
  41. DrahtBot requested review from fjahr on Feb 21, 2026
  42. sedited force-pushed on Feb 22, 2026
  43. sedited commented at 12:02 pm on February 22, 2026: contributor

    ae19284024329640a174e13079e4cc9bd0c31905 -> e5f0613503b6973dbc886eba8e999f208d84853b (txdownloadman_ibd_check_4 -> txdownloadman_ibd_check_5, compare)

  44. Add functional test exercising tx downloadman recently confirmed filter
    This documents existing behaviour before the change in the following
    commit: The bloom filter maintained by the txdownload manager tracks
    recently confirmed transasctions even during ibd. If a peer sends an INV
    once IBD is over it does not re-request them.
    
    Co-authored-by: sedited <seb.kung@gmail.com>
    ce8b692897
  45. net processing: Check if we are in ibd before processing block for txdownloadman
    This avoids wasting work on calculating bloom filters that aren't
    consumed during ibd and continuously re-calculated as now blocks get
    validated.
    
    Also update the functional test to document that transactions would now
    be requested again once out of IBD.
    
    Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
    e5f0613503
  46. sedited closed this on Feb 22, 2026

  47. sedited force-pushed on Feb 22, 2026
  48. sedited reopened this on Feb 22, 2026

  49. DrahtBot added the label CI failed on Feb 22, 2026
  50. l0rinc commented at 12:28 pm on February 22, 2026: contributor

    ACK e5f0613503b6973dbc886eba8e999f208d84853b

    Since the last review, the original test (reproducing current behavior) was adjusted.

    • Checked out and rebased locally, all tests are passing
    • Test fails correctly when only reverting the fix on the latest commit
    • Test fails correctly with the first commit + the fix applied (passes otherwise)
  51. DrahtBot removed the label CI failed on Feb 22, 2026
  52. instagibbs approved
  53. instagibbs commented at 2:34 pm on February 23, 2026: member

    ACK e5f0613503b6973dbc886eba8e999f208d84853b

    If a node gets an erroneous orphan during IBD this will likely speed things up as well due to EraseForBlock.

  54. fjahr commented at 10:19 am on February 26, 2026: contributor
    Code review ACK e5f0613503b6973dbc886eba8e999f208d84853b
  55. fanquake merged this on Feb 26, 2026
  56. fanquake closed this on Feb 26, 2026


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-03-11 09:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me