From 1e3048c6ba5b6c1b75133b8adb7574b17001ee2a Mon Sep 17 00:00:00 2001 From: Josh King Date: Mon, 13 Jan 2025 23:42:48 +1100 Subject: [PATCH 01/10] implement update_nat_open function in network_behaviour for tracking incoming peers below a given threshold count --- .../src/peer_manager/mod.rs | 3 ++ .../src/peer_manager/network_behaviour.rs | 54 ++++++++++++++++--- .../src/peer_manager/peerdb.rs | 16 ++++++ .../src/peer_manager/peerdb/peer_info.rs | 20 +++++++ .../lighthouse_network/src/types/globals.rs | 15 ++++++ 5 files changed, 100 insertions(+), 8 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..5fde1ffeb2a 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -107,6 +107,8 @@ pub struct PeerManager { metrics_enabled: bool, /// Keeps track of whether the QUIC protocol is enabled or not. quic_enabled: bool, + /// Threshold of inbound peers to consider libp2p NAT open. + libp2p_nat_open_threshold: usize, /// The logger associated with the `PeerManager`. log: slog::Logger, } @@ -172,6 +174,7 @@ impl PeerManager { discovery_enabled, metrics_enabled, quic_enabled, + libp2p_nat_open_threshold: 3, // Default to 3 inbound peers to consider NAT open log: log.clone(), }) } diff --git a/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs b/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs index 11676f9a01f..9ce96f40abe 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs @@ -209,13 +209,13 @@ impl NetworkBehaviour for PeerManager { )); } - // We have an inbound connection, this is indicative of having our libp2p NAT ports open. We - // distinguish between ipv4 and ipv6 here: - match remote_addr.iter().next() { - Some(Protocol::Ip4(_)) => set_gauge_vec(&NAT_OPEN, &["libp2p_ipv4"], 1), - Some(Protocol::Ip6(_)) => set_gauge_vec(&NAT_OPEN, &["libp2p_ipv6"], 1), - _ => {} - } + // We have an inbound connection, this is indicative of having our libp2p NAT ports open. + // We now need to check if we have enough open connections to consider our NAT open. + // This function distinguishes between ipv4 and ipv6. + self.update_nat_open_metric(&ConnectedPoint::Listener { + send_back_addr: remote_addr.clone(), + local_addr: _local_addr.clone(), + }); Ok(ConnectionHandler) } @@ -290,9 +290,10 @@ impl PeerManager { fn on_connection_closed( &mut self, peer_id: PeerId, - _endpoint: &ConnectedPoint, + endpoint: &ConnectedPoint, remaining_established: usize, ) { + self.update_nat_open_metric(endpoint); if remaining_established > 0 { return; } @@ -326,6 +327,43 @@ impl PeerManager { self.update_peer_count_metrics(); } } + fn update_nat_open_metric(&mut self, endpoint: &ConnectedPoint) { + if let ConnectedPoint::Listener { send_back_addr, .. } = endpoint { + if let Some(addr) = send_back_addr.iter().next() { + match addr { + Protocol::Ip4(_) => { + if self + .network_globals + .peers + .read() + .connected_incoming_ipv4_peers() + .count() + <= self.libp2p_nat_open_threshold + { + set_gauge_vec(&NAT_OPEN, &["libp2p_ipv4"], 0); + } else { + set_gauge_vec(&NAT_OPEN, &["libp2p_ipv4"], 1); + } + } + Protocol::Ip6(_) => { + if self + .network_globals + .peers + .read() + .connected_incoming_ipv6_peers() + .count() + <= self.libp2p_nat_open_threshold + { + set_gauge_vec(&NAT_OPEN, &["libp2p_ipv6"], 0); + } else { + set_gauge_vec(&NAT_OPEN, &["libp2p_ipv6"], 1); + } + } + _ => {} + } + } + } + } /// A dial attempt has failed. /// diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs index d2effd4d037..8fc9538dfad 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs @@ -183,6 +183,22 @@ impl PeerDB { self.peers.iter().filter(|(_, info)| info.is_connected()) } + /// Returns an iterator of all connected peers with an incoming ipv4 connection. + pub fn connected_incoming_ipv4_peers(&self) -> impl Iterator + '_ { + self.peers + .iter() + .filter(|(_, info)| info.is_connected() && info.is_incoming_ipv4_connection()) + .map(|(peer_id, _)| *peer_id) + } + + /// Returns an iterator of all connected peers with an incoming ipv6 connection. + pub fn connected_incoming_ipv6_peers(&self) -> impl Iterator + '_ { + self.peers + .iter() + .filter(|(_, info)| info.is_connected() && info.is_incoming_ipv6_connection()) + .map(|(peer_id, _)| *peer_id) + } + /// Gives the ids of all known connected peers. pub fn connected_peer_ids(&self) -> impl Iterator { self.peers diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs index ee8c27f474c..7c77b9c6ad7 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs @@ -122,6 +122,26 @@ impl PeerInfo { self.connection_direction.as_ref() } + /// Returns true if this is an incoming ipv4 connection. + pub fn is_incoming_ipv4_connection(&self) -> bool { + self.seen_multiaddrs.iter().any(|multiaddr| { + multiaddr.iter().any(|protocol| match protocol { + libp2p::core::multiaddr::Protocol::Ip4(_) => true, + _ => false, + }) + }) + } + + /// Returns true if this is an incoming ipv6 connection. + pub fn is_incoming_ipv6_connection(&self) -> bool { + self.seen_multiaddrs.iter().any(|multiaddr| { + multiaddr.iter().any(|protocol| match protocol { + libp2p::core::multiaddr::Protocol::Ip6(_) => true, + _ => false, + }) + }) + } + /// Returns the sync status of the peer. pub fn sync_status(&self) -> &SyncStatus { &self.sync_status diff --git a/beacon_node/lighthouse_network/src/types/globals.rs b/beacon_node/lighthouse_network/src/types/globals.rs index 92583b7b5d0..2acc0b71a7c 100644 --- a/beacon_node/lighthouse_network/src/types/globals.rs +++ b/beacon_node/lighthouse_network/src/types/globals.rs @@ -33,6 +33,9 @@ pub struct NetworkGlobals { pub config: Arc, /// Ethereum chain configuration. Immutable after initialization. pub spec: Arc, + /// Counts the number of connected peers. + pub connected_ipv4_peers: RwLock, + pub connected_ipv6_peers: RwLock, } impl NetworkGlobals { @@ -85,6 +88,8 @@ impl NetworkGlobals { backfill_state: RwLock::new(BackFillState::Paused), sampling_subnets, sampling_columns, + connected_ipv4_peers: RwLock::new(0), + connected_ipv6_peers: RwLock::new(0), config, spec, } @@ -121,6 +126,16 @@ impl NetworkGlobals { self.peers.read().connected_or_dialing_peers().count() } + /// Returns the number of connected peers with an incoming ipv4 connection. + pub fn connected_incoming_ipv4_peers(&self) -> usize { + *self.connected_ipv4_peers.read() + } + + /// Returns the number of connected peers with an incoming ipv6 connection. + pub fn connected_incoming_ipv6_peers(&self) -> usize { + *self.connected_ipv6_peers.read() + } + /// Returns in the node is syncing. pub fn is_syncing(&self) -> bool { self.sync_state.read().is_syncing() From 13e56514461df36bf075a14bc940f2a8124af049 Mon Sep 17 00:00:00 2001 From: Josh King Date: Mon, 13 Jan 2025 23:46:08 +1100 Subject: [PATCH 02/10] implement update_nat_open function in network_behaviour for tracking incoming peers below a given threshold count --- d fjals | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 d fjals diff --git a/d fjals b/d fjals new file mode 100644 index 00000000000..8c09d13b883 --- /dev/null +++ b/d fjals @@ -0,0 +1,113 @@ + dynamic_nat_detection +* origin/unstable + remove-IDONTWANT-promise + stable + unstable + remotes/network/HEAD -> network/stable + remotes/network/anchor + remotes/network/beacon-api-electra + remotes/network/book-security + remotes/network/builder-sse-fixes + remotes/network/component-search-loop + remotes/network/crc + remotes/network/cut-v4.2.0 + remotes/network/cut-v4.4.0 + remotes/network/dapplion-patch-1 + remotes/network/das + remotes/network/debug-jwt-auth + remotes/network/deneb-devnet-7 + remotes/network/deneb-free-blobs + remotes/network/devnet-10 + remotes/network/dont-ban-ips + remotes/network/drop-headtracker + remotes/network/drop-naughty-peer + remotes/network/ef-tests-electra + remotes/network/eip-7732 + remotes/network/eip4844 + remotes/network/eip4844-devnet-v3 + remotes/network/eip6110 + remotes/network/electra-alpha10 + remotes/network/electra-devnet-5 + remotes/network/electra-engine-api + remotes/network/electra-focil + remotes/network/electra-max-eb + remotes/network/electra_attestation_changes + remotes/network/electra_attestation_changes_borked + remotes/network/electra_devnet_0 + remotes/network/fix-release-ci + remotes/network/fix-rpc-disconnects + remotes/network/fix_block_range_math + remotes/network/freezer-tools + remotes/network/hotfix-exit-verification + remotes/network/import-blobs + remotes/network/ipv6-auto-enr + remotes/network/leveldb-1.23 + remotes/network/merge-stable + remotes/network/monitor-ci + remotes/network/observed-values + remotes/network/p2p-electra + remotes/network/peerdas-devnet-2 + remotes/network/peerdas-request-custody-by-root + remotes/network/peerdas-request-data-columns-by-root + remotes/network/proposer-boost-orphan + remotes/network/release-workflow-runners + remotes/network/reverse-publish-order + remotes/network/runners-sccache + remotes/network/separate-gossipsub + remotes/network/stable + remotes/network/staging + remotes/network/stateless-validation + remotes/network/superstruct-features + remotes/network/sync-active-request-byrange + remotes/network/tree-states + remotes/network/tree-states-archive + remotes/network/tree-states-hot-rebase + remotes/network/u256-serde + remotes/network/unit + remotes/network/unstable + remotes/network/update-discv5 + remotes/network/update-holesky-boot-enr + remotes/network/update-libp2p + remotes/origin/HEAD -> origin/stable + remotes/origin/anchor + remotes/origin/beacon-api-electra + remotes/origin/book-security + remotes/origin/component-search-loop + remotes/origin/das + remotes/origin/deneb-devnet-7 + remotes/origin/deneb-free-blobs + remotes/origin/devnet-10 + remotes/origin/drop-naughty-peer + remotes/origin/dynamic_nat_detection + remotes/origin/ef-tests-electra + remotes/origin/eip-7732 + remotes/origin/eip6110 + remotes/origin/electra-engine-api + remotes/origin/electra-max-eb + remotes/origin/electra_attestation_changes + remotes/origin/electra_attestation_changes_borked + remotes/origin/electra_devnet_0 + remotes/origin/fix-release-ci + remotes/origin/fix-rpc-disconnects + remotes/origin/hotfix-exit-verification + remotes/origin/import-blobs + remotes/origin/ipv6-auto-enr + remotes/origin/leveldb-1.23 + remotes/origin/merge-stable + remotes/origin/monitor-ci + remotes/origin/p2p-electra + remotes/origin/peerdas-devnet-2 + remotes/origin/peerdas-request-custody-by-root + remotes/origin/peerdas-request-data-columns-by-root + remotes/origin/remove-IDONTWANT-promise + remotes/origin/reverse-publish-order + remotes/origin/runners-sccache + remotes/origin/separate-gossipsub + remotes/origin/stable + remotes/origin/staging + remotes/origin/stateless-validation + remotes/origin/superstruct-features + remotes/origin/sync-active-request-byrange + remotes/origin/tree-states + remotes/origin/unit + remotes/origin/unstable From f0ee2d8686f6a6804d4e557c8a6fe0d3e7bf23fb Mon Sep 17 00:00:00 2001 From: Josh King Date: Tue, 14 Jan 2025 09:24:05 +1100 Subject: [PATCH 03/10] tidy logic and comments --- .../src/peer_manager/network_behaviour.rs | 3 +- .../lighthouse_network/src/types/globals.rs | 15 --- d fjals | 113 ------------------ 3 files changed, 1 insertion(+), 130 deletions(-) delete mode 100644 d fjals diff --git a/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs b/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs index 9ce96f40abe..52dadbe63a8 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs @@ -211,7 +211,6 @@ impl NetworkBehaviour for PeerManager { // We have an inbound connection, this is indicative of having our libp2p NAT ports open. // We now need to check if we have enough open connections to consider our NAT open. - // This function distinguishes between ipv4 and ipv6. self.update_nat_open_metric(&ConnectedPoint::Listener { send_back_addr: remote_addr.clone(), local_addr: _local_addr.clone(), @@ -293,7 +292,6 @@ impl PeerManager { endpoint: &ConnectedPoint, remaining_established: usize, ) { - self.update_nat_open_metric(endpoint); if remaining_established > 0 { return; } @@ -324,6 +322,7 @@ impl PeerManager { // Legacy standard metrics. metrics::inc_counter(&metrics::PEER_DISCONNECT_EVENT_COUNT); + self.update_nat_open_metric(endpoint); self.update_peer_count_metrics(); } } diff --git a/beacon_node/lighthouse_network/src/types/globals.rs b/beacon_node/lighthouse_network/src/types/globals.rs index 2acc0b71a7c..92583b7b5d0 100644 --- a/beacon_node/lighthouse_network/src/types/globals.rs +++ b/beacon_node/lighthouse_network/src/types/globals.rs @@ -33,9 +33,6 @@ pub struct NetworkGlobals { pub config: Arc, /// Ethereum chain configuration. Immutable after initialization. pub spec: Arc, - /// Counts the number of connected peers. - pub connected_ipv4_peers: RwLock, - pub connected_ipv6_peers: RwLock, } impl NetworkGlobals { @@ -88,8 +85,6 @@ impl NetworkGlobals { backfill_state: RwLock::new(BackFillState::Paused), sampling_subnets, sampling_columns, - connected_ipv4_peers: RwLock::new(0), - connected_ipv6_peers: RwLock::new(0), config, spec, } @@ -126,16 +121,6 @@ impl NetworkGlobals { self.peers.read().connected_or_dialing_peers().count() } - /// Returns the number of connected peers with an incoming ipv4 connection. - pub fn connected_incoming_ipv4_peers(&self) -> usize { - *self.connected_ipv4_peers.read() - } - - /// Returns the number of connected peers with an incoming ipv6 connection. - pub fn connected_incoming_ipv6_peers(&self) -> usize { - *self.connected_ipv6_peers.read() - } - /// Returns in the node is syncing. pub fn is_syncing(&self) -> bool { self.sync_state.read().is_syncing() diff --git a/d fjals b/d fjals deleted file mode 100644 index 8c09d13b883..00000000000 --- a/d fjals +++ /dev/null @@ -1,113 +0,0 @@ - dynamic_nat_detection -* origin/unstable - remove-IDONTWANT-promise - stable - unstable - remotes/network/HEAD -> network/stable - remotes/network/anchor - remotes/network/beacon-api-electra - remotes/network/book-security - remotes/network/builder-sse-fixes - remotes/network/component-search-loop - remotes/network/crc - remotes/network/cut-v4.2.0 - remotes/network/cut-v4.4.0 - remotes/network/dapplion-patch-1 - remotes/network/das - remotes/network/debug-jwt-auth - remotes/network/deneb-devnet-7 - remotes/network/deneb-free-blobs - remotes/network/devnet-10 - remotes/network/dont-ban-ips - remotes/network/drop-headtracker - remotes/network/drop-naughty-peer - remotes/network/ef-tests-electra - remotes/network/eip-7732 - remotes/network/eip4844 - remotes/network/eip4844-devnet-v3 - remotes/network/eip6110 - remotes/network/electra-alpha10 - remotes/network/electra-devnet-5 - remotes/network/electra-engine-api - remotes/network/electra-focil - remotes/network/electra-max-eb - remotes/network/electra_attestation_changes - remotes/network/electra_attestation_changes_borked - remotes/network/electra_devnet_0 - remotes/network/fix-release-ci - remotes/network/fix-rpc-disconnects - remotes/network/fix_block_range_math - remotes/network/freezer-tools - remotes/network/hotfix-exit-verification - remotes/network/import-blobs - remotes/network/ipv6-auto-enr - remotes/network/leveldb-1.23 - remotes/network/merge-stable - remotes/network/monitor-ci - remotes/network/observed-values - remotes/network/p2p-electra - remotes/network/peerdas-devnet-2 - remotes/network/peerdas-request-custody-by-root - remotes/network/peerdas-request-data-columns-by-root - remotes/network/proposer-boost-orphan - remotes/network/release-workflow-runners - remotes/network/reverse-publish-order - remotes/network/runners-sccache - remotes/network/separate-gossipsub - remotes/network/stable - remotes/network/staging - remotes/network/stateless-validation - remotes/network/superstruct-features - remotes/network/sync-active-request-byrange - remotes/network/tree-states - remotes/network/tree-states-archive - remotes/network/tree-states-hot-rebase - remotes/network/u256-serde - remotes/network/unit - remotes/network/unstable - remotes/network/update-discv5 - remotes/network/update-holesky-boot-enr - remotes/network/update-libp2p - remotes/origin/HEAD -> origin/stable - remotes/origin/anchor - remotes/origin/beacon-api-electra - remotes/origin/book-security - remotes/origin/component-search-loop - remotes/origin/das - remotes/origin/deneb-devnet-7 - remotes/origin/deneb-free-blobs - remotes/origin/devnet-10 - remotes/origin/drop-naughty-peer - remotes/origin/dynamic_nat_detection - remotes/origin/ef-tests-electra - remotes/origin/eip-7732 - remotes/origin/eip6110 - remotes/origin/electra-engine-api - remotes/origin/electra-max-eb - remotes/origin/electra_attestation_changes - remotes/origin/electra_attestation_changes_borked - remotes/origin/electra_devnet_0 - remotes/origin/fix-release-ci - remotes/origin/fix-rpc-disconnects - remotes/origin/hotfix-exit-verification - remotes/origin/import-blobs - remotes/origin/ipv6-auto-enr - remotes/origin/leveldb-1.23 - remotes/origin/merge-stable - remotes/origin/monitor-ci - remotes/origin/p2p-electra - remotes/origin/peerdas-devnet-2 - remotes/origin/peerdas-request-custody-by-root - remotes/origin/peerdas-request-data-columns-by-root - remotes/origin/remove-IDONTWANT-promise - remotes/origin/reverse-publish-order - remotes/origin/runners-sccache - remotes/origin/separate-gossipsub - remotes/origin/stable - remotes/origin/staging - remotes/origin/stateless-validation - remotes/origin/superstruct-features - remotes/origin/sync-active-request-byrange - remotes/origin/tree-states - remotes/origin/unit - remotes/origin/unstable From 134f59329d4ec42a3eb46a02e9eb2941daefc04e Mon Sep 17 00:00:00 2001 From: Josh King Date: Tue, 14 Jan 2025 14:29:56 +1100 Subject: [PATCH 04/10] move logic to existing metrics loop --- .../src/peer_manager/mod.rs | 25 ++++++++++++++++--- .../src/peer_manager/peerdb.rs | 16 ------------ 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 5fde1ffeb2a..a86bbff1c09 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -1310,7 +1310,9 @@ impl PeerManager { fn update_peer_count_metrics(&self) { let mut peers_connected = 0; let mut clients_per_peer = HashMap::new(); - let mut peers_connected_mutli: HashMap<(&str, &str), i32> = HashMap::new(); + let mut inbound_ipv4_peers_connected: usize = 0; + let mut inbound_ipv6_peers_connected: usize = 0; + let mut peers_connected_multi: HashMap<(&str, &str), i32> = HashMap::new(); let mut peers_per_custody_subnet_count: HashMap = HashMap::new(); for (_, peer_info) in self.network_globals.peers.read().connected_peers() { @@ -1339,7 +1341,7 @@ impl PeerManager { }) }) .unwrap_or("unknown"); - *peers_connected_mutli + *peers_connected_multi .entry((direction, transport)) .or_default() += 1; @@ -1348,6 +1350,23 @@ impl PeerManager { .entry(meta_data.custody_subnet_count) .or_default() += 1; } + + if peer_info.is_incoming_ipv4_connection() { + inbound_ipv4_peers_connected += 1; + if inbound_ipv4_peers_connected >= self.libp2p_nat_open_threshold { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 1); + } else { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 0); + } + } + if peer_info.is_incoming_ipv6_connection() { + inbound_ipv6_peers_connected += 1; + if inbound_ipv6_peers_connected >= self.libp2p_nat_open_threshold { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 1); + } else { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 0); + } + } } // PEERS_CONNECTED @@ -1378,7 +1397,7 @@ impl PeerManager { metrics::set_gauge_vec( &metrics::PEERS_CONNECTED_MULTI, &[direction, transport], - *peers_connected_mutli + *peers_connected_multi .get(&(direction, transport)) .unwrap_or(&0) as i64, ); diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs index 8fc9538dfad..d2effd4d037 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb.rs @@ -183,22 +183,6 @@ impl PeerDB { self.peers.iter().filter(|(_, info)| info.is_connected()) } - /// Returns an iterator of all connected peers with an incoming ipv4 connection. - pub fn connected_incoming_ipv4_peers(&self) -> impl Iterator + '_ { - self.peers - .iter() - .filter(|(_, info)| info.is_connected() && info.is_incoming_ipv4_connection()) - .map(|(peer_id, _)| *peer_id) - } - - /// Returns an iterator of all connected peers with an incoming ipv6 connection. - pub fn connected_incoming_ipv6_peers(&self) -> impl Iterator + '_ { - self.peers - .iter() - .filter(|(_, info)| info.is_connected() && info.is_incoming_ipv6_connection()) - .map(|(peer_id, _)| *peer_id) - } - /// Gives the ids of all known connected peers. pub fn connected_peer_ids(&self) -> impl Iterator { self.peers From 1e30dd12bda9d982b791a8cfe9f66943e43bca0f Mon Sep 17 00:00:00 2001 From: Josh King Date: Tue, 14 Jan 2025 14:32:41 +1100 Subject: [PATCH 05/10] revert change to network_behaviour protocol check --- .../src/peer_manager/network_behaviour.rs | 51 +++---------------- 1 file changed, 7 insertions(+), 44 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs b/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs index 52dadbe63a8..5a7030a5ba2 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs @@ -209,12 +209,13 @@ impl NetworkBehaviour for PeerManager { )); } - // We have an inbound connection, this is indicative of having our libp2p NAT ports open. - // We now need to check if we have enough open connections to consider our NAT open. - self.update_nat_open_metric(&ConnectedPoint::Listener { - send_back_addr: remote_addr.clone(), - local_addr: _local_addr.clone(), - }); + // We have an inbound connection, this is indicative of having our libp2p NAT ports open. We + // distinguish between ipv4 and ipv6 here: + match remote_addr.iter().next() { + Some(Protocol::Ip4(_)) => set_gauge_vec(&NAT_OPEN, &["libp2p_ipv4"], 1), + Some(Protocol::Ip6(_)) => set_gauge_vec(&NAT_OPEN, &["libp2p_ipv6"], 1), + _ => {} + } Ok(ConnectionHandler) } @@ -322,47 +323,9 @@ impl PeerManager { // Legacy standard metrics. metrics::inc_counter(&metrics::PEER_DISCONNECT_EVENT_COUNT); - self.update_nat_open_metric(endpoint); self.update_peer_count_metrics(); } } - fn update_nat_open_metric(&mut self, endpoint: &ConnectedPoint) { - if let ConnectedPoint::Listener { send_back_addr, .. } = endpoint { - if let Some(addr) = send_back_addr.iter().next() { - match addr { - Protocol::Ip4(_) => { - if self - .network_globals - .peers - .read() - .connected_incoming_ipv4_peers() - .count() - <= self.libp2p_nat_open_threshold - { - set_gauge_vec(&NAT_OPEN, &["libp2p_ipv4"], 0); - } else { - set_gauge_vec(&NAT_OPEN, &["libp2p_ipv4"], 1); - } - } - Protocol::Ip6(_) => { - if self - .network_globals - .peers - .read() - .connected_incoming_ipv6_peers() - .count() - <= self.libp2p_nat_open_threshold - { - set_gauge_vec(&NAT_OPEN, &["libp2p_ipv6"], 0); - } else { - set_gauge_vec(&NAT_OPEN, &["libp2p_ipv6"], 1); - } - } - _ => {} - } - } - } - } /// A dial attempt has failed. /// From ee86f0b657fbe5f11cc8248ee3b03d4c57904d44 Mon Sep 17 00:00:00 2001 From: Josh King Date: Tue, 14 Jan 2025 14:33:49 +1100 Subject: [PATCH 06/10] clippy --- .../lighthouse_network/src/peer_manager/network_behaviour.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs b/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs index 5a7030a5ba2..11676f9a01f 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs @@ -290,7 +290,7 @@ impl PeerManager { fn on_connection_closed( &mut self, peer_id: PeerId, - endpoint: &ConnectedPoint, + _endpoint: &ConnectedPoint, remaining_established: usize, ) { if remaining_established > 0 { From 79c7d2d1a9baf00ff5596fefae9b7ad358a48056 Mon Sep 17 00:00:00 2001 From: Josh King Date: Tue, 14 Jan 2025 15:13:04 +1100 Subject: [PATCH 07/10] clippy matches! macro --- .../src/peer_manager/peerdb/peer_info.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs index 7c77b9c6ad7..d92307307e7 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs @@ -125,20 +125,18 @@ impl PeerInfo { /// Returns true if this is an incoming ipv4 connection. pub fn is_incoming_ipv4_connection(&self) -> bool { self.seen_multiaddrs.iter().any(|multiaddr| { - multiaddr.iter().any(|protocol| match protocol { - libp2p::core::multiaddr::Protocol::Ip4(_) => true, - _ => false, - }) + multiaddr + .iter() + .any(|protocol| matches!(protocol, libp2p::core::multiaddr::Protocol::Ip4(_))) }) } /// Returns true if this is an incoming ipv6 connection. pub fn is_incoming_ipv6_connection(&self) -> bool { self.seen_multiaddrs.iter().any(|multiaddr| { - multiaddr.iter().any(|protocol| match protocol { - libp2p::core::multiaddr::Protocol::Ip6(_) => true, - _ => false, - }) + multiaddr + .iter() + .any(|protocol| matches!(protocol, libp2p::core::multiaddr::Protocol::Ip6(_))) }) } From b44b655bc6649b385a7878880ef4a92cd22b99cc Mon Sep 17 00:00:00 2001 From: Josh King Date: Wed, 15 Jan 2025 10:18:32 +1100 Subject: [PATCH 08/10] pull nat_open check outside of peercounting loop --- .../src/peer_manager/mod.rs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index a86bbff1c09..4163a7e8c51 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -1350,23 +1350,28 @@ impl PeerManager { .entry(meta_data.custody_subnet_count) .or_default() += 1; } - + // Check if incoming peer is ipv4 if peer_info.is_incoming_ipv4_connection() { inbound_ipv4_peers_connected += 1; - if inbound_ipv4_peers_connected >= self.libp2p_nat_open_threshold { - metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 1); - } else { - metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 0); - } } + + // Check if incoming peer is ipv6 if peer_info.is_incoming_ipv6_connection() { inbound_ipv6_peers_connected += 1; - if inbound_ipv6_peers_connected >= self.libp2p_nat_open_threshold { - metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 1); - } else { - metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 0); - } } + + // Set ipv4 nat_open metric flag if threshold of peercount is met, unset if below threshold + if inbound_ipv4_peers_connected >= self.libp2p_nat_open_threshold { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 1); + } else { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 0); + } + + // Set ipv6 nat_open metric flag if threshold of peercount is met, unset if below threshold + if inbound_ipv6_peers_connected >= self.libp2p_nat_open_threshold { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 1); + } else { + metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 0); } // PEERS_CONNECTED From cc811643fdac2d20ae136a1fa70b7aa63c64d609 Mon Sep 17 00:00:00 2001 From: Josh King Date: Wed, 15 Jan 2025 10:39:08 +1100 Subject: [PATCH 09/10] missing close bracket --- beacon_node/lighthouse_network/src/peer_manager/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon_node/lighthouse_network/src/peer_manager/mod.rs b/beacon_node/lighthouse_network/src/peer_manager/mod.rs index 4163a7e8c51..15971904524 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -1359,6 +1359,7 @@ impl PeerManager { if peer_info.is_incoming_ipv6_connection() { inbound_ipv6_peers_connected += 1; } + } // Set ipv4 nat_open metric flag if threshold of peercount is met, unset if below threshold if inbound_ipv4_peers_connected >= self.libp2p_nat_open_threshold { From a3c5ce9b1aa43088a248578a39c6a865bf598d41 Mon Sep 17 00:00:00 2001 From: Josh King Date: Wed, 15 Jan 2025 14:08:15 +1100 Subject: [PATCH 10/10] make threshold const --- beacon_node/lighthouse_network/src/peer_manager/mod.rs | 9 ++++----- 1 file changed, 4 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 15971904524..6502a8dbff5 100644 --- a/beacon_node/lighthouse_network/src/peer_manager/mod.rs +++ b/beacon_node/lighthouse_network/src/peer_manager/mod.rs @@ -63,6 +63,8 @@ pub const MIN_OUTBOUND_ONLY_FACTOR: f32 = 0.2; /// limit is 55, and we are at 55 peers, the following parameter provisions a few more slots of /// dialing priority peers we need for validator duties. pub const PRIORITY_PEER_EXCESS: f32 = 0.2; +/// The numbre of inbound libp2p peers we have seen before we consider our NAT to be open. +pub const LIBP2P_NAT_OPEN_THRESHOLD: usize = 3; /// The main struct that handles peer's reputation and connection status. pub struct PeerManager { @@ -107,8 +109,6 @@ pub struct PeerManager { metrics_enabled: bool, /// Keeps track of whether the QUIC protocol is enabled or not. quic_enabled: bool, - /// Threshold of inbound peers to consider libp2p NAT open. - libp2p_nat_open_threshold: usize, /// The logger associated with the `PeerManager`. log: slog::Logger, } @@ -174,7 +174,6 @@ impl PeerManager { discovery_enabled, metrics_enabled, quic_enabled, - libp2p_nat_open_threshold: 3, // Default to 3 inbound peers to consider NAT open log: log.clone(), }) } @@ -1362,14 +1361,14 @@ impl PeerManager { } // Set ipv4 nat_open metric flag if threshold of peercount is met, unset if below threshold - if inbound_ipv4_peers_connected >= self.libp2p_nat_open_threshold { + if inbound_ipv4_peers_connected >= LIBP2P_NAT_OPEN_THRESHOLD { metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 1); } else { metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv4"], 0); } // Set ipv6 nat_open metric flag if threshold of peercount is met, unset if below threshold - if inbound_ipv6_peers_connected >= self.libp2p_nat_open_threshold { + if inbound_ipv6_peers_connected >= LIBP2P_NAT_OPEN_THRESHOLD { metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 1); } else { metrics::set_gauge_vec(&metrics::NAT_OPEN, &["libp2p_ipv6"], 0);