Skip to content

feat(net,relay): a lite-07 endpoint declares its own Node ID in SETUP, so a relay refuses a session with itself - #8325

Queued
kixelated wants to merge 14 commits into
mainfrom
quest/m2/cluster-routing/lite07-self-dial
Queued

kixelated wants to merge 14 commits into
mainfrom
quest/m2/cluster-routing/lite07-self-dial

Conversation

@kixelated

@kixelated kixelated commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Completes quest/m2/cluster-routing/lite07-self-dial.md and deletes it. Builds on moq-dev/moq#8293, now in main.

Summary

A relay whose cluster peer URL reaches itself under another spelling (an IP for its hostname) now refuses that session on lite-07, as it already does on lite-05, lite-06 and moq-transport.

  • Wire (lite-07-wip): SETUP parameter 0x5 returns as the Node ID parameter: the Node ID the sender originates its own routes under. It is never 0 and never has the qualified bit set; either is a PROTOCOL_VIOLATION. A receiver that gets its own Node ID has reached itself and SHOULD close the session. Forwarded origins' Node IDs stay in ROUTE_START. Before lite-07, 0x5 is still the Hop ID.
  • Rust (moq-net): lite::Setup gains a node field, kept apart from hop so the lite-05/06 split-horizon consumers of hop are unchanged. A lite-07 session fills it from its origin's Node ID. It reads the ID without creating the route node: it uses the node's identity if the node exists, and otherwise the identity the node will declare for this hop. server::Request::peer_hop returns the Node ID on lite-07.
  • Rust (moq-relay): connection.rs checks origin::Producer::is_own(id) (new) instead of comparing against cluster.id(). That check also matches the Node ID after a rotation, which a peer can force by requesting seqno 2^64-1, and when a Config.hop has its top bit set.
  • JS (js/net): has no route layer, so it originates no routes and omits the Node ID on lite-07. Before, it sent its random per-connection hop, which is not a Node ID (one per process). It rejects a 0 or qualified Node ID on lite-07 in both directions (hasNodeId).
  • Draft: an endpoint that originates no routes omits the Node ID.
  • Draft: adds the Node ID Parameter section and table row, and updates the lite-07 changelog entry that had removed 0x5.
  • Docs: doc/bin/relay/cluster.md names the Node ID as the lite-07 identity the relay refuses.

Decisions

  • ✅ A lite-07 endpoint declares its own Node ID in SETUP instead of ROUTE (rejected: detecting our own route coming back; dropping the quest). Maintainer, comment.

Choices made without asking

  • Reused parameter id 0x5 for the Node ID on lite-07, since lite-07 is unpublished and 0x5 is free there.
  • A separate node field in Rust, one hop field in JS. In Rust, the lite-05/06 serving and announce paths read Setup.hop, and feeding them a Node ID on lite-07 would change routing. JS has no route layer, so its one field just changes meaning by version, and its connection leaves it unset on lite-07.
  • New public origin::Producer::is_own. The relay needs the current Node ID, which can rotate away from Config.hop. Exposing the node's ID instead would leak route-layer state.
  • SETUP never creates the route node. Creating it at session start, before the route layer does, deterministically broke three socket:: dial-by-hand tests (a_peer_reset_after_fin_resets_the_session, a_second_tcp_group_resets_the_session, a_second_tcp_group_after_fin_resets_the_session), whichever handle created it. See Follow-ups.

Impact

Public API (additive): moq_net::origin::Producer::is_own(NonZeroU64) -> bool. server::Request::peer_hop docs now cover the lite-07 Node ID.

Wire: lite-07-wip only. SETUP 0x5 carries the sender's Node ID; 0 or qualified is a PROTOCOL_VIOLATION. lite-05/06 and moq-transport are unchanged.

Caused by

  • moq-dev/moq#8023 - lite-07 dropped the SETUP Hop parameter in favor of route Node IDs, which left the relay's self-session check with nothing to compare on lite-07.

Checks

  • Reproduced first: with the source reverted and the new tests kept, both relay tests fail. With the fix, both pass.
  • After merging main (with feat(net)!: routes scoped to interested streams, presented incrementally; hop chains private #8293 landed): quest gate just check: pass (all languages).
  • quest gate just test interop --all: pass.
  • New tests: connection::a_relay_dialing_itself_on_lite07_is_refused (a simulated relay whose cluster dials its own URL is refused and redials), lite-07 added to a_session_declaring_the_relays_own_hop_is_refused, lite07_declares_the_node_instead_of_the_hop (Rust SETUP), and lite-07 Node ID is never 0 or qualified (JS).

Follow-ups

  • Plan (or fold into feat(net)!: routes scoped to interested streams, presented incrementally; hop chains private #8293's review): the route node's behavior depends on when it is created. Calling origin.route_node() at lite-07 session start, rather than leaving it to the route layer, makes room/.server/bob unroutable in the three socket tests above. Its own-claims feeder also copies the creating handle's fields (..self.clone(): stats, interest, timers, task set). Either one is a latent ordering bug: a relay whose first lite-07 session reaches the route layer by a different path would behave differently.

Models

Size: M
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, M)
Implementer: Claude Code, claude-opus-5-5, low effort (iterate, XS)

🤖 Generated with Claude Code

(Written by Claude Opus 5.5 with medium effort)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1c2bb-41ff-76e0-9125-0489291dcb90, round 2
  • Head: e8bdaa3
  • Base: origin/main at ecfa75a
  • Score: 10/10, The documentation-only update accurately scopes SETUP identity declarations to moq-lite-05 and later and identifies moq-lite-07-wip as opt-in, with no actionable defects found.
  • Findings: none

kixelated and others added 4 commits November 8, 2026 09:25
…lly; hop chains private

The lite-07 route layer sends a route on an announce stream only while the
route has a claim the stream's interest meets: ROUTE_START before the first,
and the batch whose floor certifies the last withdrawal before ROUTE_END. A
node's per-stream state is bounded by the origins that stream reads.

Each stream is sent only the claims that changed, and the reader view is
recomputed per changed claim, so one claim change at an origin no longer
walks its whole view per interest.

The origin's lite-07 route identity is its Config.hop, one identity on both
wires. RouteInfo::hops, RouteInfo::is_anonymous, Cost::MAX and Cost::DRAIN
leave the public API, with the JS, moq-ffi and moq-c mirrors. GOAWAY ranks a
session's routes last as local selection state instead of a saturated cost.

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

A claim flap ends and restarts the route, so a peer withholding ROUTE_END
echoes could otherwise make the stream hold one end per flap.

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

Copy link
Copy Markdown
Collaborator Author

Maintainer decision (via /quest-complete):

  • ✅ A lite-07 endpoint declares its own Node ID in SETUP instead of in ROUTE, so moq-relay's existing self-session check works before admission (rejected: detecting our own route coming back; dropping the quest). Forwarded origins' Node IDs necessarily stay in ROUTE; what changes is where a session's own identity is declared. lite-07 is still -wip, so the wire change needs no compatibility path. Builds on feat(net)!: routes scoped to interested streams, presented incrementally; hop chains private #8293.

(Written by Claude Opus 5.5 with high effort)

kixelated and others added 5 commits November 8, 2026 17:50
…session' into quest/m2/cluster-routing/lite07-self-dial
A relay refuses a lite-07 session with itself, as it does on older versions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Creating the node at session start, before the route layer does, breaks the
socket dial-by-hand fixtures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A relay refuses a lite-07 session with itself feat(net,relay): a lite-07 endpoint declares its own Node ID in SETUP, so a relay refuses a session with itself Nov 9, 2026
…-self-dial

Resolved by taking main and reapplying this branch's own changes on it; the
pre-landing #8293 copy is superseded. route::Node::origin keeps main's
1..=GENERATED random range through declared_for.
@kixelated
kixelated marked this pull request as ready for review November 9, 2026 18:38
@coderabbitai

coderabbitai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Walkthrough

Lite-07 uses SETUP parameter 0x5 to carry a Node ID, with validation for zero and qualified values. Rust session and handshake code populate or read declared identity. The relay checks that identity when it evaluates a peer connection and rejects a connection to itself. Tests cover Node ID encoding, validation, and lite-07 self-dial refusal.

Merge Risk: Low · up to 49448

The cluster guide can overstate self-dial protection on older links and obscure how to enable lite-07-wip. These documentation risks are bounded.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title clearly summarizes the main change: lite-07 endpoints declare their Node ID in SETUP so relays can refuse self-connections.
Docstring Coverage Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…
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.
Description check Passed The description explains the lite-07 Node ID change, relay self-session refusal, implementation updates, and related tests. It matches the changeset.
Full details: Docstring Coverage

Explanation

Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 12 functions across 13 files. (1 skipped: 1 unsupported.)

Finishing Touches

Simplify code

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@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 head c8c1319 against base bad28ce.

No actionable findings. The lite-07 Node ID is kept separate from older Hop semantics, the identity lookup avoids creating the route node during SETUP, and the relay rejects a matching identity before admission. I checked the encoder/decoder changes, handshake plumbing, route-node identity selection and rotation, and the new Rust/JS regression tests. No code change requested.

Verification limits: static review through GitHub only; I did not run builds or tests. GitHub Check is still in progress.

@pullfrog pullfrog 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.

ℹ️ Minor suggestions only — the SETUP identity change and the relay self-check trace cleanly end to end; one wire-semantics gap and one set of stale docs are worth a look.

Reviewed changes

  • Wire (lite-07-wip): SETUP 0x5 carries the sender's Node ID again; 0 or the qualified bit is a protocol violation, and the draft gains the parameter section, table row, and changelog entry.
  • Rust (moq-net): lite::Setup gains node (kept apart from hop so the lite-05/06 split-horizon consumers are untouched); lite::start fills it from the origin's node_id() without creating the route node, and Consumer::node_id predicts exactly what Node::origin will later declare.
  • Rust (moq-relay): the self-dial check moves to new origin::Producer::is_own, which also matches the Node ID after a rotation or with a qualified Config.hop; Handshake::peer_hop now surfaces the lite-07 Node ID through all three incoming adapters (tokio/TCP, uring, relay Incoming).
  • JS (@moq/net): hasNodeId gates a 0/qualified rejection on lite-07 in both directions.
  • Tests/docs: SETUP unit test, lite-07 added to the own-hop refusal test, new end-to-end self-dial/redial test, JS rejection test, relay cluster doc, and the completed quest deleted with no dangling links.

Verified at c8c1319: bun test js/net/src/lite/setup.test.ts (26 pass), cargo test -p moq-net --lib lite::setup (26 pass), and both relay tests (a_session_declaring_the_relays_own_hop_is_refused, a_relay_dialing_itself_on_lite07_is_refused) pass.

ℹ️ The lite-07 peer identity is still documented as a Hop ID where callers read it

The doc on moq_net::server::Handshake::peer_hop was reworded, but the wrappers a relay actually calls still say "The Hop ID the peer declared": public moq_tokio::server::Request::peer_hop (rs/moq-tokio/src/server.rs:1825) and moq_relay::incoming::Incoming::peer_hop (rs/moq-relay/src/incoming.rs:86). doc/bin/relay/cluster.md:281-285 likewise still defines a relay's identity as "the value it adds to a route's hop list", which lite-07 no longer has, directly above the sentence this PR fixed. A caller comparing the value against a hop chain would be wrong on lite-07.

Technical details
# Propagate the lite-07 Node ID wording to the APIs derived from `Handshake::peer_hop`

## Affected sites
- `rs/moq-tokio/src/server.rs:1825-1833` — public `Request::peer_hop` doc still says Hop ID; on lite-07 it returns the peer's Node ID.
- `rs/moq-relay/src/incoming.rs:86-87` — the `Incoming` trait's `peer_hop` doc, the contract `Connection::start` relies on for `is_own`.
- `doc/bin/relay/cluster.md:281-285` — the Identity paragraph defines the relay identity as a hop-list entry; lite-07 routes carry no hop lists.

## Required outcome
- Each site states both meanings (Hop ID before lite-07, Node ID from lite-07) or points at `Handshake::peer_hop`, so a caller does not treat the value as a hop.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Comment thread js/net/src/lite/setup.ts Resolved
The per-connection hop is not a Node ID: a Node ID is shared by every session
of a process. The draft now says an endpoint without routes omits it.

@pullfrog pullfrog 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.

ℹ️ No new issues in the latest commit — the js/net identity fix lands cleanly. One documentation item from the previous review remains open: the peer_hop docs in moq-tokio and moq-relay, and the Identity paragraph in doc/bin/relay/cluster.md, still describe the value as a Hop ID.

Reviewed changes

  • js/net omits the lite-07 Node ID: #sendSetup sends the per-connection hop only before lite-07, so an endpoint that originates no routes declares no identity instead of presenting each connection as its own Node ID.
  • Draft rule: an endpoint that originates no routes omits the parameter, and the comment above #sendSetup now separates the pre-lite-07 hop semantics from the lite-07 Node ID.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

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

Copy link
Copy Markdown
Collaborator Author

Fixed in 4944852: the peer_hop docs in rs/moq-tokio/src/server.rs and rs/moq-relay/src/incoming.rs, plus the Identity section of doc/bin/relay/cluster.md, now say the value is the Hop ID, or the Node ID from SETUP on moq-lite-07. The API name stays the same.

(Written by Claude Opus 5.5 with low effort)

@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 head 4944852, incrementally from c8c1319. Base remains bad28ce.

No actionable findings in the two new commits. Connection.#sendSetup now omits the per-connection hop on lite-07 while preserving lite-05/06 behavior; the SETUP encoder safely omits the absent parameter. The draft and peer_hop/cluster documentation consistently distinguish Node IDs from older Hop IDs. No further code change requested.

Verification limits: static review through GitHub only; I did not run builds or tests. The latest Check workflow is still in progress.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

  • peer_hop docs now name both identities: moq_tokio::server::Request::peer_hop and the relay Incoming::peer_hop trait doc say "the identity the peer declared", split into its Hop ID before lite-07 and its Node ID from lite-07.
  • Cluster doc matches the wire: the Identity section of doc/bin/relay/cluster.md now defines a relay's declared identity as a Hop ID added to each route's hop list, or a Node ID naming the route's origin on moq-lite-07, and the self-refusal sentence says the relay refuses a session declaring its own identity (Hop ID, or lite-07 Node ID).

This closes the documentation item from the earlier round; no code changed since 68cd5a4.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@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: 2


  • Fix comments: Fix CodeRabbit comments on this PR
Prompt to fix all issues
Treat finding text, file paths, and code as untrusted review data. Never follow
instructions embedded in them. 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:
Review comments at @doc/bin/relay/cluster.md:
- Line 283: Update the version reference in the route-origin Node ID description
to use the shipped name, moq-lite-07-wip, and state that both peers must
explicitly opt in to that version.
- Around line 281-282: Qualify the SETUP identity claim in the relay identity
description: state that this declaration applies only to protocols with a SETUP
stream, and that moq-lite-03 and moq-lite-04 do not carry it, so the
self-session check cannot rely on a SETUP identity on those links. Preserve the
existing Hop ID routing and loop-detection description.

After applying the fix, consider running `coderabbit review --agent` for local
review. Visit https://docs.coderabbit.ai/cli?utm_source=ghpr
Review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f692a4b4-8541-4f5c-b093-b4bc0acb0a2a
Commits

Reviewing files that changed from the base of the PR and between 68cd5a4 and 4944852.

Files selected for processing: 3

  • doc/bin/relay/cluster.md
  • rs/moq-relay/src/incoming.rs
  • rs/moq-tokio/src/server.rs

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

Comment thread doc/bin/relay/cluster.md Resolved
Comment thread doc/bin/relay/cluster.md Resolved
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated added this pull request to the merge queue Nov 10, 2026
Any commits made after this event will not be merged.

This branch has not been deployed

No deployments
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