From 3ae6288c71bc96a355ec22160246cbd4c2af1f6f Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 17 Jun 2024 14:10:03 +0200 Subject: [PATCH] refactor(iroh-net)!: Improve magicsock module visibility This audits the public visibility of items in the magicsock module. pub(super) and pub(crate) mean the same thing here, since it is a top-level module. This prefers pub(crate) to make it clear who can see the items. Likewise it marks sub-items pub(crate) as well to make the visibility clear. Some items did not need to be visible outside of the module and have been removed. Unfortunately for ControlMsg this is technically a breaking API change, even though there was no practical use for it, it only ever was pub by accident. --- iroh-net/src/endpoint.rs | 2 +- iroh-net/src/magicsock.rs | 66 +++++++++++++++--------------- iroh-net/src/magicsock/node_map.rs | 2 +- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/iroh-net/src/endpoint.rs b/iroh-net/src/endpoint.rs index 4e6ea7f2fd..a303cd4618 100644 --- a/iroh-net/src/endpoint.rs +++ b/iroh-net/src/endpoint.rs @@ -47,7 +47,7 @@ pub use quinn::{ }; pub use super::magicsock::{ - ConnectionInfo, ConnectionType, ConnectionTypeStream, ControlMsg, DirectAddr, DirectAddrInfo, + ConnectionInfo, ConnectionType, ConnectionTypeStream, DirectAddr, DirectAddrInfo, DirectAddrType, DirectAddrsStream, }; diff --git a/iroh-net/src/magicsock.rs b/iroh-net/src/magicsock.rs index 6a4d260a53..804fb1d071 100644 --- a/iroh-net/src/magicsock.rs +++ b/iroh-net/src/magicsock.rs @@ -77,7 +77,7 @@ mod udp_conn; pub use self::metrics::Metrics; pub use self::node_map::{ - ConnectionType, ConnectionTypeStream, ControlMsg, DirectAddrInfo, NodeInfo as ConnectionInfo, + ConnectionType, ConnectionTypeStream, DirectAddrInfo, NodeInfo as ConnectionInfo, }; pub(super) use self::timer::Timer; @@ -95,37 +95,37 @@ const NETCHECK_REPORT_TIMEOUT: Duration = Duration::from_secs(10); /// Contains options for `MagicSock::listen`. #[derive(derive_more::Debug)] -pub(super) struct Options { +pub(crate) struct Options { /// The port to listen on. /// Zero means to pick one automatically. - pub port: u16, + pub(crate) port: u16, /// Secret key for this node. - pub secret_key: SecretKey, + pub(crate) secret_key: SecretKey, /// The [`RelayMap`] to use, leave empty to not use a relay server. - pub relay_map: RelayMap, + pub(crate) relay_map: RelayMap, /// Path to store known nodes. - pub nodes_path: Option, + pub(crate) nodes_path: Option, /// Optional node discovery mechanism. - pub discovery: Option>, + pub(crate) discovery: Option>, /// A DNS resolver to use for resolving relay URLs. /// /// You can use [`crate::dns::default_resolver`] for a resolver that uses the system's DNS /// configuration. - pub dns_resolver: DnsResolver, + pub(crate) dns_resolver: DnsResolver, /// Proxy configuration. - pub proxy_url: Option, + pub(crate) proxy_url: Option, /// Skip verification of SSL certificates from relay servers /// /// May only be used in tests. #[cfg(any(test, feature = "test-utils"))] - pub insecure_skip_relay_cert_verify: bool, + pub(crate) insecure_skip_relay_cert_verify: bool, } impl Default for Options { @@ -146,13 +146,13 @@ impl Default for Options { /// Contents of a relay message. Use a SmallVec to avoid allocations for the very /// common case of a single packet. -pub(super) type RelayContents = SmallVec<[Bytes; 1]>; +type RelayContents = SmallVec<[Bytes; 1]>; /// Handle for [`MagicSock`]. /// /// Dereferences to [`MagicSock`], and handles closing. #[derive(Clone, Debug, derive_more::Deref)] -pub(super) struct Handle { +pub(crate) struct Handle { #[deref(forward)] msock: Arc, // Empty when closed @@ -170,7 +170,7 @@ pub(super) struct Handle { /// means any QUIC endpoints on top will be sharing as much information about nodes as /// possible. #[derive(derive_more::Debug)] -pub(super) struct MagicSock { +pub(crate) struct MagicSock { actor_sender: mpsc::Sender, relay_actor_sender: mpsc::Sender, /// String representation of the node_id of this node. @@ -246,19 +246,19 @@ pub(super) struct MagicSock { impl MagicSock { /// Creates a magic [`MagicSock`] listening on [`Options::port`]. - pub async fn spawn(opts: Options) -> Result { + pub(crate) async fn spawn(opts: Options) -> Result { Handle::new(opts).await } /// Returns the relay node we are connected to, that has the best latency. /// /// If `None`, then we are not connected to any relay nodes. - pub fn my_relay(&self) -> Option { + pub(crate) fn my_relay(&self) -> Option { self.my_relay.get() } /// Get the current proxy configuration. - pub fn proxy_url(&self) -> Option<&Url> { + pub(crate) fn proxy_url(&self) -> Option<&Url> { self.proxy_url.as_ref() } @@ -282,24 +282,24 @@ impl MagicSock { } /// Get the cached version of the Ipv4 and Ipv6 addrs of the current connection. - pub fn local_addr(&self) -> (SocketAddr, Option) { + pub(crate) fn local_addr(&self) -> (SocketAddr, Option) { *self.local_addrs.read().expect("not poisoned") } /// Returns `true` if we have at least one candidate address where we can send packets to. - pub fn has_send_address(&self, node_key: PublicKey) -> bool { + pub(crate) fn has_send_address(&self, node_key: PublicKey) -> bool { self.connection_info(node_key) .map(|info| info.has_send_address()) .unwrap_or(false) } /// Retrieve connection information about nodes in the network. - pub fn connection_infos(&self) -> Vec { + pub(crate) fn connection_infos(&self) -> Vec { self.node_map.node_infos(Instant::now()) } /// Retrieve connection information about a node in the network. - pub fn connection_info(&self, node_id: NodeId) -> Option { + pub(crate) fn connection_info(&self, node_id: NodeId) -> Option { self.node_map.node_info(node_id) } @@ -317,7 +317,7 @@ impl MagicSock { /// /// To get the current direct addresses, drop the stream after the first item was /// received. - pub fn direct_addresses(&self) -> DirectAddrsStream { + pub(crate) fn direct_addresses(&self) -> DirectAddrsStream { DirectAddrsStream { initial: Some(self.endpoints.get()), inner: self.endpoints.watch().into_stream(), @@ -328,7 +328,7 @@ impl MagicSock { /// /// Note that this can be used to wait for the initial home relay to be known. If the home /// relay is known at this point, it will be the first item in the stream. - pub fn watch_home_relay(&self) -> impl Stream { + pub(crate) fn watch_home_relay(&self) -> impl Stream { let current = futures_lite::stream::iter(self.my_relay()); let changes = self .my_relay @@ -351,7 +351,7 @@ impl MagicSock { /// /// Will return an error if there is no address information known about the /// given `node_id`. - pub fn conn_type_stream(&self, node_id: NodeId) -> Result { + pub(crate) fn conn_type_stream(&self, node_id: NodeId) -> Result { self.node_map.conn_type_stream(node_id) } @@ -359,7 +359,7 @@ impl MagicSock { /// /// Note this is a user-facing API and does not wrap the [`SocketAddr`] in a /// [`QuicMappedAddr`] as we do internally. - pub fn get_mapping_addr(&self, node_id: NodeId) -> Option { + pub(crate) fn get_mapping_addr(&self, node_id: NodeId) -> Option { self.node_map .get_quic_mapped_addr_for_node_key(node_id) .map(|a| a.0) @@ -367,22 +367,22 @@ impl MagicSock { /// Add addresses for a node to the magic socket's addresbook. #[instrument(skip_all, fields(me = %self.me))] - pub fn add_node_addr(&self, addr: NodeAddr) { + pub(crate) fn add_node_addr(&self, addr: NodeAddr) { self.node_map.add_node_addr(addr); } /// Get a reference to the DNS resolver used in this [`MagicSock`]. - pub fn dns_resolver(&self) -> &DnsResolver { + pub(crate) fn dns_resolver(&self) -> &DnsResolver { &self.dns_resolver } /// Reference to optional discovery service - pub fn discovery(&self) -> Option<&dyn Discovery> { + pub(crate) fn discovery(&self) -> Option<&dyn Discovery> { self.discovery.as_ref().map(Box::as_ref) } /// Call to notify the system of potential network changes. - pub async fn network_change(&self) { + pub(crate) async fn network_change(&self) { self.actor_sender .send(ActorMessage::NetworkChange) .await @@ -1458,7 +1458,7 @@ impl Handle { /// Polling the socket ([`AsyncUdpSocket::poll_recv`]) will return [`Poll::Pending`] /// indefinitely after this call. #[instrument(skip_all, fields(me = %self.msock.me))] - pub async fn close(&self) -> Result<()> { + pub(crate) async fn close(&self) -> Result<()> { if self.msock.is_closed() { return Ok(()); } @@ -2482,7 +2482,7 @@ fn split_packets(transmits: &[quinn_udp::Transmit]) -> RelayContents { /// Splits a packet into its component items. #[derive(Debug)] -pub(super) struct PacketSplitIter { +struct PacketSplitIter { bytes: Bytes, } @@ -2490,7 +2490,7 @@ impl PacketSplitIter { /// Create a new PacketSplitIter from a packet. /// /// Returns an error if the packet is too big. - pub fn new(bytes: Bytes) -> Self { + fn new(bytes: Bytes) -> Self { Self { bytes } } @@ -2685,7 +2685,7 @@ struct NetInfo { impl NetInfo { /// reports whether `self` and `other` are basically equal, ignoring changes in relay ServerLatency & RelayLatency. - pub fn basically_equal(&self, other: &Self) -> bool { + fn basically_equal(&self, other: &Self) -> bool { let eq_icmp_v4 = match (self.working_icmp_v4, other.working_icmp_v4) { (Some(slf), Some(other)) => slf == other, _ => true, // ignore for comparison if only one report had this info @@ -2708,7 +2708,7 @@ impl NetInfo { } #[cfg(test)] -pub(crate) mod tests { +mod tests { use anyhow::Context; use futures_lite::StreamExt; use iroh_test::CallOnDrop; diff --git a/iroh-net/src/magicsock/node_map.rs b/iroh-net/src/magicsock/node_map.rs index 7e6e2e74fc..4e9dd8f7f3 100644 --- a/iroh-net/src/magicsock/node_map.rs +++ b/iroh-net/src/magicsock/node_map.rs @@ -31,7 +31,7 @@ use crate::{ mod best_addr; mod node_state; -pub use node_state::{ConnectionType, ControlMsg, DirectAddrInfo, NodeInfo}; +pub use node_state::{ConnectionType, DirectAddrInfo, NodeInfo}; pub(super) use node_state::{DiscoPingPurpose, PingAction, PingRole, SendPing}; /// Number of nodes that are inactive for which we keep info about. This limit is enforced