Skip to content

fix(net): give an anonymous dial its own hop (backport #5025) - #5126

Merged
kixelated merged 2 commits into
releasefrom
backport/anonymous-dial-hop
Oct 10, 2026
Merged

kixelated merged 2 commits into
releasefrom
backport/anonymous-dial-hop

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Backport of #5025 (2c0cf6728) onto release for 0.17.3.

Problem

When moq-relay dials a peer that declares no hop (draft-16, or a later draft without the cluster extension), moq_net::Client excluded only Hop::UNKNOWN from what it publishes. Routes learned from that peer were attributed to hop 0 and announced straight back to it, and a SUBSCRIBE from the peer could be routed back over the session it arrived on. Both sides end up subscribing to each other and no objects flow (seen in Fastly fanout). Release's server side already assigns each accepted session a random hop; the dialing side did not.

Approach

Cherry-pick of #5025: a dialed session whose peer declares no hop gets a fresh random hop per connection, matching the accepted side. with_peer_hop still pins a stable one, and a hop the peer declares on the wire still wins.

Adaptation for release:

  • quest/m0/README.md conflicted; kept release's version (the quest file fix(net): give an anonymous dial its own hop #5025 deletes never existed on release).
  • The new test rs/moq-net/tests/dial_split_horizon.rs did not compile on release, which predates moq_net_sim and announce::Event. A second commit ports it to #[tokio::test(start_paused = true)], tokio::time::{timeout, sleep}, tokio::spawn(run(driver)), and announce::Update/announce::Kind. Assertions are unchanged.

Impact

  • Public API: none. Client::with_peer_hop keeps its signature; only its docs change, as do moq_tokio::Client::with_peer_hop's.
  • Behavior: an anonymous dial now attributes the peer to a random per-connection hop instead of Hop::UNKNOWN. The assigned hop stays local and is never written into a hop chain.
  • Wire: none.

Tests

  • dial_split_horizon passes with the fix.
  • With rs/moq-net/src/client.rs reverted to release, it fails: an anonymous dial still needs a hop, or its routes echo (left and right both Hop { id: 0 }).
  • just check against origin/release passes, including the full moq-net and moq-tokio suites (4988 Rust tests passed, 10 skipped).

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 2 commits October 9, 2026 14:53
(cherry picked from commit 2c0cf67)

Co-authored-by: Grok 4.7 <noreply@x.ai>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Release predates moq_net_sim and announce::Event, so the backported test
runs on paused tokio time and reads announce::Update/Kind instead.

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

chatgpt-codex-connector Bot commented Oct 9, 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-10-09T22:21:33.976115Z 6182551 PR opened
ℹ️ 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.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Grok review of #5126 at 61825510 (backport of #5025 onto release)

A clean cherry-pick. A dialed session whose peer declares no hop now gets a random per-connection hop (Client::assigned_hop), the same way accepted sessions already do. That stops routes from echoing back to the announcing peer and stops SUBSCRIBEs from looping back. Both connect paths and start_lite pick one hop per connection and use it for both the excluding() filter and the session's peer_hop, so the two can't drift apart. The ported test checks the actual failure (both hops 0) and was confirmed to fail without the fix.

Blocking: none.

Non-blocking

  • assigned_hop() is evaluated once per connect/accept_lite call, so a reconnect gets a new hop. For a relay that redials the same anonymous peer, routes learned on the old session and the new one count as different endpoints until the old routes expire. That matches the accepted side and is presumably intended, but a short note in the release changelog would help operators who see brief duplicate routes during a redial.
  • Behavior change on release: callers that relied on anonymous dials collapsing to Hop::UNKNOWN (several dials sharing one exclusion) now get split horizon per dial. The PR says the public API is unchanged, which is true, but this belongs in the 0.17.3 notes as a behavior fix.
  • The test only covers draft-16. A lite-03 or earlier dial goes through start_lite, which has no test on release. Not required for a backport.
  • CI is still pending.

Verdict: MERGE once CI is green.

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

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merging at the maintainer's direction: the cherry-pick matches #5025 line for line in rs/, and the test adaptation only moves to tokio paused time and release's announce API. CI green on 61825510; Codex reviewed that head with no findings. Grok's notes (a 0.17.3 changelog line, lite-03 coverage on main) are follow-ups.

(Written by Claude Opus 5.5)

@kixelated
kixelated merged commit f0d1a8e into release Oct 10, 2026
7 checks passed
@kixelated
kixelated deleted the backport/anonymous-dial-hop branch October 10, 2026 00:41
This was referenced Oct 10, 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