Skip to content

Commit

Permalink
refactor(iroh-net): remove NetworkMap (#1447)
Browse files Browse the repository at this point in the history
## Description

While having another look at the code when thinking about #1440 I
realized that the `NetworkMap`is never used apart from serving as a
source of entries for the `PeerMap`.

So I removed it. Looks much simpler now, and tests passed for me
locally. Not sure if I haven't missed something.

## Notes & open questions

* We never remove entries from the `peer_map`. This is already the case
before this PR if I'm not missing something, because we never removed
entries from the `NetworkMap`, and only the latter would ever have
triggered a removal of entries from the `peer_map`.

## Change checklist

- [x] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.
  • Loading branch information
Frando committed Sep 2, 2023
1 parent 1bcc765 commit bc26321
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 210 deletions.
1 change: 0 additions & 1 deletion iroh-net/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ pub mod magicsock;
pub mod metrics;
pub mod net;
pub mod netcheck;
pub mod netmap;
pub mod ping;
pub mod portmapper;
pub mod stun;
Expand Down
94 changes: 34 additions & 60 deletions iroh-net/src/magic_endpoint.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
//! An endpoint that leverages a [quinn::Endpoint] backed by a [magicsock::MagicSock].

use std::{
net::SocketAddr,
sync::{Arc, Mutex},
time::Duration,
};
use std::{net::SocketAddr, sync::Arc, time::Duration};

use anyhow::{anyhow, ensure, Context, Result};
use quinn_proto::VarInt;
Expand All @@ -16,12 +12,22 @@ use crate::{
derp::DerpMap,
key::{PublicKey, SecretKey},
magicsock::{self, Callbacks, MagicSock},
netmap::NetworkMap,
tls,
};

pub use super::magicsock::EndpointInfo as ConnectionInfo;

/// Adress information for a node.
#[derive(Debug)]
pub struct NodeAddr {
/// The node's public key.
pub node_id: PublicKey,
/// The node's home DERP region.
pub derp_region: Option<u16>,
/// Socket addresses where this node might be reached directly.
pub endpoints: Vec<SocketAddr>,
}

/// Builder for [MagicEndpoint]
#[derive(Debug)]
pub struct MagicEndpointBuilder {
Expand Down Expand Up @@ -202,7 +208,6 @@ pub struct MagicEndpoint {
secret_key: Arc<SecretKey>,
msock: MagicSock,
endpoint: quinn::Endpoint,
netmap: Arc<Mutex<NetworkMap>>,
keylog: bool,
}

Expand Down Expand Up @@ -245,7 +250,6 @@ impl MagicEndpoint {
secret_key: Arc::new(secret_key),
msock,
endpoint,
netmap: Arc::new(Mutex::new(NetworkMap { peers: vec![] })),
keylog,
})
}
Expand Down Expand Up @@ -296,8 +300,8 @@ impl MagicEndpoint {
/// currently communicating with that node over a `Direct` (UDP) or `Relay` (DERP) connection.
///
/// Connections are currently only pruned on user action (when we explicitly add a new address
/// to the internal [`crate::netmap::NetworkMap`] in [`MagicEndpoint::add_known_addrs`]), so
/// these connections are not necessarily active connections.
/// to the internal addressbook through [`MagicEndpoint::add_known_addrs`]), so these connections
/// are not necessarily active connections.
pub async fn connection_infos(&self) -> anyhow::Result<Vec<ConnectionInfo>> {
self.msock.tracked_endpoints().await
}
Expand Down Expand Up @@ -331,14 +335,21 @@ impl MagicEndpoint {
derp_region: Option<u16>,
known_addrs: &[SocketAddr],
) -> Result<quinn::Connection> {
if derp_region.is_some() || !known_addrs.is_empty() {
self.add_known_addrs(node_id, derp_region, known_addrs)
.await?;
}

let addr = self.msock.get_mapping_addr(&node_id).await.ok_or_else(|| {
anyhow!("failed to retrieve the mapped address from the magic socket")
})?;
self.add_known_addrs(node_id, derp_region, known_addrs)
.await?;

let addr = self.msock.get_mapping_addr(&node_id).await;
let Some(addr) = addr else {
return Err(match (known_addrs.is_empty(), derp_region) {
(true, None) => {
anyhow!("No UDP addresses or DERP region provided. Unable to dial peer {node_id:?}")
}
(true, Some(region)) if !self.msock.has_derp_region(region).await => {
anyhow!("No UDP addresses provided and we do not have any DERP configuration for DERP region {region}. Unable to dial peer {node_id:?}")
}
_ => anyhow!("Failed to retrieve the mapped address from the magic socket. Unable to dial peer {node_id:?}")
});
};

let client_config = {
let alpn_protocols = vec![alpn.to_vec()];
Expand Down Expand Up @@ -384,49 +395,12 @@ impl MagicEndpoint {
derp_region: Option<u16>,
endpoints: &[SocketAddr],
) -> Result<()> {
match (endpoints.is_empty(), derp_region) {
(true, None) => {
anyhow::bail!(
"No UDP addresses or DERP region provided. Unable to dial peer {node_id:?}"
);
}
(true, Some(region)) if !self.msock.has_derp_region(region).await => {
anyhow::bail!("No UDP addresses provided and we do not have any DERP configuration for DERP region {region}, any hole punching required to establish a connection will not be possible.");
}
(false, None) => {
tracing::warn!("No DERP region provided, any hole punching required to establish a connection will not be possible.");
}
(false, Some(region)) if !self.msock.has_derp_region(region).await => {
tracing::warn!("We do not have any DERP configuration for DERP region {region}, any hole punching required to establish a connection will not be possible.");
}
_ => {}
}

let netmap = {
let mut netmap = self.netmap.lock().unwrap();
let node = netmap.peers.iter_mut().find(|peer| peer.key == node_id);
if let Some(node) = node {
for endpoint in endpoints {
if !node.endpoints.contains(endpoint) {
node.endpoints.push(*endpoint);
node.addresses.push(endpoint.ip());
}
}
} else {
let endpoints = endpoints.to_vec();
let addresses = endpoints.iter().map(|ep| ep.ip()).collect();
let node = config::Node {
name: None,
addresses,
endpoints,
key: node_id,
derp: derp_region,
};
netmap.peers.push(node)
}
netmap.clone()
let addr = NodeAddr {
node_id,
derp_region,
endpoints: endpoints.to_vec(),
};
self.msock.set_network_map(netmap).await?;
self.msock.add_known_addr(addr).await?;
Ok(())
}
Expand Down
Loading

0 comments on commit bc26321

Please sign in to comment.