From bfccb6c66655966ab1eb503d96252fbfaa5c7e44 Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Wed, 16 Sep 2026 21:25:45 -0300 Subject: [PATCH 01/10] feat(app): wire the QUIC transport feature `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. --- crates/app/src/node/behaviour.rs | 14 +++++++++++--- crates/p2p/src/utils.rs | 15 --------------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/crates/app/src/node/behaviour.rs b/crates/app/src/node/behaviour.rs index ae4b1f89..1fbc4cbf 100644 --- a/crates/app/src/node/behaviour.rs +++ b/crates/app/src/node/behaviour.rs @@ -96,8 +96,7 @@ pub(crate) struct WireP2PParams { } /// Composes the core behaviours and builds the libp2p [`Node`]. -// TODO(#402 part B): QUIC transport (featureset-gated off at v1.7.1) and -// bandwidth metrics. +// TODO(#402 part B): bandwidth metrics. pub(crate) async fn wire_p2p( params: WireP2PParams, ) -> Result<(Node, CoreHandles), AppError> { @@ -215,10 +214,19 @@ pub(crate) async fn wire_p2p( // checker observes the same shared peer/connection state the swarm updates. let p2p_context_for_handle = p2p_context.clone(); + // A QUIC node listens on the configured UDP addresses alongside TCP and + // upgrades direct TCP connections to QUIC (Charon's `wireP2P` picks + // `NodeTypeQUIC` off the same featureset flag). + let node_type = if feature_set.enabled(pluto_featureset::Feature::Quic) { + NodeType::QUIC + } else { + NodeType::TCP + }; + let node = Node::new( p2p_config, key, - NodeType::TCP, + node_type, false, p2p_context, |builder, _keypair, relay_client| { diff --git a/crates/p2p/src/utils.rs b/crates/p2p/src/utils.rs index fdf646ca..36386d29 100644 --- a/crates/p2p/src/utils.rs +++ b/crates/p2p/src/utils.rs @@ -194,11 +194,6 @@ pub fn is_tcp_addr(addr: &Multiaddr) -> bool { addr.iter().any(|p| matches!(p, MaProtocol::Tcp(_))) } -/// Returns true if the node has QUIC enabled (listening on QUIC addresses). -pub fn is_quic_enabled<'a>(listen_addrs: impl Iterator) -> bool { - listen_addrs.into_iter().any(is_quic_addr) -} - /// Returns true if there is a direct (non-relay) QUIC connection among the /// peers. pub fn has_direct_quic_conn(peers: &[&crate::p2p_context::Peer]) -> bool { @@ -334,16 +329,6 @@ mod tests { assert!(filter_direct_quic_addrs(std::iter::empty()).is_empty()); } - #[test] - fn quic_is_enabled_only_while_listening_on_quic() { - let tcp = addr("/ip4/1.2.3.4/tcp/3610"); - let quic = addr("/ip4/1.2.3.4/udp/3610/quic-v1"); - - assert!(is_quic_enabled([&tcp, &quic].into_iter())); - assert!(!is_quic_enabled([&tcp].into_iter())); - assert!(!is_quic_enabled(std::iter::empty())); - } - #[test] fn direct_conn_checks_ignore_relayed_connections() { let quic = conn(addr("/ip4/1.2.3.4/udp/3610/quic-v1")); From 2088a3b7e096f8af515b787e86d3014eae201310 Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Wed, 16 Sep 2026 21:27:17 -0300 Subject: [PATCH 02/10] test(p2p): cover the QUIC upgrade decision logic 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. --- crates/p2p/src/quic_upgrade.rs | 235 +++++++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) diff --git a/crates/p2p/src/quic_upgrade.rs b/crates/p2p/src/quic_upgrade.rs index a7b78346..997cf3f7 100644 --- a/crates/p2p/src/quic_upgrade.rs +++ b/crates/p2p/src/quic_upgrade.rs @@ -429,12 +429,52 @@ impl NetworkBehaviour for QuicUpgradeBehaviour { #[cfg(test)] mod tests { + use libp2p::swarm::CloseConnection; + use super::*; + use crate::p2p_context::Peer; + + const RELAY_ID: &str = "16Uiu2HAkzdQ5Y9SYT91K1ue5SxXwgmajXntfScGnLYeip5hHyWmT"; + const TCP: &str = "/ip4/10.0.0.2/tcp/3610"; + const QUIC: &str = "/ip4/10.0.0.2/udp/3630/quic-v1"; fn behaviour() -> QuicUpgradeBehaviour { QuicUpgradeBehaviour::new(P2PContext::default(), PeerId::random(), true) } + fn addr(s: &str) -> Multiaddr { + s.parse().unwrap() + } + + fn relayed(transport: &str) -> Multiaddr { + addr(&format!("{transport}/p2p/{RELAY_ID}/p2p-circuit")) + } + + /// A behaviour that knows `peer`, holds the given `(connection id, remote + /// address)` connections to it, and has learned `addrs` for it via + /// identify. + fn connected( + quic_enabled: bool, + peer: PeerId, + conns: &[(usize, Multiaddr)], + addrs: &[Multiaddr], + ) -> QuicUpgradeBehaviour { + let local = PeerId::random(); + let ctx = P2PContext::new([local, peer]); + { + let mut store = ctx.peer_store_write_lock(); + for (id, remote_addr) in conns { + store.add_peer(Peer { + id: peer, + connection_id: ConnectionId::new_unchecked(*id), + remote_addr: remote_addr.clone(), + }); + } + store.set_peer_addresses(peer, addrs.to_vec()); + } + QuicUpgradeBehaviour::new(ctx, local, quic_enabled) + } + /// The reason carried by the queued `UpgradeFailed` event for `peer`. fn failure_reason(behaviour: &QuicUpgradeBehaviour, peer: &PeerId) -> Option { behaviour @@ -449,6 +489,201 @@ mod tests { }) } + /// Peers the queued `Dial` events target. + fn dialed(behaviour: &QuicUpgradeBehaviour) -> Vec { + behaviour + .pending_events + .iter() + .filter_map(|event| match event { + ToSwarm::Dial { opts } => opts.get_peer_id(), + _ => None, + }) + .collect() + } + + /// Connections the queued `CloseConnection` events close. + fn closed(behaviour: &QuicUpgradeBehaviour) -> Vec { + behaviour + .pending_events + .iter() + .filter_map(|event| match event { + ToSwarm::CloseConnection { + connection: CloseConnection::One(id), + .. + } => Some(*id), + _ => None, + }) + .collect() + } + + /// Peers the queued `Upgraded` events report. + fn upgraded(behaviour: &QuicUpgradeBehaviour) -> Vec { + behaviour + .pending_events + .iter() + .filter_map(|event| match event { + ToSwarm::GenerateEvent(QuicUpgradeEvent::Upgraded { peer }) => Some(*peer), + _ => None, + }) + .collect() + } + + #[tokio::test] + async fn tick_dials_the_quic_addrs_of_a_tcp_connected_peer() { + let peer = PeerId::random(); + let mut behaviour = connected( + true, + peer, + &[(1, addr(TCP))], + &[addr(TCP), addr(QUIC), relayed(QUIC)], + ); + + behaviour.run_upgrade_logic(); + + assert_eq!(dialed(&behaviour), vec![peer]); + assert!( + closed(&behaviour).is_empty(), + "TCP stays up until QUIC is established" + ); + let Some(UpgradeState::DialingQuic { tcp_conn_ids }) = + behaviour.pending_upgrades.get(&peer) + else { + panic!("upgrade must be armed for {peer}"); + }; + assert_eq!(tcp_conn_ids, &[ConnectionId::new_unchecked(1)]); + + // An armed upgrade is not dialed again on the next tick. + behaviour.run_upgrade_logic(); + assert_eq!(dialed(&behaviour).len(), 1); + } + + #[tokio::test] + async fn tick_is_a_noop_on_a_tcp_only_node() { + let peer = PeerId::random(); + let mut behaviour = connected(false, peer, &[(1, addr(TCP))], &[addr(QUIC)]); + + behaviour.run_upgrade_logic(); + + assert!(behaviour.pending_events.is_empty()); + assert!(behaviour.pending_upgrades.is_empty()); + } + + #[tokio::test] + async fn tick_skips_peers_that_cannot_be_upgraded() { + let peer = PeerId::random(); + let cases = [ + ("no connection", vec![], vec![addr(QUIC)]), + ( + "only a relayed TCP connection", + vec![(1, relayed(TCP))], + vec![addr(QUIC)], + ), + ( + "no direct QUIC address", + vec![(1, addr(TCP))], + vec![addr(TCP), relayed(QUIC)], + ), + ]; + + for (case, conns, addrs) in cases { + let mut behaviour = connected(true, peer, &conns, &addrs); + + behaviour.run_upgrade_logic(); + + assert!(behaviour.pending_events.is_empty(), "{case}"); + assert!(behaviour.pending_upgrades.is_empty(), "{case}"); + } + } + + #[tokio::test] + async fn tick_closes_redundant_tcp_once_direct_quic_exists() { + let peer = PeerId::random(); + let mut behaviour = connected( + true, + peer, + &[(1, addr(TCP)), (2, addr(QUIC)), (3, relayed(TCP))], + &[addr(QUIC)], + ); + + behaviour.run_upgrade_logic(); + + assert_eq!( + closed(&behaviour), + vec![ConnectionId::new_unchecked(1)], + "only the direct TCP connection is redundant" + ); + assert!(dialed(&behaviour).is_empty()); + assert!(behaviour.pending_upgrades.is_empty()); + } + + #[tokio::test] + async fn quic_connection_completes_the_upgrade() { + let peer = PeerId::random(); + let mut behaviour = connected(true, peer, &[(1, addr(TCP))], &[addr(QUIC)]); + // An elapsed backoff left over from an earlier failure. + behaviour.backoffs.insert( + peer, + QuicUpgradeBackoff { + tickers_remaining: 0, + backoff_duration: 4, + }, + ); + behaviour.run_upgrade_logic(); + assert_eq!(dialed(&behaviour), vec![peer]); + + behaviour.handle_connection_established(peer, &addr(QUIC)); + + assert_eq!(upgraded(&behaviour), vec![peer]); + assert_eq!(closed(&behaviour), vec![ConnectionId::new_unchecked(1)]); + assert!(behaviour.pending_upgrades.is_empty()); + assert!( + !behaviour.backoffs.contains_key(&peer), + "success clears the backoff" + ); + } + + #[tokio::test] + async fn non_quic_connection_during_upgrade_records_a_failure() { + let peer = PeerId::random(); + let mut behaviour = connected(true, peer, &[(1, addr(TCP))], &[addr(QUIC)]); + behaviour.run_upgrade_logic(); + + behaviour.handle_connection_established(peer, &addr(TCP)); + + assert!(upgraded(&behaviour).is_empty()); + assert!(closed(&behaviour).is_empty(), "the TCP connection is kept"); + assert_eq!( + failure_reason(&behaviour, &peer).as_deref(), + Some("connected via non-direct address instead of direct QUIC") + ); + assert!(behaviour.pending_upgrades.is_empty()); + assert!(behaviour.should_skip(&peer), "failure arms the backoff"); + } + + #[tokio::test] + async fn dial_failure_during_upgrade_records_a_failure() { + let peer = PeerId::random(); + let other = PeerId::random(); + let mut behaviour = connected(true, peer, &[(1, addr(TCP))], &[addr(QUIC)]); + behaviour.run_upgrade_logic(); + + // Dial failures for peers with no armed upgrade are not this + // behaviour's. + behaviour.handle_dial_failure(Some(other)); + behaviour.handle_dial_failure(None); + assert!(failure_reason(&behaviour, &other).is_none()); + + behaviour.handle_dial_failure(Some(peer)); + + assert_eq!( + failure_reason(&behaviour, &peer).as_deref(), + Some("dial failed") + ); + assert!(closed(&behaviour).is_empty(), "the TCP connection is kept"); + assert!(behaviour.pending_upgrades.is_empty()); + assert!(behaviour.should_skip(&peer), "failure arms the backoff"); + } + #[test] fn backoff_doubles_then_pins_at_the_cap() { let mut backoff = QuicUpgradeBackoff::new(); From dc3096bdc15035d6558f95e23485054988d45247 Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Wed, 16 Sep 2026 21:28:33 -0300 Subject: [PATCH 03/10] fix(p2p): advertise the bound listen ports, not the configured ones 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. --- crates/p2p/src/p2p.rs | 114 ++++++++++++++++--------- crates/p2p/tests/advertised_addrs.rs | 119 +++++++++++++++++++++++++++ crates/p2p/tests/common/mod.rs | 4 - crates/relay-server/src/p2p.rs | 11 +-- 4 files changed, 198 insertions(+), 50 deletions(-) create mode 100644 crates/p2p/tests/advertised_addrs.rs diff --git a/crates/p2p/src/p2p.rs b/crates/p2p/src/p2p.rs index b2896ad1..b09338df 100644 --- a/crates/p2p/src/p2p.rs +++ b/crates/p2p/src/p2p.rs @@ -291,6 +291,18 @@ pub struct Node { /// Listeners registered through [`Node::listen_on`], in registration order. listener_ids: Vec, + + /// External IP / hostname overrides, advertised on the bound ports. + cfg: P2PConfig, + + /// Whether private bound addresses are withheld from advertisement. + filter_private_addrs: bool, + + /// Addresses libp2p reported as bound, kernel-assigned ports included. + bound_addrs: Vec, + + /// Addresses this node currently registers as external on the swarm. + advertised_addrs: Vec, } impl Node { @@ -348,7 +360,7 @@ impl Node { let mut node = Self::build_client(keypair, node_type, p2p_context, behaviour_fn)?; - node.apply_config(&cfg, filter_private_addrs)?; + node.apply_config(cfg, filter_private_addrs)?; Ok(node) } @@ -389,18 +401,22 @@ impl Node { let mut node = Self::build_server(keypair, node_type, p2p_context, bandwidth, behaviour_fn)?; - node.apply_config(&cfg, filter_private_addrs)?; + node.apply_config(cfg, filter_private_addrs)?; Ok(node) } - /// Listens on and advertises the configured addresses of every transport - /// this node's [`NodeType`] installs. + /// Listens on the configured addresses of every transport this node's + /// [`NodeType`] installs. /// /// Transport and address selection are driven by the same /// [`NodeType::transports`] list, so a node can never listen on a transport /// it did not install, nor install one it never listens on. - fn apply_config(&mut self, cfg: &P2PConfig, filter_private_addrs: bool) -> Result<()> { + /// + /// Nothing is advertised yet: a configured port of 0 means the kernel picks + /// one, so the advertised set is derived from the addresses libp2p reports + /// as bound (see [`Node::readvertise`]). + fn apply_config(&mut self, cfg: P2PConfig, filter_private_addrs: bool) -> Result<()> { let mut addrs = Vec::new(); for &proto in self.node_type.transports() { @@ -419,48 +435,54 @@ impl Node { ); } - // Listen on internal addresses only - for addr in &addrs { - self.listen_on(addr.clone())?; + // Surface an unparsable external IP here, where the error can + // propagate; `readvertise` runs from the event loop and cannot. + utils::external_multiaddrs(&cfg, &addrs)?; + + for addr in addrs { + self.listen_on(addr)?; } - self.set_advertised_addrs(cfg, filter_private_addrs, &addrs) + self.cfg = cfg; + self.filter_private_addrs = filter_private_addrs; + + Ok(()) } - /// Advertises the external IP / hostname from `cfg` on the ports of - /// `listen_addrs`, together with `listen_addrs` themselves. - /// - /// Replaces everything the node advertises, including addresses added - /// through [`Node::add_external_address`]. - /// - /// Callers that listen on port 0 should call this again once libp2p has - /// reported the kernel-assigned ports: the configured addresses advertise - /// port 0, which is not dialable. - pub fn set_advertised_addrs( - &mut self, - cfg: &P2PConfig, - filter_private_addrs: bool, - listen_addrs: &[Multiaddr], - ) -> Result<()> { - let external_addrs = utils::external_multiaddrs(cfg, listen_addrs)?; + /// Re-derives the advertised set from the bound addresses: the external IP + /// / hostname on the bound ports, plus the bound addresses themselves, with + /// private ones withheld when configured. Only the difference to the + /// previously advertised set is applied, so addresses added through + /// [`Node::add_external_address`] are left alone. + fn readvertise(&mut self) { + let external_addrs = match utils::external_multiaddrs(&self.cfg, &self.bound_addrs) { + Ok(addrs) => addrs, + // Unreachable after `apply_config` validated the config, but a + // swarm event handler cannot propagate errors. + Err(err) => { + warn!(%err, "failed to derive external addresses"); + return; + } + }; - // Advertise filtered addresses (external + optionally filtered - // internal) - let advertised_addrs = utils::filter_advertised_addresses( + let next = utils::filter_advertised_addresses( utils::ExternalAddresses(external_addrs), - utils::InternalAddresses(listen_addrs.to_vec()), - filter_private_addrs, + utils::InternalAddresses(self.bound_addrs.clone()), + self.filter_private_addrs, ); - for addr in self.swarm.external_addresses().cloned().collect::>() { - self.swarm.remove_external_address(&addr); + for addr in &self.advertised_addrs { + if !next.contains(addr) { + self.swarm.remove_external_address(addr); + } } - - for addr in advertised_addrs { - self.swarm.add_external_address(addr); + for addr in &next { + if !self.advertised_addrs.contains(addr) { + self.swarm.add_external_address(addr.clone()); + } } - Ok(()) + self.advertised_addrs = next; } fn bind_local_peer_id(p2p_context: &P2PContext, local_peer_id: PeerId) -> Result<()> { @@ -517,6 +539,10 @@ impl Node { node_type, p2p_context, listener_ids: Vec::new(), + cfg: P2PConfig::default(), + filter_private_addrs: false, + bound_addrs: Vec::new(), + advertised_addrs: Vec::new(), }) } @@ -552,6 +578,10 @@ impl Node { node_type, p2p_context, listener_ids: Vec::new(), + cfg: P2PConfig::default(), + filter_private_addrs: false, + bound_addrs: Vec::new(), + advertised_addrs: Vec::new(), }) } @@ -669,12 +699,22 @@ impl Node { } } - // Listen address changes + // Listen address changes drive what the node advertises. SwarmEvent::NewListenAddr { address, .. } => { info!(%address, "listening on new address"); + if !self.bound_addrs.contains(address) { + self.bound_addrs.push(address.clone()); + self.readvertise(); + } } SwarmEvent::ExpiredListenAddr { address, .. } => { info!(%address, "listen address expired"); + self.bound_addrs.retain(|bound| bound != address); + self.readvertise(); + } + SwarmEvent::ListenerClosed { addresses, .. } => { + self.bound_addrs.retain(|bound| !addresses.contains(bound)); + self.readvertise(); } // External address discovery diff --git a/crates/p2p/tests/advertised_addrs.rs b/crates/p2p/tests/advertised_addrs.rs new file mode 100644 index 00000000..81d9af06 --- /dev/null +++ b/crates/p2p/tests/advertised_addrs.rs @@ -0,0 +1,119 @@ +//! A node configured to listen on port 0 must advertise the ports the kernel +//! assigned, never the configured 0, and must advertise its external IP on +//! those same ports. +//! +//! Checked through identify as received by a peer, which is the only view +//! other nodes ever get of what this node advertises. + +use std::time::Duration; + +use futures::StreamExt as _; +use libp2p::{Multiaddr, identify, multiaddr::Protocol, relay, swarm::SwarmEvent}; +use pluto_p2p::{ + behaviours::pluto::PlutoBehaviourEvent, + config::P2PConfig, + p2p::{Node, NodeType}, + p2p_context::P2PContext, + peer::peer_id_from_key, +}; +use pluto_testutil::random::generate_insecure_k1_key; +use tokio::time::timeout; + +type ClientNode = Node; + +const TEST_TIMEOUT: Duration = Duration::from_secs(20); +const EXTERNAL_IP: &str = "1.2.3.4"; + +fn tcp_port(addr: &Multiaddr) -> Option { + addr.iter().find_map(|p| match p { + Protocol::Tcp(port) => Some(port), + _ => None, + }) +} + +async fn first_listen_addr(node: &mut ClientNode) -> Multiaddr { + timeout(TEST_TIMEOUT, async { + loop { + if let SwarmEvent::NewListenAddr { address, .. } = node.select_next_some().await { + return address; + } + } + }) + .await + .expect("timed out waiting for a listen address") +} + +#[tokio::test] +async fn advertises_bound_ports_not_configured_port_zero() { + let key_a = generate_insecure_k1_key(21); + let key_b = generate_insecure_k1_key(22); + let peer_a = peer_id_from_key(key_a.public_key()).expect("peer id A"); + let peer_b = peer_id_from_key(key_b.public_key()).expect("peer id B"); + + // A listens on a kernel-assigned port and has an external IP override. + let mut node_a: ClientNode = Node::new( + P2PConfig::builder() + .with_tcp_addrs(vec!["127.0.0.1:0".to_owned()]) + .with_external_ip(EXTERNAL_IP.to_owned()) + .build(), + key_a, + NodeType::TCP, + false, + P2PContext::new(vec![peer_b]), + |builder, _keypair, relay_client| builder.with_inner(relay_client), + ) + .expect("build node A"); + + let mut node_b: ClientNode = Node::new( + P2PConfig::default(), + key_b, + NodeType::TCP, + false, + P2PContext::new(vec![peer_a]), + |builder, _keypair, relay_client| builder.with_inner(relay_client), + ) + .expect("build node B"); + + let bound = first_listen_addr(&mut node_a).await; + let bound_port = tcp_port(&bound).expect("bound TCP port"); + assert!(bound_port != 0, "kernel must have assigned a port"); + + node_b.dial(bound.clone()).expect("dial A"); + + // Drive both until B has A's identify payload. + let advertised = timeout(TEST_TIMEOUT, async { + loop { + tokio::select! { + _ = node_a.select_next_some() => {} + event = node_b.select_next_some() => { + if let SwarmEvent::Behaviour(PlutoBehaviourEvent::Identify( + identify::Event::Received { peer_id, info, .. }, + )) = event + && peer_id == peer_a + { + return info.listen_addrs; + } + } + } + } + }) + .await + .expect("timed out waiting for A's identify"); + + let external: Multiaddr = format!("/ip4/{EXTERNAL_IP}/tcp/{bound_port}") + .parse() + .expect("external multiaddr"); + + assert!( + advertised.contains(&bound), + "bound address {bound} missing from {advertised:?}", + ); + assert!( + advertised.contains(&external), + "external address {external} missing from {advertised:?}", + ); + assert!( + advertised.iter().all(|addr| tcp_port(addr) != Some(0)), + "port 0 must never be advertised: {advertised:?}", + ); +} diff --git a/crates/p2p/tests/common/mod.rs b/crates/p2p/tests/common/mod.rs index d24ae946..eb7f787d 100644 --- a/crates/p2p/tests/common/mod.rs +++ b/crates/p2p/tests/common/mod.rs @@ -64,10 +64,6 @@ pub async fn spawn_relay_server(key: SecretKey) -> (PeerId, Multiaddr, JoinHandl .await .expect("timed out waiting for the relay listen address"); - // Without a reachable advertised address, reservations are rejected - // client-side with `NoAddressesInReservation`. - node.add_external_address(addr.clone()); - let handle = tokio::spawn(async move { loop { node.select_next_some().await; diff --git a/crates/relay-server/src/p2p.rs b/crates/relay-server/src/p2p.rs index 8ab91601..54f818c2 100644 --- a/crates/relay-server/src/p2p.rs +++ b/crates/relay-server/src/p2p.rs @@ -249,19 +249,12 @@ pub async fn bind_relay(config: &Config, key: SecretKey) -> Result { )?; // First poll of the swarm, and so the first point at which this relay - // services anything. Every other listener is already bound. + // services anything. Every other listener is already bound, and the node + // advertises each address as libp2p reports it bound. let listen_addrs = Arc::new(RwLock::new(Vec::new())); wait_for_listen_addrs(&mut node, &listen_addrs).await?; let bound_addrs = listen_addrs.read().await.clone(); - // Advertise the ports libp2p bound rather than the configured ones, which - // carry port 0 whenever the kernel picked the port. - node.set_advertised_addrs( - &config.p2p_config, - config.filter_private_addrs, - &bound_addrs, - )?; - // Compute external multiaddrs from external_ip / external_host config so // they're advertised on `/` and folded into ENR responses on `/enr` even // when libp2p only sees private listen addresses (e.g., K8s pods behind From 91f8cd319dae1720d941b3d1c5cf6674011c4a81 Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:29:32 -0300 Subject: [PATCH 04/10] fix(p2p): keep relay circuit listeners out of the advertised set `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//tcp/`, 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. --- crates/p2p/src/p2p.rs | 89 +++++++++++++------------ crates/p2p/src/utils.rs | 99 ++++++++++++++-------------- crates/p2p/tests/advertised_addrs.rs | 67 ++++++++++++------- 3 files changed, 138 insertions(+), 117 deletions(-) diff --git a/crates/p2p/src/p2p.rs b/crates/p2p/src/p2p.rs index b09338df..9911972a 100644 --- a/crates/p2p/src/p2p.rs +++ b/crates/p2p/src/p2p.rs @@ -88,6 +88,7 @@ //! Client nodes may include relay client to support connecting via relays. use std::{ + net::IpAddr, pin::Pin, task::{Context, Poll}, time::Duration, @@ -292,14 +293,14 @@ pub struct Node { /// Listeners registered through [`Node::listen_on`], in registration order. listener_ids: Vec, - /// External IP / hostname overrides, advertised on the bound ports. - cfg: P2PConfig, + /// External IP to advertise on the bound ports. + external_ip: Option, - /// Whether private bound addresses are withheld from advertisement. - filter_private_addrs: bool, + /// External hostname to advertise on the bound ports. + external_host: Option, - /// Addresses libp2p reported as bound, kernel-assigned ports included. - bound_addrs: Vec, + /// Whether private listen addresses are withheld from advertisement. + filter_private_addrs: bool, /// Addresses this node currently registers as external on the swarm. advertised_addrs: Vec, @@ -360,7 +361,7 @@ impl Node { let mut node = Self::build_client(keypair, node_type, p2p_context, behaviour_fn)?; - node.apply_config(cfg, filter_private_addrs)?; + node.apply_config(&cfg, filter_private_addrs)?; Ok(node) } @@ -401,7 +402,7 @@ impl Node { let mut node = Self::build_server(keypair, node_type, p2p_context, bandwidth, behaviour_fn)?; - node.apply_config(cfg, filter_private_addrs)?; + node.apply_config(&cfg, filter_private_addrs)?; Ok(node) } @@ -416,7 +417,15 @@ impl Node { /// Nothing is advertised yet: a configured port of 0 means the kernel picks /// one, so the advertised set is derived from the addresses libp2p reports /// as bound (see [`Node::readvertise`]). - fn apply_config(&mut self, cfg: P2PConfig, filter_private_addrs: bool) -> Result<()> { + fn apply_config(&mut self, cfg: &P2PConfig, filter_private_addrs: bool) -> Result<()> { + self.external_ip = cfg + .external_ip + .as_deref() + .map(str::parse::) + .transpose()?; + self.external_host = cfg.external_host.clone(); + self.filter_private_addrs = filter_private_addrs; + let mut addrs = Vec::new(); for &proto in self.node_type.transports() { @@ -435,39 +444,36 @@ impl Node { ); } - // Surface an unparsable external IP here, where the error can - // propagate; `readvertise` runs from the event loop and cannot. - utils::external_multiaddrs(&cfg, &addrs)?; - for addr in addrs { self.listen_on(addr)?; } - self.cfg = cfg; - self.filter_private_addrs = filter_private_addrs; - Ok(()) } - /// Re-derives the advertised set from the bound addresses: the external IP - /// / hostname on the bound ports, plus the bound addresses themselves, with - /// private ones withheld when configured. Only the difference to the - /// previously advertised set is applied, so addresses added through - /// [`Node::add_external_address`] are left alone. + /// Re-derives the advertised set from the listen addresses libp2p reports: + /// the external IP / hostname on their ports plus the addresses themselves, + /// private ones withheld when configured. Relay circuit listeners are + /// skipped: their ports are the relay's, and identify lists them anyway. + /// Only the difference to the previous set is applied, so addresses the + /// swarm confirmed by other means (AutoNAT) survive. fn readvertise(&mut self) { - let external_addrs = match utils::external_multiaddrs(&self.cfg, &self.bound_addrs) { - Ok(addrs) => addrs, - // Unreachable after `apply_config` validated the config, but a - // swarm event handler cannot propagate errors. - Err(err) => { - warn!(%err, "failed to derive external addresses"); - return; - } - }; + let listen_addrs: Vec = self + .swarm + .listeners() + .filter(|addr| !utils::is_relay_addr(addr)) + .cloned() + .collect(); + + let external_addrs = utils::external_multiaddrs_on( + self.external_ip, + self.external_host.as_deref(), + &listen_addrs, + ); let next = utils::filter_advertised_addresses( utils::ExternalAddresses(external_addrs), - utils::InternalAddresses(self.bound_addrs.clone()), + utils::InternalAddresses(listen_addrs), self.filter_private_addrs, ); @@ -539,9 +545,9 @@ impl Node { node_type, p2p_context, listener_ids: Vec::new(), - cfg: P2PConfig::default(), + external_ip: None, + external_host: None, filter_private_addrs: false, - bound_addrs: Vec::new(), advertised_addrs: Vec::new(), }) } @@ -578,9 +584,9 @@ impl Node { node_type, p2p_context, listener_ids: Vec::new(), - cfg: P2PConfig::default(), + external_ip: None, + external_host: None, filter_private_addrs: false, - bound_addrs: Vec::new(), advertised_addrs: Vec::new(), }) } @@ -699,23 +705,16 @@ impl Node { } } - // Listen address changes drive what the node advertises. + // Listener changes drive what the node advertises. SwarmEvent::NewListenAddr { address, .. } => { info!(%address, "listening on new address"); - if !self.bound_addrs.contains(address) { - self.bound_addrs.push(address.clone()); - self.readvertise(); - } + self.readvertise(); } SwarmEvent::ExpiredListenAddr { address, .. } => { info!(%address, "listen address expired"); - self.bound_addrs.retain(|bound| bound != address); - self.readvertise(); - } - SwarmEvent::ListenerClosed { addresses, .. } => { - self.bound_addrs.retain(|bound| !addresses.contains(bound)); self.readvertise(); } + SwarmEvent::ListenerClosed { .. } => self.readvertise(), // External address discovery SwarmEvent::ExternalAddrConfirmed { address } => { diff --git a/crates/p2p/src/utils.rs b/crates/p2p/src/utils.rs index 36386d29..d3bc93ca 100644 --- a/crates/p2p/src/utils.rs +++ b/crates/p2p/src/utils.rs @@ -8,11 +8,7 @@ //! //! These utilities are primarily used internally by the [`crate::p2p`] module. -use std::{ - collections::HashSet, - net::{IpAddr, SocketAddr}, - time::Duration, -}; +use std::{collections::HashSet, net::IpAddr, time::Duration}; use libp2p::{ Multiaddr, @@ -22,10 +18,7 @@ use libp2p::{ use crate::metrics::{ConnectionType, Protocol}; -use crate::{ - config::{self, P2PConfig}, - manet::Manet, -}; +use crate::{config::P2PConfig, manet::Manet}; /// A transport a node can listen on and advertise. /// @@ -39,49 +32,35 @@ pub enum TransportProtocol { Quic, } -/// Returns the external IP and Hostname fields as `proto` multiaddrs on -/// `ports`. -/// -/// `ports` must be the ports the node actually listens on: a configured port of -/// 0 means the kernel picks one, so the configured value would advertise -/// nothing dialable. -fn external_proto_multiaddrs( +/// Returns the external IP and hostname from `cfg` as multiaddrs on the ports +/// of `listen_addrs`, TCP forms first. +pub fn external_multiaddrs( cfg: &P2PConfig, - ports: &[u16], - proto: TransportProtocol, + listen_addrs: &[Multiaddr], ) -> crate::p2p::Result> { - let mut resp = vec![]; - - if let Some(external_ip) = cfg.external_ip.as_ref() { - let ip = external_ip.parse::()?; - - for port in ports { - let maddr = config::multi_addr_from_socket_addr(SocketAddr::new(ip, *port), proto)?; - - resp.push(maddr); - } - } - - if let Some(external_host) = cfg.external_host.as_ref() { - for port in ports { - resp.push(match proto { - TransportProtocol::Tcp => multiaddr::multiaddr!(Dns(external_host), Tcp(*port)), - TransportProtocol::Quic => { - multiaddr::multiaddr!(Dns(external_host), Udp(*port), QuicV1) - } - }); - } - } - - Ok(resp) + let external_ip = cfg + .external_ip + .as_deref() + .map(str::parse::) + .transpose()?; + + Ok(external_multiaddrs_on( + external_ip, + cfg.external_host.as_deref(), + listen_addrs, + )) } -/// Returns the external IP and Hostname fields as multiaddrs on the ports of -/// `listen_addrs`, TCP forms first. -pub fn external_multiaddrs( - cfg: &P2PConfig, +/// [`external_multiaddrs`] over an already parsed external IP. +/// +/// `listen_addrs` must be the addresses the node actually listens on: a +/// configured port of 0 means the kernel picks one, so the configured value +/// would advertise nothing dialable. +pub(crate) fn external_multiaddrs_on( + external_ip: Option, + external_host: Option<&str>, listen_addrs: &[Multiaddr], -) -> crate::p2p::Result> { +) -> Vec { let mut addrs = Vec::new(); for proto in [TransportProtocol::Tcp, TransportProtocol::Quic] { @@ -90,10 +69,32 @@ pub fn external_multiaddrs( .filter_map(|addr| addr_port(addr, proto)) .collect(); - addrs.extend(external_proto_multiaddrs(cfg, &ports, proto)?); + if let Some(ip) = external_ip { + addrs.extend( + ports + .iter() + .map(|&port| with_transport(Multiaddr::from(ip), port, proto)), + ); + } + + if let Some(host) = external_host { + addrs.extend( + ports + .iter() + .map(|&port| with_transport(multiaddr::multiaddr!(Dns(host)), port, proto)), + ); + } } - Ok(addrs) + addrs +} + +/// Appends the `proto` transport on `port` to `base`. +fn with_transport(base: Multiaddr, port: u16, proto: TransportProtocol) -> Multiaddr { + match proto { + TransportProtocol::Tcp => base.with(MaProtocol::Tcp(port)), + TransportProtocol::Quic => base.with(MaProtocol::Udp(port)).with(MaProtocol::QuicV1), + } } /// Returns the port `addr` carries for `proto`, if any. diff --git a/crates/p2p/tests/advertised_addrs.rs b/crates/p2p/tests/advertised_addrs.rs index 81d9af06..c3507885 100644 --- a/crates/p2p/tests/advertised_addrs.rs +++ b/crates/p2p/tests/advertised_addrs.rs @@ -1,12 +1,12 @@ -//! A node configured to listen on port 0 must advertise the ports the kernel -//! assigned, never the configured 0, and must advertise its external IP on -//! those same ports. +//! What a node advertises, seen through identify as a peer receives it. //! -//! Checked through identify as received by a peer, which is the only view -//! other nodes ever get of what this node advertises. +//! A node configured to listen on port 0 must advertise the kernel-assigned +//! port, never the configured 0, with its external IP on that same port; and a +//! relay circuit listener must not leak the relay's port into that set. -use std::time::Duration; +mod common; +use common::{TEST_TIMEOUT, spawn_relay_server}; use futures::StreamExt as _; use libp2p::{Multiaddr, identify, multiaddr::Protocol, relay, swarm::SwarmEvent}; use pluto_p2p::{ @@ -15,13 +15,13 @@ use pluto_p2p::{ p2p::{Node, NodeType}, p2p_context::P2PContext, peer::peer_id_from_key, + utils::is_relay_addr, }; use pluto_testutil::random::generate_insecure_k1_key; use tokio::time::timeout; type ClientNode = Node; -const TEST_TIMEOUT: Duration = Duration::from_secs(20); const EXTERNAL_IP: &str = "1.2.3.4"; fn tcp_port(addr: &Multiaddr) -> Option { @@ -31,26 +31,33 @@ fn tcp_port(addr: &Multiaddr) -> Option { }) } -async fn first_listen_addr(node: &mut ClientNode) -> Multiaddr { +/// Drives `node` until it has reported `want` listen addresses. +async fn listen_addrs(node: &mut ClientNode, want: usize) -> Vec { timeout(TEST_TIMEOUT, async { - loop { + let mut addrs = Vec::with_capacity(want); + while addrs.len() < want { if let SwarmEvent::NewListenAddr { address, .. } = node.select_next_some().await { - return address; + addrs.push(address); } } + addrs }) .await - .expect("timed out waiting for a listen address") + .expect("timed out waiting for the listen addresses") } #[tokio::test] -async fn advertises_bound_ports_not_configured_port_zero() { +async fn advertises_own_bound_ports_only() { + let (relay_peer, relay_addr, relay_handle) = + spawn_relay_server(generate_insecure_k1_key(20)).await; + let key_a = generate_insecure_k1_key(21); let key_b = generate_insecure_k1_key(22); let peer_a = peer_id_from_key(key_a.public_key()).expect("peer id A"); let peer_b = peer_id_from_key(key_b.public_key()).expect("peer id B"); - // A listens on a kernel-assigned port and has an external IP override. + // A listens on a kernel-assigned TCP port, reserves a relay circuit, and + // has an external IP override. let mut node_a: ClientNode = Node::new( P2PConfig::builder() .with_tcp_addrs(vec!["127.0.0.1:0".to_owned()]) @@ -59,10 +66,17 @@ async fn advertises_bound_ports_not_configured_port_zero() { key_a, NodeType::TCP, false, - P2PContext::new(vec![peer_b]), + P2PContext::new(vec![peer_b, relay_peer]), |builder, _keypair, relay_client| builder.with_inner(relay_client), ) .expect("build node A"); + node_a + .listen_on( + relay_addr + .with(Protocol::P2p(relay_peer)) + .with(Protocol::P2pCircuit), + ) + .expect("A listen_on circuit"); let mut node_b: ClientNode = Node::new( P2PConfig::default(), @@ -74,11 +88,15 @@ async fn advertises_bound_ports_not_configured_port_zero() { ) .expect("build node B"); - let bound = first_listen_addr(&mut node_a).await; + let bound = listen_addrs(&mut node_a, 2) + .await + .into_iter() + .find(|addr| !is_relay_addr(addr)) + .expect("A must report its TCP listen address"); let bound_port = tcp_port(&bound).expect("bound TCP port"); assert!(bound_port != 0, "kernel must have assigned a port"); - node_b.dial(bound.clone()).expect("dial A"); + node_b.dial(bound).expect("dial A"); // Drive both until B has A's identify payload. let advertised = timeout(TEST_TIMEOUT, async { @@ -100,20 +118,23 @@ async fn advertises_bound_ports_not_configured_port_zero() { .await .expect("timed out waiting for A's identify"); + relay_handle.abort(); + let external: Multiaddr = format!("/ip4/{EXTERNAL_IP}/tcp/{bound_port}") .parse() .expect("external multiaddr"); - - assert!( - advertised.contains(&bound), - "bound address {bound} missing from {advertised:?}", - ); assert!( advertised.contains(&external), "external address {external} missing from {advertised:?}", ); + + // Neither the configured port 0 nor the relay's port may appear: every + // non-circuit address carries the port A actually bound. assert!( - advertised.iter().all(|addr| tcp_port(addr) != Some(0)), - "port 0 must never be advertised: {advertised:?}", + advertised + .iter() + .filter(|addr| !is_relay_addr(addr)) + .all(|addr| tcp_port(addr) == Some(bound_port)), + "advertised addresses on a port other than {bound_port}: {advertised:?}", ); } From 66fe0af73e1d4957a164b2c31a1df642007d4ebe Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:30:29 -0300 Subject: [PATCH 05/10] docs(app): trim the node type comment --- crates/app/src/node/behaviour.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/app/src/node/behaviour.rs b/crates/app/src/node/behaviour.rs index 1fbc4cbf..e6ae4954 100644 --- a/crates/app/src/node/behaviour.rs +++ b/crates/app/src/node/behaviour.rs @@ -214,9 +214,7 @@ pub(crate) async fn wire_p2p( // checker observes the same shared peer/connection state the swarm updates. let p2p_context_for_handle = p2p_context.clone(); - // A QUIC node listens on the configured UDP addresses alongside TCP and - // upgrades direct TCP connections to QUIC (Charon's `wireP2P` picks - // `NodeTypeQUIC` off the same featureset flag). + // Charon's `wireP2P` picks the node type off the same flag. let node_type = if feature_set.enabled(pluto_featureset::Feature::Quic) { NodeType::QUIC } else { From 12df20d156f63169d938389bd425af0975f0a539 Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:31:00 -0300 Subject: [PATCH 06/10] test(p2p): trim the QUIC upgrade tests to the decisions worth pinning 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`. --- crates/p2p/src/quic_upgrade.rs | 98 ++++++++++++---------------------- 1 file changed, 34 insertions(+), 64 deletions(-) diff --git a/crates/p2p/src/quic_upgrade.rs b/crates/p2p/src/quic_upgrade.rs index 997cf3f7..e1e3a176 100644 --- a/crates/p2p/src/quic_upgrade.rs +++ b/crates/p2p/src/quic_upgrade.rs @@ -558,9 +558,9 @@ mod tests { } #[tokio::test] - async fn tick_is_a_noop_on_a_tcp_only_node() { + async fn tick_leaves_relayed_only_peers_to_force_direct() { let peer = PeerId::random(); - let mut behaviour = connected(false, peer, &[(1, addr(TCP))], &[addr(QUIC)]); + let mut behaviour = connected(true, peer, &[(1, relayed(TCP))], &[addr(QUIC)]); behaviour.run_upgrade_logic(); @@ -568,33 +568,6 @@ mod tests { assert!(behaviour.pending_upgrades.is_empty()); } - #[tokio::test] - async fn tick_skips_peers_that_cannot_be_upgraded() { - let peer = PeerId::random(); - let cases = [ - ("no connection", vec![], vec![addr(QUIC)]), - ( - "only a relayed TCP connection", - vec![(1, relayed(TCP))], - vec![addr(QUIC)], - ), - ( - "no direct QUIC address", - vec![(1, addr(TCP))], - vec![addr(TCP), relayed(QUIC)], - ), - ]; - - for (case, conns, addrs) in cases { - let mut behaviour = connected(true, peer, &conns, &addrs); - - behaviour.run_upgrade_logic(); - - assert!(behaviour.pending_events.is_empty(), "{case}"); - assert!(behaviour.pending_upgrades.is_empty(), "{case}"); - } - } - #[tokio::test] async fn tick_closes_redundant_tcp_once_direct_quic_exists() { let peer = PeerId::random(); @@ -643,45 +616,42 @@ mod tests { } #[tokio::test] - async fn non_quic_connection_during_upgrade_records_a_failure() { - let peer = PeerId::random(); - let mut behaviour = connected(true, peer, &[(1, addr(TCP))], &[addr(QUIC)]); - behaviour.run_upgrade_logic(); - - behaviour.handle_connection_established(peer, &addr(TCP)); - - assert!(upgraded(&behaviour).is_empty()); - assert!(closed(&behaviour).is_empty(), "the TCP connection is kept"); - assert_eq!( - failure_reason(&behaviour, &peer).as_deref(), - Some("connected via non-direct address instead of direct QUIC") - ); - assert!(behaviour.pending_upgrades.is_empty()); - assert!(behaviour.should_skip(&peer), "failure arms the backoff"); - } + async fn failed_upgrade_keeps_tcp_and_arms_backoff() { + /// Fails `peer`'s armed upgrade. + type Fail = fn(&mut QuicUpgradeBehaviour, PeerId); + + let cases: [(&str, Fail); 2] = [ + ("non-QUIC connection", |behaviour, peer| { + behaviour.handle_connection_established(peer, &addr(TCP)); + }), + ("dial failure", |behaviour, peer| { + behaviour.handle_dial_failure(Some(peer)); + }), + ]; - #[tokio::test] - async fn dial_failure_during_upgrade_records_a_failure() { - let peer = PeerId::random(); - let other = PeerId::random(); - let mut behaviour = connected(true, peer, &[(1, addr(TCP))], &[addr(QUIC)]); - behaviour.run_upgrade_logic(); + for (case, fail) in cases { + let peer = PeerId::random(); + let mut behaviour = connected(true, peer, &[(1, addr(TCP))], &[addr(QUIC)]); + behaviour.run_upgrade_logic(); - // Dial failures for peers with no armed upgrade are not this - // behaviour's. - behaviour.handle_dial_failure(Some(other)); - behaviour.handle_dial_failure(None); - assert!(failure_reason(&behaviour, &other).is_none()); + // Failures of peers with no armed upgrade are not this behaviour's. + behaviour.handle_dial_failure(Some(PeerId::random())); + behaviour.handle_dial_failure(None); - behaviour.handle_dial_failure(Some(peer)); + fail(&mut behaviour, peer); - assert_eq!( - failure_reason(&behaviour, &peer).as_deref(), - Some("dial failed") - ); - assert!(closed(&behaviour).is_empty(), "the TCP connection is kept"); - assert!(behaviour.pending_upgrades.is_empty()); - assert!(behaviour.should_skip(&peer), "failure arms the backoff"); + assert!(upgraded(&behaviour).is_empty(), "{case}"); + assert!( + closed(&behaviour).is_empty(), + "{case}: the TCP connection is kept" + ); + assert!(failure_reason(&behaviour, &peer).is_some(), "{case}"); + assert!(behaviour.pending_upgrades.is_empty(), "{case}"); + assert!( + behaviour.backoffs.contains_key(&peer), + "{case}: failure arms the backoff" + ); + } } #[test] From cc564666f6ef4ab5b33864eac67f0b9ce35c39c8 Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:23:33 -0300 Subject: [PATCH 07/10] refactor(p2p): build listen multiaddrs by pushing protocols `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. --- crates/p2p/src/config.rs | 42 ++++++++-------------------------------- crates/p2p/src/utils.rs | 9 +++------ 2 files changed, 11 insertions(+), 40 deletions(-) diff --git a/crates/p2p/src/config.rs b/crates/p2p/src/config.rs index 91deaf02..f22023de 100644 --- a/crates/p2p/src/config.rs +++ b/crates/p2p/src/config.rs @@ -1,16 +1,11 @@ //! # Charon P2P Configuration -use std::{ - fmt, - net::{IpAddr, SocketAddr}, - str::FromStr, - time::Duration, -}; +use std::{fmt, net::SocketAddr, str::FromStr, time::Duration}; use libp2p::{Multiaddr, multiaddr, ping}; use url::Url; -use crate::utils::TransportProtocol; +use crate::utils::{TransportProtocol, with_transport}; /// Shared default relay endpoints used by commands and P2P-facing configs. pub const DEFAULT_RELAYS: [&str; 5] = [ @@ -122,10 +117,6 @@ pub enum P2PConfigError { /// Failed to parse the UDP addresses. #[error("Failed to parse the UDP addresses")] FailedToParseUdpAddresses(std::net::AddrParseError), - - /// Failed to parse the multiaddress. - #[error("Failed to parse the multiaddress")] - FailedToParseMultiaddr(#[from] multiaddr::Error), } // Note: this is only for testing purposes! @@ -141,10 +132,6 @@ impl PartialEq for P2PConfigError { P2PConfigError::FailedToParseUdpAddresses(x), P2PConfigError::FailedToParseUdpAddresses(y), ) if x == y => true, - ( - P2PConfigError::FailedToParseMultiaddr(x), - P2PConfigError::FailedToParseMultiaddr(y), - ) if x.to_string() == y.to_string() => true, _ => false, } } @@ -202,10 +189,11 @@ impl P2PConfig { /// Returns the configured listen multiaddresses for `proto`. pub fn multiaddrs(&self, proto: TransportProtocol) -> Result> { - self.parse_addrs(proto)? + Ok(self + .parse_addrs(proto)? .into_iter() .map(|addr| multi_addr_from_socket_addr(addr, proto)) - .collect() + .collect()) } } @@ -240,27 +228,13 @@ fn resolve_listen_addr(addr: impl AsRef, proto: TransportProtocol) -> Resul } /// Renders `socket_addr` as a `proto` multiaddr. -pub(crate) fn multi_addr_from_socket_addr( - socket_addr: SocketAddr, - proto: TransportProtocol, -) -> Result { - let typ = match socket_addr.ip() { - IpAddr::V4(_) => "ip4", - IpAddr::V6(_) => "ip6", - }; - - let transport = match proto { - TransportProtocol::Tcp => format!("tcp/{}", socket_addr.port()), - TransportProtocol::Quic => format!("udp/{}/quic-v1", socket_addr.port()), - }; - - Multiaddr::from_str(&format!("/{}/{}/{}", typ, socket_addr.ip(), transport)) - .map_err(P2PConfigError::FailedToParseMultiaddr) +fn multi_addr_from_socket_addr(socket_addr: SocketAddr, proto: TransportProtocol) -> Multiaddr { + with_transport(Multiaddr::from(socket_addr.ip()), socket_addr.port(), proto) } #[cfg(test)] mod tests { - use std::net::{Ipv4Addr, Ipv6Addr}; + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use super::*; diff --git a/crates/p2p/src/utils.rs b/crates/p2p/src/utils.rs index d3bc93ca..c33db185 100644 --- a/crates/p2p/src/utils.rs +++ b/crates/p2p/src/utils.rs @@ -51,11 +51,8 @@ pub fn external_multiaddrs( )) } -/// [`external_multiaddrs`] over an already parsed external IP. -/// -/// `listen_addrs` must be the addresses the node actually listens on: a -/// configured port of 0 means the kernel picks one, so the configured value -/// would advertise nothing dialable. +/// Returns `external_ip` and `external_host` as multiaddrs on the ports of +/// `listen_addrs`, TCP forms first. pub(crate) fn external_multiaddrs_on( external_ip: Option, external_host: Option<&str>, @@ -90,7 +87,7 @@ pub(crate) fn external_multiaddrs_on( } /// Appends the `proto` transport on `port` to `base`. -fn with_transport(base: Multiaddr, port: u16, proto: TransportProtocol) -> Multiaddr { +pub(crate) fn with_transport(base: Multiaddr, port: u16, proto: TransportProtocol) -> Multiaddr { match proto { TransportProtocol::Tcp => base.with(MaProtocol::Tcp(port)), TransportProtocol::Quic => base.with(MaProtocol::Udp(port)).with(MaProtocol::QuicV1), From d312a099ba30eac9cbc6341c2570f00afa0e5fea Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:23:33 -0300 Subject: [PATCH 08/10] test(p2p): fold the upgrade lifecycle test and drop unasserted setup 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. --- crates/p2p/src/quic_upgrade.rs | 103 +++++++++++---------------- crates/p2p/tests/advertised_addrs.rs | 28 ++++---- 2 files changed, 57 insertions(+), 74 deletions(-) diff --git a/crates/p2p/src/quic_upgrade.rs b/crates/p2p/src/quic_upgrade.rs index e1e3a176..3fdf1fa6 100644 --- a/crates/p2p/src/quic_upgrade.rs +++ b/crates/p2p/src/quic_upgrade.rs @@ -454,7 +454,6 @@ mod tests { /// address)` connections to it, and has learned `addrs` for it via /// identify. fn connected( - quic_enabled: bool, peer: PeerId, conns: &[(usize, Multiaddr)], addrs: &[Multiaddr], @@ -472,7 +471,7 @@ mod tests { } store.set_peer_addresses(peer, addrs.to_vec()); } - QuicUpgradeBehaviour::new(ctx, local, quic_enabled) + QuicUpgradeBehaviour::new(ctx, local, true) } /// The reason carried by the queued `UpgradeFailed` event for `peer`. @@ -528,39 +527,10 @@ mod tests { .collect() } - #[tokio::test] - async fn tick_dials_the_quic_addrs_of_a_tcp_connected_peer() { - let peer = PeerId::random(); - let mut behaviour = connected( - true, - peer, - &[(1, addr(TCP))], - &[addr(TCP), addr(QUIC), relayed(QUIC)], - ); - - behaviour.run_upgrade_logic(); - - assert_eq!(dialed(&behaviour), vec![peer]); - assert!( - closed(&behaviour).is_empty(), - "TCP stays up until QUIC is established" - ); - let Some(UpgradeState::DialingQuic { tcp_conn_ids }) = - behaviour.pending_upgrades.get(&peer) - else { - panic!("upgrade must be armed for {peer}"); - }; - assert_eq!(tcp_conn_ids, &[ConnectionId::new_unchecked(1)]); - - // An armed upgrade is not dialed again on the next tick. - behaviour.run_upgrade_logic(); - assert_eq!(dialed(&behaviour).len(), 1); - } - #[tokio::test] async fn tick_leaves_relayed_only_peers_to_force_direct() { let peer = PeerId::random(); - let mut behaviour = connected(true, peer, &[(1, relayed(TCP))], &[addr(QUIC)]); + let mut behaviour = connected(peer, &[(1, relayed(TCP))], &[addr(QUIC)]); behaviour.run_upgrade_logic(); @@ -572,7 +542,6 @@ mod tests { async fn tick_closes_redundant_tcp_once_direct_quic_exists() { let peer = PeerId::random(); let mut behaviour = connected( - true, peer, &[(1, addr(TCP)), (2, addr(QUIC)), (3, relayed(TCP))], &[addr(QUIC)], @@ -590,9 +559,13 @@ mod tests { } #[tokio::test] - async fn quic_connection_completes_the_upgrade() { + async fn upgrade_dials_quic_once_then_closes_tcp() { let peer = PeerId::random(); - let mut behaviour = connected(true, peer, &[(1, addr(TCP))], &[addr(QUIC)]); + let mut behaviour = connected( + peer, + &[(1, addr(TCP))], + &[addr(TCP), addr(QUIC), relayed(QUIC)], + ); // An elapsed backoff left over from an earlier failure. behaviour.backoffs.insert( peer, @@ -601,8 +574,24 @@ mod tests { backoff_duration: 4, }, ); + behaviour.run_upgrade_logic(); + assert_eq!(dialed(&behaviour), vec![peer]); + assert!( + closed(&behaviour).is_empty(), + "TCP stays up until QUIC is established" + ); + let Some(UpgradeState::DialingQuic { tcp_conn_ids }) = + behaviour.pending_upgrades.get(&peer) + else { + panic!("upgrade must be armed for {peer}"); + }; + assert_eq!(tcp_conn_ids, &[ConnectionId::new_unchecked(1)]); + + // An armed upgrade is not dialed again on the next tick. + behaviour.run_upgrade_logic(); + assert_eq!(dialed(&behaviour).len(), 1); behaviour.handle_connection_established(peer, &addr(QUIC)); @@ -617,41 +606,33 @@ mod tests { #[tokio::test] async fn failed_upgrade_keeps_tcp_and_arms_backoff() { - /// Fails `peer`'s armed upgrade. - type Fail = fn(&mut QuicUpgradeBehaviour, PeerId); - - let cases: [(&str, Fail); 2] = [ - ("non-QUIC connection", |behaviour, peer| { - behaviour.handle_connection_established(peer, &addr(TCP)); - }), - ("dial failure", |behaviour, peer| { - behaviour.handle_dial_failure(Some(peer)); - }), - ]; - - for (case, fail) in cases { + let armed = || { let peer = PeerId::random(); - let mut behaviour = connected(true, peer, &[(1, addr(TCP))], &[addr(QUIC)]); + let mut behaviour = connected(peer, &[(1, addr(TCP))], &[addr(QUIC)]); behaviour.run_upgrade_logic(); - - // Failures of peers with no armed upgrade are not this behaviour's. - behaviour.handle_dial_failure(Some(PeerId::random())); - behaviour.handle_dial_failure(None); - - fail(&mut behaviour, peer); - - assert!(upgraded(&behaviour).is_empty(), "{case}"); + (behaviour, peer) + }; + let check = |behaviour: &QuicUpgradeBehaviour, peer: &PeerId, case: &str| { + assert!(upgraded(behaviour).is_empty(), "{case}"); assert!( - closed(&behaviour).is_empty(), + closed(behaviour).is_empty(), "{case}: the TCP connection is kept" ); - assert!(failure_reason(&behaviour, &peer).is_some(), "{case}"); + assert!(failure_reason(behaviour, peer).is_some(), "{case}"); assert!(behaviour.pending_upgrades.is_empty(), "{case}"); assert!( - behaviour.backoffs.contains_key(&peer), + behaviour.backoffs.contains_key(peer), "{case}: failure arms the backoff" ); - } + }; + + let (mut behaviour, peer) = armed(); + behaviour.handle_connection_established(peer, &addr(TCP)); + check(&behaviour, &peer, "non-QUIC connection"); + + let (mut behaviour, peer) = armed(); + behaviour.handle_dial_failure(Some(peer)); + check(&behaviour, &peer, "dial failure"); } #[test] diff --git a/crates/p2p/tests/advertised_addrs.rs b/crates/p2p/tests/advertised_addrs.rs index c3507885..1c8043bf 100644 --- a/crates/p2p/tests/advertised_addrs.rs +++ b/crates/p2p/tests/advertised_addrs.rs @@ -31,16 +31,24 @@ fn tcp_port(addr: &Multiaddr) -> Option { }) } -/// Drives `node` until it has reported `want` listen addresses. -async fn listen_addrs(node: &mut ClientNode, want: usize) -> Vec { +/// Drives `node` until it has reported both a direct and a relayed listen +/// address, and returns the direct one. +async fn direct_listen_addr(node: &mut ClientNode) -> Multiaddr { timeout(TEST_TIMEOUT, async { - let mut addrs = Vec::with_capacity(want); - while addrs.len() < want { + let mut direct = None; + let mut relayed = false; + loop { if let SwarmEvent::NewListenAddr { address, .. } = node.select_next_some().await { - addrs.push(address); + if is_relay_addr(&address) { + relayed = true; + } else { + direct = Some(address); + } + if relayed && let Some(addr) = &direct { + return addr.clone(); + } } } - addrs }) .await .expect("timed out waiting for the listen addresses") @@ -88,11 +96,7 @@ async fn advertises_own_bound_ports_only() { ) .expect("build node B"); - let bound = listen_addrs(&mut node_a, 2) - .await - .into_iter() - .find(|addr| !is_relay_addr(addr)) - .expect("A must report its TCP listen address"); + let bound = direct_listen_addr(&mut node_a).await; let bound_port = tcp_port(&bound).expect("bound TCP port"); assert!(bound_port != 0, "kernel must have assigned a port"); @@ -128,8 +132,6 @@ async fn advertises_own_bound_ports_only() { "external address {external} missing from {advertised:?}", ); - // Neither the configured port 0 nor the relay's port may appear: every - // non-circuit address carries the port A actually bound. assert!( advertised .iter() From 0e41f044f2648332d84d7d21b51aa5616b07e0e7 Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:23:33 -0300 Subject: [PATCH 09/10] docs(app): reword the node type comment --- crates/app/src/node/behaviour.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/app/src/node/behaviour.rs b/crates/app/src/node/behaviour.rs index e6ae4954..ecd72895 100644 --- a/crates/app/src/node/behaviour.rs +++ b/crates/app/src/node/behaviour.rs @@ -214,7 +214,7 @@ pub(crate) async fn wire_p2p( // checker observes the same shared peer/connection state the swarm updates. let p2p_context_for_handle = p2p_context.clone(); - // Charon's `wireP2P` picks the node type off the same flag. + // Mirrors Charon's `wireP2P`. let node_type = if feature_set.enabled(pluto_featureset::Feature::Quic) { NodeType::QUIC } else { From 2940e78bf0176f9b90c9e613b4a2aab27a342636 Mon Sep 17 00:00:00 2001 From: "emlautarom1-agent[bot]" <292495798+emlautarom1-agent[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:34:20 -0300 Subject: [PATCH 10/10] fix(p2p): close relayed TCP too once a direct QUIC connection exists 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. --- crates/p2p/src/quic_upgrade.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/p2p/src/quic_upgrade.rs b/crates/p2p/src/quic_upgrade.rs index 3fdf1fa6..2f446e94 100644 --- a/crates/p2p/src/quic_upgrade.rs +++ b/crates/p2p/src/quic_upgrade.rs @@ -229,11 +229,10 @@ impl QuicUpgradeBehaviour { "already has direct QUIC connection to peer" ); + // Relayed TCP connections are redundant too, as in Charon. let tcp_conn_ids: Vec<_> = conns .iter() - .filter(|c| { - utils::is_tcp_addr(&c.remote_addr) && !utils::is_relay_addr(&c.remote_addr) - }) + .filter(|c| utils::is_tcp_addr(&c.remote_addr)) .map(|c| c.connection_id) .collect(); @@ -549,10 +548,12 @@ mod tests { behaviour.run_upgrade_logic(); + let mut closed = closed(&behaviour); + closed.sort(); assert_eq!( - closed(&behaviour), - vec![ConnectionId::new_unchecked(1)], - "only the direct TCP connection is redundant" + closed, + [1, 3].map(ConnectionId::new_unchecked), + "every TCP connection is redundant, relayed ones included" ); assert!(dialed(&behaviour).is_empty()); assert!(behaviour.pending_upgrades.is_empty());