fix(network): own GetBlocks reservations through request writes

[czarcas7ic]

Motivation

GetBlocks reservations must settle exactly once when publication, expiry, reset, and transport writes race. A cold peer also needs enough time to deliver its first block. This outgoing-download chunk is extracted from #892 and stacked after #943.

Solution

Reserve queue capacity before taking work, publish the exact request owner atomically, and retain its claim until transport finishes or discards it. Expiry and receipt that invalidate a queued request share the same atomic settlement, immediately returning all its unsent heights and retiring its peer obligation before the writer reaches it. Cleanup preserves received bodies and replacement owners, discards committed heights, and destroys request claims outside queue locks. The floor watchdog uses the write disposition captured during settlement, so queued requests do not trigger peer avoidance even while the last owner is being dropped. Started writes retain per-height accounting and partial-write cancellation. Publication failures log the peer, generation, and range. Keep the approved initial-probe and queued-response deadline policy. The current wire layout and serving path remain active until the final chunk.

Testing

At 429db8a6d, all 393 selected block-sync, session, and transport tests pass without retries, including buffered response validation after remote failure and neutral cleanup after local resets.

At d6831f340, all 113 selected handler and block-sync session tests pass without retries, including duplicate rejection, capacity accounting, cleanup, reopening, and paired downloads. Network all-target Clippy with warnings denied, formatting, Markdown lint, and changelog checks pass.

The receipt/liveness regression failed before its fix. The combined stack at 06fdcce37 passes 3,484 tests under the workspace CI profile, excluding three macOS address-binding tests. Workspace Clippy with warnings denied, formatting, and changelog checks pass. Coverage includes receipt before writer startup, return of unsent work, preservation of received bodies, and started writes retaining their reservations. The final activation qualification is recorded in #945. Prepared with Codex assistance.

Changelog

Added the reservation-correctness and slow-peer deadline fragment.

[v12-auditor]

Note

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

Open the full results here.

Finding	Severity	Details
F-271493	🟠 High	Filtered claim drop deadlocks work queue reset_above upgrades weak request-write entries while holding the global WorkQueueInner mutex, but its iterator retains only claims whose range extends above the new floor. An upgraded claim rejected by that filter is dropped before the mutex is released. If the transport concurrently drops the previous last strong reference because a paired stream is cancelled, its write fails, or its write future is aborted, the filtered temporary can become the final Arc<RequestWrite>. The resulting non-WRITTEN destructor calls back into WorkQueue and attempts to acquire the same non-reentrant mutex, permanently deadlocking the reset. This can occur for any queued or partially written request whose range is already at or below the reset floor.
F-271494	🟡 Medium	Reset reissues work at the new floor A destructive reset does not fully retire an unreceived request whose range includes the new floor. reset_above resets claims only when their last height is strictly above the floor and removes only queue entries strictly above it, so a request ending at the reset height can still start and a straddling request retains its prefix. When the peer routine observes the reset epoch, its generic cleanup returns those retained reserved entries to pending without enforcing the queue’s height > floor invariant. The next fill starts its search at the untrusted peer-advertised servable_low, so the stale reset-height entry can be selected and requested again. With the default one-block request geometry, a peer can repeatedly return a short response for that obsolete height, keeping the scheduler from reaching the first needed successor.
F-271495	🟡 Medium	Teardown reissues a request still being written A published GetBlocks request can already be in the transport’s STARTED state when the peer routine exits. PeerRoutine::drop nevertheless returns every unreceived height to pending and releases its byte charge without first retiring the request stream. Queue cleanup calls only expire_unwritten, which cannot stop a claim after it has transitioned to STARTED; only the reset-specific path cancels such a write. A peer can trigger this by sending a malformed inbound frame after the outbound writer starts the request: the protocol error drops the routine before the outer supervisor cancels the connection, allowing the original write to complete after its work is reissued and uncharged.
F-271497	🔵 Low	Reset cannot stop a started stale request RequestWrite::reset attempts to retire a partially written request by cancelling the block-sync session when its state is STARTED. Block sync is registered as a single ordered stream, so the transport runs it under the standalone write policy rather than as an interruptible ordered pair. The standalone worker explicitly disables its stream-token cancellation arm while the current frame write is pending. A reset racing the request write therefore cannot stop the invalidated GetBlocks frame, and a successful write subsequently records the claim as WRITTEN. The peer can then serve an obsolete range after its local work has already been released or replaced.
F-271498	🟠 High	Replacement sessions preserve a ghost preferred carrier A replacement peer routine starts with received_status = false and refuses all download work until it receives a new Status, but session admission preserves the previous registry entry’s status, servable range, latency score, and slot diagnostics while only bumping its generation and clearing outstanding heights. The superseded routine’s generation-gated teardown cannot clear those preserved eligibility facts. Other routines continue consulting the stale registry entry and defer the floor whenever it appears to be a faster unsaturated carrier. An authenticated peer can first establish a favorable RTprop and free-slot state, replace its block-sync session, and then withhold Status; its new routine never requests the floor while all slower honest routines keep yielding to it.
F-271500	🟠 High	Late bodies bypass memory admission limits Late responses for heights that have been reassigned to another peer bypass both defenses that reconcile untrusted size estimates with actual body memory. The normal matched path rejects a body whose serialized size exceeds the configured tolerance around its estimate, but the unmatched active path never performs that check. It also applies admit_received_body only when the height is pending, not when another request currently reserves it. An attacker can arrange low advertised estimates, let its requests time out and be replaced, then deliver maximum-size late bodies; the code releases each replacement’s small reservation and forwards the full decoded body despite a saturated resident lookahead gate.
F-271502	🟡 Medium	Carrier preference checks the wrong height The floor-selection code computes the critical missing successor as download_floor + 1 but asks the peer registry whether another peer can serve download_floor itself. At genesis or a pruning boundary, normal peers commonly advertise servable_low equal to the missing successor and therefore fail the preference test for the already-downloaded floor. A cold attacker can race a proven lower-latency peer for the exclusive successor request even though it should have deferred. Because an unmeasured peer now receives the normal timeout rather than the short rescue leash, each silent identity can hold the floor for the longer deadline. Rotating identities can repeat the delay.
F-271503	🟡 Medium	Backpressure cycling renews liveness forever The liveness grace for a full outbound queue is intended to be bounded, but no state records that a peer has already consumed it. Whenever the queue briefly regains capacity, the run loop clears outbound_full_since; filling it again shortly before the next liveness deadline makes the congestion interval appear fresh and grants another full request_timeout extension. A peer controls this by alternating reads and pauses and by sending GetBlocks messages that cause the node to enqueue serving responses. Repeating the cycle prevents the no-progress park indefinitely without returning an accepted block. The attacker can retain peer slots and protect other long-lived claims from the peer-wide liveness backstop.
F-271504	🟡 Medium	Stale terminators cancel replacement requests An expired request can finish writing after its heights have been reassigned because timeout cleanup calls only expire_unwritten, which cannot transition a STARTED claim and does not cancel its session. The old and replacement requests can then share the same start_height. Stream-6 terminators contain no attempt identifier, and local lookup correlates BlocksDone and RangeUnavailable only by that start height. A delayed terminator generated for the expired request is therefore applied to the live replacement and returns its work using the replacement owner. A peer can repeat the sequence around transport backpressure to cancel valid retries.
F-271505	🟡 Medium	Skipped writes remain phantom outstanding requests A late response can settle a replacement owner while that replacement’s frame is still queued. RequestWrite::try_start correctly skips the frame because its reservation is no longer Reserved, but owner_for_height continues returning its owner as long as the item is non-provisional. Both obsolete-request GC and registry publication use this weaker owner predicate, so the never-written request remains locally and globally outstanding. It later times out, is counted as a reliability failure, and can park a healthy peer that never received a GetBlocks request. A malicious late responder can use timing to attrit competing carriers.
F-271506	🟡 Medium	Watchdog races discard valid late bodies After the floor watchdog retires a height from shared ownership, the old peer routine can still hold a local range containing it. If that old peer’s valid started or written response arrives after another peer has reserved the height, range lookup takes the stale local match first, so the unmatched first-completion-wins path is never reached. The owner mismatch then drops the stale local request and returns without forwarding the already received valid body or settling the replacement. The node waits for the replacement’s full timeout even though it had a valid candidate available. A malicious replacement can deliberately withhold its response to maximize the delay.
F-271510	🔵 Low	Unbounded timeout configuration disables liveness Configuration validation imposes only a one-millisecond lower bound on request_timeout and no maximum. Cold floor requests now deliberately use that full normal timeout, while peer liveness multiplies it by four and deadline code adds these durations to Instant with unchecked +. A representable but extremely large value can let a silent peer hold the floor for years. A still larger accepted value can exceed the platform’s Instant range and panic peer routines whenever they arm a request or liveness deadline.
F-271513	🟠 High	Partial request retirement can orphan live suffix work Partial retirement of a multi-height request is incorrectly interpreted as retirement of its entire outstanding range. The floor watchdog clears and returns only its current expired height, while advance_floor deliberately removes only a committed prefix and leaves a live suffix reserved. If a body then arrives for the retired height, owner_for_height no longer matches the range owner, and the peer routine calls drop_obsolete_outstanding for the whole local request. That removes the local timeout and published outstanding bookkeeping but does not return or otherwise settle the suffix entries that remain in in_flight; after RequestWrite reaches WRITTEN, its destructor also performs no settlement. The handler must distinguish a per-height retirement from retirement of the whole request, retaining/reconciling the live suffix and its liveness metadata, or explicitly return every remaining owner-owned suffix height when retiring the range.
F-271514	🟡 Medium	Unbounded floor deadlines let peers monopolize the missing block Floor-claim deadlines are derived from a cumulative byte estimate that includes both earlier outstanding requests on the ordered stream and every block in the newly created range. Although normal serving sends the range contiguously from start_height, the deadline charges later range bodies that need not arrive before the critical floor body. Further, request_deadline selects the short floor_rescue_timeout only when a byte-rate measurement exists, while bbr_btlbw_bytes_per_sec returns None in CwndUnit::Blocks, causing floor claims in that mode to use the normal timeout. The resulting deadline is not bounded independently of request backlog or configured in-flight capacity, and the floor watchdog defers recovery until that stored deadline. Meanwhile, note_block_progress renews peer-wide liveness for any accepted block, letting a peer provide non-floor or later-range bodies while retaining an exclusively claimed floor height.
F-271515	🟠 High	Unvalidated late bodies can settle and suppress replacement responses The first-completion-wins handling lets a late response release the active replacement request's per-height reservation based on height and the header-derived block hash, while transaction/body consistency is deferred to consensus apply. Since Hash for a Block is derived from its header, that check does not establish that the supplied body satisfies the header's commitments. The late response is forwarded and credited with progress and reliability before that semantic validation occurs. Once the reservation has been released, a subsequent response from the replacement peer follows accept_already_settled_height, which marks the height received and records progress without forwarding its body for validation. Consequently, an invalid late body can both win over a valid replacement and allow either the late sender or the replacement sender to receive liveness credit despite no validated block having been supplied; recorded misbehavior does not cancel the session.

And two more auto-invalidated findings.

Analyzed seven files, diff 5b5eeb2...9fcd167.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-11T20:20:31.583433Z	c9c6531	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[evan-forbes]

The floor watchdog still penalizes requests that never started writing. In reactor.rs, run_floor_watchdog applies peer avoidance before request settlement determines whether the write started. With at least three servable peers, a queued request can expire, return its reservation, and still leave its peer excluded from the floor height. The peer-routine expiry path correctly skips that penalty, so behavior depends on which handler runs first.

I reproduced this with an additional test against the running reactor. The test confirms that the request was skipped and its reservation reached zero, then fails because the registry still excludes the peer. Settlement should return the write disposition so the watchdog can withhold avoidance for unwritten requests. Checking the status before settlement would leave a race with writer startup.

A few follow-ups:

• RequestWrite::publish scans the global request map with retain under the shared work-queue mutex on every publication. Can terminal cleanup remove the exact owner instead?
  This adds contention as queued requests accumulate, though I have not measured the throughput impact.

• Add a direct assertion for cumulative response deadlines. Queue two requests, verify that the second deadline includes the first request’s unreceived bytes, then receive a
  prefix and check the next deadline.

• Build expected_blocks before constructing RequestWrite, then move items into the claim to avoid cloning the vector.