Merge pull request #2234 from subspace/networking-tweaks

Networking tweaks

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,

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

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

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
@@ -260,8 +257,6 @@ pub struct Config<LocalRecordProvider> {
     /// 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<Multiaddr>,
-    /// Enable autonat protocol. Helps detecting whether we're behind the firewall.
-    pub enable_autonat: bool,
     /// Defines whether we should run blocking Kademlia bootstrap() operation before other requests.
     pub disable_bootstrap_on_start: bool,
 }
@@ -383,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,
         }
     }
@@ -454,7 +448,6 @@ where
         bootstrap_addresses,
         kademlia_mode,
         external_addresses,
-        enable_autonat,
         disable_bootstrap_on_start,
     } = config;
     let local_peer_id = peer_id(&keypair);
@@ -475,7 +468,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,
@@ -507,14 +500,26 @@ where
                 ..ConnectedPeersConfig::default()
             }
         }),
-        autonat: enable_autonat.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,
             ..Default::default()
         }),
     });
 
+    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(
         temporary_bans_cache_size,
         temporary_ban_backoff,
@@ -592,7 +597,6 @@ where
             general_connection_decision_handler,
             special_connection_decision_handler,
             bootstrap_addresses,
-            kademlia_mode,
             disable_bootstrap_on_start,
         });
 

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::{
@@ -29,8 +29,7 @@ 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<Multiaddr>,
     /// Ensures a single bootstrap on run() invocation.
     bootstrap_command_state: Arc<AsyncMutex<BootstrapCommandState>>,
-    /// Kademlia mode.
-    kademlia_mode: KademliaMode,
     /// Receives an event on peer address removal from the persistent storage.
     removed_addresses_rx: mpsc::UnboundedReceiver<PeerAddressRemovedEvent>,
     /// Optional storage for the [`HandlerId`] of the address removal task.
@@ -167,7 +164,6 @@ where
     pub(crate) general_connection_decision_handler: Option<ConnectedPeersHandler>,
     pub(crate) special_connection_decision_handler: Option<ConnectedPeersHandler>,
     pub(crate) bootstrap_addresses: Vec<Multiaddr>,
-    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<LocalRecordProvider>,
     ) -> 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! {
@@ -624,6 +606,15 @@ where
             }
             SwarmEvent::ExternalAddrConfirmed { address } => {
                 info!(%address, "Confirmed external address");
+
+                let connected_peers = self.swarm.connected_peers().copied().collect::<Vec<_>>();
+                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::<Vec<_>>();
+                self.swarm.behaviour_mut().identify.push(connected_peers);
             }
             other => {
                 trace!("Other swarm event: {:?}", other);
@@ -731,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::<Vec<_>>(),
+                            )
+                        })
+                    })
+                    .unwrap_or_default();
                 for address in info.listen_addrs {
                     if !self.allow_non_global_addresses_in_dht
                         && !is_global_address_or_dns(&address)
@@ -751,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,
@@ -1172,32 +1190,15 @@ 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));
+            // 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);
 
-                debug!("Kademlia mode set: {:?}.", mode);
-            };
+                let connected_peers = self.swarm.connected_peers().copied().collect::<Vec<_>>();
+                self.swarm.behaviour_mut().identify.push(connected_peers);
+            }
         }
     }