The in-flight budget tests still assumed buffer-only accounting #254

Merged

enricobuehler merged 1 commits from worktree-packet-budget-tests into main 2026-08-15 16:41:35 +00:00

Conversation 0 Commits 1 Files Changed 2

+39 -11

enricobuehler commented 2026-08-15 16:15:11 +00:00

Owner

Copy Link

Copy Source

main has been red on ci / rust since 59d8b8a6 landed. This makes it green without touching the security fix.

What is actually wrong

59d8b8a6 (security-review 2026-08-15, finding 11) started metering BlockState against the in-flight budget — correctly. Both its vectors are sized from attacker-declared header fields, so a slice-streamed frame could otherwise mint thousands of distinct-index blocks while the metered buffer stayed pinned near zero. That fix is right and stays.

What it missed is that two tests encode the old cost model as arithmetic in their own comments — "exactly 32 such frames fit; the 33rd must be refused", "four fit the budget, the fifth must be refused". Once block state is metered, fewer frames fit, so a stricter and more correct budget reads as a failure:

in_flight_buffer_budget_bounds_allocation                7 drops, expected 1
streamed_open_commits_its_own_extent_and_stays_bounded   2 drops, expected 1

Both numbers are exactly what the new metering predicts: a 512 B buffer + 104 B of block state takes 26 of the 16384 B budget rather than 32, and a ceiling-claiming 4096 B open takes 3 rather than 4. The tests were measuring the hole, not the firewall.

The fix

Derive the refusal boundary from the cost model instead of baking in a frame count. The tests now ask block_state_bytes() how much one frame commits, push exactly one frame past what fits, and assert the same property as before — everything under the budget accepted, the one past it dropped. A future field on BlockState moves the boundary and the tests follow it, rather than failing with an arithmetic puzzle that invites re-hardcoding whatever number CI last printed.

Each test also now asserts the invariant those counts were only ever a proxy for: in_flight() <= budget. That catches a release site forgetting half the cost — the accounting-drift failure in_flight's own doc comment warns about, which surfaces in the field as a permanent loss storm once the budget wedges.

IN_FLIGHT_BUF_FACTOR and block_state_bytes become pub(super), following the existing LOSS_WINDOW_NS precedent. No production behaviour changes — the diff is two visibility widenings and test bodies.

Verification

• punktfunk-core 414/414 with --all-features (the superset of CI's failing 398-test target).
• cargo fmt --all --check clean; clippy --all-targets --all-features -D warnings clean.
• Mutation-checked: inflating IN_FLIGHT_BUF_FACTOR 1000× makes both tests fail with 0 drops, so neither assertion went vacuous.

`main` has been red on `ci / rust` since `59d8b8a6` landed. This makes it green without touching the security fix. ## What is actually wrong `59d8b8a6` (security-review 2026-08-15, finding 11) started metering `BlockState` against the in-flight budget — correctly. Both its vectors are sized from attacker-declared header fields, so a slice-streamed frame could otherwise mint thousands of distinct-index blocks while the metered buffer stayed pinned near zero. **That fix is right and stays.** What it missed is that two tests encode the *old* cost model as arithmetic in their own comments — *"exactly 32 such frames fit; the 33rd must be refused"*, *"four fit the budget, the fifth must be refused"*. Once block state is metered, fewer frames fit, so a stricter and more correct budget reads as a failure: ``` in_flight_buffer_budget_bounds_allocation 7 drops, expected 1 streamed_open_commits_its_own_extent_and_stays_bounded 2 drops, expected 1 ``` Both numbers are exactly what the new metering predicts: a 512 B buffer + 104 B of block state takes 26 of the 16384 B budget rather than 32, and a ceiling-claiming 4096 B open takes 3 rather than 4. The tests were measuring the hole, not the firewall. ## The fix Derive the refusal boundary from the cost model instead of baking in a frame count. The tests now ask `block_state_bytes()` how much one frame commits, push exactly one frame past what fits, and assert the same property as before — everything under the budget accepted, the one past it dropped. A future field on `BlockState` moves the boundary and the tests follow it, rather than failing with an arithmetic puzzle that invites re-hardcoding whatever number CI last printed. Each test also now asserts the invariant those counts were only ever a proxy for: `in_flight() <= budget`. That catches a release site forgetting half the cost — the accounting-drift failure `in_flight`'s own doc comment warns about, which surfaces in the field as a permanent loss storm once the budget wedges. `IN_FLIGHT_BUF_FACTOR` and `block_state_bytes` become `pub(super)`, following the existing `LOSS_WINDOW_NS` precedent. **No production behaviour changes** — the diff is two visibility widenings and test bodies. ## Verification - `punktfunk-core` **414/414** with `--all-features` (the superset of CI's failing 398-test target). - `cargo fmt --all --check` clean; `clippy --all-targets --all-features -D warnings` clean. - **Mutation-checked**: inflating `IN_FLIGHT_BUF_FACTOR` 1000× makes both tests fail with 0 drops, so neither assertion went vacuous.

enricobuehler added 1 commit 2026-08-15 16:15:14 +00:00

test(core): the in-flight budget tests still assumed buffer-only accounting ... 37d39295aa

`59d8b8a6` (security-review 2026-08-15 finding 11) started metering BlockState against the
in-flight budget, because both its vectors are sized from attacker-declared header fields and a
slice-streamed frame could otherwise mint thousands of unmetered blocks. That fix is right and
stays. What it missed is that two tests encode the OLD cost model as arithmetic in their
comments — "exactly 32 such frames fit; the 33rd must be refused", "four fit the budget, the
fifth must be refused" — so a stricter, more correct budget reads as a failure:

  in_flight_buffer_budget_bounds_allocation          7 drops, expected 1
  streamed_open_commits_its_own_extent_and_stays_bounded   2 drops, expected 1

Both numbers are exactly what the new metering predicts (a 512 B buffer + 104 B of block state
takes 26 of the 16384 B budget, not 32; a ceiling-claiming 4096 B open takes 3, not 4), so the
tests were measuring the hole rather than the firewall. main has been red on `ci / rust` since.

Derive the refusal boundary from the cost model instead of baking in a frame count: the tests now
ask block_state_bytes() how much a frame commits, push exactly one frame past what fits, and
assert the same property as before — everything under the budget accepted, the one past it
dropped. A future field on BlockState moves the boundary and the tests follow it, rather than
failing with an arithmetic puzzle that invites re-hardcoding whatever number CI last printed.

Also assert the invariant the counts were only ever a proxy for: `in_flight() <= budget` at the
end of each. That one catches a release site forgetting half the cost — the accounting-drift
failure `in_flight`'s own doc comment warns about, which surfaces in the field as a permanent
loss storm once the budget wedges.

IN_FLIGHT_BUF_FACTOR and block_state_bytes become pub(super) (the LOSS_WINDOW_NS precedent);
no production behaviour changes.

Verified: punktfunk-core 414/414 with --all-features (the superset of CI's failing target),
`cargo fmt --all --check` clean, clippy -D warnings clean. Mutation-checked: inflating
IN_FLIGHT_BUF_FACTOR 1000x makes both tests fail with 0 drops, so neither went vacuous.

enricobuehler merged commit 4ee095220f into main 2026-08-15 16:41:35 +00:00

enricobuehler deleted branch worktree-packet-budget-tests 2026-08-15 16:41:36 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

FernandoMX enricobuehler (Enrico Bühler) kszlim nannku neoney sassycrown

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unom/punktfunk#254