feat(p2p): add a TCP (noise + yamux) transport alongside QUIC

[MegaRedHand]

What changes

A libp2p TCP transport (noise + yamux) is added alongside QUIC, bound to the same port number as the existing QUIC listener. TCP and UDP are separate namespaces, so one --gossipsub-port still names both.

The reason it matters: a peer that advertises a quic entry nothing answers on leaves a QUIC-only node with no second address to try. The dial times out and the peer is never reached. With both transports offered, libp2p races a peer's quic and tcp addresses within one dial attempt.

Area	Before	After
Swarm transports	QUIC only	.with_tcp(nodelay, noise, yamux) then .with_quic()
Listeners	/udp/<port>/quic-v1	that, plus /tcp/<port>
Admission	quic entry required (NoQuicPort)	quic or tcp (NoDialableTransport)
Dial target	one Multiaddr	DialOpts with the full list, QUIC first
Bootnode	quic_port, udp_port	plus tcp_port, same "port 0 is absent" rule
Local ENR	ip/udp/quic/secp256k1/eth2/attnets	plus tcp
as_discovery_node	hands ethrex tcp_port: 0	hands it the real advertised port

SwarmCommand::Dial and SwarmHandle::dial now carry a DialOpts (peer id plus address list) rather than a single Multiaddr, since that list is what makes the per-dial race possible. Static bootnode dialing, bootnode redialing, and the discovery dial loop all go through it.

"Peer connected" gains a transport field, read off the connection's own multiaddr rather than off which address we dialed, so a live run can show which path actually carried it.

Advertising tcp also clears lighthouse's discovery predicate, which requires enr.tcp4().is_some() || enr.tcp6().is_some() on top of the fork_digest comparison. That is only half the story: the lean fork digest is still the cross-client dummy 0x12345678, so a beacon-chain client rejects us on that instead. The docs say so rather than claiming the gap is closed.

No manifest change. The pinned libp2p fork (2f14d0ec, features = ["full"]) already carries libp2p-tcp, libp2p-noise and libp2p-yamux.

Tests

• Four new admission cases: tcp-only, quic-only, both (asserting QUIC is ordered first), and each transport at port 0 with the other absent.
• two_swarms_connect_over_tcp builds two real swarms through build_swarm (port 0, so it cannot collide with a running node), learns the listener's TCP address off its own NewListenAddr, dials it, and polls both until each reports ConnectionEstablished over tcp. A regression to QUIC-only hangs to the 10s timeout rather than passing.
• Two existing tests are inverted along with the decision they pinned: local_enr_advertises_udp_and_quic_but_no_tcp → local_enr_advertises_udp_quic_and_tcp, and the two NoQuicPort rejections are renamed to say "and no tcp".

make lint clean; make test green, spec fixtures included (122 forkchoice, 119 SSZ).

Deployment note

Nodes now bind an extra TCP port at --gossipsub-port and advertise it in their ENR. A containerised devnet that maps only -p 9001:9001/udp will publish a tcp entry that is unreachable from outside. QUIC is dialed first and still works, so this degrades to today's behaviour rather than breaking, but adding the TCP mapping is what makes the fallback usable.

[github-actions]

🤖 Kimi Code Review

⚠️ Review failed: Kimi API request failed with status 404: Not found the model kimi-k2.5 or Permission denied

---

Automated review by Kimi (Moonshot AI)

[github-actions]

🤖 Codex Code Review

No blocking findings.

• crates/net/p2p/src/discovery/mod.rs:3-6 is now stale: it still says discovered peers are dialed “over libp2p QUIC” and that static bootnode dialing is untouched, but this PR changes both paths to race QUIC and TCP. Updating that comment would avoid future confusion.
• crates/net/p2p/src/discovery/mod.rs:181-184 is also slightly stale: it still describes the advertised ENR as carrying eth2, attnets, and quic, but tcp is now part of the published record too.

The actual transport change looks internally consistent: ENR generation advertises tcp, admission accepts tcp-only peers, static bootnode dialing and discovery dialing both use a single DialOpts with QUIC-first ordering, and the existing connected_peers guard still prevents a failed raced address from tearing down bookkeeping for a live connection.

I could not run cargo test -p ethlambda-p2p here. cargo/rustup tries to write under /home/runner/.rustup and /home/runner/.cargo, which are read-only in this environment, and switching to a writable cargo home would still require fetching an unfetched git dependency (leanSig) with network access unavailable.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[github-actions]

🤖 Claude Code Review

Review: PR 600 — TCP transport alongside QUIC

Overall this is a careful, well-tested port. Admission, dial-target construction, ENR handling, and bootnode parsing were all updated consistently — I couldn't find a stale reference to the old NoQuicPort variant or single-Multiaddr Bootnode.addr anywhere in the crate. The Vec::with_capacity(2) / QUIC-first ordering in both admit() (crates/net/p2p/src/discovery/admission.rs:173-179) and bootnode_dial_addrs() (crates/net/p2p/src/lib.rs:657-666) is duplicated but consistent, and the port-0-means-absent rule is applied uniformly to both transports. The new two_swarms_connect_over_tcp integration test is a real end-to-end check (not just a compile-time proof), which is the right level of rigor for a transport change.

Findings, in descending order of severity:

1. --gossipsub-port CLI help text is now inaccurate (bin/ethlambda/src/cli.rs:24-27). The doc comment still reads "UDP port for the libp2p QUIC listener," with no mention that it now also binds a TCP listener on the same port number. docs/discovery.md and CLAUDE.md were both updated to describe the dual-transport bind, but the flag's own --help text — the first place an operator looks — wasn't. Worth a one-line addition so ethlambda --help doesn't undersell what the flag does (e.g. for the deployment-note scenario in the PR body, where the fix is adding a TCP port mapping).

2. The core benefit depends on the pinned libp2p fork's dial concurrency, which isn't verified here. The whole design rests on the claim that "libp2p races a peer's quic and tcp addresses within one dial attempt" (PR body, and the comments at crates/net/p2p/src/discovery/dial.rs:113-115 and crates/net/p2p/src/lib.rs:333-335). Upstream libp2p does this via dial_concurrency_factor (default 8, concurrent), but this project pins a custom LambdaClass fork (2f14d0ec). If that fork's Swarm::dial happens to try addresses sequentially with a full per-address timeout instead, the fallback still works correctly but takes much longer to kick in than the docs imply. Given how central this assumption is to the PR's stated motivation, it'd be worth a quick confirmation against the fork's dial_opts/pool code (or an integration test that fails a first address and asserts the second connects well under a full dial timeout) rather than relying on upstream libp2p's documented behavior.

3. Minor grammar nit in the new error message at crates/net/p2p/src/lib.rs:639: "node advertises neither a quic, tcp nor a udp port" reads awkwardly — "node advertises none of quic, tcp, or udp" would parse more cleanly. Cosmetic only.

4. Expanded attack surface is acknowledged but worth double-checking operationally, not a code defect: this opens a second raw listener (TCP+noise+yamux) on every node, in addition to QUIC. The PR's "Deployment note" already flags the devnet port-mapping implication; the same applies to any firewall/security-group rules gating the existing QUIC UDP port — those now need a matching TCP rule for the fallback to actually be reachable from outside, not just bound locally.

No issues found in the fork-choice/attestation/state-transition/signature code — this PR doesn't touch any of it, as expected for a pure networking change.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[pablodeymo]

This switch to DialOpts::peer_id(...) changes the dial condition from Always (the old From<Multiaddr> path) to the default DisconnectedAndNotDialing, but the .unwrap() stayed. A bootnodes list containing the same node twice (same ENR pasted twice, or an old+new ENR for one secp256k1 key — both decode to one PeerId, and parse_enrs doesn't dedup) now panics on the second entry with DialError::DialPeerConditionFalse before listen_on ever runs — a file that booted fine pre-PR aborts the node. Either dedup by PeerId in parse_enrs or treat DialPeerConditionFalse as non-fatal (log + continue).

[pablodeymo]

The new TCP bind is a hard .expect(), so any process already holding TCP on --gossipsub-port (e.g. sharing the port number with the API server, which was legal pre-PR since only UDP was bound; or a stale container on tcp/9001) now aborts the node at startup after QUIC bound and bootnodes were dialed, instead of degrading to today's QUIC-only behaviour. build_swarm already returns Result, so this could be a typed error — and the CLI side should follow: validate_discovery still only checks the UDP clash with --discovery.port ("both bind UDP"), and the --gossipsub-port help text still reads "UDP port for the libp2p QUIC listener".

[pablodeymo]

"QUIC first, so it stays the preferred path" doesn't hold: the pinned fork's ConcurrentDial (swarm pool/concurrent_dial.rs) pushes up to dial_concurrency_factor (default 8) addresses into a FuturesUnordered at once, so with two addresses list order confers no preference — both handshakes run on every dial (2x sockets and crypto on both ends) and the mesh nondeterministically lands on whichever finishes first. To make the ordering real and TCP a true fallback: .override_dial_concurrency_factor(NonZeroU8::new(1).unwrap()) on the DialOpts builders, or dial TCP only after a QUIC failure. (The same ordering claim appears in lib.rs, swarm_adapter.rs and docs/discovery.md.)

[pablodeymo]

With tcp-only records now dialable, a beacon-chain bootnode list (the stated use case for clearing lighthouse's predicate) becomes a set of static dial targets with no fork-digest gate — parse_enr never reads eth2; that check lives only on the discovered-peer path (admission.rs). The TCP+noise+yamux handshake to a lighthouse node succeeds, the peer occupies a target_peers slot contributing zero subnets, and when the remote drops us for having no shared protocols, ConnectionClosed re-arms RetryPeerRedial every 12s with no backoff or cap — permanent hammering of third-party mainnet infrastructure. Applying the same eth2/fork-digest admission to static bootnodes (or at least a redial backoff/cap) would close this.

[pablodeymo]

Nice, careful port overall — admission, dial construction, ENR, and bootnode parsing move together, and two_swarms_connect_over_tcp is a real end-to-end check. I left inline comments on four issues I'd consider blocking (a startup panic on duplicated bootnode entries, a startup panic on an occupied TCP port, the dial-concurrency assumption behind "QUIC first", and ungated beacon-chain bootnodes + unbounded 12s redial). A few more, mostly non-blocking:

• Dockerfile still EXPOSEs only 9001/udp — the new TCP listener on 9001 isn't exposed, so a containerized node advertises a tcp entry nothing outside can reach, and every peer racing the address list burns a TCP connect timeout on a black-holed port: the failure mode this PR exists to remove, inverted. The deployment note already says the TCP mapping is what makes the fallback usable — the image should ship it: EXPOSE 9001/tcp. (Blocking-adjacent, but it's a one-liner.)
• ENR seq never bumps: the record now differs across versions (new tcp entry) but is still signed at constant INITIAL_ENR_SEQ = 1. ethrex's WHOAREYOU responder omits the record when enr_seq matches and the peer table only accepts strictly-higher seq, so peers that stayed up across an in-place upgrade keep the tcp-less record forever — the lighthouse-predicate benefit is invisible to exactly the peers that already know us. The constant seq is pre-existing, but this PR is the first payload change that makes it bite.
• Silently dropped dial candidate (discovery/dial.rs): with the condition now DisconnectedAndNotDialing, a synchronous rejection can happen for any in-flight peer, not just NoAddresses/Denied. The candidate was already popped and marked tried, and the early return skips the peer_attnets insert, so covered_subnets under-counts permanently. The comment above the block still describes the old reality.
• tcp written into the ENR unconditionally (discovery/enr.rs): the advertised ports are copied from config, not derived from the bound listeners (contrast discovery_port, which uses bound.port()). With --gossipsub-port 0 the ENR publishes quic:0 and tcp:0 — Some(0) satisfies lighthouse's tcp4().is_some() predicate, our own admit() would reject the record as NoDialableTransport, and the two OS-assigned real ports differ, breaking the "same port number" invariant. Readers filter 0 as absent; the writer has no matching guard.
• Duplication that's now load-bearing: the QUIC-first list builder is inlined byte-for-byte in both admit() and bootnode_dial_addrs, and the .filter(|p| *p != 0) tcp rule is open-coded twice with no read_tcp_port sibling to read_quic_port. Once the ordering is made real (concurrency = 1), divergence between the two sites changes behaviour silently — worth extracting one dial_addrs(ip, quic, tcp, peer_id) helper.
• Test asserting its own inversion: parse_enrs_keeps_a_quic_less_record_as_a_discovery_seed's comment still says the record is kept "with quic_port: None telling build_swarm not to dial it", but the first embedded mainnet ENR carries tcp=9000, so post-PR it is dialed — and nothing asserts Bootnode.tcp_port or exercises bootnode_dial_addrs. Renaming it and asserting bootnodes[0].tcp_port == Some(9000) / bootnodes[1].tcp_port == None would pin the new behaviour.

One free win worth adding to the description: the DialOpts switch also fixes redial stacking — the dial condition now refuses a new dial while one is in flight.