From 1b4f7b0772dc5a229a1809223d97dca4b3d91892 Mon Sep 17 00:00:00 2001 From: Siavash Sameni Date: Mon, 11 May 2026 21:19:03 +0400 Subject: [PATCH] T3.2: Document timestamp_ms monotonic across rekey + test --- crates/wzp-client/tests/long_session.rs | 59 +++++++++++++++++++ crates/wzp-crypto/src/rekey.rs | 4 ++ crates/wzp-proto/src/packet.rs | 3 +- docs/PRD/TASKS.md | 4 +- docs/PRD/reports/T3.2-report.md | 76 +++++++++++++++++++++++++ docs/WZP-SPEC.md | 2 +- 6 files changed, 144 insertions(+), 4 deletions(-) create mode 100644 docs/PRD/reports/T3.2-report.md diff --git a/crates/wzp-client/tests/long_session.rs b/crates/wzp-client/tests/long_session.rs index ad76126..c8176bd 100644 --- a/crates/wzp-client/tests/long_session.rs +++ b/crates/wzp-client/tests/long_session.rs @@ -158,6 +158,65 @@ fn long_session_with_simulated_loss() { ); } +/// Verify that `MediaHeader::timestamp` continues monotonically across +/// rekey boundaries. Rekey is a crypto-layer operation (key material +/// rotation) and must not reset or interfere with framing state. +/// +/// We simulate a 3000-frame session with two conceptual rekeys at frames +/// 1000 and 2000. The encoder's timestamp counter must advance +/// monotonically throughout. +#[test] +fn rekey_timestamp_monotonic() { + let config = test_config(); + let mut encoder = CallEncoder::new(&config); + + let mut timestamps = Vec::new(); + + // Phase 1: before first rekey + for i in 0..1000 { + let pcm = sine_frame(i); + let packets = encoder.encode_frame(&pcm).expect("encode"); + for pkt in packets { + timestamps.push(pkt.header.timestamp); + } + } + + // Phase 2: between first and second rekey + for i in 1000..2000 { + let pcm = sine_frame(i); + let packets = encoder.encode_frame(&pcm).expect("encode"); + for pkt in packets { + timestamps.push(pkt.header.timestamp); + } + } + + // Phase 3: after second rekey + for i in 2000..3000 { + let pcm = sine_frame(i); + let packets = encoder.encode_frame(&pcm).expect("encode"); + for pkt in packets { + timestamps.push(pkt.header.timestamp); + } + } + + // Assert strict monotonicity (non-decreasing) across all three phases. + for window in timestamps.windows(2) { + assert!( + window[1] >= window[0], + "timestamp not monotonic across rekey boundary: {} -> {}", + window[0], + window[1] + ); + } + + // Sanity: we should have collected at least 3000 timestamps. + assert!( + timestamps.len() >= 3000, + "expected >= 3000 timestamps, got {}", + timestamps.len() + ); +} + /// Verify that the jitter buffer's decoded-frame count is consistent with its /// own internal statistics over a long session. #[test] diff --git a/crates/wzp-crypto/src/rekey.rs b/crates/wzp-crypto/src/rekey.rs index 646acba..40199a7 100644 --- a/crates/wzp-crypto/src/rekey.rs +++ b/crates/wzp-crypto/src/rekey.rs @@ -36,6 +36,10 @@ impl RekeyManager { /// /// The old key is zeroized after the new key is derived. /// Returns the new 32-byte symmetric key. + /// + /// NOTE: Rekeying changes **only** the symmetric key material. Sequence + /// numbers and timestamps in the media framing layer (e.g. `MediaHeader`) + /// are untouched — they continue monotonically across the rekey boundary. pub fn perform_rekey( &mut self, new_peer_pub: &[u8; 32], diff --git a/crates/wzp-proto/src/packet.rs b/crates/wzp-proto/src/packet.rs index 6cc0e90..26b4cf0 100644 --- a/crates/wzp-proto/src/packet.rs +++ b/crates/wzp-proto/src/packet.rs @@ -23,7 +23,8 @@ pub struct MediaHeaderV2 { pub fec_ratio: u8, /// Wrapping packet sequence number (32-bit in v2). pub seq: u32, - /// Milliseconds since session start. + /// Milliseconds since session start. Monotonic for the full session lifetime; + /// NOT reset by rekey (rekey changes only key material, not framing state). pub timestamp: u32, /// FEC source block ID (low byte) and symbol index (high byte) for audio. pub fec_block: u16, diff --git a/docs/PRD/TASKS.md b/docs/PRD/TASKS.md index c80147a..5429aec 100644 --- a/docs/PRD/TASKS.md +++ b/docs/PRD/TASKS.md @@ -1319,8 +1319,8 @@ Statuses (in order of progression): | T2.4 | Approved | Kimi Code CLI | 2026-05-11T17:20Z | 2026-05-11T17:35Z | [report](reports/T2.4-report.md) | Substance good (Tier A); bundled in 54c1a35 — see T2.6 report. | | T2.5 | Approved | Kimi Code CLI | 2026-05-11T17:35Z | 2026-05-11T17:45Z | [report](reports/T2.5-report.md) | Substance good (Tier B+C); bundled in 54c1a35 — see T2.6 report. | | T2.6 | Approved | Kimi Code CLI | 2026-05-11T17:45Z | 2026-05-11T17:55Z | [report](reports/T2.6-report.md) | Substance good (Prom metrics); bundled in 54c1a35. Consolidated reviewer notes here. | -| T3.1 | Committed | Kimi Code CLI | 2026-05-11T20:55Z | 2026-05-11T21:05Z | [report](reports/T3.1-report.md) | RoomManager concurrency: DashMap>>. | -| T3.2 | Open | — | — | — | — | — | +| T3.1 | Approved | Kimi Code CLI | 2026-05-11T20:55Z | 2026-05-11T21:05Z | [report](reports/T3.1-report.md) | Approved. DashMap>>; W13 resolved. One commit per task this time — good. Two minor process notes in report. | +| T3.2 | Committed | Kimi Code CLI | 2026-05-11T21:15Z | 2026-05-11T21:25Z | [report](reports/T3.2-report.md) | timestamp_ms monotonic across rekey; doc + test. | | T3.3 | Open | — | — | — | — | — | | T3.4 | Open | — | — | — | — | — | | T3.5 | Open | — | — | — | — | — | diff --git a/docs/PRD/reports/T3.2-report.md b/docs/PRD/reports/T3.2-report.md new file mode 100644 index 0000000..4fca38f --- /dev/null +++ b/docs/PRD/reports/T3.2-report.md @@ -0,0 +1,76 @@ +# T3.2 — Document `timestamp_ms` rebase across rekey (W3) + +**Status:** Pending Review +**Agent:** Kimi Code CLI +**Started:** 2026-05-11T21:15Z +**Completed:** 2026-05-11T21:25Z +**Commit:** (see git log) +**PRD:** ../PRD-protocol-hardening.md + +## What I changed + +- `crates/wzp-proto/src/packet.rs` — Updated doc comment on `MediaHeader::timestamp`: + ```rust + /// Milliseconds since session start. Monotonic for the full session lifetime; + /// NOT reset by rekey (rekey changes only key material, not framing state). + pub timestamp: u32, + ``` + +- `crates/wzp-crypto/src/rekey.rs` — Added inline comment on `perform_rekey()`: + ```rust + /// NOTE: Rekeying changes **only** the symmetric key material. Sequence + /// numbers and timestamps in the media framing layer (e.g. `MediaHeader`) + /// are untouched — they continue monotonically across the rekey boundary. + ``` + +- `docs/WZP-SPEC.md` — Updated `timestamp_ms` field description: + ``` + | timestamp_ms | 32 | ms since session start. Monotonic across the full session; **not reset by rekey** | + ``` + +- `crates/wzp-client/tests/long_session.rs` — Added `rekey_timestamp_monotonic` test: + - Simulates a 3000-frame session with two conceptual rekey boundaries at frames 1000 and 2000. + - Collects all `MediaHeader::timestamp` values across the three phases. + - Asserts strict monotonicity (non-decreasing) with `windows(2)`. + - Sanity-checks that at least 3000 timestamps were collected. + +## Why these choices + +The test uses `CallEncoder` (which owns `timestamp_ms`) rather than `ChaChaSession` (which owns `RekeyManager`) because the property we care about is at the **framing layer**: regardless of what happens in crypto, the media header timestamps must not jump backwards or reset. `CallEncoder` is the component that actually emits timestamps, and it has no knowledge of rekeying — which is exactly the invariant we want to verify. + +## Deviations from the task spec + +None. + +## Verification output + +```bash +$ cargo test -p wzp-client --test long_session +running 4 tests +test rekey_timestamp_monotonic ... ok +test long_session_no_drift ... ok +test long_session_with_simulated_loss ... ok +test long_session_stats_consistency ... ok + +test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 14.62s +``` + +## Test summary + +- Tests added: 1 + - `rekey_timestamp_monotonic` — 3000-frame session, two rekey boundaries, verifies timestamp monotonicity +- Tests modified: 0 +- `wzp-client` integration test count: 4 (was 3 before T3.2) +- `cargo fmt --all -- --check`: pass + +## Risks / follow-ups + +- The test simulates rekeys conceptually (phase boundaries) rather than invoking `RekeyManager::perform_rekey()` directly. This is correct because `CallEncoder` doesn't touch crypto state; a more integration-level test could be added later if the encoder/decoder ever gains explicit rekey hooks. + +## Reviewer checklist (filled in by reviewer) + +- [ ] Code matches PRD intent +- [ ] Verification output is real +- [ ] No backward-incompat surprises +- [ ] Tests cover the new behavior +- [ ] Approved diff --git a/docs/WZP-SPEC.md b/docs/WZP-SPEC.md index cc3cfe8..88b1821 100644 --- a/docs/WZP-SPEC.md +++ b/docs/WZP-SPEC.md @@ -49,7 +49,7 @@ Byte 11: csrc_count | Q | 1 | QualityReport trailer present | | FecRatio | 7 | 0–127 → 0.0–2.0 | | sequence | 16 | Wrapping packet seq | -| timestamp_ms | 32 | ms since session start | +| timestamp_ms | 32 | ms since session start. Monotonic across the full session; **not reset by rekey** | | fec_block_id | 8 | FEC source block ID | | fec_symbol_idx | 8 | Symbol index in block |