feat(iroh-relay)!: adjust APIs to make it easier to create RelayMaps from lists of RelayUrls (#3292)

## Description

In our examples and tests, it is enough to build `RelayMap`s from a
single `RelayUrl`, but our users will more likely be in the situation
where they have multiple relays.

Now we have `From<RelayUrl>`, `From<(RelayUrl, u16)>`,
`FromIterator<RelayUrl>` `FromIterator<(RelayUrl, u16)>` impls to create
a `RelayMap`.

Also, `From<RelayUrl>` and `From<(RelayUrl, u16)>` to create a
`RelayNode`.

## Breaking Changes

- iroh
    - remove
- `pub fn default_from_node(url: RelayUrl, stun_port: u16) -> RelayMap`
    - change
- `pub fn from_url(url: RelayUrl) -> RelayMap`, use `From<RelayUrl>`
instead
       


## Change checklist
<!-- Remove any that are not relevant. -->
- [x] Self-review.
- [x] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [x] All breaking changes documented.
- [x] List all breaking changes in the above "Breaking Changes" section.

---------

Co-authored-by: “ramfox” <“kasey@n0.computer”>
Co-authored-by: Floris Bruynooghe <flub@n0.computer>

Author: 3 people

iroh-relay/src/relay_map.rs

@@ -2,7 +2,6 @@
 
 use std::{collections::BTreeMap, fmt, sync::Arc};
 
-use anyhow::{ensure, Result};
 use iroh_base::RelayUrl;
 use serde::{Deserialize, Serialize};
 
@@ -52,50 +51,53 @@ impl RelayMap {
     pub fn is_empty(&self) -> bool {
         self.nodes.is_empty()
     }
+}
 
-    /// Creates a new [`RelayMap`] with a single relay server configured.
-    ///
-    /// Allows to set a custom STUN port and different IP addresses for IPv4 and IPv6.
-    /// If IP addresses are provided, no DNS lookup will be performed.
-    ///
-    /// Sets the port to the default [`DEFAULT_RELAY_QUIC_PORT`].
-    pub fn default_from_node(url: RelayUrl, stun_port: u16) -> Self {
-        let mut nodes = BTreeMap::new();
-        nodes.insert(
-            url.clone(),
-            RelayNode {
-                url,
-                stun_only: false,
-                stun_port,
-                quic: Some(RelayQuicConfig::default()),
-            }
-            .into(),
-        );
-
-        RelayMap {
-            nodes: Arc::new(nodes),
+impl FromIterator<RelayNode> for RelayMap {
+    fn from_iter<T: IntoIterator<Item = RelayNode>>(iter: T) -> Self {
+        Self {
+            nodes: Arc::new(
+                iter.into_iter()
+                    .map(|node| (node.url.clone(), Arc::new(node)))
+                    .collect(),
+            ),
         }
     }
+}
 
-    /// Returns a [`RelayMap`] from a [`RelayUrl`].
+impl From<RelayUrl> for RelayMap {
+    /// Creates a [`RelayMap`] from a [`RelayUrl`].
     ///
-    /// This will use the default STUN port, the default QUIC port
-    /// (as defined by the `iroh-relay` crate) and IP addresses
-    /// resolved from the URL's host name via DNS.
-    /// relay nodes are specified at <../../docs/relay_nodes.md>
-    pub fn from_url(url: RelayUrl) -> Self {
-        Self::default_from_node(url, DEFAULT_STUN_PORT)
+    /// The [`RelayNode`]s in the [`RelayMap`] will have the default STUN and QUIC address
+    /// discovery ports.
+    fn from(value: RelayUrl) -> Self {
+        Self {
+            nodes: Arc::new([(value.clone(), Arc::new(value.into()))].into()),
+        }
+    }
+}
+
+impl From<RelayNode> for RelayMap {
+    fn from(value: RelayNode) -> Self {
+        Self {
+            nodes: Arc::new([(value.url.clone(), Arc::new(value))].into()),
+        }
     }
+}
 
-    /// Constructs the [`RelayMap`] from an iterator of [`RelayNode`]s.
-    pub fn from_nodes<I: Into<Arc<RelayNode>>>(value: impl IntoIterator<Item = I>) -> Result<Self> {
-        let mut map = BTreeMap::new();
-        for node in value.into_iter() {
-            let node = node.into();
-            ensure!(!map.contains_key(&node.url), "Duplicate node url");
-            map.insert(node.url.clone(), node);
+impl FromIterator<RelayUrl> for RelayMap {
+    /// Creates a [`RelayMap`] from an iterator of [`RelayUrl`].
+    ///
+    /// The [`RelayNode`]s in the [`RelayMap`] will have the default STUN and QUIC address
+    /// discovery ports.
+    fn from_iter<T: IntoIterator<Item = RelayUrl>>(iter: T) -> Self {
+        Self {
+            nodes: Arc::new(
+                iter.into_iter()
+                    .map(|url| (url.clone(), Arc::new(url.into())))
+                    .collect(),
+            ),
         }
-        Ok(RelayMap { nodes: map.into() })
     }
 }
 
@@ -131,6 +133,17 @@ pub struct RelayNode {
     pub quic: Option<RelayQuicConfig>,
 }
 
+impl From<RelayUrl> for RelayNode {
+    fn from(value: RelayUrl) -> Self {
+        Self {
+            url: value,
+            stun_only: false,
+            stun_port: DEFAULT_STUN_PORT,
+            quic: quic_config(),
+        }
+    }
+}
+
 fn quic_config() -> Option<RelayQuicConfig> {
     Some(RelayQuicConfig::default())
 }

iroh/bench/src/iroh.rs

@@ -7,7 +7,7 @@ use anyhow::{Context, Result};
 use bytes::Bytes;
 use iroh::{
     endpoint::{Connection, ConnectionError, RecvStream, SendStream, TransportConfig},
-    Endpoint, NodeAddr, RelayMap, RelayMode, RelayUrl,
+    Endpoint, NodeAddr, RelayMode, RelayUrl,
 };
 use tracing::{trace, warn};
 
@@ -25,9 +25,9 @@ pub fn server_endpoint(
 ) -> (NodeAddr, Endpoint) {
     let _guard = rt.enter();
     rt.block_on(async move {
-        let relay_mode = relay_url.clone().map_or(RelayMode::Disabled, |url| {
-            RelayMode::Custom(RelayMap::from_url(url))
-        });
+        let relay_mode = relay_url
+            .clone()
+            .map_or(RelayMode::Disabled, |url| RelayMode::Custom(url.into()));
 
         #[allow(unused_mut)]
         let mut builder = Endpoint::builder();
@@ -87,9 +87,9 @@ pub async fn connect_client(
     relay_url: Option<RelayUrl>,
     opt: Opt,
 ) -> Result<(Endpoint, Connection)> {
-    let relay_mode = relay_url.clone().map_or(RelayMode::Disabled, |url| {
-        RelayMode::Custom(RelayMap::from_url(url))
-    });
+    let relay_mode = relay_url
+        .clone()
+        .map_or(RelayMode::Disabled, |url| RelayMode::Custom(url.into()));
     #[allow(unused_mut)]
     let mut builder = Endpoint::builder();
     #[cfg(feature = "local-relay")]

iroh/examples/transfer.rs

@@ -10,7 +10,7 @@ use indicatif::HumanBytes;
 use iroh::{
     discovery::{dns::DnsDiscovery, pkarr::PkarrPublisher},
     endpoint::{ConnectionError, PathSelection},
-    Endpoint, NodeAddr, RelayMap, RelayMode, RelayUrl, SecretKey,
+    Endpoint, NodeAddr, RelayMode, RelayUrl, SecretKey,
 };
 use iroh_base::ticket::NodeTicket;
 use tracing::info;
@@ -106,8 +106,7 @@ async fn provide(
     let relay_mode = match relay_url {
         Some(relay_url) => {
             let relay_url = RelayUrl::from_str(&relay_url)?;
-            let relay_map = RelayMap::from_url(relay_url);
-            RelayMode::Custom(relay_map)
+            RelayMode::Custom(relay_url.into())
         }
         None => RelayMode::Default,
     };
@@ -229,8 +228,7 @@ async fn fetch(
     let relay_mode = match relay_url {
         Some(relay_url) => {
             let relay_url = RelayUrl::from_str(&relay_url)?;
-            let relay_map = RelayMap::from_url(relay_url);
-            RelayMode::Custom(relay_map)
+            RelayMode::Custom(relay_url.into())
         }
         None => RelayMode::Default,
     };

iroh/src/defaults.rs

@@ -36,12 +36,11 @@ pub mod prod {
 
     /// Get the default [`RelayMap`].
     pub fn default_relay_map() -> RelayMap {
-        RelayMap::from_nodes([
+        RelayMap::from_iter([
             default_na_relay_node(),
             default_eu_relay_node(),
             default_ap_relay_node(),
         ])
-        .expect("default nodes invalid")
     }
 
     /// Get the default [`RelayNode`] for NA.
@@ -104,8 +103,7 @@ pub mod staging {
 
     /// Get the default [`RelayMap`].
     pub fn default_relay_map() -> RelayMap {
-        RelayMap::from_nodes([default_na_relay_node(), default_eu_relay_node()])
-            .expect("default nodes invalid")
+        RelayMap::from_iter([default_na_relay_node(), default_eu_relay_node()])
     }
 
     /// Get the default [`RelayNode`] for NA.

iroh/src/net_report.rs

@@ -877,11 +877,9 @@ pub(crate) mod stun_utils {
 mod test_utils {
     //! Creates a relay server against which to perform tests
 
-    use std::sync::Arc;
-
     use iroh_relay::{server, RelayNode, RelayQuicConfig};
 
-    pub(crate) async fn relay() -> (server::Server, Arc<RelayNode>) {
+    pub(crate) async fn relay() -> (server::Server, RelayNode) {
         let server = server::Server::spawn(server::testing::server_config())
             .await
             .expect("should serve relay");
@@ -895,7 +893,7 @@ mod test_utils {
             quic,
         };
 
-        (server, Arc::new(node_desc))
+        (server, node_desc)
     }
 
     /// Create a [`crate::RelayMap`] of the given size.
@@ -910,8 +908,7 @@ mod test_utils {
             servers.push(relay_server);
             nodes.push(node);
         }
-        let map = crate::RelayMap::from_nodes(nodes).expect("unuque urls");
-        (servers, map)
+        (servers, crate::RelayMap::from_iter(nodes))
     }
 }
 
@@ -983,7 +980,7 @@ mod tests {
                     quic: None,
                 }
             });
-            RelayMap::from_nodes(nodes).expect("generated invalid nodes")
+            RelayMap::from_iter(nodes)
         }
 
         /// Sets up a simple STUN server binding to `0.0.0.0:0`.

iroh/src/net_report/reportgen.rs

@@ -1455,6 +1455,8 @@ mod tests {
         let (_server_b, relay_b) = test_utils::relay().await;
 
         let mut report = Report::default();
+        let relay_a = Arc::new(relay_a);
+        let relay_b = Arc::new(relay_b);
 
         // A STUN IPv4 probe from the the first relay server.
         let probe_report_a = ProbeReport {
@@ -1539,6 +1541,8 @@ mod tests {
     async fn test_update_report_icmp() {
         let (_server_a, relay_a) = test_utils::relay().await;
         let (_server_b, relay_b) = test_utils::relay().await;
+        let relay_a = Arc::new(relay_a);
+        let relay_b = Arc::new(relay_b);
 
         let mut report = Report::default();
 
@@ -1652,7 +1656,7 @@ mod tests {
         let addr = server.stun_addr().expect("test relay serves stun");
         let probe = Probe::IcmpV4 {
             delay: Duration::from_secs(0),
-            node,
+            node: Arc::new(node),
         };
 
         // A single ICMP packet might get lost.  Try several and take the first.
@@ -1717,6 +1721,7 @@ mod tests {
     #[traced_test]
     async fn test_quic_probe() -> TestResult {
         let (server, relay) = test_utils::relay().await;
+        let relay = Arc::new(relay);
         let client_config = iroh_relay::client::make_dangerous_client_config();
         let ep = quinn::Endpoint::client(SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 0))?;
         let client_addr = ep.local_addr()?;
@@ -1730,7 +1735,7 @@ mod tests {
         let port = server.quic_addr().unwrap().port();
         let probe = Probe::QuicIpv4 {
             delay: Duration::from_secs(0),
-            node: relay.clone(),
+            node: relay,
         };
         let probe = match run_quic_probe(
             quic_addr_disc,

iroh/src/test_utils.rs

@@ -101,13 +101,14 @@ pub async fn run_relay_server_with(
     let quic = server
         .quic_addr()
         .map(|addr| RelayQuicConfig { port: addr.port() });
-    let m = RelayMap::from_nodes([RelayNode {
+    let n: RelayMap = RelayNode {
         url: url.clone(),
         stun_only: false,
         stun_port: server.stun_addr().map_or(DEFAULT_STUN_PORT, |s| s.port()),
         quic,
-    }])?;
-    Ok((m, url, server))
+    }
+    .into();
+    Ok((n, url, server))
 }
 
 pub(crate) mod dns_and_pkarr_servers {