#36350 net_processing: count presync work in chain sync eviction

ReviewabilityReviewsAgreementSize
#36350 0xShadowX net_processing: count presync work in chain sync evictionCI failing1Strong31 + 161 tests
  • Prevent nodes on high-latency networks from repeatedly evicting active peers during headers sync
  • Stop slow peers on Tor or I2P from losing all headers sync progress to a premature 22-minute timeout
  • CI is failing, so verify whether the failure is related before a deep review
  • Strong support after resolving concerns about stalled peers and unbounded renewals (cyb3ralbert)
  • Verified by testing that timeout renewals are strictly bounded in the functional test (cyb3ralbert)
  • 31 lines added or modified outside tests
  • 161 lines added or modified in tests
  • 2 lines removed in total
  • 3 files, 4 commits
P2P#8 of 71
P2 · bug fix
  • P2 because it fixes a chain sync liveness failure for nodes operating over high-latency networks
  • Slow outbound peers on Tor or I2P can stall or restart headers sync indefinitely on master
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Tracks progress across both the presync and redownload phases of anti-DoS headers sync to grant bounded chain sync timeout renewals to slow outbound peers. In `PeerManagerImpl::ConsiderEviction`, if an outbound peer has not yet updated its best-known block index entry but has made headers sync progress since the timeout was set, `CHAIN_SYNC_TIMEOUT` is renewed for another 20-minute period up to a maximum of six times (`MAX_HEADERS_SYNC_TIMEOUT_RENEWALS`).

Problem: While syncing below `-minimumchainwork`, anti-DoS headers sync holds headers in memory during presync and inside a redownload buffer, leaving `pindexBestKnownBlock` null for that peer. Chain sync eviction treats the peer as having an old or missing chain, triggering a 20-minute timeout followed by a 2-minute disconnect deadline. On high-latency networks like Tor or I2P where the ~480 mainnet round trips take longer than 22 minutes, nodes repeatedly evict active sync peers and discard all headers sync progress.

Evidence of importance:

  • PR description: 'On mainnet presync is roughly 480 headers round trips. If that takes more than ~22 minutes (about 2.5 s per round trip, e.g. over slow Tor or I2P), the sync peer gets evicted before presync can finish, and the same thing can happen with the next peer. The chain sync logic is from #11490, presync came later in #25717, and the two were never connected.'
  • PR description test results: 'On master the peer is disconnected after 24 minutes. With the fix it stays connected and presynced_headers keeps going up.'

Scope: The core logic change is only ~30 lines in headerssync.h and net_processing.cpp; the vast majority of the patch (+161 lines) consists of new functional tests in test/functional/p2p_headers_sync_with_minchainwork.py.

Risk of a mistake: high (p2p). Mistakes in outbound peer eviction logic could allow slow or malicious peers to monopolize outbound sync slots or inadvertently stall initial chain synchronization.

Changed since the previous assessment: Added commit d59713ea51 to test_headers_sync_renewals_bounded, verifying that an outbound peer stays connected after 7 periods and disconnects after 8 periods to strictly enforce MAX_HEADERS_SYNC_TIMEOUT_RENEWALS.

Reviewability

Ready · CI failing

CI is red; check whether the failure is the PR's before a deep review.

Waiting on (from the thread read):

  • nothing: all items settled and PR is ACKed 2026-10-22

Agreement

Strong

substantive support, no open objection (cyb3ralbert)

Thread notes: cyb3ralbert ACKed after testing d59713ea51 and confirming that test_headers_sync_renewals_bounded properly pins the bound, accepting the final suggestion.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
cyb3ralbert (contributor)objection, correctnessA stalled peer that stops responding to getheaders remains connected indefinitely, holding the outbound slot.fixed · fix pushed · author confirmed and pushed bf56f4fb83 renewing the timeout only if presync work increasedchangedyes2026-10-14: 'With a single outbound peer, a peer that sends one headers message and then stops responding to `getheaders` remains connected for 24 hours under mocktime, whereas master disconnects it after 24 minut'
Status from: 2026-10-14: 0xShadowX: 'Pushed bf56f4fb83. The presync work is now recorded when the timeout is set, and when it expires the timeout is only ren'
cyb3ralbert (contributor)question, scopeWhether the Tor/I2P latency scenario is expected to apply when there is only a single outbound peer.answered · author explained that logic is independent of peer countansweredyes2026-10-14: 'Is the Tor/I2P latency scenario expected to apply when this is the only outbound peer?'
Status from: 2026-10-14: 0xShadowX: 'Single outbound peer: nothing in the logic depends on how many outbound peers there are, so a slow Tor/I2P link can hit '
cyb3ralbert (contributor)question, usefulnessWhether the eviction was observed on a real node or inferred from expected round-trip times.answered · author confirmed it was inferred from mainnet round trips and reproduced in mocktime testsansweredyes2026-10-14: 'Was the eviction observed on a real node, or inferred from expected round-trip times?'
Status from: 2026-10-14: 0xShadowX: 'Real node: no, the eviction was not observed on a real node. It is inferred from the ~480 presync round trips on mainnet'
cyb3ralbert (contributor)objection, correctnessDuring the REDOWNLOAD phase, presync work stops growing and timeout renewals cease, risking eviction of slow peers before the redownload buffer releases headers.fixed · fix pushed · author implemented GetSyncProgress() to cover both presync and redownload in 69e0e7188changedyes2026-10-15: 'A peer that needs timeout renewals during presync therefore has only 22–42 minutes after the transition to deliver those headers before being disconnected.'
Status from: 2026-10-15: 0xShadowX: 'The renewal now uses a progress counter that covers both phases (`HeadersSyncState::GetSyncProgress()`, presync height +'
cyb3ralbert (contributor)objection, safetyTimeout renewals are unbounded, allowing a peer with slow but regular progress to hold an outbound slot indefinitely.fixed · fix pushed · author bounded renewals to MAX_HEADERS_SYNC_TIMEOUT_RENEWALS = 6 in 69e0e7188changedyes2026-10-15: 'There does not appear to be a bound on the number of renewals, so a peer making very slow but regular progress can hold the outbound slot for an extended period.'
Status from: 2026-10-15: 0xShadowX: 'Renewals: now bounded by `MAX_HEADERS_SYNC_TIMEOUT_RENEWALS = 6`, so a peer can hold the slot for at most ~2h22m through'
cyb3ralbert (contributor)suggestion, maintenanceCall sync_with_ping() on each step or allow extra minutes to make test_presync_peer_stalled_is_evicted less timing-sensitive.fixed · fix pushed · author updated advance_minutes to sync with ping after each mocktime step and increased loop to 50 minutesansweredyes2026-10-15: 'Calling `sync_with_ping()` on each step, or allowing a few extra minutes, would make the test less timing-sensitive.'
Status from: 2026-10-15: 0xShadowX: '`advance_minutes` now calls `sync_with_ping()` after every mocktime step and the loop runs to 50 minutes.'
cyb3ralbert (contributor)suggestion, maintenancePin the number of renewals in test_headers_sync_renewals_bounded so that the test fails if MAX_HEADERS_SYNC_TIMEOUT_RENEWALS is changed.accepted · fix pushed · cyb3ralbert verified that the test fails with bound at 5 or 7 and passes with 6answeredyes2026-10-21: 'One gap in the new bound test: `test_headers_sync_renewals_bounded` doesn't pin the number of renewals.'
Status from: 2026-10-22: cyb3ralbert: 'with 6 it passes, so the bound is now pinned from both sides.'

Support:

  • cyb3ralbert (contributor) (ACK, 2026-10-22) [both: p2p_headers_sync_with_minchainwork.py]: The new commit only touches the functional test; verified test_headers_sync_renewals_bounded fails if the renewal bound is changed to 5 or 7 and passes with 6, pinning the bound from both sides

Participants:

  • cyb3ralbert (contributor, support, 4 statements 2026-10-14 to 2026-10-22): ACKed after verifying the renewal bound test; previously raised REDOWNLOAD eviction, unbounded renewals, and test coverage concerns (all settled)

Files

162 lines under test/bench/ci.

  • test/functional/p2p_headers_sync_with_minchainwork.py +161/-1
  • src/net_processing.cpp +27/-1
  • src/headerssync.h +4/-0

Card

The short text the ranking pass compares across PRs.

Prevents premature eviction of outbound peers during anti-DoS headers presync and redownload. When syncing headers below minimum chain work, headers are held without updating the block index, causing chain sync eviction to disconnect active peers after 22 minutes. On slow links like Tor or I2P, the ~480 round trips can take longer than 22 minutes, discarding all sync progress and repeating with subsequent peers. The change inspects headers sync progress during eviction checks and grants bounded 20-minute timeout renewals (up to six) as long as progress is being made, verified by functional tests.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-23T01:17, confidence high, input hash 56892fa662a153bc
thread read through 2026-10-22 (7 statements), head d59713ea51
labels: P2P, CI failed

Categories

P2P

P2 · bug fix

Fixes a sync liveness bug where nodes on high-latency networks like Tor or I2P repeatedly evict active sync peers. Chain sync eviction previously failed to account for anti-DoS presync headers progress, disconnecting peers after ~22 minutes when ~480 round trips take longer than 2.5s each.

Membership: Matches the P2P label and modifies net_processing.cpp and headerssync.h to prevent chain sync eviction during headers presync.

Factors: security/stability 1, bug 2, performance 0, user value 2, leverage 0