refactor!: streamline discovery and node info types (#3175)

## Description

Adds a `NodeData` struct that contains the information about a node that
can be published in discovery services. This struct is both used in
`iroh_relay::dns::NodeInfo` and in `iroh::discovery::Discovery`. Also,
`iroh::discovery::DiscoveryItem` has no public fields anymore and uses
`NodeInfo` internally to again reduce duplication. With these changes,
the fields published about a node in discovery now are only defined in a
single place.

This PR also serves these prupsoes:
* We want to add some "user defined data" to discovery (see #3176). This
would add a third argument to `Discovery::publish`; with this PR instead
we can just add it as an optional field to `NodeData`.
* It makes it possible to potentially add data to discovery after 1.0.
The fields of `NodeData` are private, so we can add fields, and
setter/getter methods, without this being a semver-breaking change.

A few other places are changed for consistency: `StaticProvider` now
works with `NodeInfo` instead of `NodeAddr` (`NodeInfo` can be easily
converted into `NodeAddr`). `DnsProvider::lookup_node_by_id` etc. now
return `NodeInfo` instead of `NodeAddr`. A few method names are adjusted
for consistency. `NodeInfo` is constructed in a builder-style fashion
instead of passing all fields to `new`.

## Breaking Changes

* `iroh::discovery::Discovery::publish` now takes `data: &NodeData` as
its single argument. `iroh::discovery::NodeData` is a re-export of
`iroh_relay::dns::node_info::NodeData`, and contains relay URL and
direct addresses. See docs for `NodeData` for details.
* `iroh::discovery::DiscoveryItem` no longer has any public fields.
There are now getters for the contained data, and constructors for
createing a `DiscoveryItem` from a `NodeInfo`.
* `iroh_relay::dns::node_info::NodeInfo` is changed.
* `NodeInfo::new` now has a single `NodeId` argument. Use
`NodeInfo::with_direct_addresses` and `NodeInfo::with_relay_url` to set
addresses and relay URL. Alternatively, use `NodeInfo::from_parts` and
pass a `NodeData` struct.
* `NodeInfo` now has two public fields `node_id: NodeId` and `data:
NodeData`, and setter and getter methods for the relay URL and direct
addresses.
* ` iroh::discovery::pkarr::PkarrPublisher::update_addr_info` now takes
a `NodeData` argument

## Notes & open questions

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

## Change checklist

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

Author: Frando

iroh-dns-server/benches/write.rs

@@ -29,9 +29,9 @@ fn benchmark_dns_server(c: &mut Criterion) {
                     let node_id = secret_key.public();
 
                     let pkarr_relay = LOCALHOST_PKARR.parse().expect("valid url");
-                    let relay_url = Some("http://localhost:8080".parse().unwrap());
                     let pkarr = PkarrRelayClient::new(pkarr_relay);
-                    let node_info = NodeInfo::new(node_id, relay_url, Default::default());
+                    let relay_url = "http://localhost:8080".parse().unwrap();
+                    let node_info = NodeInfo::new(node_id).with_relay_url(Some(relay_url));
                     let signed_packet = node_info.to_pkarr_signed_packet(&secret_key, 30).unwrap();
 
                     let start = std::time::Instant::now();

iroh-dns-server/examples/publish.rs

@@ -77,7 +77,7 @@ async fn main() -> Result<()> {
     println!("publish to {pkarr_relay} ...");
 
     let pkarr = PkarrRelayClient::new(pkarr_relay);
-    let node_info = NodeInfo::new(node_id, Some(args.relay_url), Default::default());
+    let node_info = NodeInfo::new(node_id).with_relay_url(Some(args.relay_url.into()));
     let signed_packet = node_info.to_pkarr_signed_packet(&secret_key, 30)?;
     pkarr.publish(&signed_packet).await?;
 

iroh-dns-server/examples/resolve.rs

@@ -51,10 +51,10 @@ async fn main() -> anyhow::Result<()> {
         Command::Domain { domain } => resolver.lookup_node_by_domain_name(&domain).await?,
     };
     println!("resolved node {}", resolved.node_id);
-    if let Some(url) = resolved.relay_url {
+    if let Some(url) = resolved.relay_url() {
         println!("    relay={url}")
     }
-    for addr in resolved.direct_addresses {
+    for addr in resolved.direct_addresses() {
         println!("    addr={addr}")
     }
     Ok(())

iroh-dns-server/src/lib.rs

@@ -25,12 +25,11 @@ mod tests {
     use iroh::{
         discovery::pkarr::PkarrRelayClient,
         dns::{node_info::NodeInfo, DnsResolver},
-        SecretKey,
+        RelayUrl, SecretKey,
     };
     use pkarr::{PkarrClient, SignedPacket};
     use testresult::TestResult;
     use tracing_test::traced_test;
-    use url::Url;
 
     use crate::{
         config::BootstrapOption,
@@ -170,9 +169,9 @@ mod tests {
 
         let secret_key = SecretKey::generate(rand::thread_rng());
         let node_id = secret_key.public();
-        let relay_url: Url = "https://relay.example.".parse()?;
         let pkarr = PkarrRelayClient::new(pkarr_relay);
-        let node_info = NodeInfo::new(node_id, Some(relay_url.clone()), Default::default());
+        let relay_url: RelayUrl = "https://relay.example.".parse()?;
+        let node_info = NodeInfo::new(node_id).with_relay_url(Some(relay_url.clone()));
         let signed_packet = node_info.to_pkarr_signed_packet(&secret_key, 30)?;
 
         pkarr.publish(&signed_packet).await?;
@@ -181,7 +180,7 @@ mod tests {
         let res = resolver.lookup_node_by_id(&node_id, origin).await?;
 
         assert_eq!(res.node_id, node_id);
-        assert_eq!(res.relay_url.map(Url::from), Some(relay_url));
+        assert_eq!(res.relay_url(), Some(&relay_url));
 
         server.shutdown().await?;
         Ok(())
@@ -234,8 +233,8 @@ mod tests {
         // create a signed packet
         let secret_key = SecretKey::generate(rand::thread_rng());
         let node_id = secret_key.public();
-        let relay_url: Url = "https://relay.example.".parse()?;
-        let node_info = NodeInfo::new(node_id, Some(relay_url.clone()), Default::default());
+        let relay_url: RelayUrl = "https://relay.example.".parse()?;
+        let node_info = NodeInfo::new(node_id).with_relay_url(Some(relay_url.clone()));
         let signed_packet = node_info.to_pkarr_signed_packet(&secret_key, 30)?;
 
         // publish the signed packet to our DHT
@@ -252,7 +251,7 @@ mod tests {
         let res = resolver.lookup_node_by_id(&node_id, origin).await?;
 
         assert_eq!(res.node_id, node_id);
-        assert_eq!(res.relay_url.map(Url::from), Some(relay_url));
+        assert_eq!(res.relay_url(), Some(&relay_url));
 
         server.shutdown().await?;
         for mut node in testnet.nodes {
@@ -268,8 +267,8 @@ mod tests {
     fn random_signed_packet() -> Result<SignedPacket> {
         let secret_key = SecretKey::generate(rand::thread_rng());
         let node_id = secret_key.public();
-        let relay_url: Url = "https://relay.example.".parse()?;
-        let node_info = NodeInfo::new(node_id, Some(relay_url.clone()), Default::default());
+        let relay_url: RelayUrl = "https://relay.example.".parse()?;
+        let node_info = NodeInfo::new(node_id).with_relay_url(Some(relay_url.clone()));
         node_info.to_pkarr_signed_packet(&secret_key, 30)
     }
 }

iroh-relay/src/dns.rs

@@ -8,11 +8,12 @@ use std::{
 
 use anyhow::{bail, Context, Result};
 use hickory_resolver::{Resolver, TokioResolver};
-use iroh_base::{NodeAddr, NodeId};
+use iroh_base::NodeId;
 use n0_future::{
     time::{self, Duration},
     StreamExt,
 };
+use node_info::NodeInfo;
 use url::Url;
 
 pub mod node_info;
@@ -215,18 +216,18 @@ impl DnsResolver {
     ///
     /// To lookup nodes that published their node info to the DNS servers run by n0,
     /// pass [`N0_DNS_NODE_ORIGIN_PROD`] as `origin`.
-    pub async fn lookup_node_by_id(&self, node_id: &NodeId, origin: &str) -> Result<NodeAddr> {
+    pub async fn lookup_node_by_id(&self, node_id: &NodeId, origin: &str) -> Result<NodeInfo> {
         let attrs =
             node_info::TxtAttrs::<node_info::IrohAttr>::lookup_by_id(self, node_id, origin).await?;
-        let info: node_info::NodeInfo = attrs.into();
-        Ok(info.into())
+        let info = attrs.into();
+        Ok(info)
     }
 
     /// Looks up node info by DNS name.
-    pub async fn lookup_node_by_domain_name(&self, name: &str) -> Result<NodeAddr> {
+    pub async fn lookup_node_by_domain_name(&self, name: &str) -> Result<NodeInfo> {
         let attrs = node_info::TxtAttrs::<node_info::IrohAttr>::lookup_by_name(self, name).await?;
-        let info: node_info::NodeInfo = attrs.into();
-        Ok(info.into())
+        let info = attrs.into();
+        Ok(info)
     }
 
     /// Looks up node info by DNS name in a staggered fashion.
@@ -239,7 +240,7 @@ impl DnsResolver {
         &self,
         name: &str,
         delays_ms: &[u64],
-    ) -> Result<NodeAddr> {
+    ) -> Result<NodeInfo> {
         let f = || self.lookup_node_by_domain_name(name);
         stagger_call(f, delays_ms).await
     }
@@ -255,7 +256,7 @@ impl DnsResolver {
         node_id: &NodeId,
         origin: &str,
         delays_ms: &[u64],
-    ) -> Result<NodeAddr> {
+    ) -> Result<NodeInfo> {
         let f = || self.lookup_node_by_id(node_id, origin);
         stagger_call(f, delays_ms).await
     }