Skip to content

feat(l2ps): SR-4 DACS negotiation stack (WI-A/B/C combined)#99

Merged
tcsenpai merged 7 commits into
mainfrom
feat/sr4-dacs-negotiation
Jun 24, 2026
Merged

feat(l2ps): SR-4 DACS negotiation stack (WI-A/B/C combined)#99
tcsenpai merged 7 commits into
mainfrom
feat/sr4-dacs-negotiation

Conversation

@tcsenpai

@tcsenpai tcsenpai commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Combines the three stacked SR-4 DACS negotiation slices (#95 WI-A, #96 WI-B, #97 WI-C) into a single PR off main, rebased past v4.0.11.

What your code does (plain English)

L2PS lets two parties hold a private, encrypted conversation over the Demos messaging layer. This PR builds the negotiation layer on top of that: two parties can make an offer, send counter-offers back and forth, and finally accept or reject — then optionally record a tamper-proof receipt of the whole exchange on-chain. Three pieces:

  1. Transport (WI-A) — carries channel messages over the L2PS instant-messaging socket. Encrypts each message, sends it to every channel member, and on receipt decrypts, drops duplicates/foreign messages, and hands them to the session in strict order (the network can deliver out of order, so a reorder buffer fills gaps before delivery).
  2. Negotiation (WI-B) — the offer → counter → accept/reject state machine. Enforces which transitions are legal, settles the agreed terms by the exact proposal that the accept points at (so a multi-round counter chain resolves correctly on both sides), and stays agnostic to the actual term contents.
  3. Finalize (WI-C) — on a terminal (accepted) negotiation, exports the transcript and anchors an encrypted copy per the DACS-3 §8.7 disclosure policy (none / recommended / required).

What's in the diff

Area File
Transport adapter src/l2ps/channel/transport.ts
RFQ state machine src/l2ps/channel/negotiate.ts
Finalize + anchor src/l2ps/channel/finalize.ts
Public exports src/l2ps/channel/index.ts
Tests src/tests/l2ps/{transport,negotiate,finalize,transport.e2e}.test.ts

Conflict resolution

#95 conflicted with main because v4.0.11 independently landed the same TS4113 fix (BroadcastFailedError / TransportError: drop invalid override on Error.cause). Resolved by taking the chain's version — functionally identical to main (public readonly cause: unknown) but keeps the explanatory comment. The standalone build-fix commit from the chain is now redundant and folded away.

Verification

  • bun run build — green.
  • bun test transport.test.ts negotiate.test.ts — 12/12 pure unit tests pass.
  • finalize.test.ts / transport.e2e.test.ts fail to load with a pre-existing export 'ValidityData' not found in './blockchain/ValidityData' module error — reproduced identically on the original feat/sr4-wi-c-finalize-anchor branch, so not introduced here. Unblocking those is out of scope for this merge.

Scope / follow-ups (unchanged from the slices)

  • negotiate-sealed-envelope (commit-then-reveal) gated on DACS-3 §8.4.3 (not in repo).
  • WI-D (AgreementDocument co-sign) needs DACS-3 §8.5.1.
  • WI-E (devnet E2E) needs a node running the messaging server.

Closes #95, closes #96, closes #97.

Summary by CodeRabbit

  • New Features

    • Added RFQ session negotiation support, including offer/counter/accept/reject/abort flows and session state tracking.
    • Added RFQ finalization with transcript export and optional anchoring based on policy.
    • Added a channel transport adapter for encrypted message delivery, ordering, buffering, and error handling.
    • Expanded public channel exports to include the new session, transport, and finalization APIs.
  • Tests

    • Added integration and end-to-end coverage for negotiation, finalization, message ordering, and encryption/decryption behavior.
  • Documentation

    • Added inline clarifications to error classes.

Shitikyan and others added 6 commits June 23, 2026 17:31
`bun run build` is red on main: BroadcastFailedError and TransportError
declare `public override readonly cause`, but the configured lib
(tsconfig `lib: ["es2021"]`, `target: es2020`) predates ES2022's
`Error.cause`, so the base `Error` has no `cause` to override — TS4113.
The pre-commit build hook blocks every commit until this clears.

Drop the `override` modifier; the class still declares `cause` as its
own property (identical runtime behaviour). Pre-existing — landed with
#94/#91, not introduced here.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bridges the transport-agnostic DACS channel layer (ChannelSession,
which only produces/verifies signed envelopes) onto the L2PS
instant-messaging transport (L2PSMessagingPeer, which moves encrypted
bytes between subnet members).

Core is a reorder buffer. ChannelSession.receiveIncoming accepts any
sequence strictly greater than the highest seen and advances its
counter to it — so a gap (seq N+2 before N+1) silently skips the
missing messages rather than rejecting. The L2PS transport delivers by
timestamp and replays an offline queue, so out-of-order arrival is
normal. The adapter hands the session only the next contiguous
sequence and buffers anything ahead of the gap until it fills;
duplicates / already-applied sequences are dropped; envelopes for
other channels sharing the subnet are ignored.

Outgoing: session.sendOutgoing -> JSON -> AES-256-GCM under the subnet
key -> peer.send to every member except self. Incoming: decrypt ->
parse -> buffer -> drain in order -> session.receiveIncoming (still
enforces signature / sender-in-members / channelId, throwing on tamper).

Dependencies are declared as structural interfaces so the reorder
logic is unit-tested without real signing keys or a live socket. 5/5
tests: in-order, gap-buffer-and-fill, duplicate-drop,
foreign-channel-ignore, send/reply interleave.

Scope: turn-based messaging (DACS negotiation is offer->counter->accept);
concurrent same-sequence emission from both parties is out of SR-4 v1
scope. SDK stays transport-agnostic — this adapter is the opt-in bridge.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Complements the unit test (which mocks the session to isolate the
reorder buffer) with a full-stack E2E: two real Demos identities, two
real ChannelSessions, two L2PSChannelTransports over an in-process
relay that stands in for the messaging server. Exercises real CCI-key
signing, real cross-party verification (signature + sender∈members +
channelId), real AES-256-GCM under a shared subnet key, and the reorder
buffer under realistic out-of-order delivery.

3 cases:
- full offer → counter → accept round verifies on both ends; both
  transcripts agree on [1,2,3]
- a two-message burst delivered REVERSED (seq2 before seq1) is buffered
  and applied in order
- a tampered frame surfaces via onError and does not advance the session

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
RfqSession drives the request-for-quote state machine: offer/counter
proposals terminating in accept/reject/abort (CH-5 termination). Per
the SR-4 brief the RFQ message bodies are implementation-defined (only
sealed-envelope bodies are spec-locked, §8.4.3), so this fixes a
minimal body schema (proposal carries opaque `terms`; accept references
the accepted proposal's sequence) and the legal-transition rules while
staying agnostic to the actual terms.

Pure protocol state machine — transport-agnostic like ChannelSession:
the caller wires `send` to a transport (e.g. L2PSChannelTransport.send,
WI-A) and feeds verified inbound envelopes to `onIncoming`. By the time
a message reaches onIncoming the channel layer has verified signature /
sender∈members / channelId / sequence, so this layer only enforces
protocol rules (legal transitions, accept referencing an existing
proposal). Resolves accepted terms by the exact proposal sequence the
accept points at, so a multi-round counter chain settles on the right
terms on both sides.

7 tests: offer→counter→accept settles agreedTerms both sides; reject
terminates both; accept resolves the referenced proposal's terms across
a counter chain; illegal transitions rejected (accept/counter with
nothing standing, double offer); no action after terminal; inbound
accept referencing an unknown proposal throws; impl-defined terms
carried opaquely.

negotiate-sealed-envelope remains gated on DACS-3 §8.4.3 (not in repo).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Ties WI-B's terminal state to WI-3's transcript anchor: on an agreed
(accepted) negotiate-rfq, export the signed channel transcript and —
per the disclosure policy (DACS-3 §8.7) — anchor an encrypted copy,
returning the channelTranscriptRef the rfq output carries.

finalizeRfq orchestrates exportTranscript (sign the ordered messages) →
anchorEncryptedTranscript (encrypt-to-members + SR-2 anchor). Policy:
- none: transcript stays local, nothing anchored
- encrypted-anchored-recommended: anchor only on explicit consent
- encrypted-anchored-required: MUST anchor; a null result throws (the
  phase fails, §8.7)

Refuses to finalize a non-accepted negotiation; requires an L2PS
instance when the policy actually anchors. The anchor call is injectable
so the orchestration is unit-tested without a live node (the real anchor
deploys an SR-2 storage program). 6 tests cover all four policy paths +
the two guards, using a real signed offer→counter→accept so the exported
transcript is genuinely CCI-signed.

WI-D (AgreementDocument co-sign) needs DACS-3 §8.5.1; WI-E (devnet E2E)
needs a node running the messaging server.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Combines the three stacked WIP slices into one branch off main:
- WI-A: channel-over-L2PS transport adapter (transport.ts)
- WI-B: negotiate-rfq phase logic (negotiate.ts)
- WI-C: finalizeRfq transcript anchor on terminal (finalize.ts)

Conflict resolution: BroadcastFailedError/TransportError — main already
landed the equivalent TS4113 fix (drop invalid override on Error.cause);
kept the explanatory comment from the chain. Functionally identical.
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@tcsenpai, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 42 minutes and 42 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 56d12401-8f5a-4ef0-9a5d-7144a8a7182b

📥 Commits

Reviewing files that changed from the base of the PR and between 05a708f and 1a07d5e.

📒 Files selected for processing (6)
  • src/l2ps/channel/finalize.ts
  • src/l2ps/channel/negotiate.ts
  • src/l2ps/channel/transport.ts
  • src/tests/l2ps/finalize.test.ts
  • src/tests/l2ps/negotiate.test.ts
  • src/tests/l2ps/transport.test.ts

Walkthrough

Three new channel-layer modules are added under src/l2ps/channel/: transport.ts (AES-256-GCM encrypted message adapter with out-of-order buffering), negotiate.ts (RFQ offer/counter/accept state machine), and finalize.ts (policy-driven transcript anchoring). All are barrel-exported via index.ts, and each is covered by dedicated unit, integration, and E2E test suites. Two error classes receive minor inline documentation clarifications.

Changes

SR-4 negotiate-rfq channel stack

Layer / File(s) Summary
Transport interfaces and AES-256-GCM adapter
src/l2ps/channel/transport.ts
Defines ChannelSessionLike, L2PSMessagingPeerLike, wire payload types, and L2PSChannelTransportOpts; implements L2PSChannelTransport with start/send/ingest/drain logic, out-of-order sequence buffering via a serialized draining promise chain, and AES-256-GCM encrypt/decrypt with base64 wire encoding.
RfqSession negotiation state machine
src/l2ps/channel/negotiate.ts
Defines RFQ message body interfaces, StandingProposal, RfqState/RfqOutcome; implements RfqSession with offer/counter/accept/reject/abort outbound methods (enforcing open-only), internal proposal map keyed by sequence, and onIncoming inbound handler enforcing known-sequence accept references.
finalizeRfq transcript export and anchoring
src/l2ps/channel/finalize.ts
Defines RfqLike, ChannelSessionView, FinalizeRfqOpts, RfqFinalizeResult; implements finalizeRfq enforcing accepted state, exporting a signed transcript, and branching on TranscriptDisclosurePolicy to call an injectable anchor function (defaulting to anchorEncryptedTranscript) or return a null ref.
Channel barrel re-exports
src/l2ps/channel/index.ts
Extends the barrel to re-export transport, negotiate, and finalize public APIs and types.
RfqSession unit tests
src/tests/l2ps/negotiate.test.ts
Two-party in-memory harness covers offer/counter/accept settlement, rejection, per-sequence accept resolution, illegal transition guards, post-terminal guards, unknown-sequence throw, and opaque term propagation.
L2PSChannelTransport unit tests
src/tests/l2ps/transport.test.ts
FakeSession/FakePeer fakes drive five cases: in-order delivery, gap buffering/drain, duplicate dropping, channel-id filtering, and interleaved sends with inbound replies.
L2PSChannelTransport E2E tests
src/tests/l2ps/transport.e2e.test.ts
In-process Relay test double with hold/flush/tamper covers: normal round-trip, out-of-order reordering, and ciphertext tampering triggering onError with an empty session transcript.
finalizeRfq integration tests
src/tests/l2ps/finalize.test.ts
Real offer→counter→accept flow through two ChannelSession/RfqSession pairs; tests required anchoring with transcript content assertions, none policy, recommended-with-no-consent, required-failing-null, non-accepted rejection, and missing-l2ps rejection.
Error class cause override comments
src/websdk/BroadcastFailedError.ts, src/websdk/TransportError.ts
Adds inline comments explaining that override is omitted for Error.cause due to the configured lib predating ES2022 typings.

Sequence Diagram(s)

sequenceDiagram
  participant Alice as Alice RfqSession
  participant AliceCh as Alice ChannelSession / Transport
  participant Relay as L2PSMessagingPeer / Relay
  participant BobCh as Bob ChannelSession / Transport
  participant Bob as Bob RfqSession

  Alice->>AliceCh: offer(terms)
  AliceCh->>Relay: AES-256-GCM encrypt + send
  Relay->>BobCh: deliver frame
  BobCh->>BobCh: ingest → decrypt → drain → receiveIncoming
  BobCh->>Bob: onIncoming(offer)
  Bob->>Bob: record standingProposal

  Bob->>BobCh: counter(terms)
  BobCh->>Relay: AES-256-GCM encrypt + send
  Relay->>AliceCh: deliver frame
  AliceCh->>Alice: onIncoming(counter)

  Alice->>AliceCh: accept(seq)
  AliceCh->>Relay: AES-256-GCM encrypt + send
  Relay->>BobCh: deliver frame
  BobCh->>Bob: onIncoming(accept) → state=accepted

  Note over Alice,Bob: Both sides reach accepted

  Alice->>Alice: finalizeRfq(policy, session)
  Alice->>Alice: exportSignedTranscript
  Alice->>Alice: anchorEncryptedTranscript → AttestationRef
  Alice-->>Alice: RfqFinalizeResult { transcript, channelTranscriptRef }
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

  • kynesyslabs/sdks#94: finalizeRfq defaults to anchorEncryptedTranscript and branches on TranscriptDisclosurePolicy, both of which are the anchoring APIs and policy types introduced in PR #94.

Suggested labels

Review effort 4/5

Poem

🐇 Hoppity-hop through the channel we go,
Offers and counters in encrypted flow,
Sequences buffered, reordered, drained clean,
The finest RFQ state machine seen!
Transcripts anchored, the policy obeyed—
This bunny's quite proud of the protocol laid. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: an SR-4 L2PS/DACS negotiation stack combining transport, negotiation, and finalize work.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/sr4-dacs-negotiation

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.

@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR merges three stacked SR-4 DACS negotiation slices (WI-A/B/C) into a single branch off main, rebased past v4.0.11. It delivers the full offer→counter→accept/reject/abort negotiation stack on top of the L2PS messaging layer, with on-chain transcript anchoring driven by a §8.7 disclosure policy.

  • WI-A (transport.ts): Channel-over-L2PS adapter with AES-256-GCM encryption, a bounded reorder buffer (MAX_REORDER_AHEAD = 256), serialised drain chain, correct subarray-key handling via keyBytes(), and delivery-before-commit ordering so a failed peer send does not consume the channel sequence.
  • WI-B (negotiate.ts): Pure RFQ state machine enforcing legal offer/counter/accept/reject/abort transitions in both outbound and inbound directions, with self-accept prevention and stale-proposal rejection.
  • WI-C (finalize.ts): Transcript export and conditional anchor orchestration — member-signer and session-transcript match guards added, willAnchor evaluated before the l2ps guard so a no-consent recommended path returns cleanly without needing an L2PS instance.

Confidence Score: 5/5

All three modules are well-guarded and the previously raised concerns have been fully addressed; this is safe to merge.

The transport correctly handles subarray keys, bounds the reorder buffer, delivers before advancing the local sequence counter, and serialises the inbound drain chain. The negotiation state machine enforces both outbound and inbound transition rules, blocks self-accept, and rejects stale-proposal accepts. The finalise function validates signer membership and session-transcript consistency before exporting or anchoring. The only open point is a module-level style nit (base64 helpers duplicated from anchor.ts).

No files require special attention; the pre-existing ValidityData import error affecting finalize.test.ts and transport.e2e.test.ts is documented as out of scope.

Important Files Changed

Filename Overview
src/l2ps/channel/transport.ts New channel-over-L2PS transport adapter: AES-256-GCM encrypt/decrypt with correct subarray key handling (keyBytes helper), bounded reorder buffer (MAX_REORDER_AHEAD=256), serialised drain chain, and corrected send ordering that preserves appliedSeq on delivery failure. Local base64 helpers duplicate anchor.ts.
src/l2ps/channel/negotiate.ts New RFQ state machine: offer/counter/accept/reject/abort with enforced inbound and outbound transition rules, self-accept prevention, stale-proposal rejection, and proper two-sided settlement. No issues found.
src/l2ps/channel/finalize.ts New transcript finalisation with §8.7 policy dispatch: member-signer check, transcript-session match validation, no-consent fast-path that skips the l2ps guard, and required-policy failure on null anchor. No issues found.
src/l2ps/channel/index.ts Adds clean re-exports for the three new modules; no logic changes.
src/tests/l2ps/transport.test.ts Unit tests for the reorder buffer: in-order delivery, gap buffering, duplicate drops, channel isolation, send/receive interleaving, failed-send non-commit, subarray key, and MAX_REORDER_AHEAD window. Good coverage of all edge cases.
src/tests/l2ps/negotiate.test.ts Unit tests for the RFQ state machine covering the full offer→counter→accept flow, reject, illegal transitions, self-accept prevention, inbound counter before offer, second inbound offer, and stale-accept rejection. Complete protocol coverage.
src/tests/l2ps/finalize.test.ts Unit tests for finalizeRfq covering all three disclosure policies, consent gating, missing l2ps guard, non-member signer rejection, session-transcript mismatch detection, and the no-consent/no-l2ps regression path.
src/tests/l2ps/transport.e2e.test.ts E2E tests with real Demos identities, real ChannelSessions, and an in-process relay: full offer/counter/accept round, out-of-order burst replay, and tampered-frame error surfacing. Will not load until the pre-existing ValidityData export issue is resolved (noted in PR description as out of scope).
src/websdk/BroadcastFailedError.ts Adds explanatory comment for the missing override keyword on Error.cause (TS4113 fix from es2021 lib target); no logic change.
src/websdk/TransportError.ts Same TS4113 comment fix as BroadcastFailedError; no logic change.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant A as Alice (RfqSession + Transport)
    participant L as L2PS Messaging Layer
    participant B as Bob (RfqSession + Transport)

    Note over A,B: WI-A: Transport layer (encrypt, reorder buffer)
    A->>A: "sendOutgoing(offer, seq=1)"
    A->>A: AES-256-GCM encrypt
    A->>L: "peer.send(to=bob, ciphertext, hash)"
    L->>B: deliver payload (onMessage)
    B->>B: decrypt → ChannelMessage
    B->>B: "buffer.set(seq=1) → drain()"
    B->>B: "session.receiveIncoming(seq=1)"
    B->>B: "appliedSeq = 1"

    Note over A,B: WI-B: RFQ state machine
    B->>B: "onIncoming(offer, seq=1) → _standing = {seq:1, sender:A}"
    B->>B: "sendOutgoing(counter, seq=2)"
    B->>L: "peer.send(to=alice, ciphertext)"
    L->>A: deliver
    A->>A: "decrypt → receiveIncoming(seq=2)"
    A->>A: "onIncoming(counter, seq=2) → _standing = {seq:2, sender:B}"
    A->>A: "accept() checks sender≠me → sends accept{acceptedSequence:2}"
    A->>L: "peer.send(to=bob, ciphertext)"
    L->>B: deliver
    B->>B: "onIncoming(accept) → seq matches _standing.seq=2 → settle(accepted)"

    Note over A,B: WI-C: Finalize + Anchor
    A->>A: finalizeRfq(rfq, session, policy)
    A->>A: "check rfq.state=accepted"
    A->>A: check signer∈members
    A->>A: "check transcript has seq=2 proposal + accept"
    A->>A: exportTranscript → signed ChannelTranscript
    alt "policy = encrypted-anchored-required"
        A->>A: anchorEncryptedTranscript(transcript, l2ps)
        A-->>A: AttestationRef
    else "policy = none or recommended without consent"
        A-->>A: "channelTranscriptRef = null"
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant A as Alice (RfqSession + Transport)
    participant L as L2PS Messaging Layer
    participant B as Bob (RfqSession + Transport)

    Note over A,B: WI-A: Transport layer (encrypt, reorder buffer)
    A->>A: "sendOutgoing(offer, seq=1)"
    A->>A: AES-256-GCM encrypt
    A->>L: "peer.send(to=bob, ciphertext, hash)"
    L->>B: deliver payload (onMessage)
    B->>B: decrypt → ChannelMessage
    B->>B: "buffer.set(seq=1) → drain()"
    B->>B: "session.receiveIncoming(seq=1)"
    B->>B: "appliedSeq = 1"

    Note over A,B: WI-B: RFQ state machine
    B->>B: "onIncoming(offer, seq=1) → _standing = {seq:1, sender:A}"
    B->>B: "sendOutgoing(counter, seq=2)"
    B->>L: "peer.send(to=alice, ciphertext)"
    L->>A: deliver
    A->>A: "decrypt → receiveIncoming(seq=2)"
    A->>A: "onIncoming(counter, seq=2) → _standing = {seq:2, sender:B}"
    A->>A: "accept() checks sender≠me → sends accept{acceptedSequence:2}"
    A->>L: "peer.send(to=bob, ciphertext)"
    L->>B: deliver
    B->>B: "onIncoming(accept) → seq matches _standing.seq=2 → settle(accepted)"

    Note over A,B: WI-C: Finalize + Anchor
    A->>A: finalizeRfq(rfq, session, policy)
    A->>A: "check rfq.state=accepted"
    A->>A: check signer∈members
    A->>A: "check transcript has seq=2 proposal + accept"
    A->>A: exportTranscript → signed ChannelTranscript
    alt "policy = encrypted-anchored-required"
        A->>A: anchorEncryptedTranscript(transcript, l2ps)
        A-->>A: AttestationRef
    else "policy = none or recommended without consent"
        A-->>A: "channelTranscriptRef = null"
    end
Loading

Reviews (2): Last reviewed commit: "fix(l2ps): address Greptile review on SR..." | Re-trigger Greptile

Comment thread src/l2ps/channel/transport.ts
Comment thread src/l2ps/channel/transport.ts
Comment thread src/l2ps/channel/transport.ts
Comment thread src/l2ps/channel/transport.ts
Comment thread src/l2ps/channel/negotiate.ts
Comment thread src/l2ps/channel/negotiate.ts
Comment thread src/l2ps/channel/negotiate.ts
Comment thread src/l2ps/channel/finalize.ts
Comment thread src/l2ps/channel/finalize.ts
Comment thread src/l2ps/channel/finalize.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (4)
src/l2ps/channel/index.ts (1)

54-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use @/ aliases for the new barrel re-exports.

These new exports use relative paths; repo TS guidelines require alias-based imports/exports (@/...) for consistency.

As per coding guidelines: "**/*.{ts,tsx}: Use @/ path aliases instead of relative imports".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/l2ps/channel/index.ts` around lines 54 - 80, The new barrel re-exports in
the channel index file are using relative module paths instead of the required
`@/` alias style. Update the export statements in the index barrel that re-export
from transport, negotiate, and finalize to use alias-based paths consistent with
the repo’s TypeScript import/export guidelines, keeping the existing exported
symbols unchanged.

Source: Coding guidelines

src/tests/l2ps/finalize.test.ts (1)

123-135: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

This test currently masks the non-consent branch contract.

l2ps: {} as never hides whether recommended + consent:false works without L2PS. Add an assertion for the no-l2ps case and replace as never with a typed stub where needed to keep TS coverage strict.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tests/l2ps/finalize.test.ts` around lines 123 - 135, The `finalizeRfq`
test is masking the non-consent path by using `l2ps: {} as never`, so it does
not verify that `recommended` with `consent: false` works when L2PS is absent.
Update the `recommended without consent` case in `finalize.test.ts` to add a
separate assertion for the no-`l2ps` scenario using `finalizeRfq`, and replace
the `as never` cast with a properly typed stub or explicit optional omission so
TypeScript still checks the contract strictly.

Source: Coding guidelines

src/l2ps/channel/finalize.ts (1)

20-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use @/ aliases in this TypeScript module.

This file currently uses relative imports (../../, ../, ./), which conflicts with the repository TS import guideline.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/l2ps/channel/finalize.ts` around lines 20 - 32, Replace the relative
imports in finalize.ts with the repository’s `@/` alias paths so the module
follows the TypeScript import guideline. Update each import at the top of the
file, including the ones for Demos, ClaimReference, L2PS,
anchorEncryptedTranscript, exportTranscript, ChannelMessage, ChannelTranscript,
RfqOutcome, and RfqState, to use the appropriate `@/` alias equivalent while
keeping the same symbols and grouping.

Sources: Coding guidelines, Learnings

src/l2ps/channel/transport.ts (1)

34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the repo alias for this import.

Line 34 introduces a relative import in a new TypeScript file. @/l2ps/channel/types would keep this file aligned with the repo’s alias convention.

As per coding guidelines, **/*.{ts,tsx} should "Use @/ path aliases instead of relative imports".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/l2ps/channel/transport.ts` at line 34, The new import in transport.ts
uses a relative path instead of the repo alias convention. Update the
ChannelMessage and ChannelMessageType import in the transport module to use the
`@/l2ps/channel/types` alias so it matches the existing TypeScript import style
across the repo.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/l2ps/channel/finalize.ts`:
- Around line 98-102: The guard in finalizeRfq is too broad and blocks the
non-anchoring encrypted-anchored-recommended path when consent is not true.
Update the policy check around opts.l2ps so it only throws when the current flow
actually needs transcript encryption, and allow the consent !== true branch to
proceed and return channelTranscriptRef: null without requiring an L2PS
instance. Use finalizeRfq and the encrypted-anchored-recommended policy handling
to locate the conditional.
- Around line 87-92: Validate the signer’s membership before calling
exportTranscript in finalize, since FinalizeRfqOpts requires the signer to be a
member but finalize currently never checks it. In finalize.ts, use the existing
opts.session.members data (and the signer claim from opts.signer) to verify the
signer is included, and reject finalization with an error if they are not. Keep
the check colocated with the transcript export path so non-members cannot reach
exportTranscript under policy: "none".

In `@src/l2ps/channel/negotiate.ts`:
- Around line 181-194: Inbound “counter” messages are being accepted in
onIncoming even when there is no existing standing proposal, which bypasses the
RFQ transition guard used by counter(). Update the switch handling in
negotiate.ts so the "counter" branch first verifies a current standing proposal
exists (using the same state tracked by this._standing / proposals) and rejects
or ignores the message when the precondition is not met. Keep the existing
"offer" handling unchanged, and ensure the validation is applied in the
onIncoming path before constructing and storing the StandingProposal.

In `@src/l2ps/channel/transport.ts`:
- Around line 142-150: The `send()` flow in `ChannelTransport` currently
consumes a new sequence via `this.session.sendOutgoing()` before fan-out, so a
partial `peer.send()` failure makes the signed envelope unretryable and can
desync recipients. Update `send()` and the surrounding
`ChannelTransport`/session contract so the same signed envelope can be retried
idempotently after transport errors instead of minting a fresh sequence on
retry; keep the sequence/signature stable across retries and only advance the
shared channel counter once delivery is successfully committed or explicitly
acknowledged.
- Around line 111-117: The shared key handling in the L2PSChannelTransport
constructor is using the full backing ArrayBuffer instead of the intended
32-byte Uint8Array view. Update the constructor in L2PSChannelTransport to
validate that opts.sharedKey.byteLength is exactly 32, then ensure both
crypto.subtle.importKey calls use this.sharedKey itself (or a copied Uint8Array
view) rather than this.sharedKey.buffer so only the actual key bytes are
imported.

In `@src/tests/l2ps/negotiate.test.ts`:
- Around line 104-116: Add a regression test in negotiate.test.ts to cover
inbound counter before any offer: the current illegal transition coverage in
rejects illegal transitions only checks outbound aSes.counter, but not
onIncoming handling. Extend the existing harness-based test flow around
harness(), aSes, and onIncoming so that an inbound "counter" is rejected when
nothing is standing, and assert the expected rejection message. Also mirror this
coverage in the similar test block referenced by the same behavior so the
protocol rule stays enforced consistently.

---

Nitpick comments:
In `@src/l2ps/channel/finalize.ts`:
- Around line 20-32: Replace the relative imports in finalize.ts with the
repository’s `@/` alias paths so the module follows the TypeScript import
guideline. Update each import at the top of the file, including the ones for
Demos, ClaimReference, L2PS, anchorEncryptedTranscript, exportTranscript,
ChannelMessage, ChannelTranscript, RfqOutcome, and RfqState, to use the
appropriate `@/` alias equivalent while keeping the same symbols and grouping.

In `@src/l2ps/channel/index.ts`:
- Around line 54-80: The new barrel re-exports in the channel index file are
using relative module paths instead of the required `@/` alias style. Update the
export statements in the index barrel that re-export from transport, negotiate,
and finalize to use alias-based paths consistent with the repo’s TypeScript
import/export guidelines, keeping the existing exported symbols unchanged.

In `@src/l2ps/channel/transport.ts`:
- Line 34: The new import in transport.ts uses a relative path instead of the
repo alias convention. Update the ChannelMessage and ChannelMessageType import
in the transport module to use the `@/l2ps/channel/types` alias so it matches the
existing TypeScript import style across the repo.

In `@src/tests/l2ps/finalize.test.ts`:
- Around line 123-135: The `finalizeRfq` test is masking the non-consent path by
using `l2ps: {} as never`, so it does not verify that `recommended` with
`consent: false` works when L2PS is absent. Update the `recommended without
consent` case in `finalize.test.ts` to add a separate assertion for the
no-`l2ps` scenario using `finalizeRfq`, and replace the `as never` cast with a
properly typed stub or explicit optional omission so TypeScript still checks the
contract strictly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5939e492-f0a9-445e-861e-88258f1af32d

📥 Commits

Reviewing files that changed from the base of the PR and between cd4a26a and 05a708f.

📒 Files selected for processing (10)
  • src/l2ps/channel/finalize.ts
  • src/l2ps/channel/index.ts
  • src/l2ps/channel/negotiate.ts
  • src/l2ps/channel/transport.ts
  • src/tests/l2ps/finalize.test.ts
  • src/tests/l2ps/negotiate.test.ts
  • src/tests/l2ps/transport.e2e.test.ts
  • src/tests/l2ps/transport.test.ts
  • src/websdk/BroadcastFailedError.ts
  • src/websdk/TransportError.ts

Comment thread src/l2ps/channel/finalize.ts
Comment thread src/l2ps/channel/finalize.ts
Comment thread src/l2ps/channel/negotiate.ts
Comment on lines +111 to +117
constructor(opts: L2PSChannelTransportOpts) {
this.session = opts.session
this.peer = opts.peer
this.sharedKey = opts.sharedKey
this.recipients = opts.recipients
this.onMessage = opts.onMessage
this.onError = opts.onError

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major

🧩 Analysis chain

🌐 Web query:

For Web Crypto SubtleCrypto.importKey("raw", keyData, "AES-GCM"), does passing a Uint8Array's .bufferignore the view'sbyteOffset/byteLength, and what raw key sizes are accepted for AES-GCM?

💡 Result:

When you pass a TypedArray (like Uint8Array) to SubtleCrypto.importKey, the Web Crypto API correctly respects the view's byteOffset and byteLength. It does not ignore these properties; rather, it uses the specific range of the underlying ArrayBuffer defined by that view [1][2]. If you pass the view's.buffer property directly instead of the view itself, you are passing the entire underlying ArrayBuffer. This will ignore the byteOffset and byteLength of your view, potentially importing unintended data if your view was only a slice of the buffer. Therefore, you should always pass the TypedArray or DataView object itself, rather than its.buffer, to ensure the API only uses the intended range of bytes. For AES-GCM, the raw key data must be exactly 128, 192, or 256 bits (16, 24, or 32 bytes) in length [3]. If the provided key data does not match one of these specific lengths, the importKey operation will throw an error [3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== transport.ts outline ==\n'
ast-grep outline src/l2ps/channel/transport.ts --view expanded || true

printf '\n== transport.ts relevant lines ==\n'
nl -ba src/l2ps/channel/transport.ts | sed -n '1,320p'

printf '\n== sharedKey search ==\n'
rg -n "sharedKey|importKey\\(|AES-GCM" src/l2ps -S || true

Repository: kynesyslabs/sdks

Length of output: 2226


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== transport.ts outline ==\n'
ast-grep outline src/l2ps/channel/transport.ts --view expanded || true

printf '\n== transport.ts relevant lines ==\n'
nl -ba src/l2ps/channel/transport.ts | sed -n '1,320p'

printf '\n== sharedKey search ==\n'
rg -n "sharedKey|importKey\\(|AES-GCM" src/l2ps -S || true

Repository: kynesyslabs/sdks

Length of output: 2226


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== transport.ts constructor and crypto helpers ==\n'
sed -n '111,235p' src/l2ps/channel/transport.ts | cat -n

printf '\n== sharedKey length checks ==\n'
rg -n "byteLength\s*!==\s*32|byteLength\s*===\s*32|sharedKey.*byteLength|sharedKey.*slice\\(" src/l2ps -S || true

printf '\n== importKey call sites ==\n'
rg -n 'importKey\("raw"|sharedKey\.buffer|sharedKey\b' src/l2ps/channel/transport.ts -n -S || true

Repository: kynesyslabs/sdks

Length of output: 6176


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== transport.ts constructor and crypto helpers ==\n'
sed -n '111,235p' src/l2ps/channel/transport.ts | cat -n

printf '\n== sharedKey length checks ==\n'
rg -n "byteLength\s*!==\s*32|byteLength\s*===\s*32|sharedKey.*byteLength|sharedKey.*slice\\(" src/l2ps -S || true

printf '\n== importKey call sites ==\n'
rg -n 'importKey\("raw"|sharedKey\.buffer|sharedKey\b' src/l2ps/channel/transport.ts -n -S || true

Repository: kynesyslabs/sdks

Length of output: 6176


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== transport.ts type comments and exports ==\n'
sed -n '70,130p' src/l2ps/channel/transport.ts | cat -n

printf '\n== L2PSChannelTransport usage sites ==\n'
rg -n "new L2PSChannelTransport\\(" -S .

printf '\n== sharedKey producers/usages ==\n'
rg -n "sharedKey" -S src . | sed -n '1,200p'

Repository: kynesyslabs/sdks

Length of output: 4854


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== transport.ts type comments and exports ==\n'
sed -n '70,130p' src/l2ps/channel/transport.ts | cat -n

printf '\n== L2PSChannelTransport usage sites ==\n'
rg -n "new L2PSChannelTransport\\(" -S .

printf '\n== sharedKey producers/usages ==\n'
rg -n "sharedKey" -S src . | sed -n '1,200p'

Repository: kynesyslabs/sdks

Length of output: 4854


Use the 32-byte key view directly
this.sharedKey.buffer imports the whole backing buffer, not the Uint8Array slice. Enforce opts.sharedKey.byteLength === 32 in the constructor and pass this.sharedKey itself (or a copied Uint8Array) to both crypto.subtle.importKey calls.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/l2ps/channel/transport.ts` around lines 111 - 117, The shared key
handling in the L2PSChannelTransport constructor is using the full backing
ArrayBuffer instead of the intended 32-byte Uint8Array view. Update the
constructor in L2PSChannelTransport to validate that opts.sharedKey.byteLength
is exactly 32, then ensure both crypto.subtle.importKey calls use this.sharedKey
itself (or a copied Uint8Array view) rather than this.sharedKey.buffer so only
the actual key bytes are imported.

Comment thread src/l2ps/channel/transport.ts
Comment thread src/tests/l2ps/negotiate.test.ts
Transport (transport.ts):
- key-view bug (P1): importKey copied the whole backing ArrayBuffer; a
  32-byte subarray key over a larger buffer broke AES-256 import. Slice
  the exact [byteOffset, byteOffset+byteLength) window (keyBytes()).
- send-commits-early (P1): deliver to all recipients BEFORE advancing
  appliedSeq, so a rejected peer.send leaves the sequence uncommitted and
  a retry re-emits it instead of skipping ahead.
- unbounded reorder buffer (P2): drop inbound frames more than
  MAX_REORDER_AHEAD (256) ahead of the contiguous point.

Negotiate (negotiate.ts):
- inbound transitions bypass rules (P1): inbound offer rejected when a
  proposal stands; inbound counter rejected when none stands.
- stale accept (P1): inbound accept must reference the standing proposal
  sequence, not any past one.
- self-accept (P1): accept() rejects accepting your own standing proposal.

Finalize (finalize.ts):
- no-consent path throws (P2): 'recommended' without consent is a
  no-anchor outcome and no longer requires an L2PS instance.
- non-member signer (P2): reject a signer not in members up front.
- transcript mismatch (P2): require the session messages to contain the
  accepted proposal + matching accept before exporting/anchoring.

Tests: +10 regression tests (transport 4, negotiate 4, finalize 3 — one
existing terminal-state test updated to use a counterparty accept since
self-accept is now forbidden). transport+negotiate suites: 19/19 green.
@tcsenpai tcsenpai merged commit 2a6baa3 into main Jun 24, 2026
8 checks passed
@sonarqubecloud

Copy link
Copy Markdown

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.

2 participants