remove nat module and use libp2p upnp (#4840)

* remove nat module and use libp2p upnp

* update Cargo.lock

* remove no longer used dependencies

* restore nat module refactored

* log successful mapping

* only activate upnp if config enabled

reduce logs to debug!

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into libp2p-nat

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into libp2p-nat

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into libp2p-nat

* address review

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into libp2p-nat

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into libp2p-nat

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into libp2p-nat

* address review

Cargo.lock

@@ -299,9 +299,9 @@ dependencies = [
 
 [[package]]
 name = "anyhow"
-version = "1.0.79"
+version = "1.0.80"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "080e9890a082662b09c1ad45f567faeeb47f22b5fb23895fbe1e651e718e25ca"
+checksum = "5ad32ce52e4161730f7098c077cd2ed6229b5804ccf99e5366be1ab72a98b4e1"
 
 [[package]]
 name = "arbitrary"
@@ -1347,7 +1347,7 @@ checksum = "eee4243f1f26fc7a42710e7439c149e2b10b05472f88090acce52632f231a73a"
 dependencies = [
  "camino",
  "cargo-platform",
- "semver 1.0.21",
+ "semver 1.0.22",
  "serde",
  "serde_json",
  "thiserror",
@@ -4129,17 +4129,6 @@ dependencies = [
  "unicode-normalization",
 ]
 
-[[package]]
-name = "if-addrs"
-version = "0.6.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2273e421f7c4f0fc99e1934fe4776f59d8df2972f4199d703fc0da9f2a9f73de"
-dependencies = [
- "if-addrs-sys",
- "libc",
- "winapi",
-]
-
 [[package]]
 name = "if-addrs"
 version = "0.10.2"
@@ -4150,16 +4139,6 @@ dependencies = [
  "windows-sys 0.48.0",
 ]
 
-[[package]]
-name = "if-addrs-sys"
-version = "0.3.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "de74b9dd780476e837e5eb5ab7c88b49ed304126e412030a0adba99c8efe79ea"
-dependencies = [
- "cc",
- "libc",
-]
-
 [[package]]
 name = "if-watch"
 version = "3.2.0"
@@ -4170,7 +4149,7 @@ dependencies = [
  "core-foundation",
  "fnv",
  "futures",
- "if-addrs 0.10.2",
+ "if-addrs",
  "ipnet",
  "log",
  "rtnetlink",
@@ -4348,6 +4327,17 @@ version = "2.9.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3"
 
+[[package]]
+name = "is-terminal"
+version = "0.4.12"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f23ff5ef2b80d608d61efee834934d862cd92461afc0560dedf493e4c033738b"
+dependencies = [
+ "hermit-abi 0.3.6",
+ "libc",
+ "windows-sys 0.52.0",
+]
+
 [[package]]
 name = "itertools"
 version = "0.10.5"
@@ -5726,6 +5716,7 @@ dependencies = [
 name = "network"
 version = "0.2.0"
 dependencies = [
+ "anyhow",
  "beacon_chain",
  "beacon_processor",
  "delay_map",
@@ -5741,7 +5732,6 @@ dependencies = [
  "futures",
  "genesis",
  "hex",
- "if-addrs 0.6.7",
  "igd-next",
  "itertools",
  "lazy_static",
@@ -5920,15 +5910,6 @@ dependencies = [
  "libc",
 ]
 
-[[package]]
-name = "num_threads"
-version = "0.1.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5c7398b9c8b70908f6371f47ed36737907c87c52af34c268fed0bf0ceb92ead9"
-dependencies = [
- "libc",
-]
-
 [[package]]
 name = "object"
 version = "0.32.2"
@@ -7265,7 +7246,7 @@ version = "0.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "bfa0f585226d2e68097d4f95d113b15b83a82e819ab25717ec0590d9584ef366"
 dependencies = [
- "semver 1.0.21",
+ "semver 1.0.22",
 ]
 
 [[package]]
@@ -7380,9 +7361,9 @@ dependencies = [
 
 [[package]]
 name = "ryu"
-version = "1.0.16"
+version = "1.0.17"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f98d2aa92eebf49b69786be48e4477826b256916e84a57ff2a4f21923b48eb4c"
+checksum = "e86697c916019a8588c99b5fac3cead74ec0b4b819707a682fd4d23fa0ce1ba1"
 
 [[package]]
 name = "safe_arith"
@@ -7544,9 +7525,9 @@ dependencies = [
 
 [[package]]
 name = "semver"
-version = "1.0.21"
+version = "1.0.22"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b97ed7a9823b74f99c7742f5336af7be5ecd3eeafcb1507d1fa93347b1d589b0"
+checksum = "92d43fe69e652f3df9bdc2b85b2854a0825b86e4fb76bc44d945137d053639ca"
 dependencies = [
  "serde",
 ]
@@ -7691,9 +7672,9 @@ dependencies = [
 
 [[package]]
 name = "serde_yaml"
-version = "0.9.31"
+version = "0.9.32"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "adf8a49373e98a4c5f0ceb5d05aa7c648d75f63774981ed95b7c7443bbd50c6e"
+checksum = "8fd075d994154d4a774f95b51fb96bdc2832b0ea48425c92546073816cda1f2f"
 dependencies = [
  "indexmap 2.2.3",
  "itoa",
@@ -7992,11 +7973,11 @@ dependencies = [
 
 [[package]]
 name = "slog-term"
-version = "2.9.0"
+version = "2.9.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "87d29185c55b7b258b4f120eab00f48557d4d9bc814f41713f449d35b0f8977c"
+checksum = "b6e022d0b998abfe5c3782c1f03551a596269450ccd677ea51c56f8b214610e8"
 dependencies = [
- "atty",
+ "is-terminal",
  "slog",
  "term",
  "thread_local",
@@ -8526,9 +8507,7 @@ checksum = "c8248b6521bb14bc45b4067159b9b6ad792e2d6d754d6c41fb50e29fefe38749"
 dependencies = [
  "deranged",
  "itoa",
- "libc",
  "num-conv",
- "num_threads",
  "powerfmt",
  "serde",
  "time-core",

Cargo.toml

@@ -94,6 +94,7 @@ resolver = "2"
 edition = "2021"
 
 [workspace.dependencies]
+anyhow = "1"
 arbitrary = { version = "1", features = ["derive"] }
 bincode = "1"
 bitvec = "1"

beacon_node/lighthouse_network/Cargo.toml

@@ -64,7 +64,7 @@ quick-protobuf-codec = "0.3"
 [dependencies.libp2p]
 version = "0.53"
 default-features = false
-features = ["identify", "yamux", "noise", "dns", "tcp", "tokio", "plaintext", "secp256k1", "macros", "ecdsa", "metrics", "quic"]
+features = ["identify", "yamux", "noise", "dns", "tcp", "tokio", "plaintext", "secp256k1", "macros", "ecdsa", "metrics", "quic", "upnp"]
 
 [dev-dependencies]
 slog-term = { workspace = true }

beacon_node/lighthouse_network/src/service/behaviour.rs

@@ -6,6 +6,7 @@ use crate::types::SnappyTransform;
 use crate::gossipsub;
 use libp2p::identify;
 use libp2p::swarm::NetworkBehaviour;
+use libp2p::upnp::tokio::Behaviour as Upnp;
 use types::EthSpec;
 
 use super::api_types::RequestId;
@@ -32,6 +33,8 @@ where
     // NOTE: The id protocol is used for initial interop. This will be removed by mainnet.
     /// Provides IP addresses and peer information.
     pub identify: identify::Behaviour,
+    /// Libp2p UPnP port mapping.
+    pub upnp: Upnp,
     /// The routing pub-sub mechanism for eth2.
     pub gossipsub: Gossipsub,
 }

beacon_node/lighthouse_network/src/service/mod.rs

@@ -28,10 +28,9 @@ use crate::{error, metrics, Enr, NetworkGlobals, PubsubMessage, TopicHash};
 use api_types::{PeerRequestId, Request, RequestId, Response};
 use futures::stream::StreamExt;
 use gossipsub_scoring_parameters::{lighthouse_gossip_thresholds, PeerScoreSettings};
-use libp2p::multiaddr::{Multiaddr, Protocol as MProtocol};
+use libp2p::multiaddr::{self, Multiaddr, Protocol as MProtocol};
 use libp2p::swarm::{Swarm, SwarmEvent};
-use libp2p::PeerId;
+use libp2p::{identify, PeerId, SwarmBuilder};
-use libp2p::{identify, SwarmBuilder};
 use slog::{crit, debug, info, o, trace, warn};
 use std::path::PathBuf;
 use std::pin::Pin;
@@ -363,6 +362,7 @@ impl<AppReqId: ReqId, TSpec: EthSpec> Network<AppReqId, TSpec> {
                 identify,
                 peer_manager,
                 connection_limits,
+                upnp: Default::default(),
             }
         };
 
@@ -1601,6 +1601,47 @@ impl<AppReqId: ReqId, TSpec: EthSpec> Network<AppReqId, TSpec> {
         }
     }
 
+    fn inject_upnp_event(&mut self, event: libp2p::upnp::Event) {
+        match event {
+            libp2p::upnp::Event::NewExternalAddr(addr) => {
+                info!(self.log, "UPnP route established"; "addr" => %addr);
+                let mut iter = addr.iter();
+                // Skip Ip address.
+                iter.next();
+                match iter.next() {
+                    Some(multiaddr::Protocol::Udp(udp_port)) => match iter.next() {
+                        Some(multiaddr::Protocol::QuicV1) => {
+                            if let Err(e) = self.discovery_mut().update_enr_quic_port(udp_port) {
+                                warn!(self.log, "Failed to update ENR"; "error" => e);
+                            }
+                        }
+                        _ => {
+                            trace!(self.log, "UPnP address mapped multiaddr from unknown transport"; "addr" => %addr)
+                        }
+                    },
+                    Some(multiaddr::Protocol::Tcp(tcp_port)) => {
+                        if let Err(e) = self.discovery_mut().update_enr_tcp_port(tcp_port) {
+                            warn!(self.log, "Failed to update ENR"; "error" => e);
+                        }
+                    }
+                    _ => {
+                        trace!(self.log, "UPnP address mapped multiaddr from unknown transport"; "addr" => %addr);
+                    }
+                }
+            }
+            libp2p::upnp::Event::ExpiredExternalAddr(_) => {}
+            libp2p::upnp::Event::GatewayNotFound => {
+                info!(self.log, "UPnP not available");
+            }
+            libp2p::upnp::Event::NonRoutableGateway => {
+                info!(
+                    self.log,
+                    "UPnP is available but gateway is not exposed to public network"
+                );
+            }
+        }
+    }
+
     /* Networking polling */
 
     /// Poll the p2p networking stack.
@@ -1623,6 +1664,10 @@ impl<AppReqId: ReqId, TSpec: EthSpec> Network<AppReqId, TSpec> {
                     }
                     BehaviourEvent::Identify(ie) => self.inject_identify_event(ie),
                     BehaviourEvent::PeerManager(pe) => self.inject_pm_event(pe),
+                    BehaviourEvent::Upnp(e) => {
+                        self.inject_upnp_event(e);
+                        None
+                    }
                     BehaviourEvent::ConnectionLimits(le) => void::unreachable(le),
                 },
                 SwarmEvent::ConnectionEstablished { .. } => None,

beacon_node/lighthouse_network/src/service/utils.rs

@@ -8,7 +8,6 @@ use crate::{GossipTopic, NetworkConfig};
 use futures::future::Either;
 use libp2p::core::{multiaddr::Multiaddr, muxing::StreamMuxerBox, transport::Boxed};
 use libp2p::identity::{secp256k1, Keypair};
-use libp2p::quic;
 use libp2p::{core, noise, yamux, PeerId, Transport};
 use prometheus_client::registry::Registry;
 use slog::{debug, warn};
@@ -63,8 +62,8 @@ pub fn build_transport(
     let transport = if quic_support {
         // Enables Quic
         // The default quic configuration suits us for now.
-        let quic_config = quic::Config::new(&local_private_key);
+        let quic_config = libp2p::quic::Config::new(&local_private_key);
-        let quic = quic::tokio::Transport::new(quic_config);
+        let quic = libp2p::quic::tokio::Transport::new(quic_config);
         let transport = tcp
             .or_transport(quic)
             .map(|either_output, _| match either_output {

beacon_node/network/Cargo.toml

@@ -14,6 +14,7 @@ slog-async = { workspace = true }
 eth2 = { workspace = true }
 
 [dependencies]
+anyhow = { workspace = true }
 beacon_chain = { workspace = true }
 store = { workspace = true }
 lighthouse_network = { workspace = true }
@@ -35,11 +36,10 @@ lazy_static = { workspace = true }
 lighthouse_metrics = { workspace = true }
 logging = { workspace = true }
 task_executor = { workspace = true }
-igd-next = "0.14.3"
+igd-next = "0.14"
 itertools = { workspace = true }
 num_cpus = { workspace = true }
 lru_cache = { workspace = true }
-if-addrs = "0.6.4"
 lru = { workspace = true }
 strum = { workspace = true }
 tokio-util = { workspace = true }
@@ -56,4 +56,4 @@ environment = { workspace = true }
 # NOTE: This can be run via cargo build --bin lighthouse --features network/disable-backfill
 disable-backfill = []
 fork_from_env = ["beacon_chain/fork_from_env"]
-portable = ["beacon_chain/portable"]
+portable = ["beacon_chain/portable"]

beacon_node/network/src/nat.rs

@@ -3,231 +3,58 @@
 //! Currently supported strategies:
 //! - UPnP
 
-use crate::{NetworkConfig, NetworkMessage};
+use anyhow::{bail, Context, Error};
-use if_addrs::get_if_addrs;
+use igd_next::{aio::tokio as igd, PortMappingProtocol};
-use slog::{debug, info};
+use slog::debug;
-use std::net::{IpAddr, SocketAddr, SocketAddrV4};
+use std::net::{IpAddr, Ipv4Addr, SocketAddr};
-use tokio::sync::mpsc;
+use std::time::Duration;
-use types::EthSpec;
+use tokio::time::sleep;
 
-/// Configuration required to construct the UPnP port mappings.
+/// The duration in seconds of a port mapping on the gateway.
-pub struct UPnPConfig {
+const MAPPING_DURATION: u32 = 3600;
-    /// The local TCP port.
-    tcp_port: u16,
-    /// The local UDP discovery port.
-    disc_port: u16,
-    /// The local UDP quic port.
-    quic_port: u16,
-    /// Whether discovery is enabled or not.
-    disable_discovery: bool,
-    /// Whether quic is enabled or not.
-    disable_quic_support: bool,
-}
 
-/// Contains mappings that managed to be established.
+/// Renew the Mapping every half of `MAPPING_DURATION` to avoid the port being unmapped.
-#[derive(Default, Debug)]
+const MAPPING_TIMEOUT: u64 = MAPPING_DURATION as u64 / 2;
-pub struct EstablishedUPnPMappings {
-    /// A TCP port mapping for libp2p.
-    pub tcp_port: Option<u16>,
-    /// A UDP port for the QUIC libp2p transport.
-    pub udp_quic_port: Option<u16>,
-    /// A UDP port for discv5.
-    pub udp_disc_port: Option<u16>,
-}
 
-impl EstablishedUPnPMappings {
+/// Attempts to map Discovery external port mappings with UPnP.
-    /// Returns true if at least one value is set.
+pub async fn construct_upnp_mappings(
-    pub fn is_some(&self) -> bool {
+    addr: Ipv4Addr,
-        self.tcp_port.is_some() || self.udp_quic_port.is_some() || self.udp_disc_port.is_some()
+    port: u16,
-    }
-
-    // Iterator over the UDP ports
-    pub fn udp_ports(&self) -> impl Iterator<Item = &u16> {
-        self.udp_quic_port.iter().chain(self.udp_disc_port.iter())
-    }
-}
-
-impl UPnPConfig {
-    pub fn from_config(config: &NetworkConfig) -> Option<Self> {
-        config.listen_addrs().v4().map(|v4_addr| UPnPConfig {
-            tcp_port: v4_addr.tcp_port,
-            disc_port: v4_addr.disc_port,
-            quic_port: v4_addr.quic_port,
-            disable_discovery: config.disable_discovery,
-            disable_quic_support: config.disable_quic_support,
-        })
-    }
-}
-
-/// Attempts to construct external port mappings with UPnP.
-pub fn construct_upnp_mappings<T: EthSpec>(
-    config: UPnPConfig,
-    network_send: mpsc::UnboundedSender<NetworkMessage<T>>,
     log: slog::Logger,
-) {
+) -> Result<(), Error> {
-    info!(log, "UPnP Attempting to initialise routes");
+    let gateway = igd::search_gateway(Default::default())
-    match igd_next::search_gateway(Default::default()) {
+        .await
-        Err(e) => info!(log, "UPnP not available"; "error" => %e),
+        .context("Gateway does not support UPnP")?;
-        Ok(gateway) => {
-            // Need to find the local listening address matched with the router subnet
-            let interfaces = match get_if_addrs() {
-                Ok(v) => v,
-                Err(e) => {
-                    info!(log, "UPnP failed to get local interfaces"; "error" => %e);
-                    return;
-                }
-            };
-            let local_ip = interfaces.iter().find_map(|interface| {
-                // Just use the first IP of the first interface that is not a loopback and not an
-                // ipv6 address.
-                if !interface.is_loopback() {
-                    interface.ip().is_ipv4().then(|| interface.ip())
-                } else {
-                    None
-                }
-            });
 
-            let local_ip = match local_ip {
+    let external_address = gateway
-                None => {
+        .get_external_ip()
-                    info!(log, "UPnP failed to find local IP address");
+        .await
-                    return;
+        .context("Could not access gateway's external ip")?;
-                }
-                Some(v) => v,
-            };
 
-            debug!(log, "UPnP Local IP Discovered"; "ip" => ?local_ip);
+    let is_private = match external_address {
-
+        IpAddr::V4(ipv4) => ipv4.is_private(),
-            let mut mappings = EstablishedUPnPMappings::default();
+        IpAddr::V6(ipv6) => ipv6.is_loopback() || ipv6.is_unspecified(),
-
-            match local_ip {
-                IpAddr::V4(address) => {
-                    let libp2p_socket = SocketAddrV4::new(address, config.tcp_port);
-                    let external_ip = gateway.get_external_ip();
-                    // We add specific port mappings rather than getting the router to arbitrary assign
-                    // one.
-                    // I've found this to be more reliable. If multiple users are behind a single
-                    // router, they should ideally try to set different port numbers.
-                    mappings.tcp_port = add_port_mapping(
-                        &gateway,
-                        igd_next::PortMappingProtocol::TCP,
-                        libp2p_socket,
-                        "tcp",
-                        &log,
-                    ).map(|_| {
-                        let external_socket = external_ip.as_ref().map(|ip| SocketAddr::new(*ip, config.tcp_port)).map_err(|_| ());
-                        info!(log, "UPnP TCP route established"; "external_socket" => format!("{}:{}", external_socket.as_ref().map(|ip| ip.to_string()).unwrap_or_else(|_| "".into()), config.tcp_port));
-                        config.tcp_port
-                    }).ok();
-
-                    let set_udp_mapping = |udp_port| {
-                        let udp_socket = SocketAddrV4::new(address, udp_port);
-                        add_port_mapping(
-                            &gateway,
-                            igd_next::PortMappingProtocol::UDP,
-                            udp_socket,
-                            "udp",
-                            &log,
-                        ).map(|_| {
-                            info!(log, "UPnP UDP route established"; "external_socket" => format!("{}:{}", external_ip.as_ref().map(|ip| ip.to_string()).unwrap_or_else(|_| "".into()), udp_port));
-                        })
-                    };
-
-                    // Set the discovery UDP port mapping
-                    if !config.disable_discovery && set_udp_mapping(config.disc_port).is_ok() {
-                        mappings.udp_disc_port = Some(config.disc_port);
-                    }
-
-                    // Set the quic UDP port mapping
-                    if !config.disable_quic_support && set_udp_mapping(config.quic_port).is_ok() {
-                        mappings.udp_quic_port = Some(config.quic_port);
-                    }
-
-                    // report any updates to the network service.
-                    if mappings.is_some() {
-                        network_send.send(NetworkMessage::UPnPMappingEstablished{ mappings })
-                .unwrap_or_else(|e| debug!(log, "Could not send message to the network service"; "error" => %e));
-                    }
-                }
-                _ => debug!(log, "UPnP no routes constructed. IPv6 not supported"),
-            }
-        }
     };
-}
 
-/// Sets up a port mapping for a protocol returning the mapped port if successful.
+    if is_private {
-fn add_port_mapping(
+        bail!(
-    gateway: &igd_next::Gateway,
+            "Gateway's external address is a private address: {}",
-    protocol: igd_next::PortMappingProtocol,
+            external_address
-    socket: SocketAddrV4,
+        );
-    protocol_string: &'static str,
-    log: &slog::Logger,
-) -> Result<(), ()> {
-    // We add specific port mappings rather than getting the router to arbitrary assign
-    // one.
-    // I've found this to be more reliable. If multiple users are behind a single
-    // router, they should ideally try to set different port numbers.
-    let mapping_string = &format!("lighthouse-{}", protocol_string);
-    for _ in 0..2 {
-        match gateway.add_port(
-            protocol,
-            socket.port(),
-            SocketAddr::V4(socket),
-            0,
-            mapping_string,
-        ) {
-            Err(e) => {
-                match e {
-                    igd_next::AddPortError::PortInUse => {
-                        // Try and remove and re-create
-                        debug!(log, "UPnP port in use, attempting to remap"; "protocol" => protocol_string, "port" => socket.port());
-                        match gateway.remove_port(protocol, socket.port()) {
-                            Ok(()) => {
-                                debug!(log, "UPnP Removed port mapping"; "protocol" => protocol_string,  "port" => socket.port())
-                            }
-                            Err(e) => {
-                                debug!(log, "UPnP Port remove failure"; "protocol" => protocol_string, "port" => socket.port(), "error" => %e);
-                                return Err(());
-                            }
-                        }
-                    }
-                    e => {
-                        info!(log, "UPnP TCP route not set"; "error" => %e);
-                        return Err(());
-                    }
-                }
-            }
-            Ok(_) => {
-                return Ok(());
-            }
-        }
     }
-    Err(())
-}
 
-/// Removes the specified TCP and UDP port mappings.
+    loop {
-pub fn remove_mappings(mappings: &EstablishedUPnPMappings, log: &slog::Logger) {
+        gateway
-    if mappings.is_some() {
+            .add_port(
-        debug!(log, "Removing UPnP port mappings");
+                PortMappingProtocol::UDP,
-        match igd_next::search_gateway(Default::default()) {
+                port,
-            Ok(gateway) => {
+                SocketAddr::new(IpAddr::V4(addr), port),
-                if let Some(tcp_port) = mappings.tcp_port {
+                MAPPING_DURATION,
-                    match gateway.remove_port(igd_next::PortMappingProtocol::TCP, tcp_port) {
+                "Lighthouse Discovery port",
-                        Ok(()) => debug!(log, "UPnP Removed TCP port mapping"; "port" => tcp_port),
+            )
-                        Err(e) => {
+            .await
-                            debug!(log, "UPnP Failed to remove TCP port mapping"; "port" => tcp_port, "error" => %e)
+            .with_context(|| format!("Could not UPnP map port: {} on the gateway", port))?;
-                        }
+        debug!(log, "Discovery UPnP port mapped"; "port" => %port);
-                    }
+        sleep(Duration::from_secs(MAPPING_TIMEOUT)).await;
-                }
-                for udp_port in mappings.udp_ports() {
-                    match gateway.remove_port(igd_next::PortMappingProtocol::UDP, *udp_port) {
-                        Ok(()) => debug!(log, "UPnP Removed UDP port mapping"; "port" => udp_port),
-                        Err(e) => {
-                            debug!(log, "UPnP Failed to remove UDP port mapping"; "port" => udp_port, "error" => %e)
-                        }
-                    }
-                }
-            }
-            Err(e) => debug!(log, "UPnP failed to remove mappings"; "error" => %e),
-        }
     }
 }

beacon_node/network/src/service.rs

@@ -1,5 +1,5 @@
 use super::sync::manager::RequestId as SyncId;
-use crate::nat::EstablishedUPnPMappings;
+use crate::nat;
 use crate::network_beacon_processor::InvalidBlockStorage;
 use crate::persisted_dht::{clear_dht, load_dht, persist_dht};
 use crate::router::{Router, RouterMessage};
@@ -94,11 +94,6 @@ pub enum NetworkMessage<T: EthSpec> {
         /// The result of the validation
         validation_result: MessageAcceptance,
     },
-    /// Called if  UPnP managed to establish an external port mapping.
-    UPnPMappingEstablished {
-        /// The mappings that were established.
-        mappings: EstablishedUPnPMappings,
-    },
     /// Reports a peer to the peer manager for performing an action.
     ReportPeer {
         peer_id: PeerId,
@@ -188,9 +183,6 @@ pub struct NetworkService<T: BeaconChainTypes> {
     store: Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>,
     /// A collection of global variables, accessible outside of the network service.
     network_globals: Arc<NetworkGlobals<T::EthSpec>>,
-    /// Stores potentially created UPnP mappings to be removed on shutdown. (TCP port and UDP
-    /// ports).
-    upnp_mappings: EstablishedUPnPMappings,
     /// A delay that expires when a new fork takes place.
     next_fork_update: Pin<Box<OptionFuture<Sleep>>>,
     /// A delay that expires when we need to subscribe to a new fork's topics.
@@ -237,22 +229,24 @@ impl<T: BeaconChainTypes> NetworkService<T> {
             "Backfill is disabled. DO NOT RUN IN PRODUCTION"
         );
 
-        // try and construct UPnP port mappings if required.
+        if let (true, false, Some(v4)) = (
-        if let Some(upnp_config) = crate::nat::UPnPConfig::from_config(config) {
+            config.upnp_enabled,
-            let upnp_log = network_log.new(o!("service" => "UPnP"));
+            config.disable_discovery,
-            let upnp_network_send = network_senders.network_send();
+            config.listen_addrs().v4(),
-            if config.upnp_enabled {
+        ) {
-                executor.spawn_blocking(
+            let nw = network_log.clone();
-                    move || {
+            let v4 = v4.clone();
-                        crate::nat::construct_upnp_mappings(
+            executor.spawn(
-                            upnp_config,
+                async move {
-                            upnp_network_send,
+                    info!(nw, "UPnP Attempting to initialise routes");
-                            upnp_log,
+                    if let Err(e) =
-                        )
+                        nat::construct_upnp_mappings(v4.addr, v4.disc_port, nw.clone()).await
-                    },
+                    {
-                    "UPnP",
+                        info!(nw, "Could not UPnP map Discovery port"; "error" => %e);
-                );
+                    }
-            }
+                },
+                "UPnP",
+            );
         }
 
         // get a reference to the beacon chain store
@@ -358,7 +352,6 @@ impl<T: BeaconChainTypes> NetworkService<T> {
             router_send,
             store,
             network_globals: network_globals.clone(),
-            upnp_mappings: EstablishedUPnPMappings::default(),
             next_fork_update,
             next_fork_subscriptions,
             next_unsubscribe,
@@ -636,21 +629,6 @@ impl<T: BeaconChainTypes> NetworkService<T> {
             } => {
                 self.libp2p.send_error_response(peer_id, id, error, reason);
             }
-            NetworkMessage::UPnPMappingEstablished { mappings } => {
-                self.upnp_mappings = mappings;
-                // If there is an external TCP port update, modify our local ENR.
-                if let Some(tcp_port) = self.upnp_mappings.tcp_port {
-                    if let Err(e) = self.libp2p.discovery_mut().update_enr_tcp_port(tcp_port) {
-                        warn!(self.log, "Failed to update ENR"; "error" => e);
-                    }
-                }
-                // If there is an external QUIC port update, modify our local ENR.
-                if let Some(quic_port) = self.upnp_mappings.udp_quic_port {
-                    if let Err(e) = self.libp2p.discovery_mut().update_enr_quic_port(quic_port) {
-                        warn!(self.log, "Failed to update ENR"; "error" => e);
-                    }
-                }
-            }
             NetworkMessage::ValidationResult {
                 propagation_source,
                 message_id,
@@ -1009,10 +987,6 @@ impl<T: BeaconChainTypes> Drop for NetworkService<T> {
                 "Saved DHT state";
             ),
         }
-
-        // attempt to remove port mappings
-        crate::nat::remove_mappings(&self.upnp_mappings, &self.log);
-
         info!(self.log, "Network service shutdown");
     }
 }