feat(p2p): add a TCP (noise + yamux) transport alongside QUIC - #600
feat(p2p): add a TCP (noise + yamux) transport alongside QUIC#600MegaRedHand wants to merge 2 commits into
Conversation
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. Add a TCP transport (noise + yamux) alongside QUIC, bound to the same port number as the existing QUIC listener, so libp2p can race a peer's `tcp` address in the same dial attempt when its `quic` does not answer. TCP and UDP are separate namespaces, so one `--gossipsub-port` still names both listeners. Admission now accepts a peer advertising `quic`, `tcp`, or both, and orders the resulting dial list QUIC first so it stays the preferred path; it rejects only when neither is present, which renames `NoQuicPort` to `NoDialableTransport`. `SwarmCommand::Dial` and `SwarmHandle::dial` carry a full `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. `Bootnode` gains a `tcp_port`, read from the ENR under the same "a `0` port is absent" rule the other two follow, and `as_discovery_node` now hands ethrex the real port instead of the hardcoded `0`. The local ENR advertises `tcp` too, at the same port number, so a peer applying the same rule we do can reach us over the transport that just landed. That also clears lighthouse's discovery predicate, which requires `enr.tcp4().is_some() || enr.tcp6().is_some()` on top of the `fork_digest` comparison; the lean digest is still the cross-client dummy, so a beacon-chain client rejects us on that instead. "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. The libp2p fork already exposes what this needs: a `.with_tcp(...)` insertion ahead of `.with_quic()` in the SwarmBuilder chain, no manifest change. Tests: admission cases for tcp-only, quic-only, both (QUIC first), and each transport at port 0 with the other absent; a new test builds two real swarms through `build_swarm` and proves they complete a TCP connection end to end, so a regression to QUIC-only hangs to the timeout rather than passing.
discovery.md: the ENR entry table lists `tcp` alongside `quic` and `udp`, with a note on why they share a port number and what advertising `tcp` buys against lighthouse's discovery predicate. The "which peers get dialed" rule accepts either transport, a paragraph explains that a dial carries every address at once rather than one per retry, and the bootnode table drops the old "quic absent implies seed-only" framing since `tcp` can now carry a bootnode too. CLAUDE.md: the Networking section's transport line mentions TCP, and the discovery bullets track the new ENR entry and admission rule.
🤖 Kimi Code ReviewAutomated review by Kimi (Moonshot AI) |
🤖 Codex Code ReviewNo blocking findings.
The actual transport change looks internally consistent: ENR generation advertises I could not run Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewReview: PR 600 — TCP transport alongside QUICOverall 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 Findings, in descending order of severity:
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 |
1 participant
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-portstill names both.The reason it matters: a peer that advertises a
quicentry 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'squicandtcpaddresses within one dial attempt..with_tcp(nodelay, noise, yamux)then.with_quic()/udp/<port>/quic-v1/tcp/<port>quicentry required (NoQuicPort)quicortcp(NoDialableTransport)MultiaddrDialOptswith the full list, QUIC firstBootnodequic_port,udp_porttcp_port, same "port 0 is absent" ruleip/udp/quic/secp256k1/eth2/attnetstcpas_discovery_nodetcp_port: 0SwarmCommand::DialandSwarmHandle::dialnow carry aDialOpts(peer id plus address list) rather than a singleMultiaddr, 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
transportfield, 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
tcpalso clears lighthouse's discovery predicate, which requiresenr.tcp4().is_some() || enr.tcp6().is_some()on top of thefork_digestcomparison. That is only half the story: the lean fork digest is still the cross-client dummy0x12345678, 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 carrieslibp2p-tcp,libp2p-noiseandlibp2p-yamux.Tests
0with the other absent.two_swarms_connect_over_tcpbuilds two real swarms throughbuild_swarm(port0, so it cannot collide with a running node), learns the listener's TCP address off its ownNewListenAddr, dials it, and polls both until each reportsConnectionEstablishedovertcp. A regression to QUIC-only hangs to the 10s timeout rather than passing.local_enr_advertises_udp_and_quic_but_no_tcp→local_enr_advertises_udp_quic_and_tcp, and the twoNoQuicPortrejections are renamed to say "and no tcp".make lintclean;make testgreen, spec fixtures included (122 forkchoice, 119 SSZ).Deployment note
Nodes now bind an extra TCP port at
--gossipsub-portand advertise it in their ENR. A containerised devnet that maps only-p 9001:9001/udpwill publish atcpentry 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.