fix(network): stop block-sync liveness from exiling healthy peers during local commit stalls

[p0mvn]

Motivation

Block-sync's "no accepted block progress" liveness machinery blames peers for stalls that are actually local. Two related defects, both reproduced live.

Bug 1: Peer race resulting in peer disconnect due to garbage collection

Consider:

1. Peer A delivers a block at 12:00.
2. Peer A receives another request at 12:01.
3. Peer B satisfies that request.
4. Floor-GC removes Peer A’s request.
5. Peer A now has zero outstanding requests.
6. Because Peer A’s last delivery was before its latest request, the old condition does not clear its deadline.
7. The deadline expires.
8. Peer A is disconnected for making no progress—even though there is nothing left for it to deliver.
9. Peer A enters the 180-second cooldown.

As this happened repeatedly, healthy peers could be removed one at a time. In my tests, peer count went from five to zero.

The fix was to update the liveness deadline if the apply queue was non-empty, obsolete requests were removed, and the peer had no outstanding work.

Bug 2: local commit stalls looked like peer failures

Note: I do not have the answer for the commit stalls. This is fixing a different layer.

Suppose a peer sends valid blocks, but the node’s verifier or storage layer stalls. Blocks accumulate in the local applying queue, and no commits land. The liveness mechanism sees no progress and blames the peer.

Now, check the applying queue first. If non-empty, do not disconnect the peer.

If the applying queue were to become empty and the peer were truly dysfunctional, it would still be evicted.

[v12-auditor]

Note

Complete: Audit complete. V12 found two issues worth reviewing.

Open the full results here.

Finding	Severity	Details
F-99331	🟠 High	Global apply backlog disables liveness PeerRoutine::check_block_liveness now extends any expired no-progress deadline whenever the shared SequencerView.applying_len is nonzero. That counter is not peer-specific; it is the local sequencer’s global apply-map length, populated when any contiguous downloaded body moves into applying and published to every peer routine. During checkpoint-class body sync, the block-sync driver only logs after ZAKURA_BLOCK_SYNC_DRIVER_TIMEOUT and then keeps awaiting the checkpoint verifier, while the checkpoint verifier waits until a contiguous range reaches a checkpoint before resolving queued block futures. A Sybil peer set can fill block-sync slots, have one peer serve a valid next checkpoint body so applying_len remains nonzero, and have the rest withhold bodies while their timed-out requests are requeued. The new branch runs before the park/disconnect arm, so those no-progress peers are repeatedly granted deadline extensions and remain admitted while honest replacements are refused by the service caps.
F-99332	🟠 High	Floor GC clears liveness PeerRoutine::gc_committed_outstanding now calls DownloadWindow::clear_liveness_if_idle() whenever the download floor has passed a local outstanding range. clear_liveness_if_idle() only checks that outstanding is empty, unlike disarm_liveness_after_progress_if_idle() which also required the peer to have delivered a block after the last request. A malicious peer can receive its floor probe, never return the body, and keep the routine’s biased receive arm ready with valid small frames until the reactor floor watchdog force-returns the expired height and other peers commit it. The next top-of-loop GC removes the stale request and clears the liveness deadline even though last_block_at never advanced. The peer is then stuck at its no-progress request cap with no outstanding work and no liveness deadline, so it receives no more requests but also never reaches the disconnect/cooldown branch.

Analyzed one file, diff af8449e...69391ac.

[p0mvn]

Evidence record

How these two defects were independently re-identified (before finding the existing Zebra-side fixes), and what a reviewer can verify. Environment: 8-core/31 GiB host, fresh mainnet syncer peered exclusively with one local archive feeder (~825k blocks) — the single-peer limit that turns "peer-set erosion" into a hard, observable wedge. Full harness and run records: /root/unity/memtest on the test host (configs, runner scripts, per-run CSVs; summarized in REPORT.md there).

1. Symptom

Fresh syncs on main froze body verification within minutes of the first deep commit stall, while header sync continued normally. From the memory sampler of one wedged run (5 s samples; verified_height and body_received frozen for the remainder of the run, 5+ minutes to cutoff):

epoch,rss_kb,...,verified_height,...,body_received_total
...
1784001318,3341556,...,165366,...,178835   <- committer still draining
1784010111,3275364,...,166190,...,178835   <- downloads dead, tip frozen

(another capture wedged at height 4,800 ~90 s in; a third at 2,800; heights vary with when the first look-ahead backpressure stall lands.)

2. Metrics signature at the wedge

zakura_p2p_reactor_active_connections{reactor="header_sync"}  1
zakura_p2p_reactor_active_connections{reactor="block_sync"}   0
zakura_p2p_stream_parked_no_demand{stream_kind="block_sync"}  1
sync_block_outstanding                                        0

Block-sync lost its connection; the peer re-offered a block-sync stream; the local service refused it ("no demand"); no recovery.

3. The log line and the code path

Syncer log at the wedge moment:

INFO zakura_network::zakura::handler: locally parking ordered service stream
     because the service has no demand peer_id=ZakuraPeerId([...]) stream_kind=6

Stream kind 6 is block-sync. The refusal traces to wants_ordered_stream → BlockSyncService::wants_peer → !peer_is_parked(peer) (zakura-network/src/zakura/handler.rs, block_sync/service.rs:386). The only production writer of that park is the liveness path: check_block_liveness → LivenessOutcome::Disconnect → park_peer_until(now + no_progress_peer_cooldown) with error "block-sync peer made no accepted block progress before liveness deadline" (block_sync/peer_routine.rs). The deadline is 4×request_timeout (32 s at defaults) without an accepted body — which the node's own commit stall produces whenever the look-ahead gate is full. After the park, nothing re-escalates a block-sync stream when the cooldown expires, so a few-peer node never recovers.

4. Config cannot fix it

Raising request_timeout to 30 s (liveness 120 s) and cutting no_progress_peer_cooldown to 1 s only narrowed the race: the wedge still reproduced (capture at height 2,800 — the peer's stream re-open arrived milliseconds after the disconnect, inside even a 1 s park). The disconnect-on-local-stall logic itself is the defect, which is exactly what the second commit here changes.

5. Provenance of the fixes

Searching history for the mechanism (git log --all --grep=liveness) surfaced 3e5b6eb4c on the zebra remote refs — not an ancestor of main (git merge-base --is-ancestor fails). Its commit message independently documents the same mechanism observed live on the fleet: a ~45-minute commit stall exiling healthy peers "one by one into the 180 s no-progress cooldown (observed live: parks +4 on t2, t4, down to 2 peers)". Cherry-picking it exposed its dependency on a433abe62, whose message documents the floor-GC variant: "reproduced live 5→0 after a fleet-wide restart, wedging body sync at the next gap". Both sit unmerged in valargroup/zebra#498.

6. A/B confirmation

• Without these commits (main, single-peer): three independent runs wedged permanently within minutes of the first commit stall, all with the section-2/3 signature.
• With these commits (as the harness base for fix(network): bound block-sync memory with a serialized apply backlog and wire-honest admission accounting #190's validation): a single-peer node synced genesis → height 701,649 in ~2h50m through continuous look-ahead backpressure stalls with zero liveness parks (sync.block.peer.parked = 0 for the whole run). Unit tests from the original commits (floor_gc_clears_stale_liveness_deadline, the deferral test) fail on main and pass on this branch.

Repro recipe

1. Run any zakurad archive node as a feeder; point a fresh syncer at it as its only peer (v2-only avoids unrelated legacy-crawler churn).
2. Sync until the look-ahead gate first fills (any height where the committer falls behind downloads for >4×request_timeout).
3. On main: observe the disconnect log line, the stream_parked_no_demand increment, and the permanent freeze of sync_block_verified_tip_height while sync_header_* continues. On this branch: the deadline is deferred while the applying queue is non-empty and sync proceeds.

---

Posted by Claude Code on Roman's behalf; evidence gathered during the #190 validation work.

[p0mvn]

@cursor review

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.}

^{Reviewed by Cursor Bugbot for commit 5447869. Configure here.}

[cursor]

Floor GC leaves probe cap zombie

Medium Severity

After floor GC removes a peer’s last outstanding request, clear_liveness_if_idle drops the liveness deadline but leaves requests_without_block_progress at the probe cap. An unproven peer that never served a body can then neither issue another request nor be disconnected for no-progress, leaving a live but useless connection.

 

^{Reviewed by Cursor Bugbot for commit 5447869. Configure here.}

[github-actions]

Zakura PR node run

No summary was produced — see the workflow log.

Inspect the node: ssh root@167.71.109.153 (deleted by the reaper ~24h after creation).

Workflow run

[evan-forbes]

I don't think we can merge as is unfortunately. originally we had to add requests_without_block_progress as there were issues with "zombie" peers

here we're not resetting that which will result in the same bug as before if a peer is at their cap.

even if we do reset that, I think we'll have further problems as well.

to me, the cleaner solution is to simply not remove a peer's outbound request if we receive the block from a different peer. That seems like the bug here. The request is already inflight, therefore cannot be cancelled.

more generally, we are not doing a good job currently of separating active peer state and global shared state. imo

Only active entries should be used by:

• available_slots();
• outstanding_reserved_bytes();
• publish_outstanding();
• PeerRegistry::total_unreceived();
• floor-watchdog claims;
• drop-time WorkQueue cleanup.

Retired entries should be used only by:

• inbound response matching;
• response timeout cleanup;
• peer liveness/accountability;
• diagnostics.

I'm also down for adding a request id like we did in headersync, I could see that hardening a lot of state management here.

[p0mvn]

Superseded by #209