Skip to content

Commit

Permalink
refactor(iroh-net): remove iroh-net::config::Node since limited to …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
divagant-martian authored Sep 18, 2023
1 parent f2f3ead commit 00d0150
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 48 deletions.
25 changes: 1 addition & 24 deletions iroh-net/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -149,26 +149,3 @@ pub struct PingResult {
/// Did any error occur?
pub err: Option<String>,
}

/// 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<String>,
/// Direct IP addresses of this Node.
///
/// These are the IP addresses this node is listening on.
pub addresses: Vec<IpAddr>,
/// Endpoints on which we think the node is reachable.
///
/// These are populated from STUN results as well as local LAN addresses.
pub endpoints: Vec<SocketAddr>,
/// 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<u16>,
}
2 changes: 1 addition & 1 deletion iroh-net/src/magic_endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 7 additions & 15 deletions iroh-net/src/magicsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions iroh-net/src/magicsock/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 00d0150

Please sign in to comment.