#35839 p2p: prevent IBD stall after empty headers
https://github.com/bitcoin/bitcoin/pull/35839 · · +62/-11 in 2 files, 5 commits · labels: P2P
Goal
- Prevent initial headers sync from stalling indefinitely when a manual or inbound peer returns empty headers
- Allow another eligible peer to take over sync so node initial block download can continue
Fixes an initial headers sync stall where an inbound or manual peer returning empty headers retains the download slot indefinitely. When receiving an empty headers message while headers are stale, the node releases the peer from initial sync, cancels its timeout, and applies a backoff to allow other eligible peers to be queried. Includes functional tests characterizing the stall and verifying slot handoff for both inbound and manual peers.
Problem: When a node performs initial headers sync and queries a peer that has no new headers (such as an addnode peer that is itself in IBD), the peer returns a valid empty headers message. The node does not disconnect the peer or reassign the sync slot because empty headers are not considered misbehavior and inbound/manual peers are exempt from automatic eviction, leaving IBD completely stalled.
Category: P2P (#6 of 65)
P2 · bug fix
- P2 because it fixes an indefinite sync stall bug affecting manual and inbound peers during IBD
- Prevents nodes from staying stuck behind when a connected peer has no new headers to offer
P2 because it fixes an indefinite sync stall (#34096) during initial headers sync when using addnode or connecting to an inbound peer that is also behind. Under existing logic, receiving empty headers from manual or inbound peers leaves the sync slot locked without triggering eviction or timeouts, blocking node startup and progress.
Membership: Modifies PeerManagerImpl headers sync handling in src/net_processing.cpp and functional test in test/functional/p2p_initial_headers_sync.py
Factors: security/stability 1, bug 2, performance 0, user value 2, leverage 0
Reviewability: Ready
- Ready for review
- CI is passing and remaining open feedback is limited to minor formatting and a test question
The patch is clean, passes CI, and includes functional test coverage; open comments are minor code documentation requests and test clarifications.
Author status: active, last pushed code updates addressing review on 2026-09-01
Resolved concerns:
- jeanpablojp noted that setting the backoff timestamp from the original request time rather than the release time allowed peers replying after two minutes to immediately reclaim the slot; author updated logic to set the timestamp at release time.
- danielabrozzoni noted a potential stall vector where a peer sends known connecting headers to clear the backoff; author explained the difficulty of fixing this without complicating low-work sync and deferred it to follow-up work.
Agreement: Strong
- Strong consensus to fix the confirmed headers sync stall on mainnet (davidgumberg)
- Concept approval after helping refine retry backoff logic and test coverage (jeanpablojp)
- Concept approval on the latest update (hodlinator)
Positive; Concept ACKs from jeanpablojp and hodlinator with prior review concerns resolved in pushes or deferred with consent.
Multiple contributors have reviewed the approach and logic, confirming the problem described in #34096. All technical issues raised were addressed in updates or accepted as follow-up work.
- jeanpablojp: 'Concept ACK' (2026-08-31)
- hodlinator: 'Concept ACK be1f152ffe18143651ca7456113d6d684b53da17' (2026-09-15)
Objections:
| Reviewer | Kind | Harm | Status | Blocking | Author replied | Quote |
|---|---|---|---|---|---|---|
| jeanpablojp | correctness | If an empty headers message arrives after two minutes, the retained timestamp was already expired, allowing the peer to retake the sync slot immediately. | resolved | no | yes | 2026-08-31: 'If the empty response arrives more than two minutes after the request, the peer is released and takes the slot back in the same SendMessages...' Settled: 2026-08-31: author replied 'Good catch... took all your suggestions. This update starts the retry delay when a peer releases the slot' and pushed commit be1f152ffe |
| danielabrozzoni | safety | A peer could send an already-known connecting header to clear the backoff and immediately reacquire the sync slot, prolonging an IBD stall. | resolved | no | yes | 2026-08-25: 'this means that a peer can send an already-known connecting header and bypass the backoff... If P wants to stall us for as long as possible...' Settled: 2026-08-29: author replied 'I started implementing this first but it got more complicated than I was comfortable with - we should fix it after this is merged. Instead I narrowed this PR to #34096's valid empty-response stall' and reviewer did not push back |
Support:
- jeanpablojp: Concept ACK and co-authored functional test expansion
- hodlinator: Concept ACK on latest push be1f152 [not substantive]
Participants: maflcko (neutral), mzumsande (question), davidgumberg (neutral), danielabrozzoni (objection), jeanpablojp (objection), hodlinator (support)
State derived from the lists: substantive support, no open objection (jeanpablojp) (model's own read: Positive)
Review verdicts (DrahtBot): 0
- Concept ACK: jeanpablojp, hodlinator
Files
46 lines under test/bench/ci.
- test/functional/p2p_initial_headers_sync.py +39/-7
- src/net_processing.cpp +23/-4
Card
This PR prevents an initial headers sync stall by releasing an inbound or manual peer from the sync slot and imposing a backoff when it returns an empty headers message while headers are stale. It fixes #34096, where nodes syncing from an addnode peer that is also behind become stuck indefinitely because empty responses are valid and manual peers are not subject to outbound eviction. The change has Concept ACKs from two contributors, with prior review feedback incorporated into the latest push.