From abd99652b4571af05ecc19f107dd5f740476efe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Tue, 27 Feb 2024 07:29:18 +0000 Subject: [PATCH] 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 | 73 ++--- Cargo.toml | 1 + beacon_node/lighthouse_network/Cargo.toml | 2 +- .../src/service/behaviour.rs | 3 + .../lighthouse_network/src/service/mod.rs | 51 +++- .../lighthouse_network/src/service/utils.rs | 5 +- beacon_node/network/Cargo.toml | 6 +- beacon_node/network/src/nat.rs | 259 +++--------------- beacon_node/network/src/service.rs | 64 ++--- 9 files changed, 146 insertions(+), 318 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aa17e2f4c..643b275d9 100644 --- a/Cargo.lock +++ b/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", diff --git a/Cargo.toml b/Cargo.toml index a7f44551e..317c3f954 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ resolver = "2" edition = "2021" [workspace.dependencies] +anyhow = "1" arbitrary = { version = "1", features = ["derive"] } bincode = "1" bitvec = "1" diff --git a/beacon_node/lighthouse_network/Cargo.toml b/beacon_node/lighthouse_network/Cargo.toml index cd0de37d3..fe3e628f1 100644 --- a/beacon_node/lighthouse_network/Cargo.toml +++ b/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 } diff --git a/beacon_node/lighthouse_network/src/service/behaviour.rs b/beacon_node/lighthouse_network/src/service/behaviour.rs index a43678d4b..5a04d6c2d 100644 --- a/beacon_node/lighthouse_network/src/service/behaviour.rs +++ b/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, } diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index 401e43a53..3717c4973 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/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, SwarmBuilder}; +use libp2p::{identify, PeerId, SwarmBuilder}; use slog::{crit, debug, info, o, trace, warn}; use std::path::PathBuf; use std::pin::Pin; @@ -363,6 +362,7 @@ impl Network { identify, peer_manager, connection_limits, + upnp: Default::default(), } }; @@ -1601,6 +1601,47 @@ impl Network { } } + 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 Network { } 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, diff --git a/beacon_node/lighthouse_network/src/service/utils.rs b/beacon_node/lighthouse_network/src/service/utils.rs index 1dd6062ed..489c5ae52 100644 --- a/beacon_node/lighthouse_network/src/service/utils.rs +++ b/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 = quic::tokio::Transport::new(quic_config); + let quic_config = libp2p::quic::Config::new(&local_private_key); + let quic = libp2p::quic::tokio::Transport::new(quic_config); let transport = tcp .or_transport(quic) .map(|either_output, _| match either_output { diff --git a/beacon_node/network/Cargo.toml b/beacon_node/network/Cargo.toml index d8766d009..58a195aea 100644 --- a/beacon_node/network/Cargo.toml +++ b/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"] \ No newline at end of file +portable = ["beacon_chain/portable"] diff --git a/beacon_node/network/src/nat.rs b/beacon_node/network/src/nat.rs index cb81877b2..e63ff5503 100644 --- a/beacon_node/network/src/nat.rs +++ b/beacon_node/network/src/nat.rs @@ -3,231 +3,58 @@ //! Currently supported strategies: //! - UPnP -use crate::{NetworkConfig, NetworkMessage}; -use if_addrs::get_if_addrs; -use slog::{debug, info}; -use std::net::{IpAddr, SocketAddr, SocketAddrV4}; -use tokio::sync::mpsc; -use types::EthSpec; +use anyhow::{bail, Context, Error}; +use igd_next::{aio::tokio as igd, PortMappingProtocol}; +use slog::debug; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; +use std::time::Duration; +use tokio::time::sleep; -/// Configuration required to construct the UPnP port mappings. -pub struct UPnPConfig { - /// 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, -} +/// The duration in seconds of a port mapping on the gateway. +const MAPPING_DURATION: u32 = 3600; -/// Contains mappings that managed to be established. -#[derive(Default, Debug)] -pub struct EstablishedUPnPMappings { - /// A TCP port mapping for libp2p. - pub tcp_port: Option, - /// A UDP port for the QUIC libp2p transport. - pub udp_quic_port: Option, - /// A UDP port for discv5. - pub udp_disc_port: Option, -} +/// Renew the Mapping every half of `MAPPING_DURATION` to avoid the port being unmapped. +const MAPPING_TIMEOUT: u64 = MAPPING_DURATION as u64 / 2; -impl EstablishedUPnPMappings { - /// Returns true if at least one value is set. - pub fn is_some(&self) -> bool { - self.tcp_port.is_some() || self.udp_quic_port.is_some() || self.udp_disc_port.is_some() - } - - // Iterator over the UDP ports - pub fn udp_ports(&self) -> impl Iterator { - self.udp_quic_port.iter().chain(self.udp_disc_port.iter()) - } -} - -impl UPnPConfig { - pub fn from_config(config: &NetworkConfig) -> Option { - 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( - config: UPnPConfig, - network_send: mpsc::UnboundedSender>, +/// Attempts to map Discovery external port mappings with UPnP. +pub async fn construct_upnp_mappings( + addr: Ipv4Addr, + port: u16, log: slog::Logger, -) { - info!(log, "UPnP Attempting to initialise routes"); - match igd_next::search_gateway(Default::default()) { - Err(e) => info!(log, "UPnP not available"; "error" => %e), - 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 - } - }); +) -> Result<(), Error> { + let gateway = igd::search_gateway(Default::default()) + .await + .context("Gateway does not support UPnP")?; - let local_ip = match local_ip { - None => { - info!(log, "UPnP failed to find local IP address"); - return; - } - Some(v) => v, - }; + let external_address = gateway + .get_external_ip() + .await + .context("Could not access gateway's external ip")?; - debug!(log, "UPnP Local IP Discovered"; "ip" => ?local_ip); - - let mut mappings = EstablishedUPnPMappings::default(); - - 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"), - } - } + let is_private = match external_address { + IpAddr::V4(ipv4) => ipv4.is_private(), + IpAddr::V6(ipv6) => ipv6.is_loopback() || ipv6.is_unspecified(), }; -} -/// Sets up a port mapping for a protocol returning the mapped port if successful. -fn add_port_mapping( - gateway: &igd_next::Gateway, - protocol: igd_next::PortMappingProtocol, - 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(()); - } - } + if is_private { + bail!( + "Gateway's external address is a private address: {}", + external_address + ); } - Err(()) -} -/// Removes the specified TCP and UDP port mappings. -pub fn remove_mappings(mappings: &EstablishedUPnPMappings, log: &slog::Logger) { - if mappings.is_some() { - debug!(log, "Removing UPnP port mappings"); - match igd_next::search_gateway(Default::default()) { - Ok(gateway) => { - if let Some(tcp_port) = mappings.tcp_port { - match gateway.remove_port(igd_next::PortMappingProtocol::TCP, tcp_port) { - Ok(()) => debug!(log, "UPnP Removed TCP port mapping"; "port" => tcp_port), - Err(e) => { - debug!(log, "UPnP Failed to remove TCP port mapping"; "port" => tcp_port, "error" => %e) - } - } - } - 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), - } + loop { + gateway + .add_port( + PortMappingProtocol::UDP, + port, + SocketAddr::new(IpAddr::V4(addr), port), + MAPPING_DURATION, + "Lighthouse Discovery port", + ) + .await + .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; } } diff --git a/beacon_node/network/src/service.rs b/beacon_node/network/src/service.rs index 01a7e1f98..0905f4a07 100644 --- a/beacon_node/network/src/service.rs +++ b/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 { /// 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 { store: Arc>, /// A collection of global variables, accessible outside of the network service. network_globals: Arc>, - /// 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>>, /// A delay that expires when we need to subscribe to a new fork's topics. @@ -237,22 +229,24 @@ impl NetworkService { "Backfill is disabled. DO NOT RUN IN PRODUCTION" ); - // try and construct UPnP port mappings if required. - if let Some(upnp_config) = crate::nat::UPnPConfig::from_config(config) { - let upnp_log = network_log.new(o!("service" => "UPnP")); - let upnp_network_send = network_senders.network_send(); - if config.upnp_enabled { - executor.spawn_blocking( - move || { - crate::nat::construct_upnp_mappings( - upnp_config, - upnp_network_send, - upnp_log, - ) - }, - "UPnP", - ); - } + if let (true, false, Some(v4)) = ( + config.upnp_enabled, + config.disable_discovery, + config.listen_addrs().v4(), + ) { + let nw = network_log.clone(); + let v4 = v4.clone(); + executor.spawn( + async move { + info!(nw, "UPnP Attempting to initialise routes"); + if let Err(e) = + nat::construct_upnp_mappings(v4.addr, v4.disc_port, nw.clone()).await + { + info!(nw, "Could not UPnP map Discovery port"; "error" => %e); + } + }, + "UPnP", + ); } // get a reference to the beacon chain store @@ -358,7 +352,6 @@ impl NetworkService { 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 NetworkService { } => { 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 Drop for NetworkService { "Saved DHT state"; ), } - - // attempt to remove port mappings - crate::nat::remove_mappings(&self.upnp_mappings, &self.log); - info!(self.log, "Network service shutdown"); } }