refactor: remove AddrInfo (#3024)

## Description

Simplify type structures

TODO

- [x] fix tests
- [x] resolve ticket breakage

## Breaking Changes

- remove `iroh_base::node_addr::AddrInfo`
- remove `iroh_base::node_addr::AddrInfoOptions`
- introduce `ticket` feature for `iroh_base`, to use `iroh_base::ticket`

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] 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.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.

Author: dignifiedquire

iroh-base/Cargo.toml

@@ -46,8 +46,9 @@ serde_json = "1"
 serde_test = "1"
 
 [features]
-default = ["hash", "base32", "relay"]
+default = ["hash", "ticket", "relay"]
 hash = ["dep:blake3", "dep:data-encoding", "dep:postcard", "dep:derive_more", "base32"]
+ticket = ["base32", "key"]
 base32 = ["dep:data-encoding", "dep:postcard"]
 redb = ["dep:redb"]
 key = ["dep:ed25519-dalek", "dep:once_cell", "dep:rand", "dep:rand_core", "dep:ssh-key", "dep:ttl_cache", "dep:aead", "dep:crypto_box", "dep:zeroize", "dep:url", "dep:derive_more", "dep:getrandom"]

iroh-base/src/lib.rs

@@ -11,7 +11,7 @@ pub mod key;
 pub mod node_addr;
 #[cfg(feature = "relay")]
 pub mod relay_map;
-#[cfg(any(feature = "relay", feature = "key"))]
+#[cfg(feature = "relay")]
 mod relay_url;
-#[cfg(feature = "base32")]
+#[cfg(feature = "ticket")]
 pub mod ticket;

iroh-base/src/node_addr.rs

@@ -41,31 +41,34 @@ pub use crate::relay_url::RelayUrl;
 pub struct NodeAddr {
     /// The node's identifier.
     pub node_id: NodeId,
-    /// Addressing information to connect to [`Self::node_id`].
-    pub info: AddrInfo,
+    /// The node's home relay url.
+    pub relay_url: Option<RelayUrl>,
+    /// Socket addresses where the peer might be reached directly.
+    pub direct_addresses: BTreeSet<SocketAddr>,
 }
 
 impl NodeAddr {
-    /// Creates a new [`NodeAddr`] with empty [`AddrInfo`].
+    /// Creates a new [`NodeAddr`] with no `relay_url` and no `direct_addresses`.
     pub fn new(node_id: PublicKey) -> Self {
         NodeAddr {
             node_id,
-            info: Default::default(),
+            relay_url: None,
+            direct_addresses: Default::default(),
         }
     }
 
-    /// Adds a relay url to the node's [`AddrInfo`].
+    /// Adds a relay url.
     pub fn with_relay_url(mut self, relay_url: RelayUrl) -> Self {
-        self.info.relay_url = Some(relay_url);
+        self.relay_url = Some(relay_url);
         self
     }
 
-    /// Adds the given direct addresses to the peer's [`AddrInfo`].
+    /// Adds the given direct addresses.
     pub fn with_direct_addresses(
         mut self,
         addresses: impl IntoIterator<Item = SocketAddr>,
     ) -> Self {
-        self.info.direct_addresses = addresses.into_iter().collect();
+        self.direct_addresses = addresses.into_iter().collect();
         self
     }
 
@@ -77,32 +80,24 @@ impl NodeAddr {
     ) -> Self {
         Self {
             node_id,
-            info: AddrInfo {
-                relay_url,
-                direct_addresses: direct_addresses.into_iter().collect(),
-            },
+            relay_url,
+            direct_addresses: direct_addresses.into_iter().collect(),
         }
     }
 
-    /// Applies the options to `self`.
-    ///
-    /// This is used to more tightly control the information stored in a [`NodeAddr`]
-    /// received from another API.  E.g. to ensure a [discovery] service is used the
-    /// `AddrInfoOptions::Id`] option could be used to remove all other addressing details.
-    ///
-    /// [discovery]: https://docs.rs/iroh/*/iroh/index.html#node-discovery
-    pub fn apply_options(&mut self, opts: AddrInfoOptions) {
-        self.info.apply_options(opts);
+    /// Returns true, if only a [`NodeId`] is present.
+    pub fn is_empty(&self) -> bool {
+        self.relay_url.is_none() && self.direct_addresses.is_empty()
     }
 
     /// Returns the direct addresses of this peer.
     pub fn direct_addresses(&self) -> impl Iterator<Item = &SocketAddr> {
-        self.info.direct_addresses.iter()
+        self.direct_addresses.iter()
     }
 
     /// Returns the relay url of this peer.
     pub fn relay_url(&self) -> Option<&RelayUrl> {
-        self.info.relay_url.as_ref()
+        self.relay_url.as_ref()
     }
 }
 
@@ -111,10 +106,8 @@ impl From<(PublicKey, Option<RelayUrl>, &[SocketAddr])> for NodeAddr {
         let (node_id, relay_url, direct_addresses_iter) = value;
         NodeAddr {
             node_id,
-            info: AddrInfo {
-                relay_url,
-                direct_addresses: direct_addresses_iter.iter().copied().collect(),
-            },
+            relay_url,
+            direct_addresses: direct_addresses_iter.iter().copied().collect(),
         }
     }
 }
@@ -124,77 +117,3 @@ impl From<NodeId> for NodeAddr {
         NodeAddr::new(node_id)
     }
 }
-
-/// Network paths to contact an iroh node.
-///
-/// This contains zero or more network paths to establish a connection to an iroh node.
-/// Unless a [discovery service] is used at least one path is required to connect to an
-/// other node, see [`NodeAddr`] for details.
-///
-/// [discovery]: https://docs.rs/iroh/*/iroh/index.html#node-discovery
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default, PartialOrd, Ord)]
-pub struct AddrInfo {
-    /// The node's home relay url.
-    pub relay_url: Option<RelayUrl>,
-    /// Socket addresses where the peer might be reached directly.
-    pub direct_addresses: BTreeSet<SocketAddr>,
-}
-
-impl AddrInfo {
-    /// Returns whether this addressing information is empty.
-    pub fn is_empty(&self) -> bool {
-        self.relay_url.is_none() && self.direct_addresses.is_empty()
-    }
-
-    /// Applies the options to `self`.
-    ///
-    /// This is used to more tightly control the information stored in ab [`AddrInfo`]
-    /// received from another API.  E.g. to ensure a [discovery] service is used the
-    /// `AddrInfoOptions::Id`] option could be used to remove all other addressing details.
-    ///
-    /// [discovery]: https://docs.rs/iroh/*/iroh/index.html#node-discovery
-    pub fn apply_options(&mut self, opts: AddrInfoOptions) {
-        match opts {
-            AddrInfoOptions::Id => {
-                self.direct_addresses.clear();
-                self.relay_url = None;
-            }
-            AddrInfoOptions::RelayAndAddresses => {
-                // nothing to do
-            }
-            AddrInfoOptions::Relay => {
-                self.direct_addresses.clear();
-            }
-            AddrInfoOptions::Addresses => {
-                self.relay_url = None;
-            }
-        }
-    }
-}
-
-/// Options to configure what is included in a [`NodeAddr`] and [`AddrInfo`].
-#[derive(
-    Copy,
-    Clone,
-    PartialEq,
-    Eq,
-    Default,
-    Debug,
-    derive_more::Display,
-    derive_more::FromStr,
-    Serialize,
-    Deserialize,
-)]
-pub enum AddrInfoOptions {
-    /// Only the Node ID is added.
-    ///
-    /// This usually means that iroh-dns discovery is used to find address information.
-    #[default]
-    Id,
-    /// Includes the Node ID and both the relay URL, and the direct addresses.
-    RelayAndAddresses,
-    /// Includes the Node ID and the relay URL.
-    Relay,
-    /// Includes the Node ID and the direct addresses.
-    Addresses,
-}

iroh-base/src/ticket.rs

@@ -1,10 +1,12 @@
-use crate::base32;
+use std::{collections::BTreeSet, net::SocketAddr};
+
+use serde::{Deserialize, Serialize};
+
+use crate::{base32, key::NodeId, relay_url::RelayUrl};
 
-#[cfg(feature = "key")]
 mod blob;
-#[cfg(feature = "key")]
 mod node;
-#[cfg(feature = "key")]
+
 pub use self::{blob::BlobTicket, node::NodeTicket};
 
 /// A ticket is a serializable object combining information required for an operation.
@@ -70,3 +72,15 @@ pub enum Error {
     #[error("verification failed: {_0}")]
     Verify(&'static str),
 }
+
+#[derive(Serialize, Deserialize)]
+struct Variant0NodeAddr {
+    node_id: NodeId,
+    info: Variant0AddrInfo,
+}
+
+#[derive(Serialize, Deserialize)]
+struct Variant0AddrInfo {
+    relay_url: Option<RelayUrl>,
+    direct_addresses: BTreeSet<SocketAddr>,
+}

iroh-base/src/ticket/blob.rs

@@ -4,6 +4,7 @@ use std::str::FromStr;
 use anyhow::Result;
 use serde::{Deserialize, Serialize};
 
+use super::{Variant0AddrInfo, Variant0NodeAddr};
 use crate::{
     hash::{BlobFormat, Hash},
     node_addr::NodeAddr,
@@ -31,21 +32,47 @@ pub struct BlobTicket {
 /// postcard to add a discriminator.
 #[derive(Serialize, Deserialize)]
 enum TicketWireFormat {
-    Variant0(BlobTicket),
+    Variant0(Variant0BlobTicket),
+}
+
+// Legacy
+#[derive(Serialize, Deserialize)]
+struct Variant0BlobTicket {
+    node: Variant0NodeAddr,
+    format: BlobFormat,
+    hash: Hash,
 }
 
 impl Ticket for BlobTicket {
     const KIND: &'static str = "blob";
 
     fn to_bytes(&self) -> Vec<u8> {
-        let data = TicketWireFormat::Variant0(self.clone());
+        let data = TicketWireFormat::Variant0(Variant0BlobTicket {
+            node: Variant0NodeAddr {
+                node_id: self.node.node_id,
+                info: Variant0AddrInfo {
+                    relay_url: self.node.relay_url.clone(),
+                    direct_addresses: self.node.direct_addresses.clone(),
+                },
+            },
+            format: self.format,
+            hash: self.hash,
+        });
         postcard::to_stdvec(&data).expect("postcard serialization failed")
     }
 
     fn from_bytes(bytes: &[u8]) -> std::result::Result<Self, ticket::Error> {
         let res: TicketWireFormat = postcard::from_bytes(bytes).map_err(ticket::Error::Postcard)?;
-        let TicketWireFormat::Variant0(res) = res;
-        Ok(res)
+        let TicketWireFormat::Variant0(Variant0BlobTicket { node, format, hash }) = res;
+        Ok(Self {
+            node: NodeAddr {
+                node_id: node.node_id,
+                relay_url: node.info.relay_url,
+                direct_addresses: node.info.direct_addresses,
+            },
+            format,
+            hash,
+        })
     }
 }
 

iroh-base/src/ticket/node.rs

@@ -5,6 +5,7 @@ use std::str::FromStr;
 use anyhow::Result;
 use serde::{Deserialize, Serialize};
 
+use super::{Variant0AddrInfo, Variant0NodeAddr};
 use crate::{
     node_addr::NodeAddr,
     ticket::{self, Ticket},
@@ -36,21 +37,41 @@ pub struct NodeTicket {
 /// Wire format for [`NodeTicket`].
 #[derive(Serialize, Deserialize)]
 enum TicketWireFormat {
-    Variant0(NodeTicket),
+    Variant0(Variant0NodeTicket),
+}
+
+// Legacy
+#[derive(Serialize, Deserialize)]
+struct Variant0NodeTicket {
+    node: Variant0NodeAddr,
 }
 
 impl Ticket for NodeTicket {
     const KIND: &'static str = "node";
 
     fn to_bytes(&self) -> Vec<u8> {
-        let data = TicketWireFormat::Variant0(self.clone());
+        let data = TicketWireFormat::Variant0(Variant0NodeTicket {
+            node: Variant0NodeAddr {
+                node_id: self.node.node_id,
+                info: Variant0AddrInfo {
+                    relay_url: self.node.relay_url.clone(),
+                    direct_addresses: self.node.direct_addresses.clone(),
+                },
+            },
+        });
         postcard::to_stdvec(&data).expect("postcard serialization failed")
     }
 
     fn from_bytes(bytes: &[u8]) -> std::result::Result<Self, ticket::Error> {
         let res: TicketWireFormat = postcard::from_bytes(bytes).map_err(ticket::Error::Postcard)?;
-        let TicketWireFormat::Variant0(res) = res;
-        Ok(res)
+        let TicketWireFormat::Variant0(Variant0NodeTicket { node }) = res;
+        Ok(Self {
+            node: NodeAddr {
+                node_id: node.node_id,
+                relay_url: node.info.relay_url,
+                direct_addresses: node.info.direct_addresses,
+            },
+        })
     }
 }
 

iroh-dns-server/src/lib.rs

@@ -183,7 +183,7 @@ mod tests {
         let res = resolver.lookup_by_id(&node_id, origin).await?;
 
         assert_eq!(res.node_id, node_id);
-        assert_eq!(res.info.relay_url.map(Url::from), Some(relay_url));
+        assert_eq!(res.relay_url.map(Url::from), Some(relay_url));
 
         server.shutdown().await?;
         Ok(())
@@ -255,7 +255,7 @@ mod tests {
         let res = resolver.lookup_by_id(&node_id, origin).await?;
 
         assert_eq!(res.node_id, node_id);
-        assert_eq!(res.info.relay_url.map(Url::from), Some(relay_url));
+        assert_eq!(res.relay_url.map(Url::from), Some(relay_url));
 
         server.shutdown().await?;
         for mut node in testnet.nodes {