feat(zakura): add BBR-lite block-sync congestion control by evan-forbes · Pull Request #337 · valargroup/zebra

evan-forbes · 2026-06-30T17:59:58Z

Motivation

Improve block-sync behavior under heterogeneous peers by replacing the legacy per-peer congestion-control behavior with BBR-lite estimators and floor-aware scheduling, while building on the separate fuzzer/testkit scaffolding PR.

Solution

Use block progress, not just request activity, for peer liveness.
Add BBR-lite config and per-peer estimator state for RTprop, BtlBw, cwnd, ProbeRTT, delay-gradient capping, timeout dips, and byte-denominated cwnd operation.
Make BBR-lite the sole per-peer controller while preserving hard advertised request caps.
Add floor-first scheduling, cwnd bypass, best-peer bias, exact-full floor funding, and request-count cap handling under byte cwnd.
Preserve feature-branch compatibility for max_inflight_block_bytes: positive below-floor values are clamped upward before validation; zero remains invalid.
Keep close-reason tracing and committer/apply-queue work out of scope.

Tests

cargo fmt --all -- --check
cargo check -p zebra-network
cargo test -p zebra-network block_sync --no-run
cargo test -p zebra-network blocksync_fuzz -- --test-threads=1 --nocapture (9 passed, 1 ignored)
cargo test -p zebra-network bbr -- --nocapture (16 passed)
cargo test -p zebra-network block_liveness_disconnects_silent_peer_and_traces_reason -- --nocapture
cargo test -p zebra-network reactor_timeout_recovery_is_local_and_healthy_peer_keeps_filling -- --nocapture

Issue

Closes: N/A

AI Disclosure

Used OpenAI Codex to port the BBR-lite sequence onto the scaffolding branch, remove stale cubic wording, and run focused verification.

p0mvn · 2026-06-30T20:25:31Z

            }
            if items.is_empty() {
-                break;
+                break "no_work";


Can we make constants for these instead of hardcoding?

p0mvn · 2026-06-30T21:55:02Z

+            }
+            BbrPhase::ProbeRtt => {
+                // Start the hold timer the moment the queue first reaches the floor.
+                if self.probe_rtt_drained_at.is_none() && inflight <= self.params.min_cwnd {


Unit mismatch: ProbeRtt drain gate never waits under the shipped default (CwndUnit::Bytes).

inflight here is a request count (DownloadWindow::record_delivery passes self.outstanding.len()), but in byte mode self.params.min_cwnd is bbr_min_cwnd_bytes (default 4 MiB, see BbrParams::from_config). So inflight <= min_cwnd is ~tens <= 4_194_304 — always true.

Consequence: the instant ProbeRtt is entered, probe_rtt_drained_at is stamped on the first delivery and the probe_rtt_duration hold starts before the in-flight byte queue has actually drained to the floor. That defeats the stated purpose of ProbeRtt — draining the queue so one uncontended request yields a fresh base round-trip — and specifically weakens the slow-peer cwnd collapse for a peer with a deep standing queue (200 ms rarely drains a deep byte queue). cwnd is still pinned to the 4 MiB floor during the hold, so ProbeRtt isn't fully inert, but the drain gate as written measures nothing in byte mode.

Note the only test covering the drain-wait (probe_rtt_pins_min_cwnd_then_drains_and_exits, which asserts probe_rtt_drained_at.is_none() while inflight is above the floor) pins CwndUnit::Blocks, so this path is untested.

Suggested fix: make the drain check unit-aware — compare in-flight reserved bytes against min_cwnd in byte mode (thread outstanding_reserved_bytes() into advance_phase, or pass a unit-appropriate "inflight" quantity), and add a byte-mode ProbeRtt drain test mirroring the blocks-mode one.

apologies forgot to push! I also ran both models as well and it found the same issue which is a increasingly frequent occurrence

Short answers: No, we did not push — the fixes are committed locally only. And yes, this is the same bug — it's Finding #1, the ProbeRtt ship-blocker, which we already fixed in commit f936776. The description you pasted matches it exactly.

one sec

p0mvn

Agreed to iterate on the comment post-merge

Stage 0 of the BBR-lite per-peer download controller: the config knobs (bbr_enabled default on, cwnd/probe-bw/startup gains, probe-rtt cadence, rtprop/delivery-rate windows, min cwnd, delay-gradient threshold) with DEFAULT_BS_BBR_* consts, Default entries, and validation. Ships inert — no code reads them yet, so there is no behavior change; the controller stages wire them in behind bbr_enabled.

Stage 1 of the BBR-lite controller: add per-peer estimators to DownloadWindow - WindowedSamples min/max filters over request round-trips and per-ack delivery rate, a delivered counter, and a cwnd_target() = max(min_cwnd, BDP x gain). Sampled lock-free on each completed request (the existing handle_body completion hook), and emitted on block_body_received (bbr_cwnd/rtprop_ms/btlbw/delivered). available_slots() is UNCHANGED, so this is pure measurement with no behavior change: the cubic-AIMD window still controls. The control-law flip consumes these in the next stage. Verified via the fuzzer: estimators produce a sensible BDP (e.g. rtprop 6ms x btlbw 2742 blk/s => cwnd ~32) and all block_sync tests pass.

Per request, there is one congestion-control mechanism, not a flag-gated pair. This removes the cubic-AIMD machinery entirely and makes BBR-lite govern the per-peer download window directly: - DownloadWindow loses outbound_request_window, timeout_recovery_slots, the consecutive-success/timeout streak counters and growth/reduction bases, the OUTBOUND_WINDOW_* cubic constants, increase_outbound_window_after_success, and the cubic ladder in the timeout path. available_slots() now caps purely at the BBR cwnd (BDP x gain, clamped to the advertised hard cap); a timeout applies one multiplicative cwnd dip (record_timeout). - config: drop the bbr_enabled flag; BBR is unconditional. initial_inflight is the BBR cold-start cwnd. Stale cubic docs updated. - trace: request_slot effective_window now reports the BBR cwnd; the removed fields drop out; block_peer_protocol_reject reports bbr_cwnd. - tests: remove the 4 cubic-window/timeout-recovery unit tests (removed behavior). Validated: 144 block_sync tests pass; the 5 fuzzer scenarios sync correctly under BBR; the cwnd adapts from the cold-start window toward the measured BDP (e.g. 64 -> 32 at rtprop~7ms/btlbw~2.2k blk/s) and aggregate in-flight stays shallow. ProbeRTT/ProbeBW/delay-gradient remain as follow-ups.

Add a ProbeRTT phase to the per-peer BBR-lite controller. Periodically (every bbr_probe_rtt_interval) it pins the cwnd to bbr_min_cwnd to drain the peer's request queue, so an uncontended request completes and refreshes the RTprop min-filter. Without it, a peer kept continuously queued never produces an uncontended round-trip after cold start, so its RTprop drifts up to the queue-inflated minimum and the BDP-derived cwnd inflates with it - re-growing the head-of-line backlog the controller is meant to bound. The phase machine is driven from record_delivery (the only event carrying both a fresh timestamp and the post-completion inflight count): ProbeBw -> ProbeRtt when an interval elapses; ProbeRtt holds min_cwnd until the queue drains to the floor, then for bbr_probe_rtt_duration so a clean sample lands, then restores the BDP cwnd. The timeout dip is suppressed during ProbeRtt (drains are expected there, not congestion). Traced as bbr_phase on block_body_received. Unit tests cover the drain/hold/exit cycle, the slow-peer min-cwnd pin, and dip suppression. The fuzzer config scales the probe cadence to sub-second so the mechanism is exercised in a ~1s run; fuzz_one_slow_peer_hol shows the slow peer entering ProbeRtt ~40% of deliveries and holding RTprop at the honest 121ms instead of drifting to 362ms.

The BBR cwnd caps a peer's in-flight at its BDP, which is what keeps per-peer queues shallow. But it also means a peer saturated at its cwnd cannot fetch the lowest missing height (the floor) even when that height is the one gating commit — it would wait behind the full window. Add a bounded floor bypass: a floor-priority request may borrow up to `floor_bypass_slots` (default 2) in-flight slots beyond the cwnd, still clamped to the peer's advertised hard cap so it never exceeds what the peer said it will service. `available_slots_with_bonus` expresses this; `available_slots` is now `bonus == 0`. The above-floor (speculative) arm is skipped in the bypass region, so the borrowed slots fund the floor only. Pair it with a deadlock-free floor->best-peer bias: a saturated peer only borrows a bypass slot when no other servable peer has a free normal slot (`floor_has_unsaturated_other_server`). The unsaturated peer takes the floor through its normal capacity and re-fills on its next completion (try_fill runs every routine-loop turn); if every server is saturated the bias yields and the floor still moves via the bypass. Traced as `floor_bypass` on block_get_blocks_sent + a `sync.block.request.floor_bypass` counter. Unit tests cover the bypass slot arithmetic, the hard-cap clamp, and the three bias cases (defer / all-saturated / not-servable). The synthetic fuzzer peers saturate at the hard cap (their BtlBw*RTprop estimate overshoots the BDP), so the borrow itself is exercised by the unit tests and live fleet peers, not the in-process scenarios.

The BDP cwnd is BtlBw × RTprop × gain, with BtlBw a max-filter and RTprop a min-filter. When a peer's rate and round-trip vary under queueing the two extremes come from different samples, so the product overshoots the sustainable pipe and the cwnd inflates — pinning the peer at its advertised hard cap and re-growing the head-of-line queue BBR is meant to bound. (The floor-first fuzzer work saw a synthetic worker's cwnd reach 620-1166 against a hard cap of 64.) Add a delay-gradient ceiling: an EWMA of the request round-trip, and a cap that ratchets down (×0.9 per delivery) whenever the smoothed round-trip exceeds RTprop × bbr_delay_gradient_percent/100, then relaxes back up when the queue clears. The ceiling starts unbounded so it never limits an uncongested peer (the existing snap-to-BDP behavior is preserved), and a timeout ratchets it to the dipped cwnd. The effective cwnd is now min(BDP cwnd, delay ceiling), so it settles at the true operating point instead of the hard cap. Runs in ProbeBw only — ProbeRtt's drained round-trips are artificially short and would spuriously relax the ceiling. Traced as bbr_smoothed_elapsed_ms / bbr_delay_cap on block_body_received. Unit tests cover the no-bind (uncongested) and cap-on-inflation cases; the fuzzer fast peers now settle at ~21-29 effective cwnd (the operating point) instead of pinning at the hard cap 64 — which also brings the cwnd below the hard cap, giving the floor bypass its headroom.

The BBR cwnd budgets in-flight work against a unit. Today that unit is request count (one request ~ one slot); for peers serving very large bodies a byte-denominated budget is fairer (a 2 MB body should weigh more than a 10 KB one). Make the unit a config switch without disturbing the controller. Add CwndUnit{Blocks,Bytes} (config bbr_cwnd_unit, default Blocks). The controller is unit-agnostic — it sizes a cwnd from delivery rate x RTprop — so the only thing the unit changes is how outstanding work is counted against that cwnd in available_slots: Blocks subtracts the request count (unchanged), Bytes scales the cwnd's request budget by the advertised per-response byte cap and subtracts reserved body bytes. ProbeRTT, the delay-gradient, and min_cwnd stay in the controller's native unit and are untouched, which keeps the switch a small, localized change (and the existing behavior bit-identical under Blocks). OutstandingBlockRange::reserved_bytes is promoted out of #[cfg(test)]; the byte sum is recomputed on demand (the unit is experimental — a hot path would keep a running counter). Unit test covers the byte budget (big bodies fill the cwnd with fewer requests); fuzz_steady_bytes_unit drives the real reactor to the tip under the byte unit.

…xactly full The block-sync floor arm sized its take by `budget.available()`, so at `available() == 0` exactly the take came back empty, the fill loop broke, and `reserve_request_budget` — the only caller that emits `FundFloorReservation` — was never reached. The rescue shed that frees room for the starved floor never fired and the floor wedged permanently (the precise "budget full, floor starved" case the floor-rescue eviction was built for). Floor the take byte-cap at one byte: `take_in_range_budgeted` always takes its first item regardless of the cap, so the floor block itself is taken even when the budget is exactly full, reaching the funding/shed path. The `.min(response_byte_cap)` still bounds the speculative above-floor tail to the live budget, so a funded floor request never over-commits. Adds a routine-level regression test (plus a `BlockSyncPeerSession::for_test` constructor) that drives `try_fill` with the budget reserved to exactly zero and asserts the floor funding request is emitted with a non-zero need.

…cwnd unit `available_slots_with_bonus` returned remaining cwnd *bytes* under `CwndUnit::Bytes` without checking the per-peer request-count hard cap, so a peer serving tiny bodies could be issued far more in-flight requests than its advertised `max_inflight_requests`. Return no slot once the in-flight request count reaches the hard cap, mirroring the blocks-unit ceiling. Adds a unit test.

`bbr_probe_bw_gain_percent` and `bbr_startup_growth_percent` are declared and validated but not yet consumed (the ProbeBW gain cycle and a separate Startup ramp are deferred). Their doc comments read as active behaviour; note that they are currently inert so operators are not misled.

…r scheduling Congestion control: the per-peer BBR-lite cwnd is byte-denominated by default (CwndUnit::Bytes, bbr_min_cwnd_bytes), budgeting in-flight work by reserved body bytes from advertised size hints rather than a request count. The BDP's RTprop uses the raw round-trip while the size-aware delay gate keeps the size-residual, so a high-bandwidth carrier is no longer pinned at the cwnd floor. Floor scheduling: floor-watchdog avoid plus best-peer bias keep the lowest missing height on the fastest free carrier; request deadlines use a short floor-rescue leash and a body-size/measured-rate-aware above-floor leash; peer parking and the floor-gap trace round out the scheduler. Diagnostics: a block_fill_stop trace row with fill_stop_reason (no_status/cwnd_saturated/no_work/lookahead_cap/retry_avoid/budget/outbound_full/send_error) attributes carrier-idle bubbles, plus byte-cwnd/RTprop/BtlBw fields on block_body_received. Fuzzer: byte-accurate synthetic serve plus mixed_block_sizes, high_bw_fast_peer, one_slow_peer_hol, and commit_stall scenarios driving the real reactor.

Normative spec for the byte-denominated per-peer BBR-lite controller: the control law (MUST/SHOULD), edge cases and security bounds, and the defaults table. Relocated out of the gitignored docs/plans/ tree so it is tracked alongside the controller it specifies.

Under CwndUnit::Bytes (the production default) the ProbeRtt drain check compared a request count (outstanding.len()) against min_cwnd, which in byte mode is the 4 MiB floor -- a count is always far below it, so the 200 ms hold was stamped on the first ProbeRtt delivery instead of after the byte queue actually drained. The base round-trip was then sampled while the link was still contended, so the slow-peer window collapse (a spec MUST) did not reliably fire. Every existing ProbeRtt test pinned CwndUnit::Blocks, which masked this on the default path. record_delivery now reports inflight in the cwnd's own unit (reserved body bytes under Bytes, request count under Blocks) and advance_phase compares against min_cwnd in that same unit. Adds a byte-unit drain regression test (proven to fail on the old behavior) and a compile-time assert that MAX_BS_BLOCKS_PER_REQUEST fits the u128 received-offset bitset, so a future cap bump fails to build instead of silently wedging is_complete.

…order fuzz - byte_budget_concurrent_reservations_never_over_commit: a barrier phase proving exactly CAP reservations admit against a tight ceiling (no over-commit), plus a multi-thread churn phase. - received_tracker_handles_a_full_range_at_the_bitset_boundary: a full 128-block range marks every offset (including the top bit), reports complete, and releases its whole reservation. - fuzzer: assert peak_budget_reserved <= max_inflight_block_bytes in assert_core, and drive the previously-modeled-but-unexercised serve knobs via fuzz_lossy_peer (drop_probability), fuzz_reorder (reorder), and fuzz_multi_peer_tight_budget (tight global ceiling under contention).

CI's stable clippy advanced to 1.96 since #336/#337 merged, newly flagging two pre-existing issues that block this PR: - admission.rs: an `admission_decision` doc comment was orphaned above `request_deadline` (empty_line_after_doc_comments); moved it to its function. - blocksync_fuzz: allow too_many_arguments on the test-harness `spawn_action_driver`.

…ixture #337 (BBR-lite congestion control) added block-sync config fields (bbr_*, floor_bypass_slots, floor_rescue_timeout, no_progress_peer_cooldown) without updating the stored config fixture, so `last_config_is_stored` fails: the generated config matches no stored config. Regenerate the newest fixture to include them. Unrelated to this PR's auth-data-root change (which adds no config fields).

* feat(state): carry and store the per-block ZIP-244 auth-data root Propagate each block's ZIP-244 `auth_data_root` alongside its note-commitment roots, without changing how the committer authenticates blocks. This is the data-model / wire / storage half of the verified-commitment-trees successor-header authentication; a follow-up consumes it in the committer. - Add `auth_data_root` to `BlockCommitmentRoots` (the `tree_aux` header-sync payload) and to the `commitment_roots_by_height` serving-index value (64 -> 96 bytes). `FromDisk` stays backward compatible: pre-release 64-byte rows decode with a zero auth-data root. Consolidated under state DB format version 27.3.0 (no version bump). - Compute and store the auth-data root at commit, thread it through the provisional Zakura header-roots store/read, the `BlockRoots` serve paths, and the `CommitHeaderRange` plumbing. - Bump the Zakura header-sync stream format to version 5. This is a breaking wire change: a v4 and a v5 node cannot exchange header ranges. The auth-data root is carried and stored but not yet consumed by the committer, so consensus behavior is unchanged. * test(state): update commitment_roots_by_height snapshots for the auth-data root The serving-index value grew 64->96 bytes with the per-height auth-data root, so the raw-column-family snapshots gain the 32-byte auth-data root (the all-0xFF ZIP-244 placeholder for these pre-NU5 blocks). * chore(clippy): fix stable clippy 1.96 drift in upstream zakura files CI's stable clippy advanced to 1.96 since #336/#337 merged, newly flagging two pre-existing issues that block this PR: - admission.rs: an `admission_decision` doc comment was orphaned above `request_deadline` (empty_line_after_doc_comments); moved it to its function. - blocksync_fuzz: allow too_many_arguments on the test-harness `spawn_action_driver`. * test(config): store #337 BBR block-sync config fields in the config fixture #337 (BBR-lite congestion control) added block-sync config fields (bbr_*, floor_bypass_slots, floor_rescue_timeout, no_progress_peer_cooldown) without updating the stored config fixture, so `last_config_is_stored` fails: the generated config matches no stored config. Regenerate the newest fixture to include them. Unrelated to this PR's auth-data-root change (which adds no config fields).

p0mvn mentioned this pull request Jun 30, 2026

fix(sync): stop Zakura sync drivers before legacy ChainSync fallback #334

Merged

1 task

p0mvn reviewed Jun 30, 2026

View reviewed changes

p0mvn approved these changes Jun 30, 2026

View reviewed changes

p0mvn marked this pull request as ready for review June 30, 2026 22:41

evan-forbes added 21 commits June 30, 2026 16:42

fix: use block progress for sync peer liveness

ac3f2fd

fix(zakura): adapt BBR-lite port to feature branch

0b34ad5

test(zakura): stabilize BBR-lite fuzzer harness

8e712bc

docs(zakura): remove stale cubic block-sync wording

ef858a2

refactor(zakura): separate BBR block-sync controller

4142137

fix(zakura): harden BBR byte accounting

d84bf7b

docs(zakura): satisfy markdownlint on BBR congestion-control spec

711863b

p0mvn force-pushed the codex/bbr-lite-congestion-control branch from 486cf7d to 711863b Compare June 30, 2026 22:42

p0mvn changed the base branch from codex/block-sync-test-scaffolding to feat/pre-release-main June 30, 2026 22:42

p0mvn merged commit 3d307b0 into feat/pre-release-main Jun 30, 2026
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(zakura): add BBR-lite block-sync congestion control#337

feat(zakura): add BBR-lite block-sync congestion control#337
p0mvn merged 21 commits into
feat/pre-release-mainfrom
codex/bbr-lite-congestion-control

evan-forbes commented Jun 30, 2026

Uh oh!

p0mvn Jun 30, 2026

Uh oh!

p0mvn Jun 30, 2026

Uh oh!

evan-forbes Jun 30, 2026

Uh oh!

evan-forbes Jun 30, 2026

Uh oh!

p0mvn left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

evan-forbes commented Jun 30, 2026

Motivation

Solution

Tests

Issue

AI Disclosure

Uh oh!

p0mvn Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

p0mvn Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

evan-forbes Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

evan-forbes Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

p0mvn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants