wz-phone/docs/PROTOCOL-AUDIT.md

WZP Protocol Audit

Protocol-level review of WZP as of 2026-05-11. See WZP-SPEC.md for the spec being audited.

Strengths

• QUIC datagrams instead of raw UDP + SRTP — buys TLS 1.3, PLPMTUD, path migration, and ACK-based loss/RTT estimation. Quinn's PathSnapshot feeding DredTuner is something WebRTC stacks build from scratch.
• Continuous DRED tuning. Mapping RTT / loss / jitter to a continuous Opus DRED lookback window is genuinely better than discrete tiers — most stacks treat DRED as on/off.
• MiniHeader (49/50). At 50 pps that is ~400 B/s saved per stream; meaningful at scale.
• SFU never decodes. Preserves E2E. Most SFUs (LiveKit, Janus) terminate SRTP at the SFU.
• RaptorQ for low-bitrate Codec2 + DRED for Opus. Correct split — DRED is cheaper than FEC at high bitrate; RaptorQ shines when you can afford many small symbols.

Weaknesses

W1. u16 sequence wraps every ~21 minutes at 50 pps

Anti-replay window is 64 packets so wrap is safe for replay. But the jitter buffer's BTreeMap<u16, _> will misorder across the wrap boundary if a packet is delayed more than ~32 k frames. Widen to u32 (or version the field).

W2. fec_block_id: u8 wraps every 256 blocks (~25 s at 5-frame blocks)

A late-joining peer or a slow reconstructor can collide block IDs. Widen to u16 or carry an epoch counter.

W3. timestamp_ms rebase behavior at rekey is unspecified

Rekey every 65,536 packets (~22 min). If timestamp_ms resets, downstream sync glitches. If it does not, document explicitly.

W4. MiniHeader has no seq

Receiver infers absolute seq from the most recent full header + frame count. One missed full header (every 50 frames = 1 s) leaves 49 packets with unknown absolute seq. Acceptable for audio with short jitter buffers — fatal for video where one missed full header can desync an entire GOP. Add seq_delta: u8 to MiniHeader before video lands.

W5. QualityReport placement vs. AEAD

A 4-byte trailer on encrypted media is fine iff it sits inside the AEAD payload. If it is outside, anything stripping the last 4 bytes corrupts decryption and creates a downgrade vector. Verify in packet.rs; if outside, move it inside or AAD-bind it.

W6. Adaptive controller is loss / RTT-only — no bandwidth estimator

Quinn exposes cwnd and bytes_in_flight, but AdaptiveQualityController does not consume them. Under low utilization you cannot detect that you could upgrade to Opus 64 k. For video this is mandatory — without BWE you will either oscillate or never use available capacity.

W7. No NACK / explicit retransmit path

For audio with DRED + FEC this is fine. For video keyframes it is wasteful — an I-frame is 50–200 packets, protecting at 50 % FEC doubles bitrate. A NACK path is cheap and far cheaper than blanket FEC for I-frames.

W8. TrunkFrame batching multiplies AEAD cost

Each inner payload is its own AEAD operation. At 10 entries that is 10× ChaCha calls per recv. Fine on x86 / ARM with AES-NI / NEON; profile on weak Android (Nothing A059 baseline).

W9. CodecID is 4 bits → max 16 codecs; 9 already used

Adding H.264, H.265, AV1, VP9 takes you to 13. Land the widening before deployment — either steal from reserved / csrc_count to make CodecID 8-bit, or split into MediaType:2 / CodecID:6. Doing this post-deployment is painful.

W10. No MediaType field

Audio vs. video vs. data is implicit in CodecID. A 2-bit MediaType lets the SFU apply per-type policy (drop video first under congestion, prioritize audio fan-out) without a codec lookup.

W11. Anti-replay window 64 packets is tight for video

One keyframe burst can be 100+ packets; a single reordered earlier packet stalls the window. Bump to 256 or 1024 for video streams, or maintain a per-stream window.

W12. SignalMessage has no version byte

Bincode + #[serde(default, skip_serializing_if)] covers field additions but not variant removal or semantic change. Lead every variant with version: u8.

W13. RoomManager Mutex per-packet — RESOLVED

Already flagged in ARCHITECTURE.md. At ~1500 pps/sender for video this becomes a real ceiling.

Resolution (T3.1): RoomManager now stores DashMap<String, Arc<RwLock<Room>>> instead of DashMap<String, Room>. The DashMap guard is held only long enough to clone the Arc; all per-room operations (fan-out others(), quality observe_quality(), join/leave) then acquire the room-level std::sync::RwLock. This lets concurrent others() calls share a read lock while writers hold the write lock, eliminating the per-packet DashMap contention that was the original concern.

W14. No receiver → sender congestion feedback beyond inline QualityReport

For video you need REMB-style or transport-CC-style explicit BWE feedback at ~50 ms cadence, independent of media packets.

Priorities

Priority	Issue	Why
P0	W9 (CodecID width), W10 (MediaType), W4 (MiniHeader seq_delta)	Wire-format changes — must land before video, painful to change post-deploy
P0	W1 (seq u16 → u32)	Same window; audio benefits too
P1	W6 (BWE), W14 (transport feedback)	Blocking for usable video; improves audio adaptation
P1	W5 (QualityReport in AEAD)	Security correctness
P2	W2 (fec_block_id width), W11 (anti-replay window), W12 (signal version byte)	Long-tail correctness
P2	W7 (NACK path), W13 (RoomManager lock)	Video performance, not correctness
P3	W3 (timestamp rebase doc), W8 (AEAD profiling)	Documentation / measurement

Resolution status (2026-05-11)

The v2 wire format specified in ROAD-TO-VIDEO.md Phase V1 addresses:

Issue	Resolved by
W1 (seq u16 → u32)	sequence: u32 in MediaHeader v2
W4 (MiniHeader seq)	seq_delta: u8 added; MiniHeader v2 is 5 B
W9 (CodecID width)	Widened to 8-bit (room for 256)
W10 (MediaType)	Explicit media_type: u8 byte

W6 / W14 (BWE + TransportFeedback) addressed in Phase V2. W7 (NACK) addressed in Phase V2 / V4. Others remain open.

Known pre-existing clippy debt (as of T1.5.2)

Measured at commit c93d302 on experimental-ui (2026-05-11).

cargo clippy --workspace --all-targets -- -D warnings fails in two crates with pre-existing errors (verified against HEAD~1). These are not introduced by any Wave 1 task; they should be cleaned up in a dedicated hygiene sprint or accepted as known debt.

wzp-codec — 9 errors

Category	Count	Lint	Files
Manual saturating sub	1	clippy::implicit_saturating_sub	aec.rs:117
Needless range loop	2	clippy::needless_range_loop	aec.rs:164, resample.rs:51
Manual div_ceil	2	clippy::manual_div_ceil	codec2_dec.rs:48, codec2_enc.rs:48
Manual clamp	2	clippy::manual_clamp	denoise.rs:59, opus_enc.rs:250
Manual ASCII case-cmp	1	clippy::manual_ascii_check	opus_enc.rs:99
Same-item push in loop	1	clippy::same_item_push	resample.rs:184

warzone-protocol (submodule deps/featherchat) — 3 errors

Category	Count	Lint	Files
clone on Copy type	1	clippy::clone_on_copy	ratchet.rs:202
Missing Default impl	2	clippy::new_without_default	types.rs:59, types.rs:69

Policy: New tasks must not add new clippy errors in crates they touch. The 12 errors above are grandfathered; a follow-up cleanup task should be scheduled to fix them (especially the wzp-codec ones, which are straightforward mechanical replacements).