p2p: select another peer after empty headers #35839

pull l0rinc wants to merge 8 commits into bitcoin:master from l0rinc:l0rinc/p2p-empty-headers-sync changing 2 files +150 −37
  1. l0rinc commented at 8:52 PM on July 29, 2026: contributor

    Problem: During IBD, the node asks one peer at a time for the headers needed to catch up. That peer can send valid empty headers, which the node accepts while still treating the peer as active, so no replacement is selected. When no other preferred download peer is available, the headers timeout cannot release that peer either.

    Fix: Stop treating the peer as active after an empty response while the best header is at least a day old, and clear its headers timeout. Keep the time of the recent getheaders request so the same peer is not selected again immediately, while a later block announcement can still retry it. If an outbound peer is already waiting to be disconnected because its chain is old, keep that check running after another peer takes over headers sync.

    <details><summary>Manual reproducer</summary>

    Both runs use the checked-out build/bin/bitcoind; Python only supplies two inbound P2P peers.

    DATADIR="$(mktemp -d /tmp/bitcoin-empty-headers.XXXXXX)"
    trap 'build/bin/bitcoin-cli -regtest -datadir="$DATADIR" -rpcport=18454 stop >/dev/null 2>&1; rm -rf "$DATADIR"' EXIT
    
    { cmake -B build && cmake --build build -j --target bitcoind bitcoin-cli; } >/dev/null 2>&1
    build/bin/bitcoind -regtest -daemonwait -datadir="$DATADIR" -connect=0 -listen=1 -port=18455 -rpcport=18454
    time python3 '/Users/lorinc/Library/Application Support/JetBrains/CLion2026.1/scratches/scratch_758.py'
    killall bitcoind
    

    The fixed node returns as soon as the replacement peer receives getheaders. On the old node, that wait times out after 10 seconds because the initial peer is still treated as active.

    </details>

  2. DrahtBot added the label P2P on Jul 29, 2026
  3. DrahtBot commented at 8:52 PM on July 29, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35561 (net: move some CNodeState fields to Peer by Crypt-iQ)
    • #34565 (refactor: extract BlockDownloadManager from PeerManagerImpl by w0xlt)

    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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. l0rinc marked this as a draft on Jul 29, 2026
  5. test: characterize empty headers sync stall
    During IBD, the node asks one peer at a time for the headers needed to catch up.
    An empty response leaves that peer selected, so no replacement is asked and the same peer can be retried immediately.
    
    Characterize both behaviors before changing them.
    8b065bb0e2
  6. test: isolate block announcement setup
    The block-announcement test sends empty headers only to let the node ask that peer for headers again later.
    That setup depends on behavior this series changes.
    
    Advance mock time past the normal two-minute response wait so the test remains focused on selecting one additional peer per block announcement.
    ee531b2afe
  7. refactor: name stale header check
    The node treats headers sync differently while its best header is at least a day old: it selects one peer at a time and applies a download timeout.
    
    Name that check once so peer selection, timeout handling, and empty-response handling use the same condition.
    f8e76a6778
  8. refactor: extract initial headers cleanup
    Starting initial headers sync marks one peer as active, increments the number of active sync peers, and sets its download timeout.
    
    Extract the matching cleanup so timeout and empty-response handling clear those three pieces of state together.
    c71e180ee1
  9. p2p: select another peer after empty headers
    During IBD, the node asks one peer at a time for headers.
    A valid empty response leaves that peer selected, so no replacement is asked.
    When all peers are inbound, the headers timeout cannot stop waiting on that peer either.
    
    After an empty response while the best header is at least a day old, stop treating the peer as active and clear its download timeout.
    Keep the time of the recent `getheaders` request so the same peer is not selected again immediately.
    
    This exposes a separate issue for outbound peers: releasing the selected peer also stops a pending disconnect for having an old chain.
    The next two commits characterize and correct that intermediate behavior.
    f169af459b
  10. test: characterize outbound old-chain disconnect
    Bitcoin Core gives an outbound peer time to show that its chain has caught up before disconnecting it.
    Releasing that peer from initial headers sync currently stops this check.
    
    Characterize the missing follow-up request and disconnect before keeping the check active.
    9260902073
  11. p2p: continue pending old-chain disconnect
    Bitcoin Core gives an outbound peer time to show that its chain has caught up before disconnecting it.
    Releasing that peer from initial headers sync currently stops both the final `getheaders` request and the disconnect.
    
    Continue the pending check until the peer catches up or is disconnected.
    dae65d66a0
  12. p2p: delay retry after empty headers
    After an empty response, headers sync moves to another peer but the time of the last `getheaders` request remains set.
    This keeps the same peer from being selected again for two minutes.
    
    Record that the peer was released so repeated empty responses preserve the delay, while a block announcement can retry it.
    Clear the record when connecting headers arrive or a new initial request starts.
    ea55ab8c1c
  13. l0rinc marked this as ready for review on Jul 29, 2026
  14. l0rinc force-pushed on Jul 29, 2026
  15. DrahtBot added the label CI failed on Jul 29, 2026
  16. l0rinc closed this on Jul 30, 2026

  17. l0rinc reopened this on Jul 30, 2026

  18. maflcko commented at 7:19 AM on July 30, 2026: member

    Looks like CI fails on f169af459b5a1a6715fe392f270076cfaa31df0f:

    test  2026-07-30T05:28:45.141085Z TestFramework (ERROR): Unexpected exception: 
                                       Traceback (most recent call last):
                                         File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 145, in main
                                           self.run_test()
                                         File "/home/runner/work/bitcoin/bitcoin/ci_build/test/functional/p2p_segwit.py", line 261, in run_test
                                           self.test_non_witness_transaction()
                                         File "/home/runner/work/bitcoin/bitcoin/ci_build/test/functional/p2p_segwit.py", line 112, in func_wrapper
                                           func(self, *args, **kwargs)
                                         File "/home/runner/work/bitcoin/bitcoin/ci_build/test/functional/p2p_segwit.py", line 308, in test_non_witness_transaction
                                           self.generate(self.wallet, 99)  # let the block mature
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                         File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 662, in generate
                                           sync_fun() if sync_fun else self.sync_all()
                                                                       ^^^^^^^^^^^^^^^
                                         File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 743, in sync_all
                                           self.sync_blocks(nodes)
                                         File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 714, in sync_blocks
                                           raise AssertionError("Block sync timed out after {}s:{}".format(
                                       AssertionError: Block sync timed out after 60s:
                                         '44c1a1dd2c9cd9b6e27c3236cb82a66ac501e990bc4d84642fa9706b89169839'
                                         '0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206'
    

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-07-31 20:50 UTC

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