feat(app): wire the QUIC transport feature - #702
Draft
emlautarom1-agent[bot] wants to merge 10 commits into
Draft
emlautarom1-agent[bot] wants to merge 10 commits into
emlautarom1-agent[bot] wants to merge 10 commits into
Conversation
`run` builds a `NodeType::QUIC` node when the `quic` feature is enabled, mirroring Charon's `wireP2P`. The node then listens on the configured `--p2p-udp-address`es alongside TCP, and `QuicUpgradeBehaviour` upgrades direct TCP connections to QUIC. Drop the unused `is_quic_enabled` helper: the upgrade behaviour is gated on the node type at construction. Closes #619.
Drive `run_upgrade_logic` and the connection/dial callbacks directly against a seeded `P2PContext`: a TCP-connected peer with known QUIC addresses is dialed once and its TCP connection kept until QUIC is established; peers without a direct TCP connection or a direct QUIC address are skipped; a redundant TCP connection is closed once direct QUIC exists; a TCP-only node never dials; and a non-QUIC connection or dial failure during an upgrade records a failure and arms the backoff.
A configured port of 0 leaves the choice to the kernel, so the configured address is not dialable. `Node` derives what it advertises from the addresses libp2p reports as bound: the external IP / hostname on those ports plus the bound addresses themselves, private ones withheld when configured. The set is recomputed on `NewListenAddr`, `ExpiredListenAddr` and `ListenerClosed`, applying only the difference so addresses registered through `add_external_address` are left alone. The relay server gets this from its startup poll, so it needs no explicit re-advertise after `wait_for_listen_addrs`, and the p2p test fixture needs no manual external address for its relay.
`Node` derives what it advertises from the swarm's own listener table, skipping relay circuit listeners: their ports belong to the relay, so with an external IP or hostname configured they would otherwise be advertised as `/ip4/<external>/tcp/<relay-port>`, and identify lists them as listen addresses regardless. The external IP is parsed once when the config is applied, so deriving the advertised set is infallible and the node stores only the two external overrides it reads.
Keep the tests that exercise a decision compose and end-to-end runs cannot surface: TCP stays up until QUIC is established and is not re-dialed while armed, a relayed-only peer is left to force-direct, only the direct TCP connection is closed once direct QUIC exists, and a failed upgrade keeps TCP and arms the backoff. Drop the one-line-guard and helper-level cases, and check the backoff state directly rather than through the mutating `should_skip`.
![emlautarom1-agent[bot]](https://avatars.githubusercontent.com/u/31224949?s=60&v=4){kind=small}
| let p2p_context_for_handle = p2p_context.clone(); | ||
|
|
||
| // Charon's `wireP2P` picks the node type off the same flag. | ||
| let node_type = if feature_set.enabled(pluto_featureset::Feature::Quic) { |
Contributor
Author
There was a problem hiding this comment.
Differs from Charon. The node type decides both the transports installed and whether QuicUpgradeBehaviour acts, so a QUIC node with no --p2p-udp-address still dials peers' QUIC addresses (outbound QUIC works without a listener). Charon's upgrade loop instead re-checks every tick whether the host has a QUIC listen or advertised address and stays idle otherwise. Deliberate: the construction-time gate is simpler and strictly more capable, which is also why the unused address-based is_quic_enabled helper is gone.
| fn readvertise(&mut self) { | ||
| let listen_addrs: Vec<Multiaddr> = self | ||
| .swarm | ||
| .listeners() |
Contributor
Author
There was a problem hiding this comment.
Non-obvious. The listener table is the source of truth here because the swarm updates it before yielding NewListenAddr/ExpiredListenAddr/ListenerClosed, and Node::poll_next runs handle_event only after the swarm has yielded, so it is always current at this point. Circuit listeners are skipped because addr_port would pick up the relay's port and, with an external IP configured, advertise /ip4/<external>/tcp/<relay-port>.
`P2PConfig::multiaddrs` renders each socket address through the same `with_transport` builder as the external addresses, so the two shapes cannot drift and rendering cannot fail once the address has parsed.
One test follows a TCP-connected peer from the first tick to the closed TCP connection; the failure cases share a setup and a check closure; and the advertised-address test waits for a direct and a relayed listen address rather than a fixed count.
Matches Charon's upgrade loop: with a direct QUIC connection in place, every TCP connection to the peer is redundant, relay circuits included. The relay manager re-routes a peer only once its last connection drops, so the circuit returns if the direct connection fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #619.
Summary
runbuilds aNodeType::QUICnode when thequicfeature is enabled, the one conditional Charon has inwireP2P. The node then binds the configured--p2p-udp-addresses alongside TCP, andQuicUpgradeBehaviour, installed on every node and gated on the node type, upgrades direct TCP connections to QUIC.quicstays Alpha, sorunwithout--feature-set-enable=quicis unchanged.Two smaller pieces ride along. The upgrade behaviour's decision logic gets deterministic unit tests driven against a seeded peer store. And
Nodederives what it advertises from the swarm's own listener table rather than the configured addresses, so a configured port of 0 never leaks into identify; relay circuit listeners are left out of that derivation, since their ports belong to the relay. The relay server gets its advertised addresses from that same path.Differences from Charon
--p2p-udp-addressnever attempts upgrades. Pluto gates on the node type, so such a node still dials peers' QUIC addresses, which works for outbound QUIC. Deliberate: simpler, and strictly more capable.AddrsFactorybut derives external IP / hostname addresses from the configured ports. Pluto derives both from the bound ports.Out of scope
The compose harness defaults to
--feature-set=alpha, so with this change every smoke node is a QUIC-type node without a UDP listener: one startup warning, QUIC transport installed, no upgrade attempted because no peer advertises a QUIC address. Charon nodes in the mixed scenarios behave the same. Giving run nodes ap2p-udp-addressso the smoke tests exercise a real upgrade belongs in #689.The server-path
with_quic_enabledfix and QUIC server tests listed in #619 are onmainvia #694, and the upgrade behaviour's backoff tests via #666.