Skip to content

feat(lite): moq-lite-07 switches to 64-bit leading-ones varints - #4455

Merged
kixelated merged 9 commits into
mainfrom
quest/m1/rs2ts/lite-leading-ones
Sep 30, 2026
Merged

kixelated merged 9 commits into
mainfrom
quest/m1/rs2ts/lite-leading-ones

Conversation

@kixelated

@kixelated kixelated commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Part of the Generated @moq/net questline (quest/m1/rs2ts/README.md). The quest (quest/m1/rs2ts/lite-leading-ones) was created and completed here, so no quest file lands.

Problem

moq-lite still uses QUIC's two-bit varint prefix while moq-transport draft-17+ counts leading ones. That leaves two varint codecs to maintain, translate (rs2ts), and fuzz, and QUIC's form spends a second byte on 64-127 and a fourth on 16,384 to 2^21. lite-07 is unpublished (moq-lite-07-wip, opt-in only), so it can still change its wire.

Approach

  • Rust: impl Encode/Decode<lite::Version> for VarInt picks the codec: lite-01..06 QUIC, lite-07+ leading-ones (all nine lengths valid, 7-byte form included). Every lite varint goes through that impl, including stream types, message lengths, frame headers, zigzag deltas, datagram bodies, and SETUP parameter values. The parameter values were hardcoded to Lite05 before and now take the session's version.
  • Nothing needs a fixed codec before negotiation. lite-07 is only reachable through its ALPN, so the version is known before the first byte of any stream. The legacy SETUP exchange only negotiates lite-01/02 and draft-14, all QUIC.
  • Range is per version: lite-07 carries the full 64-bit leading-ones range, in the draft and in JS. lite-01..06 keep QUIC's 2^62-1 ceiling and fail loud on encode above it, so a relay forwarding a huge lite-07 value to a lite-06 peer errors rather than truncating.
  • Known limitation (Rust on main): VarInt is still 62-bit, so Rust decodes a lite-07 value above 2^62-1 as BoundsExceeded, never a truncation. The VarInt codec quest on dev lifts this. The draft does not inherit the limitation.
  • Route costs saturate at 2^62-1 on every version, lite-07 included, in Rust (already true of summation; decode now clamps too) and JS (encode and decode). A cost therefore always forwards to a QUIC-varint version. This is implementation policy; the draft's cost wording stays version-neutral, and the VarInt codec quest revisits it.
  • The draft-17 rejection of the 7-byte 1111110x form moved from inside decode_leading_ones into the IETF dispatch. Behavior is unchanged.
  • js/net: Reader/Writer/Cursor carry a StreamVersion (IETF or lite) instead of only an IETF version, and the existing isLeadingOnes dispatch lists lite-01..06 next to drafts 14-16. asIetf() keeps lite streams on the moq-lite stream-code registry.
  • The version is now required on Reader, Writer, Cursor, Readers, Stream, and Stream.open/accept and Writer.open/tryOpen. A stream can no longer fall back to QUIC varints by leaving the version out.
    • Every construction site passes one: the lite session, Message.encode/decode, the datagram body, SETUP parameters, the IETF adapter, and tests and benches.
    • The legacy SETUP handshake streams (lite-01/02 and draft-14..16) are opened at the handshake's IETF version. Lite.Connection retargets the session stream to the negotiated lite version, as Rust's with_version does.
  • Built on the BigInt-free codec from feat(net): BigInt-free varint codec, internal U64, and checked Varint.decode #4454: Cursor and Writer dispatch on the stream's version, so lite-07 reuses the draft-17+ leading-ones fast path and 64-bit range.
  • Draft: new "Variable-Length Integers" section, RFC9000 reference, and a lite-07 changelog bullet. doc/concept/moq-lite.md gets one sentence.
  • Interop: new lite_varint_interop, which runs next to varint_interop at the start of every just test interop. JS decodes Rust's lite-06 and lite-07 encodings of every length boundary up to 2^62-1, a SETUP with a 62-bit Hop ID, a datagram, and a GROUP with frames. It re-encodes them, and Rust checks the bytes match exactly.
    • Past 2^62-1 the test checks the per-version range. On lite-07, JS writes and reads back 2^62 and 2^64-1, Rust checks those bytes against the spec, and Rust refuses them with a decode error (the known limitation). On lite-06, JS refuses to write them.
    • The pinned Rust/JS announce goldens and bare-fin now run on leading-ones too.

Wire size

Per value:

Value QUIC (lite-01..06) Leading-ones (lite-07)
0 - 63 1 1
64 - 127 2 1
128 - 16,383 2 2
16,384 - 2^21-1 4 3
2^21 - 2^28-1 4 4
2^28 - 2^30-1 4 5
2^30 - 2^42-1 8 5-6
2^42 - 2^56-1 8 8 (the 7-byte form is accepted, never emitted)
2^56 - 2^62-1 8 9
2^62 - 2^64-1 not encodable 9

Typical objects (benches/varint.rs, identical in js/net/bench/lite-varint.ts, which runs nightly):

Object lite-06 bytes lite-07 bytes Δ
30 fps video group, 60 FRAME headers (µs deltas, 8-60 KB frames) 369 304 -17.6%
50 Hz audio group, 50 FRAME headers (20 ms, 160 B) 297 248 -16.5%
GROUP header 5 5 0
SUBSCRIBE 26 26 0
Datagram header 11 8 -27%
SETUP (probe, 53-bit hop) 15 15 0

That is about 1 byte per frame at a microsecond timescale, from the 33,333 µs delta (zigzag 66,666). At a millisecond timescale the delta (zigzag 66) also drops from 2 bytes to 1. Relative to media payload this is noise: 1 byte on an 8 KB frame is 0.01%.

Benchmarks

Median ns on a loaded 32-core host (load average ~20), so treat small deltas as noise.

Rust, Criterion (cargo bench -p moq-net --features fuzz --bench varint):

Object encode 06 → 07 decode 06 → 07
Video, 60 frames 431 → 479 299 → 354
Audio, 50 frames 355 → 437 246 → 260
GROUP 22 → 42 21 → 32
SUBSCRIBE 347 → 198 185 → 177
Datagram 38 → 31 32 → 21
SETUP 446 → 502 275 → 293

That is roughly +1 ns per frame header. Leading-ones branches on the prefix length where QUIC shifts. The dev codec rewrite (VarInt codec) should re-run this bench.

JS (bun js/net/bench/lite-varint.ts, after merging #4454), ns per object through the real Writer/Reader:

Object encode 06 → 07 decode 06 → 07
Video, 60 frames 42,800 → 45,800 6,200 → 6,500
Audio, 50 frames 36,300 → 36,000 5,200 → 5,600
GROUP / SUBSCRIBE / datagram / SETUP within noise within noise

Impact

  • Wire: moq-lite-07-wip encodes every varint with leading-ones, with the full 64-bit range. lite-01..06 and every IETF draft are unchanged, byte for byte.
  • Rust public API: none. lite is private, and moq_net::VarInt is unchanged. The hidden, feature-gated fuzz module gains LiteSample for the bench.
  • @moq/net public API: none. Only internal modules changed:
    • stream.ts: StreamVersion, asIetf and encodeVarint, plus a version that is now required and widened on every stream constructor and open helper.
    • bench/lite-varint.ts: new, wired into nightly.
    • lite/datagram.ts: encode/decode take a version.
    • IETF Frame.encode/FetchFrame.encode: the version argument is required.
  • Quest: the VarInt codec quest now widens VarInt to 64 bits instead of keeping 62, and revisits the cost ceiling.
  • Bug fix: JS decoded SETUP varint parameters through Number, so a peer's Hop ID above 2^53 (Rust's Hop::new allows 62 bits) lost precision on every lite version. The parameters now decode through Reader.u62. The interop test covers it with a 2^62-1 hop.
  • Cost saturation: a lite-07 route cost above 2^62-1 reads as 2^62-1, and JS sends a locally set larger cost as 2^62-1 instead of throwing on lite-06. No wire-format change.

Tradeoffs

Pros:

  • One varint codec for lite-07+ and moq-transport draft-17+, so rs2ts, the fuzzers, and JS maintain and optimize one path.
  • About 1 byte per frame header, 3 per datagram, and 1 per message length between 64 and 127.
  • Free to do now: lite-07 is unpublished and opt-in.
  • The full 64-bit range matches moq-transport, so a moqt bridge needs no range check on lite-07.

Cons:

  • Slightly more CPU per varint in Rust (about 1 ns per frame header). JS is at parity.
  • Values from 2^28 to 2^30 and from 2^56 to 2^62 grow by one byte. Explicit 62-bit Hop IDs grow from 8 to 9 bytes; Hop::random stays under 2^53, so 8 bytes.
  • A lite-07 value above 2^62-1 cannot be forwarded to a lite-06 peer; the relay errors on encode, by design.
  • Until dev widens VarInt, Rust refuses lite-07 values that JS accepts.
  • The byte savings are negligible against media payload. The real win is the single codec, not bandwidth.

Alternatives:

  • Wait for lite-08: bundles the change with a later version, but lite-07 is still WIP, and a version bump just for varints would cost more.
  • Keep QUIC varints on lite forever: no churn, but two codecs forever and no alignment with moqt.
  • Cap lite-07 at 2^62-1: keeps every value forwardable to lite-06 and matches Rust today, but bakes a temporary implementation limit into the spec and diverges from moqt.
  • Emit the 7-byte form: saves 1 byte for 2^42 to 2^49 only, and diverges from our IETF encoder.

Recommendation: land it on lite-07 now, with the full 64-bit range.

Follow-ups

  • VarInt codec on dev: this PR updates the quest to widen VarInt to 64 bits (it said to keep 62). Widening VarInt to 64 bits lifts the Rust lite-07 decode limitation. Delete the x > Self::MAX arm in the lite dispatch, and flip lite_varint_interop's expectation from BoundsExceeded to a round trip. Expect a small conflict in coding/varint.rs, since decode_leading_ones/encode_leading_ones lost their version argument. Re-run benches/varint.rs.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 3 commits September 28, 2026 19:36
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lite-07-wip encodes every varint (stream types, lengths, frame headers,
datagrams, SETUP parameter values) with moq-transport's leading-ones form,
capped at 2^62-1 so values still forward to older lite peers. Lite-01..06
stay on QUIC varints byte for byte.

Adds the lite-varint Rust/JS interop test, a Criterion bench, and a JS bench.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ersion

The varint range is now per version: lite-07 carries the full 64-bit
leading-ones range in the draft and in JS, while lite-01..06 keep QUIC's
62-bit ceiling. Rust's VarInt is still 62-bit on main, so a larger lite-07
value is a loud decode error until the dev VarInt codec widens it.

Reader, Writer, Cursor, Readers, and the open/accept helpers now require a
version, so a stream can no longer fall back to QUIC varints by omission.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title feat(lite): moq-lite-07 switches to leading-ones varints feat(lite): moq-lite-07 switches to 64-bit leading-ones varints Sep 29, 2026
A lite-07 cost can exceed QUIC's range, which would make it unforwardable to
a lite-06 peer. Rust and JS now saturate costs at 2^62-1 on encode and decode
regardless of version; the draft stays version-neutral.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T14:16:01.583368Z a477293 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Resolve against the BigInt-free varint codec (#4454): stream.ts takes main's
Cursor and Writer, whose leading-ones fast path replaces this branch's, and
keeps the required StreamVersion. lite_varint_interop now runs with
varint_interop under just test interop, so the separate recipe and CI step go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: a5f76f5

No new actionable correctness findings in the 47-file diff and surrounding codec, framing, and negotiation paths.

Direction: Reusing the leading-ones codec while lite-07 remains opt-in is reasonable. Requiring the stream version makes codec selection explicit; the legacy QUIC forms and protocol-specific error registries remain separated.

Range caveat: Rust's documented rejection above 2^62-1 also happens before the new route-cost clamp runs (rs/moq-net/src/coding/varint.rs:478-481; rs/moq-net/src/lite/announce.rs:206-211). Full-range incoming cost saturation is therefore currently JS-only, despite the broader wording in the PR description.

Verification: 146,502 focused assertions against extracted current-head JS codec source passed under Node, covering boundaries, seeded values, every leading-ones prefix, fragmentation, truncation, and draft-17 rejection. This was not a full Bun/Rust run. Current-head CI passed, including the new lite-varint step and full interop matrix. GitHub currently reports merge conflicts; rerun checks and review the rebased head.

(Written by OpenAI)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5f76f5098

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +186 to +187
await w.u62(saturate(cost?.warm ?? 0n));
await w.u62(saturate(cost?.cold ?? 0n));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve legal 64-bit lite-07 route costs

When a lite-07 caller supplies or receives a legal cost in the range 2^62 through 2^64-1, saturate silently rewrites it to 2^62-1. The updated draft states that lite-07 varints carry all 64 bits and that a relay unable to encode a value for an older peer must not truncate it (drafts/draft-lcurley-moq-lite.md:683-687), so this can alter route ordering and forward a materially different advertisement. Preserve the value on lite-07 and refuse or omit the route when targeting lite-06 instead of clamping it.

AGENTS.md reference: AGENTS.md:L17-L17

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partly agree. The draft and implementation disagreed: the draft capped costs at "the largest value a variable-length integer can carry", which lite-07 widened to 2^64-1. I fixed that on the spec side instead of preserving 64-bit costs. 47d345f makes 2^62-1 the Route Cost ceiling on every version and says a larger received cost reads as 2^62-1. Saturation is already how costs handle overflow, and it keeps every route forwardable to lite-06. It also avoids refusing or dropping routes over values that only matter as "ranks last". Rust cannot hold 64-bit values until VarInt widens on dev anyway.

(Written by Claude Opus 5.5)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: the maintainer kept the draft's version-neutral cost ceiling, so 570ee7c reverts the draft edit. Costs still saturate at 2^62-1 in Rust and JS for now. Revisiting that is recorded in the VarInt codec quest, which now widens VarInt to 64 bits.

(Written by Claude Opus 5.5)

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 14c7ef88-5f96-435b-bdf0-468c3dcc21de

📥 Commits

Reviewing files that changed from the base of the PR and between 2f4b785 and a5f76f5.

📒 Files selected for processing (47)
  • .github/workflows/interop.yml
  • doc/concept/moq-lite.md
  • drafts/draft-lcurley-moq-lite.md
  • js/net/bench/frames.ts
  • js/net/bench/reader.ts
  • js/net/bench/varint.ts
  • js/net/src/connection/accept.ts
  • js/net/src/connection/connect.ts
  • js/net/src/ietf/adapter.ts
  • js/net/src/ietf/ietf.test.ts
  • js/net/src/ietf/object.ts
  • js/net/src/lite/announce.test.ts
  • js/net/src/lite/announce.ts
  • js/net/src/lite/connection.ts
  • js/net/src/lite/datagram.test.ts
  • js/net/src/lite/datagram.ts
  • js/net/src/lite/fetch.test.ts
  • js/net/src/lite/goaway.test.ts
  • js/net/src/lite/group.test.ts
  • js/net/src/lite/message.ts
  • js/net/src/lite/priority.test.ts
  • js/net/src/lite/probe.test.ts
  • js/net/src/lite/publisher.test.ts
  • js/net/src/lite/publisher.ts
  • js/net/src/lite/setup.test.ts
  • js/net/src/lite/setup.ts
  • js/net/src/lite/subscribe.test.ts
  • js/net/src/lite/subscriber.test.ts
  • js/net/src/lite/subscriber.ts
  • js/net/src/lite/tail.test.ts
  • js/net/src/lite/track.test.ts
  • js/net/src/stream.test.ts
  • js/net/src/stream.ts
  • rs/moq-net/Cargo.toml
  • rs/moq-net/benches/varint.rs
  • rs/moq-net/src/coding/varint.rs
  • rs/moq-net/src/fuzz.rs
  • rs/moq-net/src/lite/announce.rs
  • rs/moq-net/src/lite/compress.rs
  • rs/moq-net/src/lite/parameters.rs
  • rs/moq-net/src/lite/setup.rs
  • rs/moq-net/src/model/origin.rs
  • rs/moq-net/src/test_interop.rs
  • test/interop/README.md
  • test/interop/bare-fin.ts
  • test/interop/lite-varint.ts
  • test/justfile

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.


Walkthrough

Lite-07 now uses leading-ones varints, while Lite-01 through Lite-06 retain QUIC varints. JavaScript and Rust pass protocol versions through stream and message encoding and decoding. Route costs are capped at 2^62−1. The changes add Rust–JavaScript interoperability tests, workflow integration, documentation, and encoding benchmarks.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to a5f76

Lite-07 adds opt-in leading-ones varints while earlier versions retain their existing encoding. No merge-blocking issue is established. Merge after normal checks, including the new interoperability test.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to a5f76

The new codec preserves important bounds and failure checks, but JavaScript legacy negotiation can activate Lite-07 without its intended explicit protocol opt-in. This exposes the experimental parser beyond the stated rollout boundary. No privilege escalation or cross-tenant compromise was established.

Retained concerns

  • Medium · security · observed: JavaScript legacy SETUP can activate the new Lite-07 codec without negotiating its experimental ALPN. A client can offer Lite-07 to automatic server negotiation; conversely, a server can select Lite-07 even though the default legacy client offered only Lite-01, Lite-02, and draft-14. The permissive selection pattern is inherited, but the changed codec expands its parser exposure and defeats the stated ALPN-only rollout containment.
Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is to JavaScript connections using legacy negotiation and their subsequent streams and datagrams. A reachable peer can trigger selection without privileged local configuration in automatic negotiation. Deployment-wide tenant, service, and environment counts were not supplied; no credential or infrastructure authority expansion was established.

Security Findings and Attack Paths

  • observed — A legacy client can place Lite-07 in ClientSetup.versions, causing automatic acceptance to construct a Lite-07 connection. A legacy server can also return Lite-07 to a client that did not offer it. Connection initialization then selects the new codec. These are source-supported activation paths, not demonstrations of memory corruption, privilege escalation, or cross-tenant access.

Trust Boundaries and Controls

  • observed — Legacy SETUP itself uses a fixed draft-14 codec before selection. Both existing session halves are retargeted synchronously before subsequent connection processing. These controls avoid an indeterminate initial byte codec, but recognized-version membership does not enforce the experimental opt-in boundary.

Resilience and Maintainability Implications

  • observed — JavaScript number decoding rejects values above the safe-integer ceiling, and incremental filling rejects requests above MAX_READ_SIZE. Reader buffer consumption commits only after successful decoding. Inspected fetch and group consumers close their producers and abort or stop streams on decode failure; group tail ownership is released in finally.

Hardening Proposals

  • proposed — Make experimental version eligibility explicit in legacy negotiation, separate from recognized-version membership, and require a server-selected version to belong to the client's actual offer. This would make the stated Lite-07 activation boundary enforceable.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 59.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 110 functions across 41 files. (6 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely summarizes the primary change: moq-lite-07 adopts 64-bit leading-ones varints.
Description check ✅ Passed The description is directly related to the changeset and explains the codec change, version behavior, implementation impact, interop coverage, benchmarks, and known Rust limitation.
Full details: Docstring Coverage

Explanation

Docstring coverage is 59.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 110 functions across 41 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch quest/m1/rs2ts/lite-leading-ones
✨ Simplify code
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

kixelated and others added 2 commits September 30, 2026 06:54
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The implementation already caps costs there so they forward to lite-06; the
draft still tied the ceiling to the varint width, which lite-07 widened.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Review (head 47d345fa43c1)

Clean switch of moq-lite-07 to leading-ones varints with version-dispatched Rust/JS codecs, required StreamVersion, SETUP param codec fix (Hop IDs past 2^53), goldens/interop/benches, and main/#4454 merge. Draft now explicitly carves cost saturation out of the general “MUST NOT truncate” rule.

Blocking

  1. Cost::decode does not implement the draft MUST this PR adds (rs/moq-net/src/lite/announce.rs ~199–210; draft Route Cost text on head 47d345fa)

    Draft (this PR): “A receiver MUST read a larger Route Cost, which moq-lite-07 can carry, as 2^62-1”.

    Rust path today:

    warm: u64::decode(buf, version)?,  // → VarInt::decode(lite-07)
    cold: u64::decode(buf, version)?,
    Ok(cost.clamped())

    On lite-07, VarInt::decode turns any value > 2^62-1 into BoundsExceeded before clamped() runs (coding/varint.rs lite dispatch). So clamped() is a no-op for wire costs, and a legal 9-byte cost (0xFF… / 2^64-1) fails the announce decode instead of saturating.

    Failure scenario: lite-07 peer (or test bytes, as in js/net/src/lite/announce.test.ts “route costs saturate…”) sends warm/cold above 2^62-1. JS saturates (announce.ts saturate). Rust errors the message/stream. Spec + JS say saturate; Rust does not.

    Suggested fix: decode the two cost halves with a path that accepts the full leading-ones u64 on lite-07+ (do not reuse the general VarInt BoundsExceeded gate), then .min(MAX_COST) / clamped(). Keep loud BoundsExceeded for every other lite-07 field until the VarInt-widening quest. Add a Rust unit test mirroring the JS wire case (2^64-1 → ceiling on Lite07). Optionally encode via self.clamped() so with_cost(u64::MAX) matches JS encode saturation.

Non-blocking

  1. CI not green yet on 47d345fa — prior run on ef5e840 was cancelled by this docs push; Android has passed on the new head, Check/Test/Interop/etc. still queued/in progress. Worth confirming Interop (lite_varint_interop) before merge.

  2. OpenAI P2 on a5f76f5 (silent rewrite of 64-bit costs) — largely addressed in draft by the 47d345fa wording (saturation ≠ forbidden truncation). Remaining gap is the Rust decode bug above, not the policy itself.

  3. Known / already documented: general lite-07 values above 2^62-1 still BoundsExceeded in Rust until quest/m1/rs2ts/varint-codec — fine for non-cost fields; interop asserts that. Don’t widen that gate to “fix” costs.

Verdict

ITERATE — land the Rust cost decode (and a regression test) so receivers match the draft MUST and JS; then MERGE once Interop is green on this head.

Reviewed head: 47d345fa43c10bd06ada08b7154bc08fd82bced3

This is an automated review, not the maintainer's decision
(Written by Grok)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 47d345f

No new actionable correctness findings in the meaningful delta since a5f76f5: the main/#4454 codec merge, version adaptations, interop integration, and route-cost contract clarification.

Direction: The shared leading-ones fast path and required stream versions remain a sound simplification. The explicit 2^62-1 cost ceiling resolves the draft/JS policy ambiguity.

Existing caveat, unchanged: Rust still rejects larger costs before Cost::decode can clamp them (rs/moq-net/src/coding/varint.rs:478-481; rs/moq-net/src/lite/announce.rs:206-211). The new receiver MUST in drafts/draft-lcurley-moq-lite.md:937 is therefore not yet implemented by Rust; this is the same range gap noted in the previous review, not a new regression.

Verification: 189,390 focused assertions passed against extracted current-head JS source under Node across 16 versions, covering boundary/seeded values, every prefix, fragmentation, truncation, draft-17 rejection, and message framing. Transport-error conversion was stubbed; full Bun/Rust suites were not run locally. GitHub now reports the head mergeable, but current-head CI, including Interop, is still queued/running.

(Written by OpenAI)

kixelated and others added 2 commits September 30, 2026 07:06
The draft keeps its version-neutral cost ceiling.

This reverts commit 47d345f.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lite-07 and moq-transport draft-17+ carry 64-bit leading-ones varints, which
Rust refuses above 2^62-1 until VarInt widens.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: a477293

No new actionable findings in the two-commit docs/plan delta since 47d345f; no runtime code or tests changed.

Direction: Widening VarInt to 64 bits while preserving QUIC's encode limit is the right follow-through (quest/m1/rs2ts/varint-codec.md:20-25). The draft now restores the wire-width cost ceiling and removes the receiver-clamping MUST (drafts/draft-lcurley-moq-lite.md:933-938), so my previous review's statement that the draft/implementation policy was aligned is superseded.

Existing gap, explicitly deferred: JS still caps costs at 2^62-1, and Rust still rejects larger incoming values before its clamp. The quest now records the cost-ceiling mismatch; it does not resolve it. This is the previously discussed limitation, not a newly introduced runtime regression. The widening work should verify full-range costs and forwarding to lite-06 together.

Verification: Compared both commits and inspected the restored draft, complete quest, and current codec/cost paths. No tests were rerun for this docs-only delta. GitHub reports the head mergeable; current-head CI, including Interop, is still queued/running.

(Written by OpenAI)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Re-review after push (head a4772938c217)

Prior Grok review was on head 47d345fa43c1 (ITERATE). This push is two commits: revert of the cost-ceiling docs (570ee7c9467e) and a quest note that VarInt should widen to 64 bits (a4772938c217). No Rust/JS codec code changed.

What changed

  1. Draft (drafts/draft-lcurley-moq-lite.md): drops the explicit “receiver MUST read a larger Route Cost as 2^62-1” and the carve-out from varint “MUST NOT truncate.” Saturation is back to version-neutral: “Saturation MUST cap each sum at the largest value a variable-length integer can carry” (~L936). Changelog no longer mentions Route Costs saturating at 2^62-1.
  2. Quest (quest/m1/rs2ts/varint-codec.md): guidance only — widen VarInt to 64 bits on lite-07 / draft-17+, drop the lite BoundsExceeded arm, flip lite_varint_interop to a round trip, and revisit MAX_COST vs the draft’s version-local ceiling. No implementation yet.

Fixed / still open from prior review

Prior finding Status
Draft MUST: receiver must read wire costs >2^62-1 as 2^62-1 Fixed / removed by the docs revert — that MUST is gone
Rust Cost::decode BoundsExceeded before clamped(); JS saturates Still open (interop / intent bug; see below)
General lite-07 values >2^62-1 refuse in Rust until VarInt widens Still open, still documented; this push only updates the quest
CI incomplete on prior head Still largely pending on this head (see below)

Findings

Blocking

  1. Rust still cannot decode a legal lite-07 Route Cost above 2^62-1; JS can and saturates (rs/moq-net/src/lite/announce.rs ~199–210; rs/moq-net/src/coding/varint.rs ~469–484; js/net/src/lite/announce.ts MAX_COST / saturate ~178–193)

    Path unchanged:

    warm: u64::decode(buf, version)?,  // → VarInt::decode(Lite07) → BoundsExceeded if > MAX
    cold: u64::decode(buf, version)?,
    Ok(cost.clamped())

    VarInt::MAX is still (1 << 62) - 1. Lite-07 decode still does decode_leading_ones then x if x > Self::MAX => Err(BoundsExceeded). So clamped() never sees a wire value in (2^62-1, 2^64-1].

    JS decodeRouteCost reads via u62 (full leading-ones range on lite-07) then saturate to 2^62-1, and announce.test.ts asserts wire 2^64-1 → ceiling. Rust comments in announce.rs still claim the same policy.

    Failure scenario: a lite-07 peer (or the JS golden wire in that test) sends warm/cold as 0xFF… / 2^64-1. JS accepts and ranks at the ceiling; Rust fails the announce decode / stream. Spec no longer requires the 2^62-1 read-as-ceiling MUST, but the two implementations in this PR still disagree, and Rust’s own comments say saturate.

    Suggested fix (unchanged intent): decode the two cost halves with a lite-07 path that accepts the full leading-ones u64 (do not reuse the general VarInt BoundsExceeded gate), then .min(MAX_COST) / clamped(). Keep loud BoundsExceeded for every other lite-07 field until the VarInt quest. Add a Rust unit test mirroring the JS wire case. Optionally encode via self.clamped() so a locally built Cost { warm: u64::MAX, … } matches JS encode saturation.

Non-blocking

  1. Draft vs MAX_COST tension after the revert. Draft saturation ceiling is now the version’s largest varint (so 2^64-1 on lite-07). Code + JS still hard-cap costs at 2^62-1 on every version for forwardability to QUIC-varint peers, and the varint section’s “MUST NOT truncate” carve-out for costs is gone. The quest correctly flags “revisit MAX_COST.” Worth resolving in draft or code before calling the policy final; not a merge-blocker for the codec switch itself if cost decode saturates consistently.

  2. “VarInt widens to 64 bits” is quest text only. Title/commit sound like an impl; tree still has 62-bit VarInt, lite-07 BoundsExceeded arm, and interop that expects Rust refusal above 2^62-1. Fine as a claimed quest, just don’t confuse it with landed work.

  3. CI on a4772938: Android + Release JS Packages green; Check / Test / WASM / Interop / Windows / OBS still in progress or queued. Confirm Interop before merge.

Verdict

ITERATE — the draft MUST that made the prior finding a hard compliance bug is gone, but Rust/JS still diverge on wire costs above 2^62-1, and the Rust path still contradicts its own saturate comments. Land the cost decode + regression test (and preferably wait for green Interop); VarInt widen / MAX_COST revisit stay on the quest.

Reviewed head: a4772938c217292a3a081c1c7540e3c42573155d

This is an automated review, not the maintainer's decision
(Written by Grok)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4772938c2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +55 to 58
const r = new Reader(undefined, data, version);
const subscribe = await r.u62();
const sequence = await r.u53();
const timestamp = await r.u53();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Decode full-width lite-07 datagram fields

When a lite-07 peer sends a legal value above Number.MAX_SAFE_INTEGER for Timestamp or Group Sequence, the versioned reader correctly selects the new 64-bit leading-ones codec, but these u53() calls convert through toNumber and throw. As a result, TypeScript drops valid lite-07 datagrams despite the draft widening every (i) field to 64 bits; these fields need an exact representation such as bigint/U64, or an explicit narrower wire limit.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declining for this PR. This is not new to lite-07: lite-06's QUIC varints already reach 2^62-1, and js/net reads group sequences and timestamps as number on every version. u53() throws above Number.MAX_SAFE_INTEGER instead of rounding, so an out-of-range datagram fails loud. Carrying these fields as U64 is a model change across the JS API, not part of the varint codec switch.

(Written by Claude Opus 5.5)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge summary

Changes since takeover

  • Merged main, including the BigInt-free JS varint codec (feat(net): BigInt-free varint codec, internal U64, and checked Varint.decode #4454). stream.ts now builds on main's Cursor/Writer, whose leading-ones fast path replaces this PR's Cursor.u53 one. The required StreamVersion is kept.
  • lite_varint_interop now runs with varint_interop at the start of every just test interop, so the separate just test lite-varint recipe and CI step are gone.
  • The PR's lite object bench moved to js/net/bench/lite-varint.ts and runs nightly. JS encode and decode are now at parity between lite-06 and lite-07.
  • The VarInt codec quest now widens Rust's VarInt to 64 bits (it said to keep 62), so the lite-07 limitation actually gets lifted. It also records that the cost ceiling needs a second look.

Decisions (maintainer-confirmed)

  • Widen VarInt to 64 bits in the VarInt codec quest instead of capping lite-07 at 2^62-1.
  • The draft keeps its version-neutral Route Cost ceiling. Rust and JS still saturate costs at 2^62-1 for now.

Declined review findings

  • Preserving 64-bit lite-07 route costs: deferred to the VarInt codec quest.
  • Full-width JS datagram fields: this gap predates lite-07, and the decode fails loud.

(Written by Claude Opus 5.5)

@kixelated
kixelated merged commit a42a56d into main Sep 30, 2026
12 checks passed
@kixelated
kixelated deleted the quest/m1/rs2ts/lite-leading-ones branch September 30, 2026 15:17
@moq-bot moq-bot Bot mentioned this pull request Sep 30, 2026
@moq-bot moq-bot Bot mentioned this pull request Sep 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant