fix(sync): reserve one slot for registry-miss retry instead of stalling dispatch

[p0mvn]

Motivation

A required block that is unavailable from every ready peer is parked on a
registry-miss backoff and retried later. In the current code, any pending
retry sets a global head-of-line gate:

let head_of_line_starved = !self.registry_miss_retry.is_empty();
if !past_lookahead && !head_of_line_starved && !reserve.is_empty() { ... }

While that gate is set, all reserve block dispatch is paused until the retry
clears. A peer can advertise a required block hash it will not actually serve;
once that hash becomes a registry miss, unrelated (fully available) reserve
downloads are stalled for roughly the retry budget —
MISSING_BLOCK_REGISTRY_RETRY_LIMIT (60) × REGISTRY_MISS_RETRY_BACKOFF (2s) ≈ 2 minutes. Impact is a targeted, node-level sync / catch-up delay (DoS).

Solution

Replace the all-or-nothing gate with a capacity reservation:

• New reserve_dispatch_limit() returns
  lookahead_limit − in_flight − reserved, where reserved = REGISTRY_MISS_RESERVED_DOWNLOAD_SLOTS.min(registry_miss_retry.len()) (0 or 1).
• sync_round dispatches reserve hashes whenever reserve_dispatch_limit > 0,
  so a single pending retry withholds just one slot instead of the whole window.
  Unrelated reserve hashes keep flowing while spare lookahead capacity remains.
• request_blocks now takes an explicit dispatch_limit (the reserve loop
  passes the reserved budget; obtain_tips passes the full lookahead limit,
  preserving its prior behavior).
• The registry-miss retry itself remains ungated in the select! timer arm, so
  the parked block still gets prompt head-of-line service — the reservation just
  keeps in-flight one below the limit so the retry has room without overshoot.

The change is inert in healthy sync (no miss ⇒ reserved = 0), and
saturating_sub prevents underflow.

Behavior note (steady state)

In the reserve loop, dispatch is now capped so total in-flight fills up to
lookahead_limit (accounting for in_flight), whereas previously
request_blocks split at the raw lookahead_limit and could briefly overshoot
to ~2×. This tightens in-flight concurrency to the documented limit. obtain_tips
is unchanged. See test evidence below for the throughput check.

Observability

• New sync.registry_miss.pending gauge: number of required blocks parked on a
  registry-miss backoff. A sustained non-zero value indicates required hashes are
  unavailable from all ready peers (and holding the reserved slot); a spike can
  indicate a peer feeding unavailable required hashes.
• The "requesting more blocks" debug line now logs reserve_dispatch_limit
  alongside in_flight / reserve / lookahead_limit.

Test evidence

New unit/regression tests in zebrad/src/components/sync/tests/vectors.rs:

• reserve_dispatch_limit_is_inert_without_registry_misses — no miss ⇒ full
  lookahead window is dispatchable (pins the healthy path unchanged).
• reserve_dispatch_limit_caps_reservation_at_one_slot — several pending misses
  still reserve only one slot.
• registry_miss_does_not_block_unrelated_reserve_dispatch — regression test:
  with a miss parked on backoff, unrelated available reserve hashes still
  dispatch to peers, and the retry stays scheduled (not dropped).
• request_blocks_dispatches_only_the_budgeted_hashes — request_blocks
  dispatches only the caller's budget and returns the rest to the reserve.

cargo fmt --all -- --check          # clean
cargo clippy -p zebrad --all-targets -- -D warnings   # clean
cargo test -p zebrad --lib components::sync::tests::vectors   # 31 passed

Real-node check: syncing a fresh node from genesis through 100k heights to
confirm no throughput regression from the in-flight cap change (results added as
a comment).

Related

Adjusts the head-of-line behavior introduced in #105
(fix: prioritize head-of-line block during registry-miss backoff).

AI disclosure

Used Claude Code (Opus 4.8) to review the fix, and to author the added tests,
the sync.registry_miss.pending metric, the changelog entry, and this PR
description. The contributor is the responsible author; all changes were
reviewed and verified locally.

[v12-auditor]

Note

Complete: Audit complete. No review-worthy issues remain after automatic triage. One finding was auto-invalidated.

Open the full results here.

Analyzed one file, diff 4619f83...9e17352.

[p0mvn]

Testing summary

This documents the testing strategies completed for the registry-miss dispatch fix, from unit level up to a real running node driven by a wire-level misbehaving peer.

1. Unit / property tests (zebrad/src/components/sync/tests/vectors.rs)

All pass (cargo test -p zebrad --lib components::sync::tests::vectors, 31/31 in the module):

• reserve_dispatch_limit_is_inert_without_registry_misses — with no miss pending, the full lookahead window is dispatchable (pins the healthy path as unchanged).
• reserve_dispatch_limit_caps_reservation_at_one_slot — several pending misses still reserve only one slot.
• request_blocks_dispatches_only_the_budgeted_hashes — request_blocks dispatches only the caller's budget and returns the rest to the reserve.
• registry_miss_does_not_block_unrelated_reserve_dispatch — regression test for the reported DoS: with a hash parked on its registry-miss backoff, unrelated available reserve hashes still dispatch to peers, and the retry stays scheduled.

2. In-process instrumented mock run (authoritative gauge evidence)

registry_miss_raises_pending_gauge_and_keeps_dispatching installs a tiny in-process metrics::Recorder (no new dependencies) and drives a genuine NotFoundRegistry through the real handler + update_metrics. Captured gauge trace:

step 0 (healthy, no miss):                            sync.registry_miss.pending = 0
step 1 (poison hash -> NotFoundRegistry scheduled):   sync.registry_miss.pending = 1
step 2 (after 2 unrelated hashes dispatched, parked): sync.registry_miss.pending = 1

This is the deterministic proof that the reserved-slot path raises the gauge above zero while unrelated dispatch continues.

3. Live mainnet sync through 100k heights (throughput / no-regression)

Fresh node, genesis → height 106,517, /metrics scraped throughout:

• 0 panics, no stalls; ~4,700 heights/min on checkpoint sync.
• The legacy sync_round (the modified code) was the active downloader (replace_legacy_syncer=false), so the change ran continuously.
• in_flight oscillated ~80–1000 (cap = checkpoint_verify_concurrency_limit) — dispatch stayed saturated, confirming the in-flight accounting change does not under-dispatch.
• sync_registry_miss_pending stayed 0 the entire healthy run — the reservation is correctly inert when nothing is missing.

4. Running-binary wire-level mock peer (Regtest, isolated)

A local-only harness (a misbehaving Zcash peer built on zebra_network's public connect_isolated_tcp_direct_with_inbound) dials an isolated Regtest zebrad and, as it syncs, advertises fabricated block hashes via FindBlocks then notfounds the BlocksByHash downloads. Observed on the real node:

• Handshake succeeds; the peer advertises ~100 bogus hashes.
• The real Zakura→legacy fallback watchdog fires (blocks_ahead=Some(64) → "falling back to legacy ChainSync so it can drive body sync").
• The legacy sync_round becomes the active downloader and issues real FindBlocks / BlocksByHash to the fake peer, running update_metrics (so sync_registry_miss_pending is published live on /metrics).
• The registry-miss classification path is exercised on the live binary: the sync.missing.block.registry.miss.count counter climbed to 99.

In this single-peer Regtest scenario the sync_registry_miss_pending gauge is only transiently non-zero — with every hash permanently unavailable the round makes no progress and churns (re-obtaining tips), so the per-round registry_miss_retry map is repeatedly populated and cleared faster than the /metrics HTTP scrape window. The gauge's 0→1 transition and the reserved-slot behavior are therefore asserted deterministically by strategy #2; strategy #4 confirms the same code path fires end-to-end under a real adversarial peer on a real node.

Local quality gate

cargo fmt --all -- --check, cargo clippy -p zebrad --all-targets -- -D warnings, and the full sync-module tests all pass. CI on this PR is green.

Testing artifacts (the wire-level harness and metrics config) were run locally and are not part of this PR.