Cleanup

Author: matheus23

iroh/src/magicsock/node_map/node_state.rs

@@ -1511,12 +1511,6 @@ mod tests {
                     udp_paths: NodeUdpPaths::from_parts(
                         endpoint_state,
                         UdpSendAddr::Valid(ip_port.into()),
-                        // BestAddr::from_parts(
-                        //     ip_port.into(),
-                        //     latency,
-                        //     now,
-                        //     now + Duration::from_secs(100), //TODO(matheus23): Is the 100s validity needed for the test?
-                        // ),
                     ),
                     sent_pings: HashMap::new(),
                     last_used: Some(now),
@@ -1582,7 +1576,6 @@ mod tests {
         // endpoint w/ expired best addr and relay w/ latency
         let (d_endpoint, d_socket_addr) = {
             let socket_addr: SocketAddr = "0.0.0.0:7".parse().unwrap();
-            let expired = now.checked_sub(Duration::from_secs(100)).unwrap();
             let key = SecretKey::generate(rand::thread_rng());
             let node_id = key.public();
             let endpoint_state = BTreeMap::from([(
@@ -1607,8 +1600,6 @@ mod tests {
                     udp_paths: NodeUdpPaths::from_parts(
                         endpoint_state,
                         UdpSendAddr::Outdated(socket_addr),
-                        // TODO(matheus23): Test might need adjustments
-                        // BestAddr::from_parts(socket_addr, Duration::from_millis(80), now, expired),
                     ),
                     sent_pings: HashMap::new(),
                     last_used: Some(now),

iroh/src/magicsock/node_map/path_state.rs

@@ -1,9 +1,6 @@
 //! The state kept for each network path to a remote node.
 
-use std::{
-    collections::{BTreeMap, HashMap},
-    net::SocketAddr,
-};
+use std::collections::{BTreeMap, HashMap};
 
 use iroh_base::NodeId;
 use iroh_relay::protos::stun;
@@ -87,13 +84,6 @@ impl PathState {
         }
     }
 
-    pub(super) fn udp_addr(&self) -> Option<SocketAddr> {
-        match self.path {
-            SendAddr::Udp(addr) => Some(addr),
-            SendAddr::Relay(_) => None,
-        }
-    }
-
     pub(super) fn with_last_payload(
         node_id: NodeId,
         path: SendAddr,

iroh/src/magicsock/node_map/path_validity.rs

@@ -2,7 +2,13 @@ use n0_future::time::{Duration, Instant};
 
 use crate::magicsock::node_map::node_state::PongReply;
 
-/// How long we trust a UDP address as the exclusive path (without using relay) without having heard a Pong reply.
+/// How long we trust a UDP address as the exclusive path (i.e. without also sending via the relay).
+///
+/// Trust for a UDP address begins when we receive a DISCO UDP pong on that address.
+/// It is then further extended by this duration every time we receive QUIC payload data while it's
+/// currently trusted.
+///
+/// If trust goes away, it can be brought back with another valid DISCO UDP pong.
 const TRUST_UDP_ADDR_DURATION: Duration = Duration::from_millis(6500);
 
 /// Tracks a path's validity.
@@ -30,9 +36,9 @@ pub(super) enum Source {
 
 #[derive(Debug, Clone, Copy)]
 pub enum ClearReason {
-    Reset,
+    // Reset, // TODO(matheus23): unused
     Inactive,
-    PongTimeout,
+    // PongTimeout,
     MatchesOurLocalAddr,
 }
 
@@ -117,9 +123,9 @@ impl PathValidity {
     }
 
     // TODO(matheus23): Use this to bias the choice of best outdated addr maybe?
-    pub(super) fn confirmed_at(&self) -> Option<Instant> {
-        self.0.as_ref().map(|inner| inner.confirmed_at)
-    }
+    // pub(super) fn confirmed_at(&self) -> Option<Instant> {
+    //     self.0.as_ref().map(|inner| inner.confirmed_at)
+    // }
 }
 
 impl Inner {

iroh/src/magicsock/node_map/udp_paths.rs

@@ -59,24 +59,6 @@ impl UdpSendAddr {
             UdpSendAddr::None => None,
         }
     }
-
-    pub fn is_better_than(&self, other: &Self) -> bool {
-        match (other, self) {
-            // Other being valid, we'll never be better
-            (UdpSendAddr::Valid(_), _) => false,
-            // Anything anything above outdated is better
-            (UdpSendAddr::Outdated(_), UdpSendAddr::Valid(_)) => true,
-            (UdpSendAddr::Outdated(_), _) => false,
-            // Anything above unconfirmed is better
-            (UdpSendAddr::Unconfirmed(_), UdpSendAddr::Valid(_)) => true,
-            (UdpSendAddr::Unconfirmed(_), UdpSendAddr::Outdated(_)) => true,
-            (UdpSendAddr::Unconfirmed(_), _) => false,
-            // None compared to none is equally bad
-            (UdpSendAddr::None, UdpSendAddr::None) => false,
-            // Anything above none is better
-            (UdpSendAddr::None, _) => true,
-        }
-    }
 }
 
 /// The UDP paths for a single node.
@@ -103,7 +85,10 @@ pub(super) struct NodeUdpPaths {
     ///   - the current send addr is not a validated path anymore or
     ///   - we received a pong with lower latency.
     pub(super) best: UdpSendAddr,
-    pub(super) best_non_ipv6: UdpSendAddr,
+    /// The current best address to send on from all IPv4 addresses we have available.
+    ///
+    /// Follows the same logic as `best` above, but doesn't include any IPv6 addresses.
+    pub(super) best_ipv4: UdpSendAddr,
 }
 
 impl NodeUdpPaths {
@@ -115,24 +100,21 @@ impl NodeUdpPaths {
     pub(super) fn from_parts(paths: BTreeMap<IpPort, PathState>, best: UdpSendAddr) -> Self {
         Self {
             paths,
-            best_non_ipv6: best,
+            best_ipv4: best, // we only use ipv4 addrs in tests
             best,
         }
     }
 
     /// Returns the current UDP address to send on.
     pub(super) fn send_addr(&self, have_ipv6: bool) -> &UdpSendAddr {
         if !have_ipv6 {
-            return &self.best_non_ipv6;
-        }
-        if self.best_non_ipv6.is_better_than(&self.best) {
-            return &self.best_non_ipv6;
+            return &self.best_ipv4;
         }
         &self.best
     }
 
     pub(super) fn update_to_best_addr(&mut self, now: Instant) {
-        self.best_non_ipv6 = self.best_addr(false, now);
+        self.best_ipv4 = self.best_addr(false, now);
         self.best = self.best_addr(true, now);
     }