From 00d0150ed9c3fb4b9a39b688c77336b380e23d12 Mon Sep 17 00:00:00 2001 From: Divma <26765164+divagant-martian@users.noreply.github.com> Date: Mon, 18 Sep 2023 18:48:39 -0500 Subject: [PATCH] refactor(iroh-net): remove `iroh-net::config::Node` since limited to its used fields it's redundant (#1486) ## Description the `Node` type contains: /~https://github.com/n0-computer/iroh/blob/f2f3ead655dee94bd01c9f4fdcc8730457f51ffd/iroh-net/src/config.rs#L157-L174 from these fields: - `name` is always `None`, so unused. - `addresses` is always calculated from the the `endpoints` so it can be removed. After this, the only remaining fields are the key, endpoints and derp region. This is equivalent to the `NodeAddr` type, making `Node` redundant /~https://github.com/n0-computer/iroh/blob/f2f3ead655dee94bd01c9f4fdcc8730457f51ffd/iroh-net/src/magic_endpoint.rs#L20-L29 ## Notes & open questions ## Change checklist - [x] Self-review. - [ ] Documentation updates if relevant. - [ ] Tests if relevant. --- iroh-net/src/config.rs | 25 +------------------------ iroh-net/src/magic_endpoint.rs | 2 +- iroh-net/src/magicsock.rs | 22 +++++++--------------- iroh-net/src/magicsock/endpoint.rs | 16 ++++++++-------- 4 files changed, 17 insertions(+), 48 deletions(-) diff --git a/iroh-net/src/config.rs b/iroh-net/src/config.rs index 783b8607b1..10ed448596 100644 --- a/iroh-net/src/config.rs +++ b/iroh-net/src/config.rs @@ -6,7 +6,7 @@ use std::{ net::{IpAddr, Ipv4Addr, SocketAddr}, }; -use super::{key::PublicKey, portmapper}; +use super::portmapper; /// Fake WireGuard endpoint IP address that means to /// use DERP. When used (in the Node.DERP field), the port number of @@ -149,26 +149,3 @@ pub struct PingResult { /// Did any error occur? pub err: Option, } - -/// A node or peer in the iroh network. -/// -/// Nodes are primarily identified by their [`Node::key`]. -#[derive(Clone, Debug, PartialEq)] -pub struct Node { - /// The public key or PeerID, the primary identifier of this node. - pub key: PublicKey, - /// DNS name of the peer. - pub name: Option, - /// Direct IP addresses of this Node. - /// - /// These are the IP addresses this node is listening on. - pub addresses: Vec, - /// Endpoints on which we think the node is reachable. - /// - /// These are populated from STUN results as well as local LAN addresses. - pub endpoints: Vec, - /// DERP-in-IP:port ("127.3.3.40:N") endpoint. Only stores the `N`. - /// - /// If this nodes expected to be reachable via DERP relaying. - pub derp: Option, -} diff --git a/iroh-net/src/magic_endpoint.rs b/iroh-net/src/magic_endpoint.rs index 8eaa8ff7f5..e464d3b312 100644 --- a/iroh-net/src/magic_endpoint.rs +++ b/iroh-net/src/magic_endpoint.rs @@ -17,7 +17,7 @@ use crate::{ pub use super::magicsock::EndpointInfo as ConnectionInfo; -/// Adress information for a node. +/// Address information for a node. #[derive(Debug)] pub struct NodeAddr { /// The node's public key. diff --git a/iroh-net/src/magicsock.rs b/iroh-net/src/magicsock.rs index 9c0263b2c8..9494ce923b 100644 --- a/iroh-net/src/magicsock.rs +++ b/iroh-net/src/magicsock.rs @@ -2234,29 +2234,21 @@ impl Actor { } #[instrument(skip_all)] - fn add_known_addr(&mut self, addr: NodeAddr) { - let n = config::Node { - name: None, - addresses: addr.endpoints.iter().map(|e| e.ip()).collect(), - endpoints: addr.endpoints.clone(), - key: addr.node_id, - derp: addr.derp_region, - }; - - if self.peer_map.endpoint_for_node_key(&n.key).is_none() { + fn add_known_addr(&mut self, n: NodeAddr) { + if self.peer_map.endpoint_for_node_key(&n.node_id).is_none() { info!( - peer = ?n.key, + peer = ?n.node_id, "inserting peer's endpoint in PeerMap" ); self.peer_map.insert_endpoint(EndpointOptions { msock_sender: self.inner.actor_sender.clone(), - public_key: n.key, - derp_region: n.derp, + public_key: n.node_id, + derp_region: n.derp_region, }); } - if let Some(ep) = self.peer_map.endpoint_for_node_key_mut(&n.key) { - ep.update_from_node(&n); + if let Some(ep) = self.peer_map.endpoint_for_node_key_mut(&n.node_id) { + ep.update_from_node_addr(&n); let id = ep.id; for endpoint in &n.endpoints { self.peer_map diff --git a/iroh-net/src/magicsock/endpoint.rs b/iroh-net/src/magicsock/endpoint.rs index 8e037df41f..101bf82f2f 100644 --- a/iroh-net/src/magicsock/endpoint.rs +++ b/iroh-net/src/magicsock/endpoint.rs @@ -13,8 +13,8 @@ use tokio::{sync::mpsc, time::Instant}; use tracing::{debug, info, trace, warn}; use crate::{ - config, disco, key::PublicKey, magicsock::Timer, net::ip::is_unicast_link_local, stun, - util::derp_only_mode, + config, disco, key::PublicKey, magic_endpoint::NodeAddr, magicsock::Timer, + net::ip::is_unicast_link_local, stun, util::derp_only_mode, }; use super::{ @@ -540,25 +540,25 @@ impl Endpoint { } } - pub fn update_from_node(&mut self, n: &config::Node) { + pub fn update_from_node_addr(&mut self, n: &NodeAddr) { if self.best_addr.is_none() { // we do not have a direct connection, so changing the derp information may // have an effect on our connection status - if self.derp_region.is_none() && n.derp.is_some() { + if self.derp_region.is_none() && n.derp_region.is_some() { // we did not have a relay connection before, but now we do inc!(MagicsockMetrics, num_relay_conns_added) - } else if self.derp_region.is_some() && n.derp.is_none() { + } else if self.derp_region.is_some() && n.derp_region.is_none() { // we had a relay connection before but do not have one now inc!(MagicsockMetrics, num_relay_conns_removed) } } - if n.derp.is_some() { + if n.derp_region.is_some() { debug!( "Changing derp region for {:?} from {:?} to {:?}", - self.public_key, self.derp_region, n.derp + self.public_key, self.derp_region, n.derp_region ); - self.derp_region = n.derp; + self.derp_region = n.derp_region; } for st in self.endpoint_state.values_mut() {