From 8bb2470e92da6a712f63262a08d1f1869ca4ef56 Mon Sep 17 00:00:00 2001 From: Soma <0421.soma@gmail.com> Date: Sun, 16 Aug 2026 08:48:50 +0900 Subject: [PATCH 1/2] feat(state-node): let nodes behind NAT take part, via AutoNAT v2 + relay + DCUtR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now a node could only join if it was directly dialable, which in practice means a machine with a public address. That is a real limit on "anyone can run a node": most people are behind a home router or a NAT gateway. This adds the three libp2p pieces that together remove it. **AutoNAT v2 decides the role, by measurement.** A node asks peers to dial a specific address of its own back; they answer only for addresses they actually reached. So "am I reachable?" is answered by observation rather than by the node's own assumption — which matters because a node that wrongly believes it is reachable advertises relay service it cannot provide. v2 rather than v1 because v1 takes the server's word for it; there is no external-compatibility reason to prefer v1, since every peer here runs this same binary. **Circuit relay v2 turns every reachable node into a relay.** No node is designated: the role follows from measured reachability, the same way the existing application-level relay role follows from whether a node is a member of a content network. Reservations and circuits are capped (128 / 32) because relaying is work done for strangers. **DCUtR gets the relay back out of the path.** Once a relayed connection exists, both sides hole-punch to a direct one, so a relay carries traffic only until the upgrade lands. Nothing here changes how directly-reachable nodes talk to each other. The existing Monas relay — forwarding *requests* to a content network's members — is a separate, application-level mechanism and is untouched; this is about establishing the *connection* underneath it. `--disable-nat-traversal` turns the machinery off for a deployment that does not need it. Two implementation mistakes worth recording, both caught by writing tests that exercise the code rather than restate it: - The circuit address was built from the relay's peer id alone. Every `listen_on` would have failed with `MissingRelayAddr` — the client transport needs to know *where* to open the circuit — and the failure was logged at debug, so the private-node path would have been silently dead. Candidates now come from the connection table, which has the addresses we reached peers on, filtered to ones a third party could dial. - Releasing a reservation only logged what it would drop: `Swarm::listeners()` yields addresses, not `ListenerId`s, so nothing was removed and a relay kept a slot allocated for a node that no longer needed it. The ids are now kept alongside the peer. Verified on a local 4-node cluster with NAT traversal on and mDNS off: AutoNAT converged all four to Public (correct — they are mutually dialable), no relay reservations were taken (also correct), zero errors, and the reconvergence behaviour from the parent branch still works. **Hole punching itself is not verified.** It needs two nodes behind *different* NATs, which a single host cannot provide — and a local cluster would "pass" for the wrong reason, the way mDNS masked the reconvergence bug on the parent branch. Treat the two-sided NAT path as untested until it is run across real networks. cargo test --workspace: 807 passed, 0 failed binaries. clippy (1.97) clean. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 76 ++++ monas-state-node/Cargo.toml | 4 + .../src/application_service/node.rs | 4 + monas-state-node/src/bin/state_node.rs | 12 + .../src/infrastructure/network/behaviour.rs | 224 +++++++++- .../infrastructure/network/libp2p_network.rs | 418 +++++++++++++++++- .../src/infrastructure/network/transport.rs | 49 +- .../tests/create_content_push_race_test.rs | 1 + monas-state-node/tests/e2e_multi_node_test.rs | 1 + monas-state-node/tests/integration_test.rs | 3 + .../tests/public_key_exchange_test.rs | 2 + 11 files changed, 785 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4c87ae9..1950615 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2467,8 +2467,10 @@ dependencies = [ "futures-timer", "getrandom 0.2.16", "libp2p-allow-block-list", + "libp2p-autonat", "libp2p-connection-limits", "libp2p-core", + "libp2p-dcutr", "libp2p-dns", "libp2p-gossipsub", "libp2p-identify", @@ -2478,6 +2480,7 @@ dependencies = [ "libp2p-metrics", "libp2p-noise", "libp2p-quic", + "libp2p-relay", "libp2p-request-response", "libp2p-swarm", "libp2p-tcp", @@ -2500,6 +2503,31 @@ dependencies = [ "libp2p-swarm", ] +[[package]] +name = "libp2p-autonat" +version = "0.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fab5e25c49a7d48dac83d95d8f3bac0a290d8a5df717012f6e34ce9886396c0b" +dependencies = [ + "async-trait", + "asynchronous-codec", + "either", + "futures", + "futures-bounded", + "futures-timer", + "libp2p-core", + "libp2p-identity", + "libp2p-request-response", + "libp2p-swarm", + "quick-protobuf", + "quick-protobuf-codec", + "rand 0.8.5", + "rand_core 0.6.4", + "thiserror 2.0.17", + "tracing", + "web-time", +] + [[package]] name = "libp2p-connection-limits" version = "0.6.0" @@ -2536,6 +2564,28 @@ dependencies = [ "web-time", ] +[[package]] +name = "libp2p-dcutr" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f4f0eec23bc79cabfdf6934718f161fc42a1d98e2c9d44007c80eb91534200c" +dependencies = [ + "asynchronous-codec", + "either", + "futures", + "futures-bounded", + "futures-timer", + "libp2p-core", + "libp2p-identity", + "libp2p-swarm", + "lru", + "quick-protobuf", + "quick-protobuf-codec", + "thiserror 2.0.17", + "tracing", + "web-time", +] + [[package]] name = "libp2p-dns" version = "0.44.0" @@ -2675,10 +2725,12 @@ checksum = "805a555148522cb3414493a5153451910cb1a146c53ffbf4385708349baf62b7" dependencies = [ "futures", "libp2p-core", + "libp2p-dcutr", "libp2p-gossipsub", "libp2p-identify", "libp2p-identity", "libp2p-kad", + "libp2p-relay", "libp2p-swarm", "pin-project", "prometheus-client", @@ -2730,6 +2782,30 @@ dependencies = [ "tracing", ] +[[package]] +name = "libp2p-relay" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "551b24ae04c63859bf5e25644acdd6aa469deb5c5cd872ca21c2c9b45a5a5192" +dependencies = [ + "asynchronous-codec", + "bytes", + "either", + "futures", + "futures-bounded", + "futures-timer", + "libp2p-core", + "libp2p-identity", + "libp2p-swarm", + "quick-protobuf", + "quick-protobuf-codec", + "rand 0.8.5", + "static_assertions", + "thiserror 2.0.17", + "tracing", + "web-time", +] + [[package]] name = "libp2p-request-response" version = "0.29.0" diff --git a/monas-state-node/Cargo.toml b/monas-state-node/Cargo.toml index c687f9b..a92f518 100644 --- a/monas-state-node/Cargo.toml +++ b/monas-state-node/Cargo.toml @@ -88,6 +88,10 @@ features = [ "macros", "cbor", "quic", + # NAT traversal: reachability probing, circuit relay v2, and hole punching. + "autonat", + "relay", + "dcutr", ] # WebRTC transport (alpha - for future browser-to-server communication) diff --git a/monas-state-node/src/application_service/node.rs b/monas-state-node/src/application_service/node.rs index 9194e67..ee39883 100644 --- a/monas-state-node/src/application_service/node.rs +++ b/monas-state-node/src/application_service/node.rs @@ -694,6 +694,7 @@ mod tests { listen_addrs: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()], bootstrap_nodes: vec![], enable_mdns: false, + enable_nat_traversal: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }, @@ -723,6 +724,7 @@ mod tests { listen_addrs: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()], bootstrap_nodes: vec![], enable_mdns: false, + enable_nat_traversal: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }, @@ -753,6 +755,7 @@ mod tests { listen_addrs: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()], bootstrap_nodes: vec![], enable_mdns: false, + enable_nat_traversal: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }, @@ -784,6 +787,7 @@ mod tests { listen_addrs: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()], bootstrap_nodes: vec![], enable_mdns: false, + enable_nat_traversal: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }, diff --git a/monas-state-node/src/bin/state_node.rs b/monas-state-node/src/bin/state_node.rs index 9683151..7716075 100644 --- a/monas-state-node/src/bin/state_node.rs +++ b/monas-state-node/src/bin/state_node.rs @@ -57,6 +57,14 @@ struct Args { #[arg(long)] disable_mdns: bool, + /// Disable NAT traversal (AutoNAT v2, circuit relay v2, DCUtR). + /// + /// NAT traversal is what lets a node behind a home router or a NAT gateway + /// join at all. A deployment where every node is publicly reachable does + /// not need it and can turn the machinery off. + #[arg(long)] + disable_nat_traversal: bool, + /// Log level (trace, debug, info, warn, error). #[arg(long, default_value = "info")] log_level: String, @@ -83,8 +91,12 @@ async fn main() -> Result<()> { .parse::() .context("Failed to parse P2P listen address")?], enable_mdns: !args.disable_mdns, + enable_nat_traversal: !args.disable_nat_traversal, ..Default::default() }; + if args.disable_nat_traversal { + tracing::info!("NAT traversal disabled; this node can only reach directly-dialable peers"); + } if args.disable_mdns { tracing::info!( "mDNS disabled; discovery relies on bootstrap peers, Kademlia and the peer store" diff --git a/monas-state-node/src/infrastructure/network/behaviour.rs b/monas-state-node/src/infrastructure/network/behaviour.rs index ea66c7c..dcbf3fa 100644 --- a/monas-state-node/src/infrastructure/network/behaviour.rs +++ b/monas-state-node/src/infrastructure/network/behaviour.rs @@ -10,7 +10,7 @@ use super::protocol::{ContentRequest, ContentResponse}; use super::public_key_protocol::{PublicKeyRequest, PublicKeyResponse}; use libp2p::{ - gossipsub, identify, kad, + autonat, dcutr, gossipsub, identify, kad, relay, request_response::{self, ProtocolSupport}, swarm::{behaviour::toggle::Toggle, NetworkBehaviour}, StreamProtocol, @@ -26,6 +26,20 @@ pub const CONTENT_PROTOCOL_NAME: &str = "/monas/content/1.0.0"; /// Protocol name for public key exchange. pub const PUBLIC_KEY_PROTOCOL_NAME: &str = "/monas/public-key/1.0.0"; +/// Simultaneous relay reservations this node will hold for others. +/// +/// A reservation costs a slot and some memory, and relaying is a favour done +/// for peers we may know nothing about, so it is capped. Reached only on a +/// node that many private peers picked as their relay. +const MAX_RELAY_RESERVATIONS: usize = 128; + +/// Simultaneous relayed circuits carried for others. +/// +/// Circuits carry real traffic, so this is the tighter of the two limits. +/// DCUtR normally promotes a circuit to a direct connection shortly after it +/// is established, which is what keeps this number small in practice. +const MAX_RELAY_CIRCUITS: usize = 32; + /// Combined network behaviour for the state node. #[derive(NetworkBehaviour)] #[behaviour(to_swarm = "NodeBehaviourEvent")] @@ -44,6 +58,19 @@ pub struct NodeBehaviour { /// config — see `BehaviourConfig::enable_mdns`. #[cfg(not(target_arch = "wasm32"))] pub mdns: Toggle, + /// AutoNAT v2 client: asks other nodes to dial us back so we learn whether + /// we are actually reachable, instead of assuming it. + pub autonat_client: Toggle, + /// AutoNAT v2 server: answers those requests for other nodes. + pub autonat_server: Toggle, + /// Circuit relay v2 server — lets other nodes reach peers we can reach. + /// Only enabled once we believe we are publicly reachable. + pub relay_server: Toggle, + /// Circuit relay v2 client — used when we cannot be dialled directly. + pub relay_client: Toggle, + /// DCUtR: upgrades a relayed connection to a direct one via hole punching, + /// so the relay is only needed to get started. + pub dcutr: Toggle, } /// Events generated by the combined behaviour. @@ -56,6 +83,11 @@ pub enum NodeBehaviourEvent { Identify(Box), #[cfg(not(target_arch = "wasm32"))] Mdns(mdns::Event), + AutonatClient(autonat::v2::client::Event), + AutonatServer(autonat::v2::server::Event), + RelayServer(Box), + RelayClient(relay::client::Event), + Dcutr(dcutr::Event), } impl From for NodeBehaviourEvent { @@ -95,6 +127,36 @@ impl From for NodeBehaviourEvent { } } +impl From for NodeBehaviourEvent { + fn from(event: autonat::v2::client::Event) -> Self { + NodeBehaviourEvent::AutonatClient(event) + } +} + +impl From for NodeBehaviourEvent { + fn from(event: autonat::v2::server::Event) -> Self { + NodeBehaviourEvent::AutonatServer(event) + } +} + +impl From for NodeBehaviourEvent { + fn from(event: relay::Event) -> Self { + NodeBehaviourEvent::RelayServer(Box::new(event)) + } +} + +impl From for NodeBehaviourEvent { + fn from(event: relay::client::Event) -> Self { + NodeBehaviourEvent::RelayClient(event) + } +} + +impl From for NodeBehaviourEvent { + fn from(event: dcutr::Event) -> Self { + NodeBehaviourEvent::Dcutr(event) + } +} + /// Configuration for creating a NodeBehaviour. #[derive(Debug, Clone)] pub struct BehaviourConfig { @@ -111,6 +173,13 @@ pub struct BehaviourConfig { /// it off is what lets a local 4-node cluster reproduce production /// conditions. pub enable_mdns: bool, + /// Enable NAT traversal: AutoNAT v2 reachability probing, circuit relay v2 + /// and DCUtR hole punching. + /// + /// This is what allows a node behind NAT to take part at all. It is + /// deliberately separable so a deployment of publicly reachable nodes — + /// which needs none of it — can leave the machinery off. + pub enable_nat_traversal: bool, } impl Default for BehaviourConfig { @@ -119,17 +188,24 @@ impl Default for BehaviourConfig { protocol_version: "/monas/1.0.0".to_string(), agent_version: format!("monas-state-node/{}", env!("CARGO_PKG_VERSION")), enable_mdns: true, + enable_nat_traversal: true, } } } impl NodeBehaviour { /// Create a new NodeBehaviour with the given peer ID and configuration. + /// `relay_client` must be the behaviour half returned by + /// `relay::client::new` alongside the transport that was passed to + /// `transport::build_transport_with_relay`. Passing `None` disables + /// relayed dialling even when `enable_nat_traversal` is set — the two + /// halves only work as a pair. #[cfg(not(target_arch = "wasm32"))] pub fn new( local_peer_id: libp2p::PeerId, keypair: &libp2p::identity::Keypair, config: BehaviourConfig, + relay_client: Option, ) -> anyhow::Result { // Kademlia configuration let mut kad_config = kad::Config::new(StreamProtocol::new("/monas/kad/1.0.0")); @@ -194,6 +270,49 @@ impl NodeBehaviour { } .into(); + // NAT traversal. + // + // Both AutoNAT roles are enabled together on purpose. A node cannot + // know in advance which side it will be on, and a network where only + // some nodes answer probes degrades for everyone: reachability is a + // service peers provide to each other, like relaying. + // + // The relay *server* is created here but only ever used once AutoNAT + // reports us as reachable (see libp2p_network.rs). Relaying from + // behind NAT would not work anyway, and advertising it would waste + // other nodes' reservation attempts. + let (autonat_client, autonat_server, relay_server, dcutr) = if config.enable_nat_traversal { + ( + Some(autonat::v2::client::Behaviour::new( + rand::rngs::OsRng, + autonat::v2::client::Config::default(), + )), + Some(autonat::v2::server::Behaviour::new(rand::rngs::OsRng)), + Some(relay::Behaviour::new( + local_peer_id, + // Bound what a relay will carry. Relaying is done for + // strangers, so the limits are what stop it from becoming + // free bandwidth for anyone who asks. + relay::Config { + max_reservations: MAX_RELAY_RESERVATIONS, + max_circuits: MAX_RELAY_CIRCUITS, + ..Default::default() + }, + )), + Some(dcutr::Behaviour::new(local_peer_id)), + ) + } else { + (None, None, None, None) + }; + + // The client half is only meaningful when its transport half was built + // too, so it is threaded in from the caller rather than created here. + let relay_client = if config.enable_nat_traversal { + relay_client + } else { + None + }; + Ok(Self { kademlia, gossipsub, @@ -201,6 +320,11 @@ impl NodeBehaviour { public_key_protocol, identify, mdns, + autonat_client: autonat_client.into(), + autonat_server: autonat_server.into(), + relay_server: relay_server.into(), + relay_client: relay_client.into(), + dcutr: dcutr.into(), }) } @@ -297,6 +421,7 @@ mod tests { protocol_version: "/custom/1.0.0".to_string(), agent_version: "custom-agent/1.0.0".to_string(), enable_mdns: true, + enable_nat_traversal: false, }; let cloned = config.clone(); @@ -344,6 +469,7 @@ mod tests { enable_mdns: false, ..BehaviourConfig::default() }, + None, ) .unwrap(); assert!( @@ -358,6 +484,7 @@ mod tests { enable_mdns: true, ..BehaviourConfig::default() }, + None, ) .unwrap(); assert!( @@ -366,6 +493,96 @@ mod tests { ); } + /// The NAT-traversal behaviours must follow the flag, both ways. + /// + /// Same failure mode as `enable_mdns`, which sat in the config being read + /// by nobody: a flag that silently does nothing is worse than no flag, + /// because it makes a deployment look configurable when it is not. + #[cfg(not(target_arch = "wasm32"))] + #[tokio::test] + async fn nat_traversal_behaviours_follow_the_flag() { + let keypair = Keypair::generate_ed25519(); + let local_peer_id = keypair.public().to_peer_id(); + + let off = NodeBehaviour::new( + local_peer_id, + &keypair, + BehaviourConfig { + enable_nat_traversal: false, + ..BehaviourConfig::default() + }, + None, + ) + .unwrap(); + assert!(off.autonat_client.as_ref().is_none()); + assert!(off.autonat_server.as_ref().is_none()); + assert!(off.relay_server.as_ref().is_none()); + assert!(off.dcutr.as_ref().is_none()); + + let on = NodeBehaviour::new( + local_peer_id, + &keypair, + BehaviourConfig { + enable_nat_traversal: true, + ..BehaviourConfig::default() + }, + None, + ) + .unwrap(); + assert!(on.autonat_client.as_ref().is_some()); + assert!(on.autonat_server.as_ref().is_some()); + assert!(on.relay_server.as_ref().is_some()); + assert!(on.dcutr.as_ref().is_some()); + } + + /// The relay client only exists when its transport half was built too. + /// + /// `relay::client::new` returns a transport and a behaviour that are + /// useless apart: the behaviour negotiates a reservation the transport + /// would then have to dial through. Enabling one without the other would + /// advertise a circuit address nothing can actually dial. + #[cfg(not(target_arch = "wasm32"))] + #[tokio::test] + async fn relay_client_is_absent_unless_its_transport_half_is_supplied() { + let keypair = Keypair::generate_ed25519(); + let local_peer_id = keypair.public().to_peer_id(); + + let without = + NodeBehaviour::new(local_peer_id, &keypair, BehaviourConfig::default(), None).unwrap(); + assert!(without.relay_client.as_ref().is_none()); + + let (_transport, client) = relay::client::new(local_peer_id); + let with = NodeBehaviour::new( + local_peer_id, + &keypair, + BehaviourConfig::default(), + Some(client), + ) + .unwrap(); + assert!(with.relay_client.as_ref().is_some()); + } + + /// Passing a relay client while NAT traversal is off must not enable it. + #[cfg(not(target_arch = "wasm32"))] + #[tokio::test] + async fn relay_client_is_ignored_when_nat_traversal_is_disabled() { + let keypair = Keypair::generate_ed25519(); + let local_peer_id = keypair.public().to_peer_id(); + + let (_transport, client) = relay::client::new(local_peer_id); + let behaviour = NodeBehaviour::new( + local_peer_id, + &keypair, + BehaviourConfig { + enable_nat_traversal: false, + ..BehaviourConfig::default() + }, + Some(client), + ) + .unwrap(); + assert!(behaviour.relay_client.as_ref().is_none()); + } + #[cfg(not(target_arch = "wasm32"))] #[tokio::test] async fn test_node_behaviour_creation() { @@ -373,7 +590,7 @@ mod tests { let local_peer_id = keypair.public().to_peer_id(); let config = BehaviourConfig::default(); - let result = NodeBehaviour::new(local_peer_id, &keypair, config); + let result = NodeBehaviour::new(local_peer_id, &keypair, config, None); assert!(result.is_ok()); let behaviour = result.unwrap(); @@ -396,9 +613,10 @@ mod tests { protocol_version: "/test/1.0.0".to_string(), agent_version: "test-agent/0.1.0".to_string(), enable_mdns: true, + enable_nat_traversal: false, }; - let result = NodeBehaviour::new(local_peer_id, &keypair, config); + let result = NodeBehaviour::new(local_peer_id, &keypair, config, None); assert!(result.is_ok()); } diff --git a/monas-state-node/src/infrastructure/network/libp2p_network.rs b/monas-state-node/src/infrastructure/network/libp2p_network.rs index 66108ff..f3ef8fd 100644 --- a/monas-state-node/src/infrastructure/network/libp2p_network.rs +++ b/monas-state-node/src/infrastructure/network/libp2p_network.rs @@ -70,6 +70,39 @@ fn reusable_addr(endpoint: &ConnectedPoint) -> Option<&Multiaddr> { } } +/// How many relays a private node keeps reservations with. +/// +/// More than one because a relay can vanish, and a private node with no +/// reservation is unreachable; not many more because each reservation costs +/// the relay a slot. +const TARGET_RELAY_RESERVATIONS: usize = 2; + +/// What AutoNAT has told us about our own reachability. +/// +/// Deliberately three-valued. "We have not been told yet" is not the same as +/// "we are private": acting on the former would have every node briefly +/// announce itself as needing a relay at startup, and a node that never gets +/// an answer (no AutoNAT server around) should not silently behave as if it +/// were behind NAT. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +enum Reachability { + #[default] + Unknown, + Public, + Private, +} + +/// Tracks reachability and the relay reservations that follow from it. +#[derive(Default)] +struct NatState { + reachability: Reachability, + /// Relays we currently listen through, with the listener that holds each + /// reservation. The `ListenerId` is kept because releasing a reservation + /// means removing its listener, and `Swarm::listeners()` only yields + /// addresses — there is no way back to the id from those. + reserved_with: HashMap, +} + /// A relay request received from a remote peer via P2P protocol. /// The swarm loop sends these through a channel to the application layer (node.rs), /// which processes them using StateNodeService. @@ -168,6 +201,9 @@ pub struct Libp2pNetworkConfig { pub bootstrap_nodes: Vec<(PeerId, Multiaddr)>, /// Enable mDNS for local peer discovery. pub enable_mdns: bool, + /// Enable NAT traversal (AutoNAT v2 + circuit relay v2 + DCUtR), which is + /// what lets a node behind NAT participate at all. + pub enable_nat_traversal: bool, /// Gossipsub topics to subscribe to. pub gossipsub_topics: Vec, /// Externally reachable addresses to advertise to peers (e.g. a public @@ -191,6 +227,7 @@ impl Default for Libp2pNetworkConfig { ], bootstrap_nodes: vec![], enable_mdns: true, + enable_nat_traversal: true, gossipsub_topics: vec!["monas-events".to_string()], external_addrs: vec![], } @@ -494,9 +531,26 @@ impl Libp2pNetwork { info!("Local peer ID: {}", local_peer_id); - // Build transport - let transport = - transport::build_transport(&keypair).context("Failed to build transport")?; + // Build transport, and the relay client if NAT traversal is on. + // + // `relay::client::new` hands back a transport half and a behaviour + // half that only work together: the transport is what actually dials + // `/…/p2p-circuit/…`, the behaviour is what negotiates the reservation. + // They are created here so both can be handed to their respective + // owners. + let (transport, relay_client) = if config.enable_nat_traversal { + let (relay_transport, relay_behaviour) = libp2p::relay::client::new(local_peer_id); + ( + transport::build_transport_with_relay(&keypair, relay_transport) + .context("Failed to build transport with relay client")?, + Some(relay_behaviour), + ) + } else { + ( + transport::build_transport(&keypair).context("Failed to build transport")?, + None, + ) + }; // Build behaviour // `enable_mdns` used to be ignored here: mDNS was always on, so a local @@ -508,8 +562,10 @@ impl Libp2pNetwork { &keypair, BehaviourConfig { enable_mdns: config.enable_mdns, + enable_nat_traversal: config.enable_nat_traversal, ..BehaviourConfig::default() }, + relay_client, )?; // Create swarm with connection limits to prevent FD/memory exhaustion (M-3). @@ -705,6 +761,7 @@ impl Libp2pNetwork { } } let mut peer_store_dirty = false; + let mut nat_state = NatState::default(); // Periodically re-establish connectivity. Without this a node that // loses every connection stays isolated forever: `ConnectionClosed` @@ -734,7 +791,7 @@ impl Libp2pNetwork { } } } - Self::handle_swarm_event(&mut swarm, &mut pending, &connected_peers, &event_tx, &crdt_repo, &data_dir, &p256_signing_key, &relay_channels, &content_network_repo, event).await; + Self::handle_swarm_event(&mut swarm, &mut pending, &connected_peers, &event_tx, &crdt_repo, &data_dir, &p256_signing_key, &relay_channels, &content_network_repo, &mut nat_state, event).await; } // Periodic cleanup of stale pending requests _ = cleanup_interval.tick() => { @@ -758,6 +815,146 @@ impl Libp2pNetwork { } } + /// Record what AutoNAT concluded about our own reachability. + /// + /// This is the one place the public/private decision is made, and it is + /// made from *measurement*: AutoNAT v2 asks other nodes to dial a specific + /// address of ours back, and they answer only for addresses they actually + /// reached. A node cannot talk itself into being considered reachable, + /// which matters because the alternative — self-reported reachability — + /// lets a node advertise relay service it cannot provide. + async fn handle_autonat_client_event( + swarm: &mut Swarm, + connected_peers: &Arc>>>, + nat_state: &mut NatState, + event: libp2p::autonat::v2::client::Event, + ) { + let observed = if event.result.is_ok() { + Reachability::Public + } else { + Reachability::Private + }; + + if nat_state.reachability == observed { + return; + } + + info!( + "AutoNAT: reachability {:?} -> {:?} (tested {})", + nat_state.reachability, observed, event.tested_addr + ); + nat_state.reachability = observed; + + match observed { + Reachability::Public => { + // We can be dialled, so the address is worth announcing and + // there is no reason to occupy someone else's relay slot. + swarm.add_external_address(event.tested_addr.clone()); + Self::release_relay_reservations(swarm, nat_state); + } + Reachability::Private => { + Self::ensure_relay_reservations(swarm, connected_peers, nat_state).await + } + Reachability::Unknown => {} + } + } + + /// Reserve a slot on connected relays so other nodes can reach us. + /// + /// Any connected peer may be asked: a relay is not a special kind of node, + /// just one that happens to be reachable. Reservations are attempted with + /// peers we are already connected to, since a peer we cannot dial cannot + /// relay for us either. + async fn ensure_relay_reservations( + swarm: &mut Swarm, + connected_peers: &Arc>>>, + nat_state: &mut NatState, + ) { + if swarm.behaviour().relay_client.as_ref().is_none() { + return; + } + + // A circuit address must carry the relay's *dialable* address, not + // just its peer id: the client transport rejects `/p2p//p2p-circuit` + // with `MissingRelayAddr`, because it has to know where to open the + // circuit. So candidates come from the connection table, which is + // where the addresses we actually reached them on are recorded. + let candidates: Vec<(PeerId, Multiaddr)> = connected_peers + .read() + .await + .iter() + .filter(|(p, _)| !nat_state.reserved_with.contains_key(p)) + .filter_map(|(p, addrs)| { + addrs + .iter() + .find(|a| Self::is_relayable_addr(a)) + .map(|a| (*p, a.clone())) + }) + .collect(); + + for (peer, addr) in candidates { + if nat_state.reserved_with.len() >= TARGET_RELAY_RESERVATIONS { + break; + } + // Listening on a circuit address is what actually requests the + // reservation; the relay accepts or refuses, and a refusal simply + // means this peer will not be one of our relays. + let circuit = addr + .with(libp2p::multiaddr::Protocol::P2p(peer)) + .with(libp2p::multiaddr::Protocol::P2pCircuit); + match swarm.listen_on(circuit.clone()) { + Ok(listener_id) => { + info!("Requesting relay reservation via {}", circuit); + nat_state.reserved_with.insert(peer, listener_id); + } + Err(e) => debug!("Cannot listen via relay {}: {}", peer, e), + } + } + } + + /// Whether an address can serve as the relay hop of a circuit address. + /// + /// Excludes addresses that are already relayed (no circuits through + /// circuits) and ones that only work from where we are standing — a relay + /// has to be reachable by the *third* party we want to be reached by. + fn is_relayable_addr(addr: &Multiaddr) -> bool { + use libp2p::multiaddr::Protocol; + let mut routable = true; + for p in addr.iter() { + match p { + Protocol::P2pCircuit => return false, + Protocol::Ip4(ip) + if ip.is_loopback() || ip.is_link_local() || ip.is_unspecified() => + { + routable = false; + } + Protocol::Ip6(ip) if ip.is_loopback() || ip.is_unspecified() => { + routable = false; + } + _ => {} + } + } + routable + } + + /// Stop using relays once we know we are directly reachable. + fn release_relay_reservations(swarm: &mut Swarm, nat_state: &mut NatState) { + if nat_state.reserved_with.is_empty() { + return; + } + info!( + "Reachable directly; releasing {} relay reservation(s)", + nat_state.reserved_with.len() + ); + // Removing the circuit listener is what actually drops the + // reservation, freeing the slot on a relay for a node that needs it. + for (peer, listener_id) in nat_state.reserved_with.drain() { + if swarm.remove_listener(listener_id) { + debug!("Released relay reservation with {}", peer); + } + } + } + /// Re-dial bootstrap peers and re-run the Kademlia bootstrap when we are /// short on connections. /// @@ -1064,6 +1261,7 @@ impl Libp2pNetwork { content_network_repo: &Option< Arc>, >, + nat_state: &mut NatState, event: SwarmEvent, ) { match event { @@ -1092,6 +1290,26 @@ impl Libp2pNetwork { SwarmEvent::Behaviour(NodeBehaviourEvent::Identify(identify_event)) => { Self::handle_identify_event(swarm, *identify_event).await; } + SwarmEvent::Behaviour(NodeBehaviourEvent::AutonatClient(ev)) => { + Self::handle_autonat_client_event(swarm, connected_peers, nat_state, ev).await; + } + SwarmEvent::Behaviour(NodeBehaviourEvent::Dcutr(ev)) => { + // A successful upgrade means the pair is now talking directly + // and the relay is out of the path — the outcome the whole + // relay machinery exists to reach. + match ev.result { + Ok(_) => info!("Hole punch to {} succeeded; now direct", ev.remote_peer_id), + Err(e) => debug!("Hole punch to {} failed: {}", ev.remote_peer_id, e), + } + } + SwarmEvent::Behaviour(NodeBehaviourEvent::RelayClient( + libp2p::relay::client::Event::ReservationReqAccepted { relay_peer_id, .. }, + )) => { + info!( + "Relay reservation accepted by {}; reachable through it now", + relay_peer_id + ); + } #[cfg(not(target_arch = "wasm32"))] SwarmEvent::Behaviour(NodeBehaviourEvent::Mdns(mdns_event)) => { Self::handle_mdns_event(swarm, connected_peers, mdns_event).await; @@ -2702,12 +2920,204 @@ mod tests { ); } + /// Build a swarm with NAT traversal on, matching how the real one is + /// assembled (relay transport and behaviour created as a pair). + fn nat_swarm() -> Swarm { + let keypair = libp2p::identity::Keypair::generate_ed25519(); + let peer_id = PeerId::from(keypair.public()); + let (relay_transport, relay_client) = libp2p::relay::client::new(peer_id); + let transport = + super::super::transport::build_transport_with_relay(&keypair, relay_transport).unwrap(); + let behaviour = NodeBehaviour::new( + peer_id, + &keypair, + BehaviourConfig { + enable_mdns: false, + ..BehaviourConfig::default() + }, + Some(relay_client), + ) + .unwrap(); + Swarm::new( + transport, + behaviour, + peer_id, + libp2p::swarm::Config::with_tokio_executor(), + ) + } + + /// Build the circuit address a reservation listens on. + /// + /// The relay's dialable address has to be in there, not just its peer id: + /// `/p2p//p2p-circuit` is rejected by the client transport with + /// `MissingRelayAddr`, since it would not know where to open the circuit. + fn circuit_addr(relay: PeerId) -> Multiaddr { + "/ip4/192.0.2.10/tcp/9001" + .parse::() + .unwrap() + .with(libp2p::multiaddr::Protocol::P2p(relay)) + .with(libp2p::multiaddr::Protocol::P2pCircuit) + } + + /// A circuit address must be accepted by the relay client transport. + /// + /// `listen_on` is where the shape of the address is validated, and it is + /// the step the first implementation got wrong: it built + /// `/p2p//p2p-circuit` from the peer id alone, which every relay + /// client rejects with `MissingRelayAddr` because it does not say where to + /// open the circuit. Reservations would have failed on every attempt. + #[tokio::test] + async fn a_circuit_address_needs_the_relay_address_not_just_its_peer_id() { + let mut swarm = nat_swarm(); + let relay_peer = PeerId::random(); + + // What the implementation builds now: relay address + peer id. + assert!( + swarm.listen_on(circuit_addr(relay_peer)).is_ok(), + "a circuit address carrying the relay's address must be accepted" + ); + + // What it built before — rejected, so no reservation was ever made. + let peer_id_only = Multiaddr::empty() + .with(libp2p::multiaddr::Protocol::P2p(relay_peer)) + .with(libp2p::multiaddr::Protocol::P2pCircuit); + assert!( + swarm.listen_on(peer_id_only).is_err(), + "a circuit address without the relay's address must be rejected" + ); + } + + /// `ensure_relay_reservations` must produce a circuit address the relay + /// client accepts, and must skip peers whose only known address is one a + /// third party could not dial. + /// + /// This exercises the implementation, unlike the address-shape test above: + /// building the circuit from the peer id alone made every `listen_on` fail + /// with `MissingRelayAddr`, so no reservation was ever taken and the whole + /// private-node path was dead. That failure was silent — the error was + /// logged at debug and the loop moved on. + #[tokio::test] + async fn reservations_are_only_taken_via_dialable_relay_addresses() { + let mut swarm = nat_swarm(); + let mut nat_state = NatState::default(); + + let routable_peer = PeerId::random(); + let loopback_peer = PeerId::random(); + let connected: Arc>>> = Arc::new(RwLock::new( + [ + ( + routable_peer, + vec!["/ip4/198.51.100.7/tcp/9001".parse().unwrap()], + ), + ( + loopback_peer, + vec!["/ip4/127.0.0.1/tcp/9001".parse().unwrap()], + ), + ] + .into_iter() + .collect(), + )); + + Libp2pNetwork::ensure_relay_reservations(&mut swarm, &connected, &mut nat_state).await; + + assert!( + nat_state.reserved_with.contains_key(&routable_peer), + "a reservation must be taken via the routable peer" + ); + assert!( + !nat_state.reserved_with.contains_key(&loopback_peer), + "a loopback-only peer cannot relay for us" + ); + } + + /// Reservations stop at the target so we do not occupy every relay slot + /// in sight. + #[tokio::test] + async fn reservations_stop_at_the_target_count() { + let mut swarm = nat_swarm(); + let mut nat_state = NatState::default(); + + let peers: HashMap> = (0..TARGET_RELAY_RESERVATIONS + 3) + .map(|i| { + ( + PeerId::random(), + vec![format!("/ip4/198.51.100.{}/tcp/9001", i + 1) + .parse() + .unwrap()], + ) + }) + .collect(); + let connected = Arc::new(RwLock::new(peers)); + + Libp2pNetwork::ensure_relay_reservations(&mut swarm, &connected, &mut nat_state).await; + + assert_eq!(nat_state.reserved_with.len(), TARGET_RELAY_RESERVATIONS); + } + + /// Releasing clears the bookkeeping and leaves unrelated listeners alone. + /// + /// Note this asserts the bookkeeping, not the listener teardown itself: + /// the relay client's `remove_listener` marks a listener closed but keeps + /// the entry, so a second call still reports `true` and cannot distinguish + /// "removed" from "never removed". Confirming the teardown needs a live + /// relay, which is part of the manual NAT verification in the README + /// rather than something this unit test can honestly claim. + #[tokio::test] + async fn releasing_clears_reservations_without_touching_other_listeners() { + let mut swarm = nat_swarm(); + let mut nat_state = NatState::default(); + let relay_peer = PeerId::random(); + + let plain = swarm + .listen_on("/ip4/127.0.0.1/tcp/0".parse().unwrap()) + .unwrap(); + let circuit = swarm.listen_on(circuit_addr(relay_peer)).unwrap(); + nat_state.reserved_with.insert(relay_peer, circuit); + nat_state.reachability = Reachability::Private; + + Libp2pNetwork::release_relay_reservations(&mut swarm, &mut nat_state); + + assert!( + nat_state.reserved_with.is_empty(), + "released reservations must be forgotten, or we never retry them" + ); + assert!( + swarm.remove_listener(plain), + "a non-circuit listener must survive the release" + ); + } + + /// A circuit address is only built from an address a third party could + /// dial — a relay reachable only from where we stand relays nothing. + #[test] + fn only_routable_non_circuit_addresses_can_host_a_reservation() { + let routable: Multiaddr = "/ip4/198.51.100.7/tcp/9001".parse().unwrap(); + assert!(Libp2pNetwork::is_relayable_addr(&routable)); + + for bad in [ + "/ip4/127.0.0.1/tcp/9001", + "/ip4/0.0.0.0/tcp/9001", + "/ip6/::1/tcp/9001", + ] { + assert!( + !Libp2pNetwork::is_relayable_addr(&bad.parse().unwrap()), + "{bad} is not reachable by a third party" + ); + } + + // No circuits through circuits. + assert!(!Libp2pNetwork::is_relayable_addr(&circuit_addr( + PeerId::random() + ))); + } + #[tokio::test] async fn test_network_creation() { let config = Libp2pNetworkConfig { listen_addrs: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()], bootstrap_nodes: vec![], enable_mdns: false, + enable_nat_traversal: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }; diff --git a/monas-state-node/src/infrastructure/network/transport.rs b/monas-state-node/src/infrastructure/network/transport.rs index e0ba39e..ceffbff 100644 --- a/monas-state-node/src/infrastructure/network/transport.rs +++ b/monas-state-node/src/infrastructure/network/transport.rs @@ -19,6 +19,28 @@ use libp2p::{ /// - QUIC: Modern, efficient transport with built-in encryption /// - WebRTC: Required for browser communication (future) pub fn build_transport(keypair: &Keypair) -> anyhow::Result> { + build_transport_inner(keypair, None) +} + +/// Build the transport with circuit-relay dialling folded in. +/// +/// The relay client is a *transport*, not just a behaviour: dialling +/// `/…/p2p-circuit/p2p/` has to be handled at the transport layer, and +/// the transport half can only be obtained together with the behaviour half +/// from `relay::client::new`. So a node that wants to reach peers through a +/// relay must build its transport with this function and register the matching +/// behaviour — the two halves are useless apart. +pub fn build_transport_with_relay( + keypair: &Keypair, + relay_transport: libp2p::relay::client::Transport, +) -> anyhow::Result> { + build_transport_inner(keypair, Some(relay_transport)) +} + +fn build_transport_inner( + keypair: &Keypair, + relay_transport: Option, +) -> anyhow::Result> { use rand::rngs::OsRng; // TCP transport with DNS resolution @@ -58,8 +80,31 @@ pub fn build_transport(keypair: &Keypair) -> anyhow::Result { (peer_id, StreamMuxerBox::new(muxer)) } - }) - .boxed(); + }); + + // Relay connections still need to be authenticated and multiplexed: the + // circuit only carries bytes, and the relay itself must not be able to + // read or tamper with what flows through it. Noise runs end-to-end between + // the two clients, so a relay sees ciphertext only. + let transport = match relay_transport { + Some(relay) => { + let relay_upgraded = relay + .upgrade(upgrade::Version::V1) + .authenticate(noise::Config::new(keypair)?) + .multiplex(yamux::Config::default()) + .timeout(std::time::Duration::from_secs(20)); + transport + .or_transport(relay_upgraded) + .map(|either, _| match either { + futures::future::Either::Left((peer_id, muxer)) => (peer_id, muxer), + futures::future::Either::Right((peer_id, muxer)) => { + (peer_id, StreamMuxerBox::new(muxer)) + } + }) + .boxed() + } + None => transport.boxed(), + }; Ok(transport) } diff --git a/monas-state-node/tests/create_content_push_race_test.rs b/monas-state-node/tests/create_content_push_race_test.rs index d8c97e7..c365d14 100644 --- a/monas-state-node/tests/create_content_push_race_test.rs +++ b/monas-state-node/tests/create_content_push_race_test.rs @@ -133,6 +133,7 @@ async fn spawn_test_node() -> TestNode { listen_addrs: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()], bootstrap_nodes: vec![], enable_mdns: false, + enable_nat_traversal: false, gossipsub_topics: vec!["test-events".to_string()], external_addrs: vec![], }; diff --git a/monas-state-node/tests/e2e_multi_node_test.rs b/monas-state-node/tests/e2e_multi_node_test.rs index e7ff429..7cf6d07 100644 --- a/monas-state-node/tests/e2e_multi_node_test.rs +++ b/monas-state-node/tests/e2e_multi_node_test.rs @@ -29,6 +29,7 @@ async fn create_test_node() -> (StateNode, TempDir) { listen_addrs: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()], bootstrap_nodes: vec![], enable_mdns: false, // Disable mDNS to avoid interference between tests + enable_nat_traversal: false, gossipsub_topics: vec![EVENTS_TOPIC.to_string()], external_addrs: vec![], }, diff --git a/monas-state-node/tests/integration_test.rs b/monas-state-node/tests/integration_test.rs index 53b8034..3942b59 100644 --- a/monas-state-node/tests/integration_test.rs +++ b/monas-state-node/tests/integration_test.rs @@ -150,6 +150,7 @@ async fn create_test_service() -> (Arc, Arc, Te listen_addrs: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()], bootstrap_nodes: vec![], enable_mdns: false, // Disable mDNS for isolated tests + enable_nat_traversal: false, gossipsub_topics: vec!["test-events".to_string()], external_addrs: vec![], }; @@ -540,6 +541,7 @@ async fn create_test_service_with_ac() -> (Arc, Arc (Arc, Arc Date: Sun, 16 Aug 2026 14:46:20 +0900 Subject: [PATCH 2/2] fix(state-node): make relay service opt-in and reservations self-healing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the NAT traversal work found four defects, three of them sharing one root: the relay reservation path was an edge-triggered, one-way state machine that could only ever move forwards. **Relaying for strangers is now opt-in.** The relay *server* was created whenever NAT traversal was on — which is the default — so every node became an open circuit relay for anonymous peers from startup. A comment claimed it was "only ever used once AutoNAT reports us as reachable (see libp2p_network.rs)", but no such gating existed anywhere. That gate cannot be built as described: `Toggle` cannot enable a behaviour after the swarm is constructed and `relay::Behaviour` has no runtime mute. So the decision is made where it can be made honestly — from an operator assertion, `--relay-service`, which requires `--external-address` and is refused alongside `--disable-nat-traversal`. Needing to traverse NAT oneself is not the same as volunteering to carry other people's traffic. `entrypoint.sh` now passes `DISABLE_NAT_TRAVERSAL`, `RELAY_SERVICE` and `EXTERNAL_ADDR`. Neither flag was reachable from a deployment before, so the only way to change either was a code change. **A reservation is recorded when the relay grants it, not when we ask.** `listen_on` returning `Ok` only means the address parsed; the relay answers later and may refuse. Recording the attempt as if it were the reservation let a refusal occupy one of the two slots for the life of the process — and since the entry also filtered that peer out of the candidate list, two refusals left the node permanently unreachable. Entries are now `Pending` until `ReservationReqAccepted`, and `ListenerClosed` drops them so the relay can be replaced. This is the same "the call is not the effect" mistake as releasing a reservation by logging about it. **Reservations are topped up from the maintenance tick**, not only on an AutoNAT transition. A relay can refuse, expire or vanish at any time, and the transition will not fire twice. This is why `maintain_connectivity` exists. **Reachability is the aggregate over addresses.** AutoNAT v2 tests one address at a time, so a multi-homed node legitimately gets a failure for its LAN address and a success for its public one. Collapsing each event into a single last-event-wins verdict made the node flip Public/Private and churn reservations against other people's relays. One reachable address is enough. The redundant `add_external_address` is gone with it: the AutoNAT client already emits `ExternalAddrConfirmed`, and the manual set it was writing to is never expired, so a node would advertise a dead address indefinitely. **Circuit addresses strip any `/p2p/` before appending the relay's.** The connection table stores addresses as dialled and the conventional bootstrap form ends in `/p2p/`, so appending blindly produced `/p2p//p2p//p2p-circuit` — rejected as malformed, silently, at debug level. `bootstrap.rs` already strips exactly this way. Failures to open a circuit are now `warn!`: a node that cannot reserve anywhere is unreachable, and burying that is how the private-node path stayed dead the first time. `peer_store` no longer persists circuit addresses. They say where a peer can be reached through a relay right now, not where it lives, so a restart would dial a circuit through a relay that has moved on. Five new tests, each verified to fail when its fix is reverted. That includes the release path, which the previous test could not constrain — it asserted the bookkeeping only, and still passed with `remove_listener` removed. The new one observes the `ListenerClosed` the swarm emits, so it fails on a log-only implementation. cargo test --workspace: 814 passed, 0 failed. clippy (1.97) clean. Co-Authored-By: Claude Opus 5 (1M context) --- docs/design.md | 13 +- monas-state-node/infra/docker/entrypoint.sh | 24 + .../src/application_service/node.rs | 4 + monas-state-node/src/bin/state_node.rs | 30 ++ .../src/infrastructure/network/behaviour.rs | 105 ++++- .../infrastructure/network/libp2p_network.rs | 416 ++++++++++++++++-- .../src/infrastructure/network/peer_store.rs | 30 +- .../tests/create_content_push_race_test.rs | 1 + monas-state-node/tests/e2e_multi_node_test.rs | 1 + monas-state-node/tests/integration_test.rs | 3 + .../tests/public_key_exchange_test.rs | 2 + 11 files changed, 568 insertions(+), 61 deletions(-) diff --git a/docs/design.md b/docs/design.md index dc6ad0f..8ac161a 100644 --- a/docs/design.md +++ b/docs/design.md @@ -181,7 +181,7 @@ presentation/ Axum HTTP API (port: 4001) | 機能 | 実装 | |------|------| -| P2Pネットワーク | libp2p（Kademlia DHT / Gossipsub / RequestResponse / mDNS） | +| P2Pネットワーク | libp2p（Kademlia DHT / Gossipsub / RequestResponse / mDNS / AutoNAT v2 / circuit relay v2 / DCUtR） | | CRDT状態管理 | crsl-lib（CIDネイティブDAG CRDT） | | コンテンツ配置 | sha256(content_id)によるDHTキー空間への決定論的配置 | | 認証 | P-256 ECDSA、自己完結型鍵ID | @@ -320,8 +320,19 @@ XOR距離によるノード選択には以下の特性がある： | Gossipsub | イベント伝播（ContentCreated, ContentUpdated等） | | RequestResponse | ノード間の直接通信（CRDT操作の同期） | | mDNS | ローカルネットワークでのピア探索 | +| AutoNAT v2 | 自ノードが外から到達可能かを**実測**で判定（自己申告ではない） | +| circuit relay v2 | NAT 配下のノードへの接続を中継。提供側は opt-in（`--relay-service`） | +| DCUtR | relay 経由の接続を hole punching で直接接続へ昇格させる | | TCP / QUIC / WebRTC | トランスポート | +#### NAT traversal と 2 つの「relay」 + +この表の **circuit relay v2 は接続層**の仕組みで、本書が[relay先の信頼度](#relay先の信頼度)以下で論じる **Monas relay（application 層のリクエスト転送）とは別物**である。前者は「そもそも TCP 接続を張れない相手にどう繋ぐか」、後者は「コンテンツを持たないノードが受けたリクエストをどう member へ渡すか」を扱う。両者は独立しており、circuit relay を無効にしても Monas relay は動く。 + +**役割は測定から従属的に決まる。** AutoNAT v2 は自ノードのアドレスを他ノードに dial back させ、**実際に到達できたアドレスについてのみ**肯定を返す。到達不能なノードが自分を到達可能と誤認すれば、提供できない relay サービスを広告してしまうため、ここは自己申告であってはならない。 + +ただし**中継する側になることは opt-in** である。NAT を越えたいこと（client 側）と、見知らぬ相手のトラフィックを運ぶ用意があること（server 側）は別の判断なので、後者は `--relay-service`（`--external-address` 必須）でのみ有効になる。既定では、NAT traversal が有効でも中継役は担わない。 + --- ## 10. セキュリティモデル diff --git a/monas-state-node/infra/docker/entrypoint.sh b/monas-state-node/infra/docker/entrypoint.sh index 3731615..47e42e2 100644 --- a/monas-state-node/infra/docker/entrypoint.sh +++ b/monas-state-node/infra/docker/entrypoint.sh @@ -10,6 +10,9 @@ BOOTSTRAP_ADDR="${BOOTSTRAP_ADDR:-}" BOOTSTRAP_DNS="${BOOTSTRAP_DNS:-}" BOOTSTRAP_PEER_ID="${BOOTSTRAP_PEER_ID:-}" DISABLE_MDNS="${DISABLE_MDNS:-}" +DISABLE_NAT_TRAVERSAL="${DISABLE_NAT_TRAVERSAL:-}" +RELAY_SERVICE="${RELAY_SERVICE:-}" +EXTERNAL_ADDR="${EXTERNAL_ADDR:-}" ARGS=( --data-dir "$DATA_DIR" @@ -26,6 +29,27 @@ case "$DISABLE_MDNS" in 1|true|TRUE|yes|YES) ARGS+=(--disable-mdns) ;; esac +# Externally reachable addresses to advertise (comma-separated multiaddrs). +if [ -n "$EXTERNAL_ADDR" ]; then + IFS=',' read -ra EXT_ADDRS <<< "$EXTERNAL_ADDR" + for a in "${EXT_ADDRS[@]}"; do + a="$(echo "$a" | tr -d '[:space:]')" + [ -n "$a" ] && ARGS+=(--external-address "$a") + done +fi + +# NAT traversal is on by default; this is the switch that turns it off +# without a code change. +if [ -n "$DISABLE_NAT_TRAVERSAL" ]; then + ARGS+=(--disable-nat-traversal) +fi + +# Acting as a relay for other peers is opt-in: it carries traffic for nodes +# we know nothing about. Requires EXTERNAL_ADDR. +if [ -n "$RELAY_SERVICE" ]; then + ARGS+=(--relay-service) +fi + # Bootstrap addresses. # # BOOTSTRAP_ADDR accepts a comma-separated list of full multiaddrs, so a node diff --git a/monas-state-node/src/application_service/node.rs b/monas-state-node/src/application_service/node.rs index ee39883..e6fe7b7 100644 --- a/monas-state-node/src/application_service/node.rs +++ b/monas-state-node/src/application_service/node.rs @@ -695,6 +695,7 @@ mod tests { bootstrap_nodes: vec![], enable_mdns: false, enable_nat_traversal: false, + enable_relay_service: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }, @@ -725,6 +726,7 @@ mod tests { bootstrap_nodes: vec![], enable_mdns: false, enable_nat_traversal: false, + enable_relay_service: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }, @@ -756,6 +758,7 @@ mod tests { bootstrap_nodes: vec![], enable_mdns: false, enable_nat_traversal: false, + enable_relay_service: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }, @@ -788,6 +791,7 @@ mod tests { bootstrap_nodes: vec![], enable_mdns: false, enable_nat_traversal: false, + enable_relay_service: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }, diff --git a/monas-state-node/src/bin/state_node.rs b/monas-state-node/src/bin/state_node.rs index 7716075..2395af1 100644 --- a/monas-state-node/src/bin/state_node.rs +++ b/monas-state-node/src/bin/state_node.rs @@ -65,6 +65,16 @@ struct Args { #[arg(long)] disable_nat_traversal: bool, + /// Offer circuit relay service to other nodes. + /// + /// A relay carries traffic for peers it knows nothing about, so this is + /// opt-in and separate from `--disable-nat-traversal`: needing a relay + /// oneself is not the same as being willing to be one. Only a node that + /// is actually reachable from outside can serve, so this requires + /// `--external-address`. + #[arg(long)] + relay_service: bool, + /// Log level (trace, debug, info, warn, error). #[arg(long, default_value = "info")] log_level: String, @@ -141,6 +151,26 @@ async fn main() -> Result<()> { } } + // Relay service is only meaningful on a node that is actually reachable + // from outside. Refusing here rather than starting a relay nobody can + // reach keeps the "advertise only what you can provide" rule honest. + if args.relay_service { + if network_config.external_addrs.is_empty() { + anyhow::bail!( + "--relay-service requires at least one --external-address: a node that \ + cannot be reached from outside cannot relay for anyone" + ); + } + if args.disable_nat_traversal { + anyhow::bail!("--relay-service cannot be used with --disable-nat-traversal"); + } + network_config.enable_relay_service = true; + tracing::info!( + "Relay service enabled; this node will carry circuits for other peers \ + (up to 128 reservations / 32 circuits)" + ); + } + let config = StateNodeConfig { data_dir: args.data_dir, http_addr: args.listen, diff --git a/monas-state-node/src/infrastructure/network/behaviour.rs b/monas-state-node/src/infrastructure/network/behaviour.rs index dcbf3fa..fbde18e 100644 --- a/monas-state-node/src/infrastructure/network/behaviour.rs +++ b/monas-state-node/src/infrastructure/network/behaviour.rs @@ -180,6 +180,13 @@ pub struct BehaviourConfig { /// deliberately separable so a deployment of publicly reachable nodes — /// which needs none of it — can leave the machinery off. pub enable_nat_traversal: bool, + /// Offer circuit relay service to other nodes. + /// + /// Separate from `enable_nat_traversal`, and off by default: needing a + /// relay oneself is not the same as being willing to carry strangers' + /// traffic. Only a node that is genuinely publicly reachable can serve, + /// so this is gated on the operator also declaring an external address. + pub enable_relay_service: bool, } impl Default for BehaviourConfig { @@ -189,6 +196,8 @@ impl Default for BehaviourConfig { agent_version: format!("monas-state-node/{}", env!("CARGO_PKG_VERSION")), enable_mdns: true, enable_nat_traversal: true, + // Off by default: carrying other people's traffic is opt-in. + enable_relay_service: false, } } } @@ -276,33 +285,47 @@ impl NodeBehaviour { // know in advance which side it will be on, and a network where only // some nodes answer probes degrades for everyone: reachability is a // service peers provide to each other, like relaying. - // - // The relay *server* is created here but only ever used once AutoNAT - // reports us as reachable (see libp2p_network.rs). Relaying from - // behind NAT would not work anyway, and advertising it would waste - // other nodes' reservation attempts. - let (autonat_client, autonat_server, relay_server, dcutr) = if config.enable_nat_traversal { + let (autonat_client, autonat_server, dcutr) = if config.enable_nat_traversal { ( Some(autonat::v2::client::Behaviour::new( rand::rngs::OsRng, autonat::v2::client::Config::default(), )), Some(autonat::v2::server::Behaviour::new(rand::rngs::OsRng)), - Some(relay::Behaviour::new( - local_peer_id, - // Bound what a relay will carry. Relaying is done for - // strangers, so the limits are what stop it from becoming - // free bandwidth for anyone who asks. - relay::Config { - max_reservations: MAX_RELAY_RESERVATIONS, - max_circuits: MAX_RELAY_CIRCUITS, - ..Default::default() - }, - )), Some(dcutr::Behaviour::new(local_peer_id)), ) } else { - (None, None, None, None) + (None, None, None) + }; + + // The relay *server* is separate, and off unless asked for. + // + // Serving as a relay means carrying traffic for peers we know nothing + // about, so it is not something to switch on as a side effect of + // wanting NAT traversal for ourselves — a node behind NAT needs the + // client half and cannot usefully provide the server half anyway. + // + // It cannot be decided from AutoNAT at runtime: `Toggle` has no way to + // enable a behaviour after the swarm is built, and `relay::Behaviour` + // has no runtime mute. So the decision is made here, from an operator + // assertion (`--relay-service`, which requires `--external-address`), + // rather than being claimed in a comment and never implemented. + let relay_server = if config.enable_relay_service { + Some(relay::Behaviour::new( + local_peer_id, + // Bound what a relay will carry. Relaying is done for + // strangers, so the limits are what stop it from becoming + // free bandwidth for anyone who asks. The per-peer limits + // left at their defaults (4 reservations / 4 circuits) are + // what stop a single peer taking everything. + relay::Config { + max_reservations: MAX_RELAY_RESERVATIONS, + max_circuits: MAX_RELAY_CIRCUITS, + ..Default::default() + }, + )) + } else { + None }; // The client half is only meaningful when its transport half was built @@ -422,6 +445,7 @@ mod tests { agent_version: "custom-agent/1.0.0".to_string(), enable_mdns: true, enable_nat_traversal: false, + enable_relay_service: false, }; let cloned = config.clone(); @@ -531,8 +555,50 @@ mod tests { .unwrap(); assert!(on.autonat_client.as_ref().is_some()); assert!(on.autonat_server.as_ref().is_some()); - assert!(on.relay_server.as_ref().is_some()); assert!(on.dcutr.as_ref().is_some()); + // The relay *server* is not part of this flag: see below. + assert!(on.relay_server.as_ref().is_none()); + } + + /// Serving as a relay is opt-in and independent of NAT traversal. + /// + /// Wanting to traverse NAT oneself is not the same as volunteering to + /// carry strangers' traffic. Tying the two together is what would turn + /// every publicly reachable node into an open relay by default. + #[cfg(not(target_arch = "wasm32"))] + #[tokio::test] + async fn relay_service_is_opt_in_and_independent_of_nat_traversal() { + let keypair = Keypair::generate_ed25519(); + let local_peer_id = keypair.public().to_peer_id(); + + let nat_only = NodeBehaviour::new( + local_peer_id, + &keypair, + BehaviourConfig { + enable_nat_traversal: true, + enable_relay_service: false, + ..BehaviourConfig::default() + }, + None, + ) + .unwrap(); + assert!( + nat_only.relay_server.as_ref().is_none(), + "NAT traversal alone must not make this node a relay for others" + ); + + let serving = NodeBehaviour::new( + local_peer_id, + &keypair, + BehaviourConfig { + enable_nat_traversal: true, + enable_relay_service: true, + ..BehaviourConfig::default() + }, + None, + ) + .unwrap(); + assert!(serving.relay_server.as_ref().is_some()); } /// The relay client only exists when its transport half was built too. @@ -614,6 +680,7 @@ mod tests { agent_version: "test-agent/0.1.0".to_string(), enable_mdns: true, enable_nat_traversal: false, + enable_relay_service: false, }; let result = NodeBehaviour::new(local_peer_id, &keypair, config, None); diff --git a/monas-state-node/src/infrastructure/network/libp2p_network.rs b/monas-state-node/src/infrastructure/network/libp2p_network.rs index f3ef8fd..0b91fa0 100644 --- a/monas-state-node/src/infrastructure/network/libp2p_network.rs +++ b/monas-state-node/src/infrastructure/network/libp2p_network.rs @@ -92,15 +92,70 @@ enum Reachability { Private, } +/// Where a reservation attempt has got to. +/// +/// `listen_on` returning `Ok` only means the transport accepted the address +/// shape; the reservation is negotiated afterwards and the relay may still +/// refuse. Recording the attempt as if it were the reservation is what would +/// let a refusal occupy one of `TARGET_RELAY_RESERVATIONS` forever — the same +/// "the call is not the effect" mistake as releasing a reservation by logging. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ReservationState { + /// `listen_on` accepted the address; the relay has not answered yet. + Pending, + /// The relay sent `ReservationReqAccepted`. + Confirmed, +} + +/// A reservation attempt against one relay. +struct Reservation { + /// Kept because releasing a reservation means removing its listener, and + /// `Swarm::listeners()` only yields addresses — there is no way back to + /// the id from those. + listener_id: libp2p::core::transport::ListenerId, + state: ReservationState, +} + /// Tracks reachability and the relay reservations that follow from it. #[derive(Default)] struct NatState { + /// Per-address AutoNAT verdicts. + /// + /// AutoNAT v2 tests *one address at a time* and reports the result for + /// that address, so a single event is not a verdict about the node. A + /// multi-homed node legitimately gets a failure for its LAN address and a + /// success for its public one; collapsing those into one last-event-wins + /// boolean makes the node flip between Public and Private and churn + /// reservations against other people's relays. The node-level answer is + /// the aggregate: reachable if *any* address is. + address_reachability: HashMap, reachability: Reachability, - /// Relays we currently listen through, with the listener that holds each - /// reservation. The `ListenerId` is kept because releasing a reservation - /// means removing its listener, and `Swarm::listeners()` only yields - /// addresses — there is no way back to the id from those. - reserved_with: HashMap, + reserved_with: HashMap, +} + +impl NatState { + /// Fold the per-address verdicts into a node-level one. + /// + /// One reachable address is enough to be reachable. Only once every + /// address we have heard about has failed do we conclude we are private. + fn aggregate_reachability(&self) -> Reachability { + if self.address_reachability.is_empty() { + return Reachability::Unknown; + } + if self.address_reachability.values().any(|ok| *ok) { + Reachability::Public + } else { + Reachability::Private + } + } + + /// Reservations that are still worth counting towards the target. + /// + /// Both pending and confirmed count, so a node does not open a third + /// circuit while two are still being negotiated. + fn active_reservations(&self) -> usize { + self.reserved_with.len() + } } /// A relay request received from a remote peer via P2P protocol. @@ -204,6 +259,9 @@ pub struct Libp2pNetworkConfig { /// Enable NAT traversal (AutoNAT v2 + circuit relay v2 + DCUtR), which is /// what lets a node behind NAT participate at all. pub enable_nat_traversal: bool, + /// Offer circuit relay service to other nodes. Off by default; see + /// `BehaviourConfig::enable_relay_service`. + pub enable_relay_service: bool, /// Gossipsub topics to subscribe to. pub gossipsub_topics: Vec, /// Externally reachable addresses to advertise to peers (e.g. a public @@ -228,6 +286,7 @@ impl Default for Libp2pNetworkConfig { bootstrap_nodes: vec![], enable_mdns: true, enable_nat_traversal: true, + enable_relay_service: false, gossipsub_topics: vec!["monas-events".to_string()], external_addrs: vec![], } @@ -563,6 +622,7 @@ impl Libp2pNetwork { BehaviourConfig { enable_mdns: config.enable_mdns, enable_nat_traversal: config.enable_nat_traversal, + enable_relay_service: config.enable_relay_service, ..BehaviourConfig::default() }, relay_client, @@ -800,6 +860,16 @@ impl Libp2pNetwork { // Periodic reconnection / re-bootstrap _ = peer_maintenance.tick() => { Self::maintain_connectivity(&mut swarm, &connected_peers, &bootstrap_nodes, &peer_store).await; + // Top relay reservations back up to the target. + // + // Reconciled here rather than only on the AutoNAT + // transition: a relay can refuse, expire or vanish at any + // time, and a private node that reacted only to the edge + // would stay unreachable for the life of the process. This + // is the same reason `maintain_connectivity` exists. + if nat_state.reachability == Reachability::Private { + Self::ensure_relay_reservations(&mut swarm, &connected_peers, &mut nat_state).await; + } // Flush at most once per tick rather than on every new // address, so a busy node does not rewrite the file // constantly. @@ -829,29 +899,37 @@ impl Libp2pNetwork { nat_state: &mut NatState, event: libp2p::autonat::v2::client::Event, ) { - let observed = if event.result.is_ok() { - Reachability::Public - } else { - Reachability::Private - }; - + // Record the verdict for *this address*. The node-level answer is the + // aggregate over every address we have heard about; see + // `NatState::aggregate_reachability`. + // + // The address itself needs no handling here: on success the AutoNAT + // client behaviour already emits `ExternalAddrConfirmed`, so the swarm + // has recorded it. Calling `add_external_address` as well would put it + // in the swarm's *manual* set, which is never expired — the node would + // keep advertising an address long after it stopped working. + nat_state + .address_reachability + .insert(event.tested_addr.clone(), event.result.is_ok()); + + let observed = nat_state.aggregate_reachability(); if nat_state.reachability == observed { return; } info!( - "AutoNAT: reachability {:?} -> {:?} (tested {})", - nat_state.reachability, observed, event.tested_addr + "AutoNAT: reachability {:?} -> {:?} (last tested {}, {} address(es) known)", + nat_state.reachability, + observed, + event.tested_addr, + nat_state.address_reachability.len() ); nat_state.reachability = observed; match observed { - Reachability::Public => { - // We can be dialled, so the address is worth announcing and - // there is no reason to occupy someone else's relay slot. - swarm.add_external_address(event.tested_addr.clone()); - Self::release_relay_reservations(swarm, nat_state); - } + // No reason to occupy someone else's relay slot once we can be + // dialled directly. + Reachability::Public => Self::release_relay_reservations(swarm, nat_state), Reachability::Private => { Self::ensure_relay_reservations(swarm, connected_peers, nat_state).await } @@ -874,6 +952,10 @@ impl Libp2pNetwork { return; } + if nat_state.active_reservations() >= TARGET_RELAY_RESERVATIONS { + return; + } + // A circuit address must carry the relay's *dialable* address, not // just its peer id: the client transport rejects `/p2p//p2p-circuit` // with `MissingRelayAddr`, because it has to know where to open the @@ -893,25 +975,110 @@ impl Libp2pNetwork { .collect(); for (peer, addr) in candidates { - if nat_state.reserved_with.len() >= TARGET_RELAY_RESERVATIONS { + if nat_state.active_reservations() >= TARGET_RELAY_RESERVATIONS { break; } - // Listening on a circuit address is what actually requests the - // reservation; the relay accepts or refuses, and a refusal simply - // means this peer will not be one of our relays. - let circuit = addr - .with(libp2p::multiaddr::Protocol::P2p(peer)) - .with(libp2p::multiaddr::Protocol::P2pCircuit); + let circuit = Self::circuit_addr_via(&addr, peer); + // `listen_on` only accepts the address shape; the relay answers + // later with `ReservationReqAccepted` or by closing the listener. + // So this is recorded as `Pending` and confirmed on that event — + // recording it as held here is what would let a refusal occupy a + // slot for the life of the process. match swarm.listen_on(circuit.clone()) { Ok(listener_id) => { info!("Requesting relay reservation via {}", circuit); - nat_state.reserved_with.insert(peer, listener_id); + nat_state.reserved_with.insert( + peer, + Reservation { + listener_id, + state: ReservationState::Pending, + }, + ); } - Err(e) => debug!("Cannot listen via relay {}: {}", peer, e), + // Not `debug!`: a node that cannot open any circuit is + // unreachable, and burying that is exactly how the + // private-node path stayed silently dead before. + Err(e) => warn!("Cannot listen via relay {}: {}", peer, e), } } } + /// Build the circuit address that reserves a slot on `relay`. + /// + /// Any `/p2p/` already in the address is dropped before the relay's own is + /// appended. The connection table stores addresses as they were dialled, + /// and the conventional bootstrap form already ends in `/p2p/`, so + /// appending blindly yields `/p2p//p2p//p2p-circuit`, which + /// `listen_on` rejects as a malformed multiaddr. `bootstrap.rs` strips the + /// same way for the same reason. + fn circuit_addr_via(addr: &Multiaddr, relay: PeerId) -> Multiaddr { + use libp2p::multiaddr::Protocol; + addr.iter() + .filter(|p| !matches!(p, Protocol::P2p(_))) + .collect::() + .with(Protocol::P2p(relay)) + .with(Protocol::P2pCircuit) + } + + /// Note that a relay accepted our reservation. + /// + /// Until this arrives the entry is `Pending`: `listen_on` succeeding says + /// nothing about whether the relay agreed. + fn confirm_relay_reservation(nat_state: &mut NatState, relay_peer_id: PeerId, renewal: bool) { + match nat_state.reserved_with.get_mut(&relay_peer_id) { + Some(reservation) => { + reservation.state = ReservationState::Confirmed; + if !renewal { + info!( + "Relay reservation accepted by {}; reachable through it now", + relay_peer_id + ); + } + } + // A reservation we are not tracking (e.g. taken before a restart + // of the bookkeeping). Nothing to confirm, but worth knowing. + None => debug!( + "Relay reservation accepted by untracked relay {}", + relay_peer_id + ), + } + } + + /// Forget a reservation whose listener has gone away. + /// + /// A relay that refuses, expires or disconnects closes the circuit + /// listener. Without this the entry would sit in `reserved_with` forever, + /// counting towards the target and blocking that peer from being retried — + /// so two refusals would leave the node permanently unreachable. + fn forget_closed_reservation( + nat_state: &mut NatState, + listener_id: libp2p::core::transport::ListenerId, + ) { + let Some(peer) = nat_state + .reserved_with + .iter() + .find(|(_, r)| r.listener_id == listener_id) + .map(|(p, _)| *p) + else { + return; + }; + let was = nat_state.reserved_with.remove(&peer).map(|r| r.state); + match was { + Some(ReservationState::Confirmed) => { + info!( + "Relay reservation with {} ended; will look for another", + peer + ) + } + // Never confirmed: the relay refused, or the circuit could not be + // opened. Retried from the maintenance tick. + _ => info!( + "Relay {} did not grant a reservation; will try another", + peer + ), + } + } + /// Whether an address can serve as the relay hop of a circuit address. /// /// Excludes addresses that are already relayed (no circuits through @@ -948,8 +1115,8 @@ impl Libp2pNetwork { ); // Removing the circuit listener is what actually drops the // reservation, freeing the slot on a relay for a node that needs it. - for (peer, listener_id) in nat_state.reserved_with.drain() { - if swarm.remove_listener(listener_id) { + for (peer, reservation) in nat_state.reserved_with.drain() { + if swarm.remove_listener(reservation.listener_id) { debug!("Released relay reservation with {}", peer); } } @@ -1303,12 +1470,19 @@ impl Libp2pNetwork { } } SwarmEvent::Behaviour(NodeBehaviourEvent::RelayClient( - libp2p::relay::client::Event::ReservationReqAccepted { relay_peer_id, .. }, + libp2p::relay::client::Event::ReservationReqAccepted { + relay_peer_id, + renewal, + .. + }, )) => { - info!( - "Relay reservation accepted by {}; reachable through it now", - relay_peer_id - ); + Self::confirm_relay_reservation(nat_state, relay_peer_id, renewal); + } + // A circuit listener closing is how a refused, expired or lost + // reservation reports itself; drop the entry so the relay can be + // replaced on the next maintenance tick. + SwarmEvent::ListenerClosed { listener_id, .. } => { + Self::forget_closed_reservation(nat_state, listener_id); } #[cfg(not(target_arch = "wasm32"))] SwarmEvent::Behaviour(NodeBehaviourEvent::Mdns(mdns_event)) => { @@ -3072,7 +3246,13 @@ mod tests { .listen_on("/ip4/127.0.0.1/tcp/0".parse().unwrap()) .unwrap(); let circuit = swarm.listen_on(circuit_addr(relay_peer)).unwrap(); - nat_state.reserved_with.insert(relay_peer, circuit); + nat_state.reserved_with.insert( + relay_peer, + Reservation { + listener_id: circuit, + state: ReservationState::Confirmed, + }, + ); nat_state.reachability = Reachability::Private; Libp2pNetwork::release_relay_reservations(&mut swarm, &mut nat_state); @@ -3087,6 +3267,169 @@ mod tests { ); } + /// Releasing must actually tear the listener down, not just forget it. + /// + /// The bookkeeping assertion above cannot tell "removed" from "never + /// removed" — which is exactly the bug this PR fixed, and exactly what + /// that test would not have caught. `remove_listener` is observable + /// though: the swarm reports `ListenerClosed` for the listener it closed. + /// Reverting `release_relay_reservations` to a log-only implementation + /// turns this test red. + #[tokio::test] + async fn releasing_actually_closes_the_circuit_listener() { + use futures::StreamExt; + + let mut swarm = nat_swarm(); + let mut nat_state = NatState::default(); + let relay_peer = PeerId::random(); + + let circuit = swarm.listen_on(circuit_addr(relay_peer)).unwrap(); + nat_state.reserved_with.insert( + relay_peer, + Reservation { + listener_id: circuit, + state: ReservationState::Confirmed, + }, + ); + nat_state.reachability = Reachability::Private; + + Libp2pNetwork::release_relay_reservations(&mut swarm, &mut nat_state); + + let closed = tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let SwarmEvent::ListenerClosed { listener_id, .. } = + swarm.select_next_some().await + { + return listener_id; + } + } + }) + .await + .expect("releasing a reservation must close its listener"); + + assert_eq!( + closed, circuit, + "the closed listener must be the circuit we reserved through" + ); + } + + /// A reservation is only "held" once the relay says so. + /// + /// `listen_on` returning `Ok` means the address parsed, nothing more. + /// Recording it as held is how a refused reservation would occupy a slot + /// for the life of the process. + #[tokio::test] + async fn a_reservation_is_pending_until_the_relay_accepts_it() { + let mut swarm = nat_swarm(); + let mut nat_state = NatState::default(); + let relay = PeerId::random(); + let connected = Arc::new(RwLock::new(HashMap::from([( + relay, + vec!["/ip4/198.51.100.7/tcp/9001".parse::().unwrap()], + )]))); + + Libp2pNetwork::ensure_relay_reservations(&mut swarm, &connected, &mut nat_state).await; + assert_eq!( + nat_state.reserved_with.get(&relay).map(|r| r.state), + Some(ReservationState::Pending), + "listen_on succeeding is not the relay agreeing" + ); + + Libp2pNetwork::confirm_relay_reservation(&mut nat_state, relay, false); + assert_eq!( + nat_state.reserved_with.get(&relay).map(|r| r.state), + Some(ReservationState::Confirmed) + ); + } + + /// A refused or lost reservation must free its slot and allow a retry. + /// + /// Without this the entry sits in `reserved_with` forever, counting + /// towards the target and filtering that peer out of the candidate list — + /// so two refusals would leave the node permanently unreachable. + #[tokio::test] + async fn a_closed_listener_frees_the_reservation_slot() { + let mut swarm = nat_swarm(); + let mut nat_state = NatState::default(); + let relay = PeerId::random(); + let connected = Arc::new(RwLock::new(HashMap::from([( + relay, + vec!["/ip4/198.51.100.7/tcp/9001".parse::().unwrap()], + )]))); + + Libp2pNetwork::ensure_relay_reservations(&mut swarm, &connected, &mut nat_state).await; + let listener = nat_state.reserved_with[&relay].listener_id; + assert_eq!(nat_state.active_reservations(), 1); + + Libp2pNetwork::forget_closed_reservation(&mut nat_state, listener); + + assert!( + nat_state.reserved_with.is_empty(), + "a closed circuit listener must free its slot, or we never retry" + ); + } + + /// The circuit address must survive a relay address that already carries + /// its own `/p2p/`. + /// + /// The connection table stores addresses as they were dialled, and the + /// conventional bootstrap form ends in `/p2p/`. Appending blindly + /// yields `/p2p//p2p//p2p-circuit`, which `listen_on` rejects as + /// malformed — so every reservation via such a peer would fail. + #[tokio::test] + async fn a_relay_address_carrying_its_own_p2p_still_yields_a_valid_circuit() { + let mut swarm = nat_swarm(); + let relay = PeerId::random(); + let with_p2p: Multiaddr = format!("/ip4/198.51.100.7/tcp/9001/p2p/{relay}") + .parse() + .unwrap(); + + let circuit = Libp2pNetwork::circuit_addr_via(&with_p2p, relay); + + assert_eq!( + circuit + .iter() + .filter(|p| matches!(p, libp2p::multiaddr::Protocol::P2p(_))) + .count(), + 1, + "the relay's peer id must appear exactly once: {circuit}" + ); + assert!( + swarm.listen_on(circuit.clone()).is_ok(), + "circuit built from a bootstrap-form address must be dialable: {circuit}" + ); + } + + /// Reachability is the aggregate over addresses, not the last event. + /// + /// AutoNAT tests one address at a time, so a multi-homed node gets a + /// failure for its LAN address and a success for its public one. Treating + /// each event as a verdict makes the node flip Public/Private and churn + /// reservations against other people's relays. + #[test] + fn one_reachable_address_is_enough_to_be_public() { + let mut nat_state = NatState::default(); + assert_eq!(nat_state.aggregate_reachability(), Reachability::Unknown); + + let lan: Multiaddr = "/ip4/10.0.0.5/tcp/9001".parse().unwrap(); + let public: Multiaddr = "/ip4/198.51.100.7/tcp/9001".parse().unwrap(); + + nat_state.address_reachability.insert(lan.clone(), false); + assert_eq!(nat_state.aggregate_reachability(), Reachability::Private); + + // The public address succeeding must win, whatever order it arrives in. + nat_state.address_reachability.insert(public, true); + assert_eq!(nat_state.aggregate_reachability(), Reachability::Public); + + // A later failure for the LAN address must not undo it. + nat_state.address_reachability.insert(lan, false); + assert_eq!( + nat_state.aggregate_reachability(), + Reachability::Public, + "a failing address must not override a reachable one" + ); + } + /// A circuit address is only built from an address a third party could /// dial — a relay reachable only from where we stand relays nothing. #[test] @@ -3118,6 +3461,7 @@ mod tests { bootstrap_nodes: vec![], enable_mdns: false, enable_nat_traversal: false, + enable_relay_service: false, gossipsub_topics: vec!["test".to_string()], external_addrs: vec![], }; diff --git a/monas-state-node/src/infrastructure/network/peer_store.rs b/monas-state-node/src/infrastructure/network/peer_store.rs index ced3699..a2e8799 100644 --- a/monas-state-node/src/infrastructure/network/peer_store.rs +++ b/monas-state-node/src/infrastructure/network/peer_store.rs @@ -156,17 +156,18 @@ impl PeerStore { /// /// Rejects addresses that cannot get us back to the peer from a fresh process: /// loopback, link-local and unspecified addresses, and relayed -/// (`/p2p-circuit`) addresses — a circuit is only valid while that particular -/// relay connection lives, so persisting one just means re-dialling a dead -/// path every maintenance tick. +/// (`/p2p-circuit`) addresses — a circuit is only meaningful while that relay +/// is up and still holds a reservation for that peer, so persisting one just +/// means re-dialling a dead path every maintenance tick. The address we want +/// to remember is where the peer itself lives. fn is_shareable(addr: &Multiaddr) -> bool { use libp2p::multiaddr::Protocol; !addr.iter().any(|p| match p { + Protocol::P2pCircuit => true, Protocol::Ip4(ip) => ip.is_loopback() || ip.is_link_local() || ip.is_unspecified(), Protocol::Ip6(ip) => { ip.is_loopback() || ip.is_unspecified() || (ip.segments()[0] & 0xffc0) == 0xfe80 } - Protocol::P2pCircuit => true, _ => false, }) } @@ -237,7 +238,8 @@ mod tests { /// A relayed address is only good while that relay connection lives. /// Persisting one means re-dialling a dead circuit on every tick — the - /// same "frozen address" class this store exists to escape. + /// same "frozen address" class this store exists to escape. Covers the + /// relay-side suffix shape (…/p2p//p2p-circuit). #[test] fn skips_relayed_circuit_addresses() { let mut store = PeerStore::default(); @@ -249,6 +251,24 @@ mod tests { assert!(store.is_empty()); } + /// A circuit address says where a peer could be reached *through a relay + /// right now*, not where it lives. Persisting one would have a restart + /// dial a circuit through a relay that no longer holds the reservation. + /// Covers the full-circuit shape (…/p2p-circuit/p2p/). + #[test] + fn skips_circuit_addresses() { + let mut store = PeerStore::default(); + let p = peer(1); + assert!(!store.record( + p, + addr( + "/ip4/198.51.100.7/tcp/9001/p2p-circuit/p2p/12D3KooWA8EXV3KjBxEU5EnsPfneLx84vMWA\ + Gyid2iykpFYbdcVN" + ) + )); + assert!(store.is_empty()); + } + #[test] fn keeps_a_bounded_number_of_addresses_per_peer() { let mut store = PeerStore::default(); diff --git a/monas-state-node/tests/create_content_push_race_test.rs b/monas-state-node/tests/create_content_push_race_test.rs index c365d14..ff499f2 100644 --- a/monas-state-node/tests/create_content_push_race_test.rs +++ b/monas-state-node/tests/create_content_push_race_test.rs @@ -134,6 +134,7 @@ async fn spawn_test_node() -> TestNode { bootstrap_nodes: vec![], enable_mdns: false, enable_nat_traversal: false, + enable_relay_service: false, gossipsub_topics: vec!["test-events".to_string()], external_addrs: vec![], }; diff --git a/monas-state-node/tests/e2e_multi_node_test.rs b/monas-state-node/tests/e2e_multi_node_test.rs index 7cf6d07..fcffe5d 100644 --- a/monas-state-node/tests/e2e_multi_node_test.rs +++ b/monas-state-node/tests/e2e_multi_node_test.rs @@ -30,6 +30,7 @@ async fn create_test_node() -> (StateNode, TempDir) { bootstrap_nodes: vec![], enable_mdns: false, // Disable mDNS to avoid interference between tests enable_nat_traversal: false, + enable_relay_service: false, gossipsub_topics: vec![EVENTS_TOPIC.to_string()], external_addrs: vec![], }, diff --git a/monas-state-node/tests/integration_test.rs b/monas-state-node/tests/integration_test.rs index 3942b59..69f15ed 100644 --- a/monas-state-node/tests/integration_test.rs +++ b/monas-state-node/tests/integration_test.rs @@ -151,6 +151,7 @@ async fn create_test_service() -> (Arc, Arc, Te bootstrap_nodes: vec![], enable_mdns: false, // Disable mDNS for isolated tests enable_nat_traversal: false, + enable_relay_service: false, gossipsub_topics: vec!["test-events".to_string()], external_addrs: vec![], }; @@ -542,6 +543,7 @@ async fn create_test_service_with_ac() -> (Arc, Arc (Arc, Arc