From 410457a7e6ab598e96780db2b5bab28d2a66d9aa Mon Sep 17 00:00:00 2001 From: Age Manning Date: Mon, 13 Jan 2025 15:44:59 +1100 Subject: [PATCH 1/9] Fix some small annoyances --- beacon_node/lighthouse_network/src/service/mod.rs | 5 +++++ beacon_node/src/cli.rs | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index afcbfce1732..0df07544e4a 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -197,6 +197,11 @@ impl Network { &ctx.chain_spec, )?; + // Pre-initializes metrics + // This is is an initial value to specify NAT ports start closed until proven otherwise. + set_gauge_vec(&NAT_OPEN, &["libp2p_ipv4"], 0), + set_gauge_vec(&NAT_OPEN, &["libp2p_ipv6"], 0), + // Construct the metadata let custody_subnet_count = ctx.chain_spec.is_peer_das_scheduled().then(|| { if config.subscribe_all_data_column_subnets { diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index cecfcee868f..125b5daa0ce 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -185,8 +185,8 @@ pub fn cli_app() -> Command { .long("port6") .value_name("PORT") .help("The TCP/UDP ports to listen on over IPv6 when listening over both IPv4 and \ - IPv6. Defaults to 9090 when required. The Quic UDP port will be set to this value + 1.") - .default_value("9090") + IPv6. Defaults to 9000. The Quic UDP port will be set to this value + 1.") + .default_value("9000") .action(ArgAction::Set) .display_order(0) ) From 86bf069f52dee8304e89212078a6e9f6bc55e46c Mon Sep 17 00:00:00 2001 From: Age Manning Date: Mon, 13 Jan 2025 15:56:21 +1100 Subject: [PATCH 2/9] Fix port6 CLI --- beacon_node/lighthouse_network/src/service/mod.rs | 4 ++-- beacon_node/src/config.rs | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index 0df07544e4a..ad8c498d0f0 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -199,8 +199,8 @@ impl Network { // Pre-initializes metrics // This is is an initial value to specify NAT ports start closed until proven otherwise. - set_gauge_vec(&NAT_OPEN, &["libp2p_ipv4"], 0), - set_gauge_vec(&NAT_OPEN, &["libp2p_ipv6"], 0), + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 0); + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 0); // Construct the metadata let custody_subnet_count = ctx.chain_spec.is_peer_das_scheduled().then(|| { diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 8d8a44a6fd6..9c2febe7f03 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -934,10 +934,9 @@ pub fn parse_listening_addresses( .map_err(|parse_error| format!("Failed to parse --port as an integer: {parse_error}"))?; let port6 = cli_args .get_one::("port6") - .map(|s| str::parse::(s)) - .transpose() - .map_err(|parse_error| format!("Failed to parse --port6 as an integer: {parse_error}"))? - .unwrap_or(9090); + .expect("--port6 has a default value") + .parse::() + .map_err(|parse_error| format!("Failed to parse --port6 as an integer: {parse_error}"))?; // parse the possible discovery ports. let maybe_disc_port = cli_args @@ -1070,7 +1069,7 @@ pub fn parse_listening_addresses( ipv4_tcp_port + 1 }); - // Defaults to 9090 when required + // Defaults to 9000 when required let ipv6_tcp_port = use_zero_ports .then(unused_port::unused_tcp6_port) .transpose()? From 2f0e4d651d56bacea5070b5cca41e4d98a08527c Mon Sep 17 00:00:00 2001 From: Age Manning Date: Mon, 13 Jan 2025 17:12:15 +1100 Subject: [PATCH 3/9] Report NAT when its not functioning --- .../src/peer_manager/mod.rs | 32 +++++++++++++++++++ .../lighthouse_network/src/service/mod.rs | 5 --- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 4df2566dacb..5d251485a90 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -21,6 +21,7 @@ use types::{DataColumnSubnetId, EthSpec, SyncSubnetId}; pub use libp2p::core::Multiaddr; pub use libp2p::identity::Keypair; +use libp2p::multiaddr::Protocol as MProtocol; pub mod peerdb; @@ -1306,6 +1307,8 @@ impl PeerManager { // Update peer count related metrics. fn update_peer_count_metrics(&self) { let mut peers_connected = 0; + let mut inbound_ipv4_peers_connected = 0; + let mut inbound_ipv6_peers_connected = 0; let mut clients_per_peer = HashMap::new(); let mut peers_connected_mutli: HashMap<(&str, &str), i32> = HashMap::new(); let mut peers_per_custody_subnet_count: HashMap = HashMap::new(); @@ -1322,6 +1325,27 @@ impl PeerManager { Some(ConnectionDirection::Outgoing) => "outbound", None => "none", }; + + // Add NAT statistics for inbound peers. + if matches!( + peer_info.connection_direction(), + Some(ConnectionDirection::Incoming) + ) { + // Helper closure to simplify the code + let multiaddr_is_ipv6 = |multiaddr: &Multiaddr| { + matches!(multiaddr.iter().next(), Some(MProtocol::Ip6(_))) + }; + + if peer_info + .seen_multiaddrs() + .any(|addr| multiaddr_is_ipv6(addr)) + { + inbound_ipv6_peers_connected += 1; + } else { + inbound_ipv4_peers_connected += 1; + } + } + // Note: the `transport` is set to `unknown` if the `listening_addresses` list is empty. // This situation occurs when the peer is initially registered in PeerDB, but the peer // info has not yet been updated at `PeerManager::identify`. @@ -1347,6 +1371,14 @@ impl PeerManager { } } + // If we have no inbound peers, we consider the NAT closed. + if inbound_ipv4_peers_connected < 1 { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 0); + } + if inbound_ipv6_peers_connected < 1 { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 0); + } + // PEERS_CONNECTED metrics::set_gauge(&metrics::PEERS_CONNECTED, peers_connected); diff --git a/beacon_node/lighthouse_network/src/service/mod.rs b/beacon_node/lighthouse_network/src/service/mod.rs index ad8c498d0f0..afcbfce1732 100644 --- a/beacon_node/lighthouse_network/src/service/mod.rs +++ b/beacon_node/lighthouse_network/src/service/mod.rs @@ -197,11 +197,6 @@ impl Network { &ctx.chain_spec, )?; - // Pre-initializes metrics - // This is is an initial value to specify NAT ports start closed until proven otherwise. - metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 0); - metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 0); - // Construct the metadata let custody_subnet_count = ctx.chain_spec.is_peer_das_scheduled().then(|| { if config.subscribe_all_data_column_subnets { From b2fc472dc6341c93c0e6ff58c96080d83ce7f8b0 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Tue, 14 Jan 2025 14:00:32 +1100 Subject: [PATCH 4/9] Fix lint --- beacon_node/lighthouse_network/src/peer_manager/mod.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 5d251485a90..889ee365ba7 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -1336,10 +1336,7 @@ impl PeerManager { matches!(multiaddr.iter().next(), Some(MProtocol::Ip6(_))) }; - if peer_info - .seen_multiaddrs() - .any(|addr| multiaddr_is_ipv6(addr)) - { + if peer_info.seen_multiaddrs().any(multiaddr_is_ipv6) { inbound_ipv6_peers_connected += 1; } else { inbound_ipv4_peers_connected += 1; From 78f9c3d634d6d5fff13bd55b9b25651c9460c55d Mon Sep 17 00:00:00 2001 From: Age Manning Date: Tue, 14 Jan 2025 14:43:52 +1100 Subject: [PATCH 5/9] Update CLI docs --- book/src/help_bn.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/book/src/help_bn.md b/book/src/help_bn.md index a4ab44748c3..6541fea48ee 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -298,8 +298,8 @@ Options: [default: 9000] --port6 The TCP/UDP ports to listen on over IPv6 when listening over both IPv4 - and IPv6. Defaults to 9090 when required. The Quic UDP port will be - set to this value + 1. [default: 9090] + and IPv6. Defaults to 9000. The Quic UDP port will be set to this + value + 1. [default: 9000] --prepare-payload-lookahead The time before the start of a proposal slot at which payload attributes should be sent. Low values are useful for execution nodes From c742405f63c7b3a613226fafacc6139ece073ebc Mon Sep 17 00:00:00 2001 From: Age Manning Date: Tue, 14 Jan 2025 14:57:23 +1100 Subject: [PATCH 6/9] Ipv6 port to default to ipv4 port when used --- beacon_node/src/cli.rs | 3 +-- beacon_node/src/config.rs | 21 ++++++++++++++++++--- book/src/help_bn.md | 4 ++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 125b5daa0ce..fc67aff6187 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -185,8 +185,7 @@ pub fn cli_app() -> Command { .long("port6") .value_name("PORT") .help("The TCP/UDP ports to listen on over IPv6 when listening over both IPv4 and \ - IPv6. Defaults to 9000. The Quic UDP port will be set to this value + 1.") - .default_value("9000") + IPv6. Defaults to --port. The Quic UDP port will be set to this value + 1.") .action(ArgAction::Set) .display_order(0) ) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 9c2febe7f03..ec0f5689038 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -932,10 +932,10 @@ pub fn parse_listening_addresses( .expect("--port has a default value") .parse::() .map_err(|parse_error| format!("Failed to parse --port as an integer: {parse_error}"))?; - let port6 = cli_args + let maybe_port6 = cli_args .get_one::("port6") - .expect("--port6 has a default value") - .parse::() + .map(|s| str::parse::(s)) + .transpose() .map_err(|parse_error| format!("Failed to parse --port6 as an integer: {parse_error}"))?; // parse the possible discovery ports. @@ -984,6 +984,14 @@ pub fn parse_listening_addresses( warn!(log, "When listening only over IPv6, use the --port flag. The value of --port6 will be ignored."); } + // If we are only listening on ipv6 and the user has specified --port6, lets just use + // that. + let port = if let Some(port6) = maybe_port6 { + port6 + } else { + port + }; + // use zero ports if required. If not, use the given port. let tcp_port = use_zero_ports .then(unused_port::unused_tcp6_port) @@ -1050,6 +1058,13 @@ pub fn parse_listening_addresses( }) } (Some(ipv4), Some(ipv6)) => { + // If --port6 is not set, we use --port + let port6 = if let Some(port6) = maybe_port6 { + port6 + } else { + port + }; + let ipv4_tcp_port = use_zero_ports .then(unused_port::unused_tcp4_port) .transpose()? diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 6541fea48ee..adc609340f9 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -298,8 +298,8 @@ Options: [default: 9000] --port6 The TCP/UDP ports to listen on over IPv6 when listening over both IPv4 - and IPv6. Defaults to 9000. The Quic UDP port will be set to this - value + 1. [default: 9000] + and IPv6. Defaults to --port. The Quic UDP port will be set to this + value + 1. --prepare-payload-lookahead The time before the start of a proposal slot at which payload attributes should be sent. Low values are useful for execution nodes From 1faf0e79b16e034125f99be32993c43db36f5c9c Mon Sep 17 00:00:00 2001 From: Tan Chee Keong Date: Wed, 15 Jan 2025 11:22:41 +0800 Subject: [PATCH 7/9] Update as per #6796 --- book/src/advanced_networking.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/book/src/advanced_networking.md b/book/src/advanced_networking.md index c0f6b5485ef..0dc1000aa04 100644 --- a/book/src/advanced_networking.md +++ b/book/src/advanced_networking.md @@ -162,8 +162,8 @@ To listen over both IPv4 and IPv6: > > **IPv6**: > -> It listens on the default value of --port6 (`9090`) for both UDP and TCP. -> QUIC will use port `9091` for UDP, which is the default `--port6` value (`9090`) + 1. +> It listens on the default value of --port6 (`9000`) for both UDP and TCP. +> QUIC will use port `9001` for UDP, which is the default `--port6` value (`9000`) + 1. > When using `--listen-address :: --listen-address --port 9909 --discovery-port6 9999`, listening will be set up as follows: > @@ -174,8 +174,8 @@ To listen over both IPv4 and IPv6: > > **IPv6**: > -> It listens on the default value of `--port6` (`9090`) for TCP, and port `9999` for UDP. -> QUIC will use port `9091` for UDP, which is the default `--port6` value (`9090`) + 1. +> It listens on the default value of `--port6` (`9000`) for TCP, and port `9999` for UDP. +> QUIC will use port `9001` for UDP, which is the default `--port6` value (`9000`) + 1. ### Configuring Lighthouse to advertise IPv6 reachable addresses From feac9196382acf4357c13d344924ffb7ba9203ac Mon Sep 17 00:00:00 2001 From: Age Manning Date: Thu, 16 Jan 2025 12:51:52 +1100 Subject: [PATCH 8/9] Prevent duplication of NAT changes --- .../src/peer_manager/mod.rs | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 11668ad22b7..07c4be7959b 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -21,7 +21,6 @@ use types::{DataColumnSubnetId, EthSpec, SyncSubnetId}; pub use libp2p::core::Multiaddr; pub use libp2p::identity::Keypair; -use libp2p::multiaddr::Protocol as MProtocol; pub mod peerdb; @@ -1347,8 +1346,6 @@ impl PeerManager { // Update peer count related metrics. fn update_peer_count_metrics(&self) { let mut peers_connected = 0; - let mut inbound_ipv4_peers_connected = 0; - let mut inbound_ipv6_peers_connected = 0; let mut clients_per_peer = HashMap::new(); let mut inbound_ipv4_peers_connected: usize = 0; let mut inbound_ipv6_peers_connected: usize = 0; @@ -1367,24 +1364,6 @@ impl PeerManager { Some(ConnectionDirection::Outgoing) => "outbound", None => "none", }; - - // Add NAT statistics for inbound peers. - if matches!( - peer_info.connection_direction(), - Some(ConnectionDirection::Incoming) - ) { - // Helper closure to simplify the code - let multiaddr_is_ipv6 = |multiaddr: &Multiaddr| { - matches!(multiaddr.iter().next(), Some(MProtocol::Ip6(_))) - }; - - if peer_info.seen_multiaddrs().any(multiaddr_is_ipv6) { - inbound_ipv6_peers_connected += 1; - } else { - inbound_ipv4_peers_connected += 1; - } - } - // Note: the `transport` is set to `unknown` if the `listening_addresses` list is empty. // This situation occurs when the peer is initially registered in PeerDB, but the peer // info has not yet been updated at `PeerManager::identify`. @@ -1433,14 +1412,6 @@ impl PeerManager { metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 0); } - // If we have no inbound peers, we consider the NAT closed. - if inbound_ipv4_peers_connected < 1 { - metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 0); - } - if inbound_ipv6_peers_connected < 1 { - metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 0); - } - // PEERS_CONNECTED metrics::set_gauge(&metrics::PEERS_CONNECTED, peers_connected); From 0d2e6319ccd064553aefd626d8aea7b9cb2cdb71 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Mon, 3 Feb 2025 16:21:51 +1100 Subject: [PATCH 9/9] Simplify code as per reviewers suggestions --- beacon_node/src/config.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 86505ba5e43..0f8f3a8012a 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -990,11 +990,7 @@ pub fn parse_listening_addresses( // If we are only listening on ipv6 and the user has specified --port6, lets just use // that. - let port = if let Some(port6) = maybe_port6 { - port6 - } else { - port - }; + let port = maybe_port6.unwrap_or(port); // use zero ports if required. If not, use the given port. let tcp_port = use_zero_ports @@ -1063,11 +1059,7 @@ pub fn parse_listening_addresses( } (Some(ipv4), Some(ipv6)) => { // If --port6 is not set, we use --port - let port6 = if let Some(port6) = maybe_port6 { - port6 - } else { - port - }; + let port6 = maybe_port6.unwrap_or(port); let ipv4_tcp_port = use_zero_ports .then(unused_port::unused_tcp4_port)