From c9f48f37cb6442dccdf7ba2f55a0d0ae78290ad2 Mon Sep 17 00:00:00 2001 From: Jack McPherson Date: Fri, 29 Sep 2023 06:39:03 +0000 Subject: [PATCH] Allow libp2p to determine listening addresses (#4700) ## Issue Addressed #4675 ## Proposed Changes - Update local ENR (**only port numbers**) with local addresses received from libp2p (via `SwarmEvent::NewListenAddr`) - Only use the zero port for CLI tests ## Additional Info ### See Also ### - #4705 - #4402 - #4745 --- beacon_node/lighthouse_network/src/config.rs | 13 ++- .../lighthouse_network/src/discovery/mod.rs | 109 +++++++++++++++--- lighthouse/tests/beacon_node.rs | 93 +++++++++------ 3 files changed, 158 insertions(+), 57 deletions(-) diff --git a/beacon_node/lighthouse_network/src/config.rs b/beacon_node/lighthouse_network/src/config.rs index c3f6b60b045..c5077448823 100644 --- a/beacon_node/lighthouse_network/src/config.rs +++ b/beacon_node/lighthouse_network/src/config.rs @@ -16,6 +16,11 @@ use std::sync::Arc; use std::time::Duration; use types::{ForkContext, ForkName}; +pub const DEFAULT_IPV4_ADDRESS: Ipv4Addr = Ipv4Addr::UNSPECIFIED; +pub const DEFAULT_TCP_PORT: u16 = 9000u16; +pub const DEFAULT_DISC_PORT: u16 = 9000u16; +pub const DEFAULT_QUIC_PORT: u16 = 9001u16; + /// The cache time is set to accommodate the circulation time of an attestation. /// /// The p2p spec declares that we accept attestations within the following range: @@ -304,10 +309,10 @@ impl Default for Config { .expect("The total rate limit has been specified"), ); let listen_addresses = ListenAddress::V4(ListenAddr { - addr: Ipv4Addr::UNSPECIFIED, - disc_port: 9000, - quic_port: 9001, - tcp_port: 9000, + addr: DEFAULT_IPV4_ADDRESS, + disc_port: DEFAULT_DISC_PORT, + quic_port: DEFAULT_QUIC_PORT, + tcp_port: DEFAULT_TCP_PORT, }); let discv5_listen_config = diff --git a/beacon_node/lighthouse_network/src/discovery/mod.rs b/beacon_node/lighthouse_network/src/discovery/mod.rs index 4d8807336bf..77fba905660 100644 --- a/beacon_node/lighthouse_network/src/discovery/mod.rs +++ b/beacon_node/lighthouse_network/src/discovery/mod.rs @@ -21,10 +21,11 @@ pub use libp2p::identity::{Keypair, PublicKey}; use enr::{ATTESTATION_BITFIELD_ENR_KEY, ETH2_ENR_KEY, SYNC_COMMITTEE_BITFIELD_ENR_KEY}; use futures::prelude::*; use futures::stream::FuturesUnordered; +use libp2p::multiaddr::Protocol; use libp2p::swarm::behaviour::{DialFailure, FromSwarm}; use libp2p::swarm::THandlerInEvent; pub use libp2p::{ - core::{ConnectedPoint, Multiaddr}, + core::{transport::ListenerId, ConnectedPoint, Multiaddr}, identity::PeerId, swarm::{ dummy::ConnectionHandler, ConnectionId, DialError, NetworkBehaviour, NotifyHandler, @@ -77,6 +78,19 @@ pub struct DiscoveredPeers { pub peers: HashMap>, } +/// Specifies which port numbers should be modified after start of the discovery service +#[derive(Debug)] +pub struct UpdatePorts { + /// TCP port associated wih IPv4 address (if present) + pub tcp4: bool, + /// TCP port associated wih IPv6 address (if present) + pub tcp6: bool, + /// QUIC port associated wih IPv4 address (if present) + pub quic4: bool, + /// QUIC port associated wih IPv6 address (if present) + pub quic6: bool, +} + #[derive(Clone, PartialEq)] struct SubnetQuery { subnet: Subnet, @@ -177,12 +191,8 @@ pub struct Discovery { /// always false. pub started: bool, - /// This keeps track of whether an external UDP port change should also indicate an internal - /// TCP port change. As we cannot detect our external TCP port, we assume that the external UDP - /// port is also our external TCP port. This assumption only holds if the user has not - /// explicitly set their ENR TCP port via the CLI config. The first indicates tcp4 and the - /// second indicates tcp6. - update_tcp_port: (bool, bool), + /// Specifies whether various port numbers should be updated after the discovery service has been started + update_ports: UpdatePorts, /// Logger for the discovery behaviour. log: slog::Logger, @@ -300,10 +310,12 @@ impl Discovery { } } - let update_tcp_port = ( - config.enr_tcp4_port.is_none(), - config.enr_tcp6_port.is_none(), - ); + let update_ports = UpdatePorts { + tcp4: config.enr_tcp4_port.is_none(), + tcp6: config.enr_tcp6_port.is_none(), + quic4: config.enr_quic4_port.is_none(), + quic6: config.enr_quic6_port.is_none(), + }; Ok(Self { cached_enrs: LruCache::new(50), @@ -314,7 +326,7 @@ impl Discovery { discv5, event_stream, started: !config.disable_discovery, - update_tcp_port, + update_ports, log, enr_dir, }) @@ -1006,8 +1018,8 @@ impl NetworkBehaviour for Discovery { // Discv5 will have updated our local ENR. We save the updated version // to disk. - if (self.update_tcp_port.0 && socket_addr.is_ipv4()) - || (self.update_tcp_port.1 && socket_addr.is_ipv6()) + if (self.update_ports.tcp4 && socket_addr.is_ipv4()) + || (self.update_ports.tcp6 && socket_addr.is_ipv6()) { // Update the TCP port in the ENR self.discv5.update_local_enr_socket(socket_addr, true); @@ -1036,12 +1048,79 @@ impl NetworkBehaviour for Discovery { FromSwarm::DialFailure(DialFailure { peer_id, error, .. }) => { self.on_dial_failure(peer_id, error) } + FromSwarm::NewListenAddr(ev) => { + let addr = ev.addr; + let listener_id = ev.listener_id; + + trace!(self.log, "Received NewListenAddr event from swarm"; "listener_id" => ?listener_id, "addr" => ?addr); + + let mut addr_iter = addr.iter(); + + let attempt_enr_update = match addr_iter.next() { + Some(Protocol::Ip4(_)) => match (addr_iter.next(), addr_iter.next()) { + (Some(Protocol::Tcp(port)), None) => { + if !self.update_ports.tcp4 { + debug!(self.log, "Skipping ENR update"; "multiaddr" => ?addr); + return; + } + + self.update_enr_tcp_port(port) + } + (Some(Protocol::Udp(port)), Some(Protocol::QuicV1)) => { + if !self.update_ports.quic4 { + debug!(self.log, "Skipping ENR update"; "multiaddr" => ?addr); + return; + } + + self.update_enr_quic_port(port) + } + _ => { + debug!(self.log, "Encountered unacceptable multiaddr for listening (unsupported transport)"; "addr" => ?addr); + return; + } + }, + Some(Protocol::Ip6(_)) => match (addr_iter.next(), addr_iter.next()) { + (Some(Protocol::Tcp(port)), None) => { + if !self.update_ports.tcp6 { + debug!(self.log, "Skipping ENR update"; "multiaddr" => ?addr); + return; + } + + self.update_enr_tcp_port(port) + } + (Some(Protocol::Udp(port)), Some(Protocol::QuicV1)) => { + if !self.update_ports.quic6 { + debug!(self.log, "Skipping ENR update"; "multiaddr" => ?addr); + return; + } + + self.update_enr_quic_port(port) + } + _ => { + debug!(self.log, "Encountered unacceptable multiaddr for listening (unsupported transport)"; "addr" => ?addr); + return; + } + }, + _ => { + debug!(self.log, "Encountered unacceptable multiaddr for listening (no IP)"; "addr" => ?addr); + return; + } + }; + + let local_enr: Enr = self.discv5.local_enr(); + + match attempt_enr_update { + Ok(_) => { + info!(self.log, "Updated local ENR"; "enr" => local_enr.to_base64(), "seq" => local_enr.seq(), "id"=> %local_enr.node_id(), "ip4" => ?local_enr.ip4(), "udp4"=> ?local_enr.udp4(), "tcp4" => ?local_enr.tcp4(), "tcp6" => ?local_enr.tcp6(), "udp6" => ?local_enr.udp6()) + } + Err(e) => warn!(self.log, "Failed to update ENR"; "error" => ?e), + } + } FromSwarm::ConnectionEstablished(_) | FromSwarm::ConnectionClosed(_) | FromSwarm::AddressChange(_) | FromSwarm::ListenFailure(_) | FromSwarm::NewListener(_) - | FromSwarm::NewListenAddr(_) | FromSwarm::ExpiredListenAddr(_) | FromSwarm::ListenerError(_) | FromSwarm::ListenerClosed(_) diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index bc6b6284e5a..4140a3f6b42 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -22,9 +22,14 @@ use types::{ Address, Checkpoint, Epoch, ExecutionBlockHash, ForkName, Hash256, MainnetEthSpec, ProgressiveBalancesMode, }; -use unused_port::{unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port}; const DEFAULT_ETH1_ENDPOINT: &str = "http://localhost:8545/"; +const DUMMY_ENR_TCP_PORT: u16 = 7777; +const DUMMY_ENR_UDP_PORT: u16 = 8888; +const DUMMY_ENR_QUIC_PORT: u16 = 9999; + +const _: () = + assert!(DUMMY_ENR_QUIC_PORT != 0 && DUMMY_ENR_TCP_PORT != 0 && DUMMY_ENR_UDP_PORT != 0); /// Returns the `lighthouse beacon_node` command. fn base_cmd() -> Command { @@ -1004,7 +1009,7 @@ fn network_listen_address_flag_wrong_double_v6_value_config() { } #[test] fn network_port_flag_over_ipv4() { - let port = unused_tcp4_port().expect("Unable to find unused port."); + let port = 0; CommandLineTest::new() .flag("port", Some(port.to_string().as_str())) .run() @@ -1021,7 +1026,7 @@ fn network_port_flag_over_ipv4() { } #[test] fn network_port_flag_over_ipv6() { - let port = unused_tcp6_port().expect("Unable to find unused port."); + let port = 0; CommandLineTest::new() .flag("listen-address", Some("::1")) .flag("port", Some(port.to_string().as_str())) @@ -1039,8 +1044,8 @@ fn network_port_flag_over_ipv6() { } #[test] fn network_port_and_discovery_port_flags_over_ipv4() { - let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); - let disc4_port = unused_udp4_port().expect("Unable to find unused port."); + let tcp4_port = 0; + let disc4_port = 0; CommandLineTest::new() .flag("port", Some(tcp4_port.to_string().as_str())) .flag("discovery-port", Some(disc4_port.to_string().as_str())) @@ -1058,8 +1063,8 @@ fn network_port_and_discovery_port_flags_over_ipv4() { } #[test] fn network_port_and_discovery_port_flags_over_ipv6() { - let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); - let disc6_port = unused_udp6_port().expect("Unable to find unused port."); + let tcp6_port = 0; + let disc6_port = 0; CommandLineTest::new() .flag("listen-address", Some("::1")) .flag("port", Some(tcp6_port.to_string().as_str())) @@ -1078,10 +1083,10 @@ fn network_port_and_discovery_port_flags_over_ipv6() { } #[test] fn network_port_and_discovery_port_flags_over_ipv4_and_ipv6() { - let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); - let disc4_port = unused_udp4_port().expect("Unable to find unused port."); - let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); - let disc6_port = unused_udp6_port().expect("Unable to find unused port."); + let tcp4_port = 0; + let disc4_port = 0; + let tcp6_port = 0; + let disc6_port = 0; CommandLineTest::new() .flag("listen-address", Some("::1")) .flag("listen-address", Some("127.0.0.1")) @@ -1113,12 +1118,12 @@ fn network_port_and_discovery_port_flags_over_ipv4_and_ipv6() { #[test] fn network_port_discovery_quic_port_flags_over_ipv4_and_ipv6() { - let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); - let disc4_port = unused_udp4_port().expect("Unable to find unused port."); - let quic4_port = unused_udp4_port().expect("Unable to find unused port."); - let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); - let disc6_port = unused_udp6_port().expect("Unable to find unused port."); - let quic6_port = unused_udp6_port().expect("Unable to find unused port."); + let tcp4_port = 0; + let disc4_port = 0; + let quic4_port = 0; + let tcp6_port = 0; + let disc6_port = 0; + let quic6_port = 0; CommandLineTest::new() .flag("listen-address", Some("::1")) .flag("listen-address", Some("127.0.0.1")) @@ -1264,7 +1269,8 @@ fn network_load_flag() { // Tests for ENR flags. #[test] fn enr_udp_port_flag() { - let port = unused_udp4_port().expect("Unable to find unused port."); + let port = DUMMY_ENR_UDP_PORT; + assert!(port != 0); CommandLineTest::new() .flag("enr-udp-port", Some(port.to_string().as_str())) .run_with_zero_port() @@ -1272,7 +1278,7 @@ fn enr_udp_port_flag() { } #[test] fn enr_quic_port_flag() { - let port = unused_udp4_port().expect("Unable to find unused port."); + let port = DUMMY_ENR_QUIC_PORT; CommandLineTest::new() .flag("enr-quic-port", Some(port.to_string().as_str())) .run_with_zero_port() @@ -1280,7 +1286,7 @@ fn enr_quic_port_flag() { } #[test] fn enr_tcp_port_flag() { - let port = unused_tcp4_port().expect("Unable to find unused port."); + let port = DUMMY_ENR_TCP_PORT; CommandLineTest::new() .flag("enr-tcp-port", Some(port.to_string().as_str())) .run_with_zero_port() @@ -1288,7 +1294,7 @@ fn enr_tcp_port_flag() { } #[test] fn enr_udp6_port_flag() { - let port = unused_udp6_port().expect("Unable to find unused port."); + let port = DUMMY_ENR_UDP_PORT; CommandLineTest::new() .flag("enr-udp6-port", Some(port.to_string().as_str())) .run_with_zero_port() @@ -1296,7 +1302,7 @@ fn enr_udp6_port_flag() { } #[test] fn enr_quic6_port_flag() { - let port = unused_udp6_port().expect("Unable to find unused port."); + let port = DUMMY_ENR_QUIC_PORT; CommandLineTest::new() .flag("enr-quic6-port", Some(port.to_string().as_str())) .run_with_zero_port() @@ -1304,7 +1310,7 @@ fn enr_quic6_port_flag() { } #[test] fn enr_tcp6_port_flag() { - let port = unused_tcp6_port().expect("Unable to find unused port."); + let port = DUMMY_ENR_TCP_PORT; CommandLineTest::new() .flag("enr-tcp6-port", Some(port.to_string().as_str())) .run_with_zero_port() @@ -1313,8 +1319,11 @@ fn enr_tcp6_port_flag() { #[test] fn enr_match_flag_over_ipv4() { let addr = "127.0.0.2".parse::().unwrap(); - let udp4_port = unused_udp4_port().expect("Unable to find unused port."); - let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); + + // the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports). + let udp4_port = DUMMY_ENR_UDP_PORT; + let tcp4_port = DUMMY_ENR_TCP_PORT; + CommandLineTest::new() .flag("enr-match", None) .flag("listen-address", Some("127.0.0.2")) @@ -1338,8 +1347,11 @@ fn enr_match_flag_over_ipv4() { fn enr_match_flag_over_ipv6() { const ADDR: &str = "::1"; let addr = ADDR.parse::().unwrap(); - let udp6_port = unused_udp6_port().expect("Unable to find unused port."); - let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); + + // the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports). + let udp6_port = DUMMY_ENR_UDP_PORT; + let tcp6_port = DUMMY_ENR_TCP_PORT; + CommandLineTest::new() .flag("enr-match", None) .flag("listen-address", Some(ADDR)) @@ -1362,13 +1374,18 @@ fn enr_match_flag_over_ipv6() { #[test] fn enr_match_flag_over_ipv4_and_ipv6() { const IPV6_ADDR: &str = "::1"; + + // the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports). + let udp6_port = DUMMY_ENR_UDP_PORT; + let tcp6_port = DUMMY_ENR_TCP_PORT; let ipv6_addr = IPV6_ADDR.parse::().unwrap(); - let udp6_port = unused_udp6_port().expect("Unable to find unused port."); - let tcp6_port = unused_tcp6_port().expect("Unable to find unused port."); + const IPV4_ADDR: &str = "127.0.0.1"; + // the reason we use the ENR dummy values is because, due to the nature of the `--enr-match` flag, these will eventually become ENR ports (as well as listening ports). + let udp4_port = DUMMY_ENR_UDP_PORT; + let tcp4_port = DUMMY_ENR_TCP_PORT; let ipv4_addr = IPV4_ADDR.parse::().unwrap(); - let udp4_port = unused_udp4_port().expect("Unable to find unused port."); - let tcp4_port = unused_tcp4_port().expect("Unable to find unused port."); + CommandLineTest::new() .flag("enr-match", None) .flag("listen-address", Some(IPV4_ADDR)) @@ -1406,7 +1423,7 @@ fn enr_match_flag_over_ipv4_and_ipv6() { #[test] fn enr_address_flag_with_ipv4() { let addr = "192.167.1.1".parse::().unwrap(); - let port = unused_udp4_port().expect("Unable to find unused port."); + let port = DUMMY_ENR_UDP_PORT; CommandLineTest::new() .flag("enr-address", Some("192.167.1.1")) .flag("enr-udp-port", Some(port.to_string().as_str())) @@ -1419,7 +1436,7 @@ fn enr_address_flag_with_ipv4() { #[test] fn enr_address_flag_with_ipv6() { let addr = "192.167.1.1".parse::().unwrap(); - let port = unused_udp4_port().expect("Unable to find unused port."); + let port = DUMMY_ENR_UDP_PORT; CommandLineTest::new() .flag("enr-address", Some("192.167.1.1")) .flag("enr-udp-port", Some(port.to_string().as_str())) @@ -1433,7 +1450,7 @@ fn enr_address_flag_with_ipv6() { fn enr_address_dns_flag() { let addr = Ipv4Addr::LOCALHOST; let ipv6addr = Ipv6Addr::LOCALHOST; - let port = unused_udp4_port().expect("Unable to find unused port."); + let port = DUMMY_ENR_UDP_PORT; CommandLineTest::new() .flag("enr-address", Some("localhost")) .flag("enr-udp-port", Some(port.to_string().as_str())) @@ -1482,8 +1499,8 @@ fn http_address_ipv6_flag() { } #[test] fn http_port_flag() { - let port1 = unused_tcp4_port().expect("Unable to find unused port."); - let port2 = unused_tcp4_port().expect("Unable to find unused port."); + let port1 = 0; + let port2 = 0; CommandLineTest::new() .flag("http", None) .flag("http-port", Some(port1.to_string().as_str())) @@ -1639,8 +1656,8 @@ fn metrics_address_ipv6_flag() { } #[test] fn metrics_port_flag() { - let port1 = unused_tcp4_port().expect("Unable to find unused port."); - let port2 = unused_tcp4_port().expect("Unable to find unused port."); + let port1 = 0; + let port2 = 0; CommandLineTest::new() .flag("metrics", None) .flag("metrics-port", Some(port1.to_string().as_str()))