Fix: ipv4/ipv6_contactable metrics never updated

src/config.rs

@@ -352,7 +352,7 @@ impl ConfigBuilder {
 
         // If enr-update is set to false, then it is non-intuitive for discv5 to revoke ENR details
         // when determining NAT status. So we will not do this.
-        if self.config.enr_update {
+        if !self.config.enr_update {
             self.config.auto_nat_listen_duration = None;
         }
 

src/service/test.rs

@@ -7,6 +7,7 @@ use crate::{
     handler::Handler,
     kbucket,
     kbucket::{BucketInsertResult, KBucketsTable, NodeStatus},
+    metrics::METRICS,
     node_info::NodeContact,
     query_pool::{QueryId, QueryPool},
     rpc::RequestId,
@@ -20,7 +21,7 @@ use rand;
 use std::{
     collections::HashMap,
     net::{Ipv4Addr, Ipv6Addr},
-    sync::Arc,
+    sync::{atomic::Ordering, Arc},
     time::Duration,
 };
 use tokio::sync::{
@@ -52,15 +53,13 @@ fn init() {
         .try_init();
 }
 
-async fn build_service(
+async fn build_service_with(
     local_enr: Arc<RwLock<Enr>>,
     enr_key: Arc<RwLock<CombinedKey>>,
     filters: bool,
+    listen_config: ListenConfig,
+    ip_mode: IpMode,
 ) -> Service {
-    let listen_config = ListenConfig::Ipv4 {
-        ip: local_enr.read().ip4().unwrap(),
-        port: local_enr.read().udp4().unwrap(),
-    };
     let config = ConfigBuilder::new(listen_config)
         .executor(Box::<crate::executor::TokioExecutor>::default())
         .build();
@@ -109,11 +108,35 @@ async fn build_service(
         event_stream: None,
         exit,
         config,
-        ip_mode: Default::default(),
+        ip_mode,
         connectivity_state,
     }
 }
 
+async fn build_service(
+    local_enr: Arc<RwLock<Enr>>,
+    enr_key: Arc<RwLock<CombinedKey>>,
+    filters: bool,
+) -> Service {
+    let listen_config = ListenConfig::Ipv4 {
+        ip: local_enr.read().ip4().unwrap(),
+        port: local_enr.read().udp4().unwrap(),
+    };
+    build_service_with(local_enr, enr_key, filters, listen_config, IpMode::Ip4).await
+}
+
+async fn build_service_ipv6(
+    local_enr: Arc<RwLock<Enr>>,
+    enr_key: Arc<RwLock<CombinedKey>>,
+    filters: bool,
+) -> Service {
+    let listen_config = ListenConfig::Ipv6 {
+        ip: local_enr.read().ip6().unwrap(),
+        port: local_enr.read().udp6().unwrap(),
+    };
+    build_service_with(local_enr, enr_key, filters, listen_config, IpMode::Ip6).await
+}
+
 fn build_non_handler_service(
     local_enr: Arc<RwLock<Enr>>,
     enr_key: Arc<RwLock<CombinedKey>>,
@@ -510,3 +533,122 @@ async fn test_ipv6_update_amongst_ipv4_dominated_network() {
     // Should be 10 ipv6 pings
     assert_eq!(v6_pings, 10)
 }
+
+// Tests that METRICS.ipv4_contactable is set to true once enough incoming connections are received
+// during an active auto-NAT listen window.
+#[tokio::test]
+async fn test_connectivity_metrics_updated_on_incoming_connections() {
+    init();
+
+    // Ensure a clean state for the global METRICS.
+    METRICS.ipv4_contactable.store(false, Ordering::Relaxed);
+
+    let enr_key = CombinedKey::generate_secp256k1();
+    let ip = Ipv4Addr::LOCALHOST;
+    let enr = Enr::builder()
+        .ip4(ip)
+        .udp4(DEFAULT_UDP_PORT)
+        .build(&enr_key)
+        .unwrap();
+
+    let mut service = build_service(
+        Arc::new(RwLock::new(enr)),
+        Arc::new(RwLock::new(enr_key)),
+        false,
+    )
+    .await;
+
+    let ipv4_socket: SocketAddr = SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 9000);
+
+    // Simulate an ENR socket update to start the auto-NAT timer. Without this,
+    // received_incoming_connection() returns early because the wait timer is None.
+    service.connectivity_state.enr_socket_update(&ipv4_socket);
+
+    // Inject incoming connections. Two are required to reach
+    // NUMBER_OF_INCOMING_CONNECTIONS_REQUIRED_TO_BE_VALID.
+    let peer_key = CombinedKey::generate_secp256k1();
+    let peer_enr = Enr::builder()
+        .ip4(ip)
+        .udp4(DEFAULT_UDP_PORT)
+        .build(&peer_key)
+        .unwrap();
+
+    service.inject_session_established(
+        peer_enr.clone(),
+        &ipv4_socket,
+        ConnectionDirection::Incoming,
+    );
+    service.inject_session_established(
+        peer_enr.clone(),
+        &ipv4_socket,
+        ConnectionDirection::Incoming,
+    );
+
+    // ipv4_contactable should now be true.
+    assert!(
+        METRICS.ipv4_contactable.load(Ordering::Relaxed),
+        "ipv4_contactable should be true after receiving enough incoming connections"
+    );
+
+    // Clean up to avoid affecting other tests.
+    METRICS.ipv4_contactable.store(false, Ordering::Relaxed);
+}
+
+// Tests that METRICS.ipv6_contactable is set to true once enough incoming connections are received
+// during an active auto-NAT listen window.
+#[tokio::test]
+async fn test_connectivity_metrics_updated_on_incoming_connections_ipv6() {
+    init();
+
+    // Ensure a clean state for the global METRICS.
+    METRICS.ipv6_contactable.store(false, Ordering::Relaxed);
+
+    let enr_key = CombinedKey::generate_secp256k1();
+    let ip6 = Ipv6Addr::LOCALHOST;
+    let enr = Enr::builder()
+        .ip6(ip6)
+        .udp6(DEFAULT_UDP_PORT)
+        .build(&enr_key)
+        .unwrap();
+
+    let mut service = build_service_ipv6(
+        Arc::new(RwLock::new(enr)),
+        Arc::new(RwLock::new(enr_key)),
+        false,
+    )
+    .await;
+
+    let ipv6_socket: SocketAddr = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 9000);
+
+    // Simulate an ENR socket update to start the auto-NAT timer for IPv6.
+    service.connectivity_state.enr_socket_update(&ipv6_socket);
+
+    // Inject incoming connections. Two are required to reach
+    // NUMBER_OF_INCOMING_CONNECTIONS_REQUIRED_TO_BE_VALID.
+    let peer_key = CombinedKey::generate_secp256k1();
+    let peer_enr = Enr::builder()
+        .ip6(ip6)
+        .udp6(DEFAULT_UDP_PORT)
+        .build(&peer_key)
+        .unwrap();
+
+    service.inject_session_established(
+        peer_enr.clone(),
+        &ipv6_socket,
+        ConnectionDirection::Incoming,
+    );
+    service.inject_session_established(
+        peer_enr.clone(),
+        &ipv6_socket,
+        ConnectionDirection::Incoming,
+    );
+
+    // ipv6_contactable should now be true.
+    assert!(
+        METRICS.ipv6_contactable.load(Ordering::Relaxed),
+        "ipv6_contactable should be true after receiving enough incoming connections"
+    );
+
+    // Clean up to avoid affecting other tests.
+    METRICS.ipv6_contactable.store(false, Ordering::Relaxed);
+}