[biafra23]

Why

#579 / #588 added QUIC to the Rust engine, but only as a fallback after TCP. The reason was one measurement against Nimbus v26.9.1. This PR revisits that against the other clients:

Client	QUIC status
Lighthouse	On by default since v4.5 (2023). Uses rust-libp2p, the same stack as our host.
Grandine	Built on Lighthouse's network stack; QUIC on by default.
Prysm	quic-go, on by default since v5.2.0.
Lodestar	On by default since v1.42 (introduced as opt-in in v1.41).
Teku	jvm-libp2p, on by default in 26.9. Its missing TCP fallback (#11397, #11403) was fixed in October.
Nimbus	QUIC is new in 26.8/26.9; the stall was measured on v26.9.1. v26.10.1+ vendors nim-lsquic 0.11.2: the no-progress abort is off (#180) and connections with no open stream get keep-alive (#181). Neither fix is known to address stream opens hanging.

The spec changed too. consensus-specs#5330, merged 2026-07-06, makes QUIC a MUST and the primary transport, TCP a SHOULD fallback, and says to dial QUIC first.

One network-level fact shapes the design. Teku #11397's go-libp2p census found that about 30 % (183 and 177 of 600) of QUIC-advertising mainnet peers answered over TCP but timed out over QUIC: their UDP port isn't forwarded. So QUIC first needs a fallback that learns, not one that waits out the 5 s QUIC handshake on every redial.

What changes

• QUIC-only peers are candidates (discovery::filter_candidate). A record with a quic/quic6 port and no TCP port becomes a candidate, with its QUIC address as the peer's addr. IPv4 is still preferred over IPv6.
• QUIC first (sync::Peer::dial_addrs). The order is now QUIC, then TCP. The pool follows a record that gains or drops TCP without losing the other transport. A QUIC-only peer that later gains TCP takes the TCP address as its identity, so its cache key changes once to a form the Java engine can dial.
• reqresp::TcpFirst: a per-process set of peers that are dialed TCP first. It is shared by the client and the swarm and capped at 4096. A peer goes into it when:
  • its QUIC dial fails and TCP then reaches it (unforwarded UDP). A failure that breaks every dial, such as no network, moves no one;
  • a request times out on its QUIC connection. That connection is then closed (demote_stalled_quic), because requests ride any existing connection and a stalled one would otherwise carry all of them. This closes the old TODO "TCP-first is a dial order, not a connection choice";
  • it identifies as Nimbus. A QUIC connection it identified on is closed once.
• One retry. A request lost to a connection this node closed is retried once, TCP first, instead of being counted as a failure against the peer.
• Java engine. It is TCP-only, so it now skips QUIC-only keys in the shared CL peer cache (ChainStack.tcpDialable). Without this it would dial them and count failures that evict them from the file for both engines. The filter also covers the proven-bootstrap and served-range maps.
• Docs. Updated the TODO entry, the reimplementation spec §10.1, the privacy/Tor table (QUIC is now the main CL path, so routing CL over Tor would need QUIC off), lc-server-design and implementation-status.

Testing

• cargo test -p myotis-net --lib: 833 passed.
• New end-to-end test: a dead QUIC address falls back to a live TCP one and moves the peer to TCP first.
• New unit tests:
  • QUIC-only ENRs (IPv4, IPv6, and an IPv4-QUIC-only endpoint preferred over an IPv6-TCP one);
  • pool behaviour as a record gains and drops transports;
  • dial reordering;
  • Nimbus agent detection;
  • closing a QUIC connection after a timeout (TCP left alone);
  • closing a Nimbus QUIC connection once;
  • the one retry after a demotion.
• cargo clippy --workspace --all-targets -- -D clippy::incompatible_msrv (the rust-clippy job's command) is clean. cargo clippy -p myotis-net --all-targets adds no new warnings in the changed code, and roost clippy with -D warnings is clean.
• The Java test (TcpDialableClPeersTest) could not run locally: Maven Central returned HTTP 429 to this sandbox. CI will run it.

Not verified / owed

• No live run. The sandbox has no UDP egress, so neither QUIC first nor the fallback was tried against a real peer.
• Re-measure Nimbus. Run against zbox's Nimbus on v26.10.1 or later over QUIC alone, then drop reqresp::is_nimbus_agent if a QUIC connection holds across many streams.
• Still open from before: roost does not listen on QUIC yet, and the shipped pins are still /tcp/.

https://claude.ai/code/session_013jBQepLws2hUP2yijLdQjs

---

Generated by Claude Code

[claude]

Minor (hidden coupling): agent here is log_client_id(&info.agent_version) — the sanitized display form. That function's contract (el/pool.rs:553-570) says "Display text only — never a key, never compared", and where it concedes an exception it enumerates it explicitly ("The family prefix agent_serves_one_period matches on survives both the strip and the cap"). This adds a second comparing consumer without updating that note, so a future sanitizer change (different prefix handling, a tag prepended, …) would silently break Nimbus detection with no local hint.

Behavior today is correct — control-char stripping and the 64-char cap both preserve a leading nimbus — so this is doc/robustness only. Either match on the raw info.agent_version (in scope one line up), or add is_nimbus_agent to the enumerated prefix-matching consumers in log_client_id's doc and in the sanitization comment above (lines 1780-1784).

[biafra23]

Accepted: fixed in 56ad76d. The Identify handler now matches on the raw info.agent_version, not the display form from log_client_id, and a comment there explains why. That function's "never compared" contract holds again, so is_nimbus_agent no longer depends on how the sanitizer treats prefixes.

---

Generated by Claude Code

[claude]

Minor (stale doc): add_with_quic's docstring (lines 1856-1860) still ends "Never a cache key, so no identity changes." — this hunk makes both halves false for the QUIC-only case: a QUIC-only peer's QUIC address is its cache key (it sits in addr, and cache_key() formats {addr}/p2p/{id}), and this line changes the identity exactly once when the record gains TCP. The adjacent test even pins the key move ("key moved to TCP once").

Cache-key stability is precisely what a reader consults that doc for, so it's worth rewording — e.g. "The QUIC side-address is never a cache key; the one identity change is a pooled QUIC-only peer whose record gains TCP (its key moves once, to the shape the Java engine can dial)."

[biafra23]

Accepted: fixed in 56ad76d. The add_with_quic doc now says the QUIC side address is never a cache key, and that there is exactly one identity change: a pooled QUIC-only peer whose record gains TCP moves its key once, from the QUIC address to the TCP one, a shape the Java engine can dial. It also covers the reverse case: a QUIC-only record for a peer pooled at TCP keeps the TCP key and learns the QUIC address as quic. Both cases are pinned in a_quic_only_peer_is_pooled_and_dialed_at_its_quic_address.

---

Generated by Claude Code

[claude]

Observation (pre-existing, adjacent — follow-up material, not a blocker): this fixes the read side, but the Java engine's write side can still mint dead TCP keys for the same peers. The discv5 callback above (line 1067) caches Enr.toLibp2pMultiaddr(), which for a record with no tcp field falls back to the discv5 UDP port rendered as /tcp/<udp-port> (Enr.java:227-239, "CL convention: same port for both"). For a genuinely QUIC-only peer that fabricated address is dead: this engine dials it and charges it failures, and the Rust engine replays it too (as a distinct key beside its own /udp/<port>/quic-v1 entry, first-in-file wins the pool identity) — the same eviction noise this PR removes on the read path, just under a fabricated key.

The fallback predates this PR and was a reasonable heuristic while every client listened on TCP; now that QUIC-only records are an expected production shape (the premise of this PR), an ENR carrying a quic field but no tcp field is positive evidence the fallback is wrong for that record. Worth a follow-up: skip the UDP fallback when the record advertises QUIC without TCP.

[biafra23]

Accepted: fixed in this PR (56ad76d) rather than as a follow-up. I checked Enr.toLibp2pMultiaddr (core/.../enr/Enr.java): with no tcp field it falls back to udpAddress() and renders it as /tcp/<udp-port>, exactly as you describe. That fabricated key reaches both the discv5 callback (ChainStack line 1066, clPeerCache.add) and the DNS-tree seeding (buildAndStartBeacon). The fallback is now skipped when the record carries a quic field: such a record yields no multiaddr. A record with udp and neither tcp nor quic keeps the old same-port fallback. EnrTest.aQuicOnlyRecordYieldsNoFabricatedTcpAddress pins both. It uses hand-built records with ENR_STRING's key and a zero signature, since decode doesn't verify signatures. :core compiles locally, but its tests couldn't run in my sandbox (Maven Central returned 429), so CI has to confirm them.

---

Generated by Claude Code

[claude]

Review: QUIC first, QUIC-only peers dialable

Independent review of the full diff, held against the repo rules in CLAUDE.md and read in context of the surrounding code. No blocking defects found. Three minor inline comments posted (one stale docstring, one hidden coupling, one pre-existing adjacent gap worth a follow-up).

Standing rules

• Trust model — clean: this is transport selection only. No new path returns peer data without verification; sync-committee/accumulator checks are untouched. The TcpFirst set is peer-influenceable (self-reported agent string, dial behavior), but it only steers which connection this node opens, never what is accepted — worst case for an adversary is changing its own dial order, and a Sybil filling the 4096 cap merely freezes demotions (others keep the spec order: slower dials, never wrong answers).
• Data sources — still libp2p only; no HTTP or trusted-RPC fallback anywhere in the diff.
• API boundary — all changes are inside the engines (node-core, rust/myotis-net); no host module touched.
• Android — the ChainStack additions use ArrayList/LinkedHashMap/String.contains (minSdk-29 safe); List.of/Map.of in the new test are on the documented safe list.
• Panic-free FFI — every new lock access handles poisoning via into_inner(); no unwrap/expect on production paths (tests only); the one debug_assert! predates the PR.

Correctness points verified (all sound)

• demote_stalled_quic runs before complete(...) in on_rr_event, so the demotion is visible to the failing caller when its error arrives — the client retry condition cannot miss it (no race).
• quic_conns is removed both in demote_stalled_quic and on ConnectionClosed, so it is bounded by open connections and the close-once semantics hold (pinned by tests, including the many-timeouts-one-poll case).
• No lock is held across an await; prefer_tcp/dial_order critical sections are short and never nest with peer_agents.
• Peer::dial_addrs can never return empty, and the quic == addr dedup covers the re-announced QUIC-only pin (test pins it).
• The pool identity transition (QUIC-only gaining TCP) correctly excludes statics, and the dns-name-pin protection still holds; a static numeric pin whose record goes QUIC-only follows the record via the existing refresh path (PR 322), consistent with its intent.
• ChainStack passes lightClientConfirmed/Denied through unfiltered, and that is fine: in BeaconLightClient they are membership sets used only to rank peers already in the dial list, so a QUIC-only key there is inert. The three filtered spots are exactly the ones that produce dial targets or preference.
• isTcp as a /tcp/ substring check is safe for this cache: hostnames and base58 peer ids cannot contain a /tcp/ segment.

Bounded oddities — fine to ship, noting them for the record

1. Retry guard is slightly wider than its comment. It also retries (a) the request whose own QUIC timeout caused the demotion — intended per the design docs, and it is what gives the triggering request its TCP shot — and (b) a request that timed out on a TCP connection while an unrelated QUIC timeout demoted the peer concurrently: a spurious extra attempt, bounded to one per caller and only in the demotion window. Worst-case added latency is one extra updates_by_range timeout (60 s), once per peer.
2. At TCP_FIRST_CAP the doc promises "a slower dial, never a wrong answer", but a stalling peer at cap actually degrades to a close-per-timeout redial loop (close still happens, memory does not). Academic at 4096 vs pool size.
3. A dual-stack record with IPv4-QUIC-only plus IPv6-TCP now loses its TCP fallback entirely (IPv4 still wins the family order, and families never mix). The new test pins this as intended; with ~30 % unforwarded UDP it is the one shape strictly worse than before — but the shape is synthetic (a client with TCP on advertises tcp4 when it has ip4).
4. A QUIC-only peer whose QUIC port later moves keeps the dead old address as identity/cache key (dialed second, forever this process). Consistent with the existing evict-and-rediscover self-healing trade-off for ordinary peers; just noting the QUIC-only variant inherits it, and the shared cache can carry one orphaned key per QUIC-to-TCP transition until eviction ages it out.
5. request_in_order now clones the wire bytes on every attempt (the old last-attempt mem::take is gone) plus one upfront clone for the possible retry. Request payloads here are tens of bytes, so this is noise — flagging only in case the regression was not deliberate.

Tests
Good coverage: an end-to-end dead-QUIC-to-TCP fallback (costs the ~5 s QUIC handshake timeout, documented in the test), demote/close-once, Nimbus close-once with the second-connection exception, the one-retry contract including the no-demotion case, pool transitions in both directions, and the QUIC-only ENR shapes including the port-0 and no-transport rejections. The dead-UDP-port trick (bind then release) has the usual tiny re-bind flake window; acceptable. The Java test could not run in the author sandbox per the description — CI must confirm it before merge.

Verdict: ready once CI is green and the inline nits are dispositioned — nothing found that blocks.

[biafra23]

Reply to the review summary's "bounded oddities". The three inline comments are answered on their threads and fixed in 56ad76d.

1. Retry guard is slightly wider than its comment … (b) a request that timed out on a TCP connection while an unrelated QUIC timeout demoted the peer concurrently

Accepted as is, no change. (a) is intended: the request whose own timeout stalled the QUIC connection is exactly the one that should get a TCP attempt. (b) is real, but it is capped at one extra attempt per caller and needs a QUIC and a TCP connection to the same peer timing out in the same window. Narrowing it would mean tagging each failure with its connection's transport through complete(). That costs more code than the rare extra attempt it would save.

2. At TCP_FIRST_CAP … a stalling peer at cap actually degrades to a close-per-timeout redial loop

Accepted as is. It's true: past the cap, the close still happens but the peer isn't remembered. Reaching 4096 distinct demoted peers takes a pool of at most MAX_POOL running for a very long time. Even at the cap, each loop iteration costs one request timeout and one redial, the same per-request cost as before this PR, and it never produces a wrong answer.

3. A dual-stack record with IPv4-QUIC-only plus IPv6-TCP now loses its TCP fallback

Accepted as is. As you note, a client with TCP enabled advertises tcp beside ip. The test pins the family order on purpose: IPv4 first, families never mixed, unchanged from before.

4. A QUIC-only peer whose QUIC port later moves keeps the dead old address as identity/cache key (dialed second …)

Accepted as is. I checked add_inner: the record's new QUIC addr becomes quic, so the new port is dialed first and the old one second. That is the evict-and-rediscover trade-off ordinary peers already have, as you say.

5. request_in_order now clones the wire bytes on every attempt

Deliberate. The retry needs the bytes again, and request payloads are tens of bytes (a Status, a by-range header, an empty finality request).

---

Generated by Claude Code

[biafra23]

CI note: Build macOS .dmg (arm64) shows as cancelled on both heads of this PR (runs 42172514502 and 42174270991). That is not this PR's doing. The job is cancelled within a second of being created on macos-14, before a runner picks it up and with no log. Every recent Desktop DMG run ends the same way, including the push runs on main for #723 (42144703078), #728 (42145199236), #725 (42174198719) and #730 (42174331306), so it looks like a macOS runner availability or billing problem for the repository. There is no fix for it inside this PR. The diff doesn't touch the desktop host or the jpackage path, but the dmg leg still hasn't built on any head of this PR.

---

Generated by Claude Code