Skip to content

Commit

Permalink
refactor(iroh-net)!: Rename endpoint for nodes to node_state (#2222)
Browse files Browse the repository at this point in the history
## Description

We still have too many things named "endpoint", this targets cleaning
up the naming of the state for each node that is stored in the NodeMap.
This is now called `NodeState` instead of `Endpoint`.  All related APIs
now talk about nodes and node states instead of endpoints.

Another minor cleanup is in the `NodeState` we had private fields with
accessor functions, except for one field which was directly
accessible.  This is migrated to accessor functions for consistency as
well.

Finally it marks the visibility of some functions more explicitly, the
visibility of those was already as such.  This makes it easier to work
and realise the impact changes have however.

## Breaking Changes

* `MagicSock::tracked_endpoints` -> `MagicSock::connection_infos`
* `MagicSock::tracked_endpoint` -> `MagicSock::connection_info`
* `magicsock::EndpointInfo` -> `magicsock::ConnectionInfo`

## Notes & open questions

Finally had to courage to change this, it's much less worse than I
feared and to me it really helps calling this `NodeState`.  I find
it a noticeable improvement.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
- [x] All breaking changes documented.
  • Loading branch information
flub authored Apr 23, 2024
1 parent 93bcaa5 commit 26e4564
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 185 deletions.
4 changes: 2 additions & 2 deletions iroh-cli/src/commands/doctor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use iroh::{
dns::default_resolver,
key::{PublicKey, SecretKey},
magic_endpoint,
magicsock::EndpointInfo,
magicsock::ConnectionInfo,
netcheck, portmapper,
relay::{RelayMap, RelayMode, RelayUrl},
util::AbortingJoinHandle,
Expand Down Expand Up @@ -390,7 +390,7 @@ impl Gui {
.unwrap_or_else(|| "unknown".to_string())
};
let msg = match endpoint.connection_info(*node_id) {
Some(EndpointInfo {
Some(ConnectionInfo {
relay_url,
conn_type,
latency,
Expand Down
6 changes: 3 additions & 3 deletions iroh-net/src/magic_endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{
tls, NodeId,
};

pub use super::magicsock::{EndpointInfo as ConnectionInfo, LocalEndpointsStream};
pub use super::magicsock::{ConnectionInfo, LocalEndpointsStream};

pub use iroh_base::node_addr::{AddrInfo, NodeAddr};

Expand Down Expand Up @@ -376,7 +376,7 @@ impl MagicEndpoint {
/// to the internal addressbook through [`MagicEndpoint::add_node_addr`]), so these connections
/// are not necessarily active connections.
pub fn connection_infos(&self) -> Vec<ConnectionInfo> {
self.msock.tracked_endpoints()
self.msock.connection_infos()
}

/// Get connection information about a specific node.
Expand All @@ -385,7 +385,7 @@ impl MagicEndpoint {
/// latency, and its [`crate::magicsock::ConnectionType`], which let's us know if we are
/// currently communicating with that node over a `Direct` (UDP) or `Relay` (relay) connection.
pub fn connection_info(&self, node_id: PublicKey) -> Option<ConnectionInfo> {
self.msock.tracked_endpoint(node_id)
self.msock.connection_info(node_id)
}

pub(crate) fn cancelled(&self) -> WaitForCancellationFuture<'_> {
Expand Down
32 changes: 16 additions & 16 deletions iroh-net/src/magicsock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub use crate::net::UdpSocket;

pub use self::metrics::Metrics;
pub use self::node_map::{
ConnectionType, ConnectionTypeStream, ControlMsg, DirectAddrInfo, EndpointInfo,
ConnectionType, ConnectionTypeStream, ControlMsg, DirectAddrInfo, NodeInfo as ConnectionInfo,
};
pub use self::timer::Timer;

Expand Down Expand Up @@ -153,7 +153,7 @@ pub(crate) type RelayContents = SmallVec<[Bytes; 1]>;
/// This is responsible for routing packets to nodes based on node IDs, it will initially
/// route packets via a relay and transparently try and establish a node-to-node
/// connection and upgrade to it. It will also keep looking for better connections as the
/// network details of both endpoints change.
/// network details of both nodes change.
///
/// It is usually only necessary to use a single [`MagicSock`] instance in an application, it
/// means any QUIC endpoints on top will be sharing as much information about nodes as
Expand Down Expand Up @@ -342,7 +342,7 @@ impl Inner {
let mut transmits_sent = 0;
match self
.node_map
.get_send_addrs_for_quic_mapped_addr(&dest, self.ipv6_reported.load(Ordering::Relaxed))
.get_send_addrs(&dest, self.ipv6_reported.load(Ordering::Relaxed))
{
Some((public_key, udp_addr, relay_url, mut msgs)) => {
let mut pings_sent = false;
Expand Down Expand Up @@ -443,10 +443,10 @@ impl Inner {
Poll::Ready(Ok(transmits_sent))
}
None => {
error!(dst=%dest, "no endpoint for mapped address");
error!(dst=%dest, "no node_state for mapped address");
Poll::Ready(Err(io::Error::new(
io::ErrorKind::NotConnected,
"trying to send to unknown endpoint",
"trying to send to unknown node",
)))
}
}
Expand Down Expand Up @@ -720,15 +720,15 @@ impl Inner {
let handled = self.node_map.handle_ping(*sender, addr.clone(), dm.tx_id);
match handled.role {
PingRole::Duplicate => {
debug!(%src, tx = %hex::encode(dm.tx_id), "received ping: endpoint already confirmed, skip");
debug!(%src, tx = %hex::encode(dm.tx_id), "received ping: path already confirmed, skip");
return;
}
PingRole::LikelyHeartbeat => {}
PingRole::NewEndpoint => {
debug!(%src, tx = %hex::encode(dm.tx_id), "received ping: new endpoint");
PingRole::NewPath => {
debug!(%src, tx = %hex::encode(dm.tx_id), "received ping: new path");
}
PingRole::Reactivate => {
debug!(%src, tx = %hex::encode(dm.tx_id), "received ping: endpoint active");
debug!(%src, tx = %hex::encode(dm.tx_id), "received ping: path active");
}
}

Expand Down Expand Up @@ -1310,13 +1310,13 @@ impl MagicSock {
}

/// Retrieve connection information about nodes in the network.
pub fn tracked_endpoints(&self) -> Vec<EndpointInfo> {
self.inner.node_map.endpoint_infos(Instant::now())
pub fn connection_infos(&self) -> Vec<ConnectionInfo> {
self.inner.node_map.node_infos(Instant::now())
}

/// Retrieve connection information about a node in the network.
pub fn tracked_endpoint(&self, node_key: PublicKey) -> Option<EndpointInfo> {
self.inner.node_map.endpoint_info(&node_key)
pub fn connection_info(&self, node_key: PublicKey) -> Option<ConnectionInfo> {
self.inner.node_map.node_info(&node_key)
}

/// Returns the local endpoints as a stream.
Expand Down Expand Up @@ -1719,7 +1719,7 @@ impl Actor {
// TODO: this might trigger too many packets at once, pace this

self.inner.node_map.prune_inactive();
let msgs = self.inner.node_map.endpoints_stayin_alive();
let msgs = self.inner.node_map.nodes_stayin_alive();
self.handle_ping_actions(msgs).await;
}
_ = endpoints_update_receiver.changed() => {
Expand Down Expand Up @@ -2281,7 +2281,7 @@ impl Actor {
/// This is called when connectivity changes enough that we no longer trust the old routes.
#[instrument(skip_all, fields(me = %self.inner.me))]
fn reset_endpoint_states(&mut self) {
self.inner.node_map.reset_endpoint_states()
self.inner.node_map.reset_node_states()
}

/// Tells the relay actor to close stale relay connections.
Expand Down Expand Up @@ -2605,7 +2605,7 @@ pub(crate) mod tests {
fn tracked_endpoints(&self) -> Vec<PublicKey> {
self.endpoint
.magic_sock()
.tracked_endpoints()
.connection_infos()
.into_iter()
.map(|ep| ep.node_id)
.collect()
Expand Down
Loading

0 comments on commit 26e4564

Please sign in to comment.