test(core): the in-flight budget tests still assumed buffer-only accounting
apple / swift (pull_request) Successful in 1m59s
apple / distribute (pull_request) Skipped
apple / screenshots (pull_request) Skipped
ci / rust-arm64 (pull_request) Successful in 1m41s
ci / web (pull_request) Successful in 1m26s
android / android (pull_request) Successful in 5m19s
ci / bun-nix (pull_request) Successful in 22s
ci / docs-site (pull_request) Successful in 1m25s
windows-client / client (arm64, --no-default-features, aarch64-pc-windows-msvc, C:\t-a64) (pull_request) Successful in 3m9s
windows-client / client (x64, , x86_64-pc-windows-msvc, C:\t) (pull_request) Successful in 6m33s
ci / rust (pull_request) Successful in 20m48s
apple / swift (pull_request) Successful in 1m59s
apple / distribute (pull_request) Skipped
apple / screenshots (pull_request) Skipped
ci / rust-arm64 (pull_request) Successful in 1m41s
ci / web (pull_request) Successful in 1m26s
android / android (pull_request) Successful in 5m19s
ci / bun-nix (pull_request) Successful in 22s
ci / docs-site (pull_request) Successful in 1m25s
windows-client / client (arm64, --no-default-features, aarch64-pc-windows-msvc, C:\t-a64) (pull_request) Successful in 3m9s
windows-client / client (x64, , x86_64-pc-windows-msvc, C:\t) (pull_request) Successful in 6m33s
ci / rust (pull_request) Successful in 20m48s
`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.
This commit is contained in:
@@ -194,7 +194,7 @@ struct ReassemblyWindow {
|
||||
/// partially-arrived frames of ACTUAL size (≪ max); without this cap, [`HARD_LOSS_WINDOW`]
|
||||
/// max-sized declarations from one header-sized packet each could commit gigabytes — an
|
||||
/// amplification the old sparse per-shard allocation didn't have.
|
||||
const IN_FLIGHT_BUF_FACTOR: usize = 4;
|
||||
pub(super) const IN_FLIGHT_BUF_FACTOR: usize = 4;
|
||||
|
||||
/// Recovery-shard buffer pool ceiling (shard-sized buffers): enough for several max-recovery
|
||||
/// blocks in flight, small enough (~720 KB at a 1408-byte shard) to keep after a loss burst.
|
||||
@@ -208,7 +208,12 @@ const RECOVERY_POOL_MAX: usize = 512;
|
||||
/// can mint thousands of distinct-index blocks while its `FrameBuf::buf` stays pinned near zero —
|
||||
/// they must be metered exactly like the buffer, or the firewall meters only half the allocation
|
||||
/// (security-review 2026-08-15 finding 11).
|
||||
fn block_state_bytes(data_shards: usize, recovery_shards: usize) -> usize {
|
||||
///
|
||||
/// `pub(super)` so the budget tests can locate the refusal boundary from the cost model itself
|
||||
/// rather than from a baked-in frame count — [`BlockState`] gaining a field moves that boundary,
|
||||
/// and a test that hard-codes it answers such a change with an arithmetic puzzle instead of the
|
||||
/// question actually worth asking.
|
||||
pub(super) fn block_state_bytes(data_shards: usize, recovery_shards: usize) -> usize {
|
||||
std::mem::size_of::<BlockState>()
|
||||
+ data_shards // have_data: Vec<bool>
|
||||
+ recovery_shards * std::mem::size_of::<Option<Vec<u8>>>() // recovery slot table
|
||||
|
||||
@@ -520,13 +520,19 @@ fn e2e_unrecoverable_loss_ages_out() {
|
||||
/// gigabytes (the eager whole-frame buffer's amplification defense).
|
||||
#[test]
|
||||
fn in_flight_buffer_budget_bounds_allocation() {
|
||||
let lim = limits(); // max_frame_bytes 4096, shards 16 B, ≤8 data shards × ≤4 blocks
|
||||
// limits(): max_frame_bytes 4096, shards 16 B, ≤8 data shards × ≤4 blocks → budget 16384 B.
|
||||
let lim = limits();
|
||||
let budget = IN_FLIGHT_BUF_FACTOR * lim.max_frame_bytes;
|
||||
// What ONE such frame commits: the largest geometry-consistent buffer (4 blocks × 8 shards
|
||||
// × 16 B = 512 B) plus the state of the single block this first shard opens. Both are sized
|
||||
// from header fields, so the firewall meters both — counting only the buffer is precisely
|
||||
// the hole security-review 2026-08-15 #11 closed, and the boundary moved when it did.
|
||||
let per_frame = 512 + block_state_bytes(8, 0);
|
||||
let fits = budget / per_frame;
|
||||
let mut r = Reassembler::new(lim);
|
||||
let coder = coder_for(FecScheme::Gf8);
|
||||
let stats = StatsCounters::default();
|
||||
// Largest geometry-consistent frame: 4 blocks × 8 shards × 16 B = 512 B per buffer.
|
||||
// Budget = 4 × 4096 = 16384 B → exactly 32 such frames fit; the 33rd must be refused.
|
||||
for i in 0..33u32 {
|
||||
for i in 0..=fits as u32 {
|
||||
let mut h = base_header();
|
||||
h.frame_index = i;
|
||||
h.frame_bytes = 512;
|
||||
@@ -539,6 +545,14 @@ fn in_flight_buffer_budget_bounds_allocation() {
|
||||
1,
|
||||
"the frame past the budget is dropped, everything under it accepted"
|
||||
);
|
||||
// The point of the whole exercise: whatever the geometry, the commitment stays under the
|
||||
// ceiling. Asserted on the live figure, so a release site that forgets half the cost (the
|
||||
// 0.23.0 accounting-drift lesson on `in_flight`) fails here and not in the field.
|
||||
assert!(
|
||||
r.in_flight() <= budget,
|
||||
"in-flight commitment {} must never exceed the {budget} B budget",
|
||||
r.in_flight(),
|
||||
);
|
||||
}
|
||||
|
||||
/// A header whose (data_shards, block_count) disagree with the geometry derived from its own
|
||||
@@ -1519,11 +1533,15 @@ fn streamed_open_commits_its_own_extent_and_stays_bounded() {
|
||||
);
|
||||
|
||||
// A SLICE sentinel whose wire base sits just under the ceiling really does commit a
|
||||
// max-sized frame (base 3968 B + K 8 = 256 shards = 4096 B) — four fit the budget, the
|
||||
// fifth must be refused.
|
||||
let mut r = Reassembler::new(limits());
|
||||
// max-sized frame (base 3968 B + K 8 = 256 shards = 4096 B), plus the state of the block it
|
||||
// opens — so the budget takes fewer of these than the buffer alone would suggest, and the
|
||||
// first one past it must be refused.
|
||||
let lim = limits();
|
||||
let budget = IN_FLIGHT_BUF_FACTOR * lim.max_frame_bytes;
|
||||
let fits = budget / (4096 + block_state_bytes(8, 0));
|
||||
let mut r = Reassembler::new(lim);
|
||||
let stats = StatsCounters::default();
|
||||
for fi in 0..5u32 {
|
||||
for fi in 0..=fits as u32 {
|
||||
let mut h = base_header();
|
||||
h.user_flags = USER_FLAG_SLICE_STREAM;
|
||||
h.block_count = 0;
|
||||
@@ -1537,10 +1555,15 @@ fn streamed_open_commits_its_own_extent_and_stays_bounded() {
|
||||
.unwrap()
|
||||
.is_none());
|
||||
}
|
||||
assert!(
|
||||
r.in_flight() <= budget,
|
||||
"in-flight commitment {} must never exceed the {budget} B budget",
|
||||
r.in_flight(),
|
||||
);
|
||||
assert_eq!(
|
||||
stats.snapshot().packets_dropped,
|
||||
1,
|
||||
"the fifth ceiling-claiming open must be refused by the in-flight budget"
|
||||
"the first ceiling-claiming open past the budget must be refused"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user