From c3d940db624eada0bc71fef341905803bee78a10 Mon Sep 17 00:00:00 2001 From: max143672 Date: Thu, 9 Nov 2023 12:55:42 +0400 Subject: [PATCH] Refactor p2p port handling Changed the handling of default p2p port. Previously, a fixed port was being used which was not serving the purpose when the operator runs a node and specifies a non-standard local port. The revised code implements variable 'desired_external_port' to set the port number from the normalized peer-to-peer listening address. This allows the use of a non-standard public address. This change makes the system more adaptable to operator wishes and avoids potential conflicts. --- components/addressmanager/src/lib.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/components/addressmanager/src/lib.rs b/components/addressmanager/src/lib.rs index 9998ee85f7..c85909d3ea 100644 --- a/components/addressmanager/src/lib.rs +++ b/components/addressmanager/src/lib.rs @@ -163,18 +163,18 @@ impl AddressManager { } info!("[UPnP] Got external ip from gateway using upnp: {ip}"); - let default_port = self.config.default_p2p_port(); - - let normalized_p2p_listen_address = self.config.p2p_listen_address.normalize(default_port); + let normalized_p2p_listen_address = self.config.p2p_listen_address.normalize(self.config.default_p2p_port()); let local_addr = if normalized_p2p_listen_address.ip.is_unspecified() { SocketAddr::new(local_ip_address::local_ip().unwrap(), normalized_p2p_listen_address.port) } else { normalized_p2p_listen_address.into() }; + // If an operator runs a node and specifies a non-standard local port, it implies that they also wish to use a non-standard public address. The variable 'desired_external_port' is set to the port number from the normalized peer-to-peer listening address. + let desired_external_port = normalized_p2p_listen_address.port; // This loop checks for existing port mappings in the UPnP-enabled gateway. // - // The goal of this loop is to identify if the desired external port (`default_port`) is + // The goal of this loop is to identify if the desired external port (`desired_external_port`) is // already mapped to any device inside the local network. This is crucial because, in // certain scenarios, gateways might not throw the `PortInUse` error but rather might // silently remap the external port when there's a conflict. By iterating through the @@ -189,7 +189,7 @@ impl AddressManager { let already_in_use = loop { match gateway.get_generic_port_mapping_entry(index) { Ok(entry) => { - if entry.enabled && entry.external_port == default_port { + if entry.enabled && entry.external_port == desired_external_port { info!("[UPnP] Found existing mapping that uses the same external port. Description: {}, external port: {}, internal port: {}, client: {}, lease duration: {}", entry.port_mapping_description, entry.external_port, entry.internal_port, entry.internal_client, entry.lease_duration); break true; } @@ -215,14 +215,17 @@ impl AddressManager { match gateway.add_port( igd::PortMappingProtocol::TCP, - default_port, + desired_external_port, local_addr, UPNP_DEADLINE_SEC as u32, UPNP_REGISTRATION_NAME, ) { Ok(_) => { - info!("[UPnP] Added port mapping to default external port: {ip}:{default_port}"); - Ok(Some((NetAddress { ip, port: default_port }, ExtendHelper { gateway, local_addr, external_port: default_port }))) + info!("[UPnP] Added port mapping to default external port: {ip}:{desired_external_port}"); + Ok(Some(( + NetAddress { ip, port: desired_external_port }, + ExtendHelper { gateway, local_addr, external_port: desired_external_port }, + ))) } Err(AddPortError::PortInUse {}) => { let port = gateway.add_any_port(