net_processing: count presync work in chain sync eviction #36350

While our best header is below -minimumchainwork, a peer's headers first go through the PRESYNC phase of HeadersSyncState, and none of them are stored. So pindexBestKnownBlock stays null for the peer we're syncing from, and ConsiderEviction (net_processing.cpp:5504) treats it like an outbound peer that never announced a block:

1. it starts CHAIN_SYNC_TIMEOUT (20 min)
2. it sends a getheaders to "verify chain work"
3. after HEADERS_RESPONSE_TIME it disconnects with Outbound peer has old chain, best known block = <none>

This happens even though the peer is still sending headers. The presync progress is lost and the next peer starts from scratch.

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.

The timeout is not reset on presync work alone. When it is set, the work the peer has shown during presync is recorded, and when it expires the timeout is renewed for another CHAIN_SYNC_TIMEOUT only if that work has increased since. A peer that stops sending headers is therefore still asked once with getheaders and then disconnected, within about two periods (~42 minutes), as before. m_headers_sync_mutex is never held while taking cs_main, and p2p_headers_sync_with_minchainwork.py and p2p_sendheaders.py also pass with DEBUG_LOCKORDER.

The new test_presync_peer_not_evicted in p2p_headers_sync_with_minchainwork.py restarts a node with -minimumchainwork=0x20000 (65536 headers). An outbound peer then sends it one full headers message per mocked minute for 25 minutes, so presync is still running at the end. On master the peer is disconnected after 24 minutes. With the fix it stays connected and presynced_headers keeps going up. p2p_outbound_eviction.py, p2p_eviction.py and feature_minchainwork.py still pass.

test_presync_peer_stalled_is_evicted covers the other side: a single outbound peer sends one headers message and then only answers pings, and it is disconnected after about 42 minutes of mocktime. That test fails without the progress check.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

External sites

• Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36350
• Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36350.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36350#author-ralph

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline and AI policy for information on the review process.

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Tested 34ef37a (also merged with master 8406af1c).

The new test passes on this PR but fails on master. 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 minutes. The peer still responds to pings.

With a second outbound peer, the headers download timeout fires after 29 minutes, for the slow peer as well.

Could we reset the timeout only when presync work has increased since the previous timeout checkpoint? This might preserve the intended behavior for slow peers without allowing a stalled peer to hold the only outbound slot.

A couple of questions:

• Is the Tor/I2P latency scenario expected to apply when this is the only outbound peer?
• Was the eviction observed on a real node, or inferred from expected round-trip times?

Thanks for testing, and you're right: resetting whenever presync work was at least the tip's work let a stalled peer hold the slot indefinitely.

Pushed bf56f4fb83. The presync work is now recorded when the timeout is set, and when it expires the timeout is only renewed if that work has increased since. Otherwise it goes through the usual getheaders and disconnect. A new test, test_presync_peer_stalled_is_evicted, sends one headers message and then only answers pings; the peer is disconnected after ~42 minutes of mocktime (one renewal because of the first headers, then the normal timeout), and the test fails on the previous revision.

On the questions:

• Single outbound peer: nothing in the logic depends on how many outbound peers there are, so a slow Tor/I2P link can hit it with one peer as well, and then it has no other peer to fall back on. I haven't measured that case.
• Real node: no, the eviction was not observed on a real node. It is inferred from the ~480 presync round trips on mainnet against the ~22 minute window, and reproduced with mocktime in the functional test.

Once presync reaches the minimum chain work and HeadersSyncState switches to REDOWNLOAD, GetPresyncWork() stops growing. As a result, the new branch cannot renew the timeout until the redownload buffer releases headers and pindexBestKnownBlock advances. On mainnet, this means 15,400 headers (8 full messages); on regtest, 7,017 headers (4 messages).

A peer that needs timeout renewals during presync therefore has only 22–42 minutes after the transition to deliver those headers before being disconnected. HeadersSyncState already tracks m_redownload_chain_work and m_redownload_buffer_last_height; should progress tracked by these fields also count towards timeout renewal?

Tested bf56f4fb8, also merged with master f13ae519d.

The regression from my previous comment is fixed: test_presync_peer_stalled_is_evicted fails on 34ef37a55 and passes here. With one full headers message followed only by pongs, the peer is disconnected 42 min 15 s after connecting (master: 22 min 10 s).

The runs behind the REDOWNLOAD question, on regtest with one outbound peer, mocktime, and -minimumchainwork=0x20000:

• One full headers message every 10 minutes reaches the transition at 66,000 headers, then the peer is disconnected 40 minutes later with Outbound peer has old chain, best known block = <none>.
• Every 19 minutes: disconnected 25 minutes after the transition.
• Every 1 or 5 minutes: sync completes.
• On master, the peer sending one message per minute is disconnected at minute 24.

A separate question is repeated renewals. With -minimumchainwork=0x40000, one full message every 19 minutes kept the peer connected for 8 hours, while every 25 minutes resulted in disconnection at minute 150. 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.

Is this the intended trade-off? Should renewals be bounded, or should the timeout account for progress without allowing indefinitely repeated extensions?

A note on the stalled test: its loop advances mocktime by one minute per 0.2 seconds without synchronizing pings, and the timeout comparisons are strict. The expected sequence is renewal at minute 21, getheaders at minute 42, and disconnection at minute 45 (the last iteration). A late SendMessages pass at minute 21 or 42 can push the timeout beyond minute 45, leaving mocktime stuck there until wait_for_disconnect times out. Calling sync_with_ping() on each step, or allowing a few extra minutes, would make the test less timing-sensitive.

test_presync_peer_not_evicted fails on master and passes here. p2p_headers_sync_with_minchainwork.py, p2p_eviction.py, p2p_outbound_eviction.py, p2p_initial_headers_sync.py, and feature_minchainwork.py pass on the head, the head merged with master, and a clang Debug build with DEBUG_LOCKORDER.

Not tested: mainnet round-trip times, Tor/I2P, or multiple outbound peers.

Thanks for the detailed testing.

REDOWNLOAD: agreed. The renewal now uses a progress counter that covers both phases (HeadersSyncState::GetSyncProgress(), presync height + redownload height), so progress in the redownload phase also counts. New test test_redownload_peer_not_evicted sends one message every 15 minutes after the transition; it fails on bf56f4fb8 and passes on 69e0e7188.

Renewals: now bounded by MAX_HEADERS_SYNC_TIMEOUT_RENEWALS = 6, so a peer can hold the slot for at most ~2h22m through this path. test_headers_sync_renewals_bounded (one message every 19 minutes) is disconnected after 7 periods. The trade-off is that a peer has to finish both phases within that window; on mainnet that is ~950 messages, i.e. ~7 messages/min (~150 kbps). I picked 6 as a value that leaves plenty of room for slow links while bounding the slot; happy to change it if a different bound (or a rate-based rule) is preferred.

Stalled test: advance_minutes now calls sync_with_ping() after every mocktime step and the loop runs to 50 minutes.

One gap in the new bound test: test_headers_sync_renewals_bounded doesn't pin the number of renewals. With MAX_HEADERS_SYNC_TIMEOUT_RENEWALS set to 5 or to 7 the whole file still passes; the test peer is then disconnected at minute 129 or 171 instead of 150. Two ways to pin it, both tried on 69e0e7188 (Linux x86_64, GCC 14.2, with 5, 6 and 7): asserting inside the existing loop that the peer is still connected after 7 steps of 19 minutes (minute 133) and gone after 8 (minute 152) fails on 5 and on 7 and passes on 6; and, since the renewal branch in ConsiderEviction logs nothing, adding one LogDebug there and asserting exactly 6 matches in debug.log fails on 5 (saw 5) and on 7 (saw 7) and passes on 6. The second one checks the renewal count itself rather than the timing. I can send either as a diff if useful.

Tested 69e0e7188 (Linux x86_64, GCC 14.2 and 12.2, Release build, regtest, mocktime, one outbound peer):

• test_redownload_peer_not_evicted fails on bf56f4fb8 (assert peer.is_connected) and passes on 69e0e7188; p2p_headers_sync_with_minchainwork.py passes as a whole (3 runs).
• The bound matches your ~2h22m. A peer that keeps serving a full 2000-header message every mocktime minute, with -minimumchainwork out of reach so it stays in presync, is disconnected at minute 150 with 300,000 headers; by my count the extra minutes come from the one-minute mocktime steps, since the comparisons are strict. On bf56f4fb8 the same peer is still connected at minute 160.

Thanks, good catch. Pushed d59713ea51: the loop in test_headers_sync_renewals_bounded now asserts the peer is still connected after 7 periods of 19 minutes (minute 133) and gone after the 8th (minute 152). I ran the file with MAX_HEADERS_SYNC_TIMEOUT_RENEWALS at 5, 6 and 7: it passes on 6 and fails on 5 and 7. I went with the timing check so no production code is touched for a log line.

ACK d59713ea518655031e2160026d85316ae19900fe

The new commit only touches the functional test (no src/ change). p2p_headers_sync_with_minchainwork.py passes. With MAX_HEADERS_SYNC_TIMEOUT_RENEWALS set to 5 test_headers_sync_renewals_bounded fails at assert peer.is_connected, and with 7 it fails at peer.wait_for_disconnect(); with 6 it passes, so the bound is now pinned from both sides.

Tested on Debian 13 x86_64, gcc 14.2.0, Release build.