From 9f0526ce456f80c6008d10bd69449ebfcde75aa3 Mon Sep 17 00:00:00 2001
From: Floris Bruynooghe <flub@n0.computer>
Date: Fri, 16 Feb 2024 10:34:29 +0100
Subject: [PATCH] refactor(iroh-net): Delete some unused testing infrastructure
 (#2028)

## Description

This came from the tailscale code but we never seem to have used it
and our routers haven't exploded yet.

## Notes & open questions

Admittedly it would be nice not to keep hitting so many external
things on the network.  But our entire testsuite would need a big
review for that.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
---
 iroh-net/src/netcheck.rs           |  6 -----
 iroh-net/src/netcheck/reportgen.rs | 40 +++++++++++-------------------
 2 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/iroh-net/src/netcheck.rs b/iroh-net/src/netcheck.rs
index 693ff630a18..47b3b6cd746 100644
--- a/iroh-net/src/netcheck.rs
+++ b/iroh-net/src/netcheck.rs
@@ -383,10 +383,6 @@ struct Actor {
     reports: Reports,
 
     // Actor configuration.
-    /// Whether the client should try to reach things other than localhost.
-    ///
-    /// This is set to true in tests to avoid probing the local LAN's router, etc.
-    skip_external_network: bool,
     /// The port mapper client, if those are requested.
     ///
     /// The port mapper is responsible for talking to routers via UPnP and the like to try
@@ -414,7 +410,6 @@ impl Actor {
             receiver,
             sender,
             reports: Default::default(),
-            skip_external_network: false,
             port_mapper,
             in_flight_stun_requests: Default::default(),
             current_report_run: None,
@@ -516,7 +511,6 @@ impl Actor {
             self.addr(),
             self.reports.last.clone(),
             self.port_mapper.clone(),
-            self.skip_external_network,
             derp_map,
             stun_sock_v4,
             stun_sock_v6,
diff --git a/iroh-net/src/netcheck/reportgen.rs b/iroh-net/src/netcheck/reportgen.rs
index 9d86851cfbd..03128d3a9f6 100644
--- a/iroh-net/src/netcheck/reportgen.rs
+++ b/iroh-net/src/netcheck/reportgen.rs
@@ -85,7 +85,6 @@ impl Client {
         netcheck: netcheck::Addr,
         last_report: Option<Arc<Report>>,
         port_mapper: Option<portmapper::Client>,
-        skip_external_network: bool,
         derp_map: DerpMap,
         stun_sock4: Option<Arc<UdpSocket>>,
         stun_sock6: Option<Arc<UdpSocket>>,
@@ -101,7 +100,6 @@ impl Client {
             netcheck: netcheck.clone(),
             last_report,
             port_mapper,
-            skip_external_network,
             incremental,
             derp_map,
             stun_sock4,
@@ -172,13 +170,6 @@ struct Actor {
     last_report: Option<Arc<Report>>,
     /// The portmapper client, if there is one.
     port_mapper: Option<portmapper::Client>,
-    /// Whether the actor should try to reach things other than localhost.
-    ///
-    /// This is set to true in tests to avoid probing the local LAN's router etc.
-    ///
-    /// TODO: currently this only skips the portmapper, but still does STUN, HTTP and ICMP
-    /// probes.  Furthermore none of our tests actually use this feature.
-    skip_external_network: bool,
     /// The DERP configuration.
     derp_map: DerpMap,
     /// Socket to send IPv4 STUN requests from.
@@ -234,7 +225,6 @@ impl Actor {
     async fn run_inner(&mut self) -> Result<()> {
         debug!(
             port_mapper = %self.port_mapper.is_some(),
-            skip_external_network=%self.skip_external_network,
             "reportstate actor starting",
         );
 
@@ -500,23 +490,21 @@ impl Actor {
         &mut self,
     ) -> MaybeFuture<Pin<Box<impl Future<Output = Option<portmapper::ProbeOutput>>>>> {
         let mut port_mapping = MaybeFuture::default();
-        if !self.skip_external_network {
-            if let Some(port_mapper) = self.port_mapper.clone() {
-                port_mapping.inner = Some(Box::pin(async move {
-                    match port_mapper.probe().await {
-                        Ok(Ok(res)) => Some(res),
-                        Ok(Err(err)) => {
-                            warn!("skipping port mapping: {err:?}");
-                            None
-                        }
-                        Err(recv_err) => {
-                            warn!("skipping port mapping: {recv_err:?}");
-                            None
-                        }
+        if let Some(port_mapper) = self.port_mapper.clone() {
+            port_mapping.inner = Some(Box::pin(async move {
+                match port_mapper.probe().await {
+                    Ok(Ok(res)) => Some(res),
+                    Ok(Err(err)) => {
+                        warn!("skipping port mapping: {err:?}");
+                        None
                     }
-                }));
-                self.outstanding_tasks.port_mapper = true;
-            }
+                    Err(recv_err) => {
+                        warn!("skipping port mapping: {recv_err:?}");
+                        None
+                    }
+                }
+            }));
+            self.outstanding_tasks.port_mapper = true;
         }
         port_mapping
     }