From ec704a9bf4235f7f04b5c53856fc9309e46dda56 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 15 Nov 2023 12:49:34 +0200 Subject: [PATCH 1/6] Disable autonat in case external addresses are provided --- crates/subspace-networking/src/constructor.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/subspace-networking/src/constructor.rs b/crates/subspace-networking/src/constructor.rs index 3a70bda4f4..451470833d 100644 --- a/crates/subspace-networking/src/constructor.rs +++ b/crates/subspace-networking/src/constructor.rs @@ -261,6 +261,8 @@ pub struct Config { /// and enable peer to notify others about its reachable address. pub external_addresses: Vec, /// Enable autonat protocol. Helps detecting whether we're behind the firewall. + /// + /// NOTE: Ignored and implied to be `false` in case `external_addresses` is not empty. pub enable_autonat: bool, /// Defines whether we should run blocking Kademlia bootstrap() operation before other requests. pub disable_bootstrap_on_start: bool, @@ -507,7 +509,7 @@ where ..ConnectedPeersConfig::default() } }), - autonat: enable_autonat.then(|| AutonatConfig { + autonat: (enable_autonat && external_addresses.is_empty()).then(|| AutonatConfig { use_connected: true, only_global_ips: !config.allow_non_global_addresses_in_dht, confidence_max: AUTONAT_MAX_CONFIDENCE, From e0d9a9d3afccb6bbcd6cb7145d1481f7bc0b6f5f Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 15 Nov 2023 12:58:37 +0200 Subject: [PATCH 2/6] Replace manual kademlia mode management with more correct built-in implementation --- .../bin/subspace-farmer/commands/farm/dsn.rs | 6 +-- crates/subspace-networking/src/constructor.rs | 12 ++--- crates/subspace-networking/src/node_runner.rs | 51 ++----------------- 3 files changed, 11 insertions(+), 58 deletions(-) diff --git a/crates/subspace-farmer/src/bin/subspace-farmer/commands/farm/dsn.rs b/crates/subspace-farmer/src/bin/subspace-farmer/commands/farm/dsn.rs index 48b7d4f699..cf275e7975 100644 --- a/crates/subspace-farmer/src/bin/subspace-farmer/commands/farm/dsn.rs +++ b/crates/subspace-farmer/src/bin/subspace-farmer/commands/farm/dsn.rs @@ -8,7 +8,7 @@ use subspace_farmer::piece_cache::PieceCache; use subspace_farmer::utils::readers_and_pieces::ReadersAndPieces; use subspace_farmer::{NodeClient, NodeRpcClient}; use subspace_networking::libp2p::identity::Keypair; -use subspace_networking::libp2p::kad::{Mode, RecordKey}; +use subspace_networking::libp2p::kad::RecordKey; use subspace_networking::libp2p::metrics::Metrics; use subspace_networking::libp2p::multiaddr::Protocol; use subspace_networking::utils::multihash::ToMultihash; @@ -196,9 +196,7 @@ pub(super) fn configure_dsn( // Allow to maintain some extra farmer connections beyond direct interest too special_connected_peers_limit: target_connections + in_connections / 4, bootstrap_addresses: bootstrap_nodes, - kademlia_mode: KademliaMode::Dynamic { - initial_mode: Mode::Client, - }, + kademlia_mode: KademliaMode::Dynamic, external_addresses, metrics, disable_bootstrap_on_start, diff --git a/crates/subspace-networking/src/constructor.rs b/crates/subspace-networking/src/constructor.rs index 451470833d..e3bb66bf96 100644 --- a/crates/subspace-networking/src/constructor.rs +++ b/crates/subspace-networking/src/constructor.rs @@ -105,10 +105,7 @@ pub enum KademliaMode { /// The Kademlia mode is static for the duration of the application. Static(Mode), /// Kademlia mode will be changed using Autonat protocol when max confidence reached. - Dynamic { - /// Defines initial Kademlia mode. - initial_mode: Mode, - }, + Dynamic, } /// Trait to be implemented on providers of local records @@ -477,7 +474,7 @@ where debug!(?connection_limits, "DSN connection limits set."); - let behaviour = Behavior::new(BehaviorConfig { + let mut behaviour = Behavior::new(BehaviorConfig { peer_id: local_peer_id, identify, kademlia, @@ -517,6 +514,10 @@ where }), }); + if let KademliaMode::Static(mode) = kademlia_mode { + behaviour.kademlia.set_mode(Some(mode)); + }; + let temporary_bans = Arc::new(Mutex::new(TemporaryBans::new( temporary_bans_cache_size, temporary_ban_backoff, @@ -594,7 +595,6 @@ where general_connection_decision_handler, special_connection_decision_handler, bootstrap_addresses, - kademlia_mode, disable_bootstrap_on_start, }); diff --git a/crates/subspace-networking/src/node_runner.rs b/crates/subspace-networking/src/node_runner.rs index fece9dbb98..a6e0c607e3 100644 --- a/crates/subspace-networking/src/node_runner.rs +++ b/crates/subspace-networking/src/node_runner.rs @@ -7,7 +7,7 @@ use crate::behavior::{ }; use crate::constructor; use crate::constructor::temporary_bans::TemporaryBans; -use crate::constructor::{ConnectedPeersHandler, KademliaMode, LocalOnlyRecordStore}; +use crate::constructor::{ConnectedPeersHandler, LocalOnlyRecordStore}; use crate::protocols::connected_peers::Event as ConnectedPeersEvent; use crate::protocols::peer_info::{Event as PeerInfoEvent, PeerInfoSuccess}; use crate::protocols::request_response::request_response_factory::{ @@ -22,15 +22,14 @@ use event_listener_primitives::HandlerId; use futures::channel::mpsc; use futures::future::Fuse; use futures::{FutureExt, StreamExt}; -use libp2p::autonat::{Event as AutonatEvent, NatStatus}; +use libp2p::autonat::Event as AutonatEvent; use libp2p::core::ConnectedPoint; use libp2p::gossipsub::{Event as GossipsubEvent, TopicHash}; use libp2p::identify::Event as IdentifyEvent; use libp2p::kad::{ Behaviour as Kademlia, BootstrapOk, Event as KademliaEvent, GetClosestPeersError, GetClosestPeersOk, GetProvidersError, GetProvidersOk, GetRecordError, GetRecordOk, - InboundRequest, Mode, PeerRecord, ProgressStep, PutRecordOk, QueryId, QueryResult, Quorum, - Record, + InboundRequest, PeerRecord, ProgressStep, PutRecordOk, QueryId, QueryResult, Quorum, Record, }; use libp2p::metrics::{Metrics, Recorder}; use libp2p::swarm::{DialError, SwarmEvent}; @@ -138,8 +137,6 @@ where bootstrap_addresses: Vec, /// Ensures a single bootstrap on run() invocation. bootstrap_command_state: Arc>, - /// Kademlia mode. - kademlia_mode: KademliaMode, /// Receives an event on peer address removal from the persistent storage. removed_addresses_rx: mpsc::UnboundedReceiver, /// Optional storage for the [`HandlerId`] of the address removal task. @@ -167,7 +164,6 @@ where pub(crate) general_connection_decision_handler: Option, pub(crate) special_connection_decision_handler: Option, pub(crate) bootstrap_addresses: Vec, - pub(crate) kademlia_mode: KademliaMode, pub(crate) disable_bootstrap_on_start: bool, } @@ -190,7 +186,6 @@ where general_connection_decision_handler, special_connection_decision_handler, bootstrap_addresses, - kademlia_mode, disable_bootstrap_on_start, }: NodeRunnerConfig, ) -> Self { @@ -231,7 +226,6 @@ where rng: StdRng::seed_from_u64(KADEMLIA_PEERS_ADDRESSES_BATCH_SIZE as u64), // any seed bootstrap_addresses, bootstrap_command_state: Arc::new(AsyncMutex::new(BootstrapCommandState::default())), - kademlia_mode, removed_addresses_rx, _address_removal_task_handler_id: address_removal_task_handler_id, disable_bootstrap_on_start, @@ -322,18 +316,6 @@ where debug!("Bootstrap started."); - let initial_mode = match self.kademlia_mode { - KademliaMode::Static(mode) => mode, - KademliaMode::Dynamic { initial_mode } => initial_mode, - }; - - self.swarm - .behaviour_mut() - .kademlia - .set_mode(Some(initial_mode)); - - debug!("Kademlia mode set: {:?}.", initial_mode); - let mut bootstrap_step = 0; loop { futures::select! { @@ -1171,33 +1153,6 @@ where if let AutonatEvent::StatusChanged { old, new } = event { info!(?old, ?new, "Public address status changed."); - - if matches!(self.kademlia_mode, KademliaMode::Dynamic { .. }) { - let mode = match &new { - NatStatus::Public(address) => { - if is_global_address_or_dns(address) - || self.allow_non_global_addresses_in_dht - { - Mode::Server - } else { - debug!( - ?old, - ?new, - ?address, - non_global_addresses=%self.allow_non_global_addresses_in_dht, - "Kademlia mode wasn't set." - ); - Mode::Client - } - } - NatStatus::Private => Mode::Client, - NatStatus::Unknown => Mode::Client, - }; - - self.swarm.behaviour_mut().kademlia.set_mode(Some(mode)); - - debug!("Kademlia mode set: {:?}.", mode); - }; } } From 61dd80a6257fe170df6467d4a552c86caa1574b6 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 15 Nov 2023 13:00:36 +0200 Subject: [PATCH 3/6] Add workaround for non-expiring autonat addresses --- crates/subspace-networking/src/node_runner.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/subspace-networking/src/node_runner.rs b/crates/subspace-networking/src/node_runner.rs index a6e0c607e3..94e6eaae70 100644 --- a/crates/subspace-networking/src/node_runner.rs +++ b/crates/subspace-networking/src/node_runner.rs @@ -22,7 +22,7 @@ use event_listener_primitives::HandlerId; use futures::channel::mpsc; use futures::future::Fuse; use futures::{FutureExt, StreamExt}; -use libp2p::autonat::Event as AutonatEvent; +use libp2p::autonat::{Event as AutonatEvent, NatStatus}; use libp2p::core::ConnectedPoint; use libp2p::gossipsub::{Event as GossipsubEvent, TopicHash}; use libp2p::identify::Event as IdentifyEvent; @@ -1153,6 +1153,13 @@ where if let AutonatEvent::StatusChanged { old, new } = event { info!(?old, ?new, "Public address status changed."); + + // TODO: Remove block once https://github.com/libp2p/rust-libp2p/issues/4863 is resolved + if let (NatStatus::Public(old_address), NatStatus::Private) = (old, new) { + self.swarm.remove_external_address(&old_address); + // Trigger potential mode change manually + self.swarm.behaviour_mut().kademlia.set_mode(None); + } } } From 6eadee741f581e14346b4f70c139ee74c9443a23 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 15 Nov 2023 13:51:25 +0200 Subject: [PATCH 4/6] Notify peer about changes to external addresses --- crates/subspace-networking/src/node_runner.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/subspace-networking/src/node_runner.rs b/crates/subspace-networking/src/node_runner.rs index 94e6eaae70..b95adc24fb 100644 --- a/crates/subspace-networking/src/node_runner.rs +++ b/crates/subspace-networking/src/node_runner.rs @@ -606,6 +606,15 @@ where } SwarmEvent::ExternalAddrConfirmed { address } => { info!(%address, "Confirmed external address"); + + let connected_peers = self.swarm.connected_peers().copied().collect::>(); + self.swarm.behaviour_mut().identify.push(connected_peers); + } + SwarmEvent::ExternalAddrExpired { address } => { + info!(%address, "External address expired"); + + let connected_peers = self.swarm.connected_peers().copied().collect::>(); + self.swarm.behaviour_mut().identify.push(connected_peers); } other => { trace!("Other swarm event: {:?}", other); @@ -1159,6 +1168,9 @@ where self.swarm.remove_external_address(&old_address); // Trigger potential mode change manually self.swarm.behaviour_mut().kademlia.set_mode(None); + + let connected_peers = self.swarm.connected_peers().copied().collect::>(); + self.swarm.behaviour_mut().identify.push(connected_peers); } } } From 683f215216f91bec22b1a4cddb3603179ac0c14f Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 15 Nov 2023 15:00:16 +0200 Subject: [PATCH 5/6] Clean up old addresses of a peer --- crates/subspace-networking/src/node_runner.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/crates/subspace-networking/src/node_runner.rs b/crates/subspace-networking/src/node_runner.rs index b95adc24fb..357c4d148f 100644 --- a/crates/subspace-networking/src/node_runner.rs +++ b/crates/subspace-networking/src/node_runner.rs @@ -722,6 +722,22 @@ where }); if full_kademlia_support { + let old_addresses = kademlia + .kbucket(peer_id) + .and_then(|peers| { + let key = peer_id.into(); + peers.iter().find_map(|peer| { + (peer.node.key == &key).then_some( + peer.node + .value + .iter() + .filter(|address| info.listen_addrs.contains(address)) + .cloned() + .collect::>(), + ) + }) + }) + .unwrap_or_default(); for address in info.listen_addrs { if !self.allow_non_global_addresses_in_dht && !is_global_address_or_dns(&address) @@ -742,8 +758,19 @@ where protocol_names = ?kademlia.protocol_names(), "Adding self-reported address to Kademlia DHT", ); + kademlia.add_address(&peer_id, address); } + for old_address in old_addresses { + trace!( + %local_peer_id, + %peer_id, + %old_address, + "Removing old self-reported address from Kademlia DHT", + ); + + kademlia.remove_address(&peer_id, &old_address); + } } else { debug!( %local_peer_id, From f3b7401b436fc55edf5f7658e6236804008eb139 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Wed, 15 Nov 2023 16:07:29 +0200 Subject: [PATCH 6/6] Infer `enable_autonat` from `external_addresses` --- .../subspace-networking/examples/benchmark.rs | 1 - .../examples/random-walker.rs | 1 - crates/subspace-networking/src/constructor.rs | 20 ++++++++++--------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/subspace-networking/examples/benchmark.rs b/crates/subspace-networking/examples/benchmark.rs index d0aaaa37bd..4216851029 100644 --- a/crates/subspace-networking/examples/benchmark.rs +++ b/crates/subspace-networking/examples/benchmark.rs @@ -274,7 +274,6 @@ pub async fn configure_dsn( allow_non_global_addresses_in_dht: enable_private_ips, request_response_protocols: vec![PieceByIndexRequestHandler::create(|_, _| async { None })], bootstrap_addresses, - enable_autonat: false, max_pending_outgoing_connections: pending_out_peers, max_established_outgoing_connections: out_peers, ..default_config diff --git a/crates/subspace-networking/examples/random-walker.rs b/crates/subspace-networking/examples/random-walker.rs index 6526427b4c..b56c45cbac 100644 --- a/crates/subspace-networking/examples/random-walker.rs +++ b/crates/subspace-networking/examples/random-walker.rs @@ -367,7 +367,6 @@ async fn configure_dsn( allow_non_global_addresses_in_dht: enable_private_ips, request_response_protocols: vec![PieceByIndexRequestHandler::create(|_, _| async { None })], bootstrap_addresses, - enable_autonat: false, max_pending_outgoing_connections: pending_out_peers, max_established_outgoing_connections: out_peers, ..default_config diff --git a/crates/subspace-networking/src/constructor.rs b/crates/subspace-networking/src/constructor.rs index e3bb66bf96..28a2d367c0 100644 --- a/crates/subspace-networking/src/constructor.rs +++ b/crates/subspace-networking/src/constructor.rs @@ -257,10 +257,6 @@ pub struct Config { /// Known external addresses to the local peer. The addresses will be added on the swarm start /// and enable peer to notify others about its reachable address. pub external_addresses: Vec, - /// Enable autonat protocol. Helps detecting whether we're behind the firewall. - /// - /// NOTE: Ignored and implied to be `false` in case `external_addresses` is not empty. - pub enable_autonat: bool, /// Defines whether we should run blocking Kademlia bootstrap() operation before other requests. pub disable_bootstrap_on_start: bool, } @@ -382,7 +378,6 @@ where bootstrap_addresses: Vec::new(), kademlia_mode: KademliaMode::Static(Mode::Client), external_addresses: Vec::new(), - enable_autonat: true, disable_bootstrap_on_start: false, } } @@ -453,7 +448,6 @@ where bootstrap_addresses, kademlia_mode, external_addresses, - enable_autonat, disable_bootstrap_on_start, } = config; let local_peer_id = peer_id(&keypair); @@ -506,7 +500,7 @@ where ..ConnectedPeersConfig::default() } }), - autonat: (enable_autonat && external_addresses.is_empty()).then(|| AutonatConfig { + autonat: external_addresses.is_empty().then(|| AutonatConfig { use_connected: true, only_global_ips: !config.allow_non_global_addresses_in_dht, confidence_max: AUTONAT_MAX_CONFIDENCE, @@ -514,8 +508,16 @@ where }), }); - if let KademliaMode::Static(mode) = kademlia_mode { - behaviour.kademlia.set_mode(Some(mode)); + match (kademlia_mode, external_addresses.is_empty()) { + (KademliaMode::Static(mode), _) => { + behaviour.kademlia.set_mode(Some(mode)); + } + (KademliaMode::Dynamic, false) => { + behaviour.kademlia.set_mode(Some(Mode::Server)); + } + _ => { + // Autonat will figure it out + } }; let temporary_bans = Arc::new(Mutex::new(TemporaryBans::new(