Remove unused structs/variants, rename Source::Udp -> Source::QuicPayload

Author: matheus23

iroh/src/magicsock/node_map.rs

@@ -11,10 +11,7 @@ use serde::{Deserialize, Serialize};
 use stun_rs::TransactionId;
 use tracing::{debug, info, instrument, trace, warn};
 
-use self::{
-    node_state::{NodeState, Options, PingHandled},
-    path_validity::ClearReason,
-};
+use self::node_state::{NodeState, Options, PingHandled};
 use super::{metrics::Metrics, ActorMessage, DiscoMessageSource, NodeIdMappedAddr};
 #[cfg(any(test, feature = "test-utils"))]
 use crate::endpoint::PathSelection;
@@ -395,22 +392,22 @@ impl NodeMapInner {
         now: Instant,
     ) {
         for addr in discovered {
-            self.remove_by_ipp(addr.into(), ClearReason::MatchesOurLocalAddr, now)
+            self.remove_by_ipp(addr.into(), now, "matches our local addr")
         }
     }
 
     /// Removes a direct address from a node.
-    fn remove_by_ipp(&mut self, ipp: IpPort, reason: ClearReason, now: Instant) {
+    fn remove_by_ipp(&mut self, ipp: IpPort, now: Instant, why: &'static str) {
         if let Some(id) = self.by_ip_port.remove(&ipp) {
             if let Entry::Occupied(mut entry) = self.by_id.entry(id) {
                 let node = entry.get_mut();
-                node.remove_direct_addr(&ipp, reason, now);
+                node.remove_direct_addr(&ipp, now, why);
                 if node.direct_addresses().count() == 0 {
                     let node_id = node.public_key();
                     let mapped_addr = node.quic_mapped_addr();
                     self.by_node_key.remove(node_id);
                     self.by_quic_mapped_addr.remove(mapped_addr);
-                    debug!(node_id=%node_id.fmt_short(), ?reason, "removing node");
+                    debug!(node_id=%node_id.fmt_short(), why, "removing node");
                     entry.remove();
                 }
             }

iroh/src/magicsock/node_map/node_state.rs

@@ -18,7 +18,6 @@ use tracing::{debug, event, info, instrument, trace, warn, Level};
 
 use super::{
     path_state::{summarize_node_paths, PathState},
-    path_validity::ClearReason,
     udp_paths::{NodeUdpPaths, UdpSendAddr},
     IpPort, Source,
 };
@@ -372,19 +371,14 @@ impl NodeState {
     /// Removes a direct address for this node.
     ///
     /// If this is also the best address, it will be cleared as well.
-    pub(super) fn remove_direct_addr(
-        &mut self,
-        ip_port: &IpPort,
-        reason: ClearReason,
-        now: Instant,
-    ) {
+    pub(super) fn remove_direct_addr(&mut self, ip_port: &IpPort, now: Instant, why: &'static str) {
         let Some(state) = self.udp_paths.paths.remove(ip_port) else {
             return;
         };
 
         match state.last_alive().map(|instant| instant.elapsed()) {
-            Some(last_alive) => debug!(%ip_port, ?last_alive, ?reason, "pruning address"),
-            None => debug!(%ip_port, last_seen=%"never", ?reason, "pruning address"),
+            Some(last_alive) => debug!(%ip_port, ?last_alive, why, "pruning address"),
+            None => debug!(%ip_port, last_seen=%"never", why, "pruning address"),
         }
 
         self.udp_paths.update_to_best_addr(now);
@@ -873,7 +867,7 @@ impl NodeState {
         prune_candidates.sort_unstable_by_key(|(_ip_port, last_alive)| *last_alive);
         prune_candidates.truncate(prune_count);
         for (ip_port, _last_alive) in prune_candidates.into_iter() {
-            self.remove_direct_addr(&ip_port, ClearReason::Inactive, now);
+            self.remove_direct_addr(&ip_port, now, "inactive");
         }
         debug!(
             paths = %summarize_node_paths(&self.udp_paths.paths),

iroh/src/magicsock/node_map/path_state.rs

@@ -47,13 +47,10 @@ pub(super) struct PathState {
     /// The time this endpoint was last advertised via a call-me-maybe DISCO message.
     pub(super) call_me_maybe_time: Option<Instant>,
 
-    // /// The most recent [`PongReply`].
-    // ///
-    // /// Previous replies are cleared when they are no longer relevant to determine whether
-    // /// this path can still be used to reach the remote node.
-    // pub(super) recent_pong: Option<PongReply>,
     /// Tracks whether this path is valid.
     ///
+    /// Also stores the latest [`PongReply`], if there is one.
+    ///
     /// See [`PathValidity`] docs.
     pub(super) validity: PathValidity,
     /// When the last payload data was **received** via this path.
@@ -134,10 +131,8 @@ impl PathState {
 
     pub(super) fn receive_payload(&mut self, now: Instant) {
         self.last_payload_msg = Some(now);
-        // TODO(matheus23): It's not necessarily UDP. Kinda weird to have this thing
-        // Also, it all results in the same 6.5s timeout anyways, maybe we just remove it?
         self.validity
-            .receive_payload(now, path_validity::Source::Udp);
+            .receive_payload(now, path_validity::Source::QuicPayload);
     }
 
     #[cfg(test)]

iroh/src/magicsock/node_map/path_validity.rs

@@ -30,25 +30,14 @@ struct Inner {
 #[derive(Debug)]
 pub(super) enum Source {
     ReceivedPong,
-    // BestCandidate,
-    Udp,
-}
-
-#[derive(Debug, Clone, Copy)]
-pub enum ClearReason {
-    // Reset, // TODO(matheus23): unused
-    Inactive,
-    // PongTimeout,
-    MatchesOurLocalAddr,
+    QuicPayload,
 }
 
 impl Source {
     fn trust_duration(&self) -> Duration {
         match self {
             Source::ReceivedPong => TRUST_UDP_ADDR_DURATION,
-            // // TODO: Fix time
-            // Source::BestCandidate => Duration::from_secs(60 * 60),
-            Source::Udp => TRUST_UDP_ADDR_DURATION,
+            Source::QuicPayload => TRUST_UDP_ADDR_DURATION,
         }
     }
 }