Pin the rekey counter arm's boundary against the real trigger

The send-counter arm of the rekey trigger had no test that could fail for
the reason it existed. The integration suites never reached it: they
inject after_messages = 65536 against a ping-driven workload, so every
rekey they observe is time-triggered. The unit test that looked like the
guard reproduced the trigger's OR predicate as a local closure and
asserted against its own copy, so deleting the arm from the real
predicate left it green.

That is the coverage hole that let the data-plane drop across a
message-count-triggered rekey survive: the defect is precisely a rekey of
a young session, which only the counter arm can produce.

Replaces the existing at-threshold check with a boundary test driving
poll_rekey, holding the time arm off so the counter is provably what
decides. The silence-below half is the part that is new, and it earns its
place by measurement: widening the bound to `p.counter >= 1` reds exactly
one test in the library suite, this one. Deletion, narrowing to `>` and
inversion are caught here too, though the time arm's negative tests
already catch a bound removed altogether.

Removes the closure-based test rather than repairing it. Its one
assertion over real code, that a fresh session entry's jitter lies within
the symmetric bound, is already covered over 100 samples elsewhere.

Note this leaves the deployed maintenance line untested on the same arm:
its trigger is inline in an async method that reads the clock directly,
with no pure function to drive, so the same test cannot be written there
without the sans-IO seam this line already has.
This commit is contained in:
Johnathan Corgan
2026-07-25 00:35:16 +00:00
parent 9213cce6c4
commit b29908ba8a
2 changed files with 37 additions and 35 deletions
+8 -30
View File
@@ -1315,36 +1315,14 @@ mod overlapping_epoch_tests {
); );
} }
// Rekey-trigger threshold: elapsed time (with symmetric jitter applied) // The rekey trigger's own threshold arithmetic is tested against the real
// OR the send counter crossing its configured bound. // predicate in src/proto/fmp/tests/core.rs, which drives poll_rekey. A test
#[test] // here previously reproduced that OR predicate as a local closure and
fn rekey_trigger_threshold_arithmetic() { // asserted against its own copy, so it could not fail for the reason it
let after_secs = 100u64; // existed: deleting the counter arm from the trigger left it green. Its one
let after_messages = 1_000u64; // assertion over real code, that a fresh entry's jitter lies within the
// symmetric bound, is covered over 100 samples by
// Jitter is always within [-REKEY_JITTER_SECS, +REKEY_JITTER_SECS]. // test_session_entry_rekey_jitter_in_range in src/node/tests/session.rs.
let (_s, recv) = xk_pair(1, 2);
let entry = entry_with_current(recv);
let jitter = entry.rekey_jitter_secs();
assert!(
jitter.abs() <= REKEY_JITTER_SECS,
"jitter within configured bound"
);
// Effective time threshold applies the symmetric jitter.
let effective_after = after_secs.saturating_add_signed(jitter);
// Reproduce the policy's OR predicate directly.
let triggers =
|elapsed: u64, counter: u64| elapsed >= effective_after || counter >= after_messages;
// Time arm: fires at/after the effective threshold, not below it.
assert!(!triggers(effective_after - 1, 0), "below time threshold");
assert!(triggers(effective_after, 0), "at time threshold");
// Counter arm: fires independently of elapsed time.
assert!(!triggers(0, after_messages - 1), "below counter threshold");
assert!(triggers(0, after_messages), "at counter threshold");
}
// Dampening boundary: within `dampening_ms` of the peer's rekey msg1, local // Dampening boundary: within `dampening_ms` of the peer's rekey msg1, local
// initiation is suppressed; at/after the window it is not. // initiation is suppressed; at/after the window it is not.
+29 -5
View File
@@ -158,13 +158,37 @@ fn rekey_expired_drain_and_trigger_both_fire() {
); );
} }
/// The send-counter arm of the rekey trigger, pinned at its boundary through
/// the real `poll_rekey` rather than a reimplementation of its predicate.
///
/// Firing at the threshold catches the arm being deleted, its comparison
/// narrowed to `>`, or its comparison inverted. The silence-below half is what
/// this test adds over that: it catches the arm being *widened* below its
/// configured bound. Measured rather than assumed — replacing the bound with
/// `p.counter >= 1` reds exactly one test in the whole library suite, this one,
/// while a bound removed altogether is already caught by the time arm's own
/// negative tests.
///
/// The time arm is held off in both halves — `elapsed_secs` stays 0 against a
/// 100 s threshold with no jitter — so the counter is provably what decides.
#[test] #[test]
fn rekey_triggers_on_counter() { fn rekey_counter_arm_fires_at_threshold_and_is_silent_below() {
let fmp = Fmp::new(); let fmp = Fmp::new();
let mut p = peer_snapshot(0x13);
p.counter = 1_000; // == after_messages let mut below = peer_snapshot(0x13);
let actions = fmp.poll_rekey(vec![p], &cfg()); below.counter = 999; // one below after_messages
assert!(matches!(actions[0], ConnAction::InitiateRekey { .. })); assert!(
fmp.poll_rekey(vec![below], &cfg()).is_empty(),
"a send counter one below the threshold must not trigger a rekey"
);
let mut at = peer_snapshot(0x13);
at.counter = 1_000; // == after_messages
let actions = fmp.poll_rekey(vec![at], &cfg());
assert!(
matches!(actions[0], ConnAction::InitiateRekey { .. }),
"a send counter at the threshold must trigger a rekey"
);
} }
#[test] #[test]