review comments

iroh-net/src/endpoint.rs

@@ -531,7 +531,10 @@ impl Endpoint {
     /// This updates the local state for the remote node.  If the provided [`NodeAddr`]
     /// contains a [`RelayUrl`] this will be used as the new relay server for this node.  If
     /// it contains any new IP endpoints they will also be stored and tried when next
-    /// connecting to this node.
+    /// connecting to this node. Any address that matches this node's direct addresses will be
+    /// silently ignored.
+    ///
+    /// See also [`Endpoint::add_node_addr_with_source`].
     ///
     /// # Errors
     ///
@@ -545,8 +548,9 @@ impl Endpoint {
     ///
     /// This updates the local state for the remote node.  If the provided [`NodeAddr`] contains a
     /// [`RelayUrl`] this will be used as the new relay server for this node.  If it contains any
-    /// new IP endpoints they will also be stored and tried when next connecting to this node. The
-    /// source is used for logging exclusively and will not be stored.
+    /// new IP endpoints they will also be stored and tried when next connecting to this node. Any
+    /// address that matches this node's direct addresses will be silently ignored. The *source* is
+    /// used for logging exclusively and will not be stored.
     ///
     /// # Errors
     ///

iroh-net/src/magicsock.rs

@@ -80,7 +80,7 @@ pub use self::node_map::{
     ConnectionType, ConnectionTypeStream, ControlMsg, DirectAddrInfo, NodeInfo as ConnectionInfo,
 };
 pub(super) use self::timer::Timer;
-pub(super) use node_map::Source;
+pub(crate) use node_map::Source;
 
 /// How long we consider a STUN-derived endpoint valid for. UDP NAT mappings typically
 /// expire at 30 seconds, so this is a few seconds shy of that.
@@ -381,16 +381,18 @@ impl MagicSock {
             self.node_map.add_node_addr(addr, source);
             Ok(())
         } else if pruned != 0 {
-            anyhow::bail!("empty addressing info, {pruned} direct addresses have been pruned")
+            Err(anyhow::anyhow!(
+                "empty addressing info, {pruned} direct addresses have been pruned"
+            ))
         } else {
-            anyhow::bail!("empty addressing info")
+            Err(anyhow::anyhow!("empty addressing info"))
         }
     }
 
     /// Updates our direct addresses.
     ///
     /// On a successful update, our address is published to discovery.
-    pub(super) fn update_endpoints(&self, eps: Vec<DirectAddr>) {
+    pub(super) fn update_direct_addresses(&self, eps: Vec<DirectAddr>) {
         let updated = self.endpoints.update(DiscoveredEndpoints::new(eps)).is_ok();
         if updated {
             let eps = self.endpoints.read();
@@ -2125,7 +2127,7 @@ impl Actor {
             // The STUN address(es) are always first.
             // Despite this sorting, clients are not relying on this sorting for decisions;
 
-            msock.update_endpoints(eps);
+            msock.update_direct_addresses(eps);
 
             // Regardless of whether our local endpoints changed, we now want to send any queued
             // call-me-maybe messages.

iroh-net/src/magicsock/node_map.rs

@@ -362,7 +362,7 @@ impl NodeMapInner {
         }
     }
 
-    /// Prunes nodes that claim to share a address we know points to us,
+    /// Prunes direct addresses from nodes that claim to share an address we know points to us.
     pub(super) fn on_direct_addr_discovered(
         &mut self,
         discovered: impl Iterator<Item = impl Into<IpPort>>,
@@ -372,7 +372,7 @@ impl NodeMapInner {
         }
     }
 
-    /// Removes a node by its IpPort
+    /// Removes a direct address from a node.
     fn remove_by_ipp(&mut self, ipp: IpPort, reason: ClearReason) {
         if let Some(id) = self.by_ip_port.remove(&ipp) {
             if let Entry::Occupied(mut entry) = self.by_id.entry(id) {

iroh-net/src/magicsock/node_map/node_state.rs

@@ -313,7 +313,7 @@ impl NodeState {
 
     /// Removes a direct address for this node.
     ///
-    /// If this is also de best address, it will be cleared as well.
+    /// 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) {
         let Some(state) = self.direct_addr_state.remove(ip_port) else {
             return;