QLV2 - #61
Open
nicoburniske wants to merge 70 commits into
Open
Conversation
nicoburniske
force-pushed
the
ql-v2
branch
2 times, most recently
from
March 3, 2026 19:20
aa7e806 to
1b84f43
Compare
nicoburniske
force-pushed
the
ql-v2
branch
5 times, most recently
from
March 24, 2026 22:23
a0af892 to
6db8123
Compare
nicoburniske
force-pushed
the
ql-v2
branch
3 times, most recently
from
March 26, 2026 18:35
b968b19 to
08625df
Compare
nicoburniske
force-pushed
the
ql-v2
branch
12 times, most recently
from
April 9, 2026 21:14
72f06ba to
1c955e5
Compare
nicoburniske
force-pushed
the
ql-v2
branch
2 times, most recently
from
April 17, 2026 03:08
517194a to
d1b767f
Compare
bijoybangladeshmsd-cyber
approved these changes
Apr 18, 2026
bijoybangladeshmsd-cyber
approved these changes
Apr 18, 2026
bijoybangladeshmsd-cyber
approved these changes
Apr 18, 2026
nicoburniske
force-pushed
the
ql-v2
branch
2 times, most recently
from
April 20, 2026 13:42
c190916 to
44d63cc
Compare
bijoybangladeshmsd-cyber
approved these changes
Apr 21, 2026
+ remove handshake metadata wrapper
badicsalex
reviewed
Jul 22, 2026
| } | ||
| } | ||
|
|
||
| macro_rules! impl_codec { |
There was a problem hiding this comment.
Note: this is kind of a weird way to do it, instead of having 3 macros.
badicsalex
reviewed
Jul 22, 2026
| @@ -0,0 +1,376 @@ | |||
| # QuantumLink V2 | |||
There was a problem hiding this comment.
These two md files should be updated to be consistent with the implementation, or dropped.
|
|
||
| impl Encode for Xx1 { | ||
| fn encoded_len(&self) -> usize { | ||
| HandshakeId::WIRE_SIZE |
There was a problem hiding this comment.
I'm not fond of these special cases, it would be better to self.handshake_id.encoded_len() + ..., and it would compile down to the same thing.
Splitting through `&mut self` means the reader never has to move `B` out of itself, so `ManuallyDrop` and the `Drop` impl go with it. Losing the `Drop` impl ends the reader's borrow of `bytes` in `fsm::receive` at its last use, making the `drop(reader)` there dead code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The plain integer codecs are fixed width, so `decode::<u64>()` and `decode_varint::<u64>()` are one word apart and produce different wire formats, with nothing in a bare `u64` field to say which one applies. The `VarInt` trait is renamed `Primitive` so it doesn't read as a typo for the `Varint` struct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Groundwork for generating these with a macro: `encoded_len` sums the fields instead of restating a constant, discriminant enums all fail with ql_codec::Error, and both byte-container encode bounds are now BufView. Nothing checked `encoded_len` against `encode` before, since encode_record_vec only uses it to size the buffer. The new test covers every type and passes against the pre-change impls, so this is byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Most Encode/Decode impls just walked the fields in order. What stays hand-written reads or writes something that isn't a field: a flags byte (StreamData), a trailing optional (Ik1), a block count (RecordAck), or the rest of the buffer (EncryptedMessage, EncryptedPeerBundle). PairingInvite loses its codec entirely, since nothing ever called it, and array_wrapper is gone because codec_newtype covers it without #[macro_use]. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MIN_WIRE_SIZE was a sum of maximums subtracted from the remaining record capacity, so the name had it backwards, and bounding the length prefixes as usize made it 41 on the host but 31 on the 32-bit target, giving the simulator and the device different framing. Three other WIRE_SIZE constants lost their last caller when `encoded_len` started summing fields; the survivors are the ones no value is in hand for. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A usize varint is capped at ten bytes on a 64-bit host and five on the 32-bit target, so a length a host emits can be rejected outright by the device's decoder. Only per-record lengths and counts move; offsets, sequence numbers and ack spans stay u64, since those accumulate. Encoded bytes are unchanged for anything under 4 GiB, as a varint does not depend on the width it was declared with. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The payload budget reserved room for the header's length prefix but not the header itself, so a stream opened with a header larger than the remaining record space built a frame that would not fit, and pushing it tripped the "builder has capacity" assert. Reserving stops once offset 0 is acked rather than once the header is sent, because until then a loss there is retransmitted and carries the header again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
QLv2 alex's proposed changes
The record seq is the GCM nonce, so no retransmission can reuse one: every transmission carries a fresh seq. A repeated seq is therefore network duplication or a replay, never evidence that our ack was lost, which made the immediate ack it triggered pure reflection. Lost acks already recover through restore_acked_ranges when our own record times out. A replay is also no proof the peer is alive, so it must not refresh the liveness timers: one captured record replayed every 20s was enough to hold a session open against a peer that had gone away. Rejecting replays before decrypt_record saves spending an AES-GCM on them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016YFf732Q7Rq4bf2PJR83LN
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016YFf732Q7Rq4bf2PJR83LN
The session_* fields were a field-for-field mirror of SessionConfig, so every new knob had to be declared in five places and copied across by hand at handshake time. local_parity and the peer's initial stream receive window can't come along: the handshake settles them per session, so a nested config would let a caller set values we then silently overwrite. They move to SessionParams instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016YFf732Q7Rq4bf2PJR83LN
TrackedRecord kept four parallel records of what a record carried, so restoring one meant three ad-hoc blocks plus a loop, and asking whether a record still referenced a stream meant two separate queries. Window updates and pings are ack-eliciting like the other two, so they belong in frames, and should_track is then just a non-empty check. Acking a record no longer requires it to be confirmed sent. An ack that overtook its own write completion was dropped on the floor, and the record went on to be retransmitted at the RTO even though the peer already had it. The Writable emit in handle_stream_window could never fire, since nothing between the two send_capacity calls changes the send buffer. collect_timeouts drains into a Vec so the restore can hang off SessionState. That costs an allocation, but only on the loss path: collect sizes from a lower bound of zero. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016YFf732Q7Rq4bf2PJR83LN
The round-robin cursor was an index into the stream map, so every removal had to shift it, and StreamOps had to carry the index alongside the id in order to reap on drop. Keying the cursor on a stream id costs a lookup per record and leaves reaping as a retain plus a shift_remove. The drop reap goes with it: reset() sets pending_reset and commit_read() sets pending_window, and either one disqualifies a stream, so it never reaped anything. Asserting that in Drop held across the suite. The reaps in handle_stream_data and handle_stream_reset stay. They look redundant next to the sweep in collect_timeouts, but receive sweeps before it processes frames, so a stream that one frame finishes is still around for the next frame in the same record, and a stale frame there fails as a protocol error instead of reaching classify_missing_stream. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZEW7twTbm7F1dAvfESBNi
This reverts commit fe9c65b.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
workspace crates:
ql-wire: wire-format definitionsql-fsm: synchronous protocol state machineql-runtime: async runtime wrapper aroundql-fsmql-rpc: RPC modality layer over streams