fix(iroh-net)!: DiscoveredDirectAddrs need to update the timestamp (#…

…2808)

## Description

DiscoverdDirectAddrs has a timestamp of when it was last changed, which
is used to decide if the direct addr information is recent enough to use
in call-me-maybe messages. However when it was updated this timestamp
was not included in the comparison to see if there were changes, so that
watchers would not get updates when the direct addresses did not change.
Unfortunately this meant the timestamp also never updated and the
DiscoveredDirectAddrs was always considered out of date (unless there
were changes in the discovered addresses).

This fixes this by putting only the DirectAddrs in the watcher.

It does a few other improvements:

- Moves all logic about DiscoveredDirectAddrs into one place. This could
be split out to a module now if desired.

- Tidies up the code which creates the DirectAddrs from the portmapper,
netcheck and the local addresses. Again this is just a first pass in
cleaning up this code, there is so much more that could be done.

## Breaking Changes

* `iroh_net::Endpoint::direct_addresses` now returns a stream yielding
Items of `BTreeSet<DirectAddr>` instead of `Vec<DirectAddr>`.
* `iroh_base::node_addr::NodeAddr::from_parts` now takes a `impl
IntoIterator<Item = DirectAddr>` instead of `Vec<DirectAddr>`. But since
`Vec` implements `IntoIterator` that is not really a breaking change.

## Notes & open questions

The crux of this fix is moving from
`Watchable<DiscoveredDirectAddresses>` to
`DiscoveredDirectAddresses::addrs` being `Watchable<Vec<DirectAddr>>`
instead of the whole struct being watchable.

I could split the refactoring of store_direct_addr_update and the
renaming of it's related functions into a separate PR in front of this
bugfix if desired. I had to change that code because of the format
change from `Vec<DirectAddrs>` to `BTreeSet<DirectAddr>` and it was just
way clearer doing it with the refactoring.

As indicated in the (already existing) comments that whole direct
address updating logic can still be more refactored and improved. But
many small steps is how we improve.

I went very opinionated on the imports... apologies.

## 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.

---------

Co-authored-by: Asmir Avdicevic <asmir.avdicevic64@gmail.com>
Co-authored-by: Divma <26765164+divagant-martian@users.noreply.github.com>
Co-authored-by: Diva M <divma@protonmail.com>

iroh-base/src/node_addr.rs

@@ -74,7 +74,7 @@ impl NodeAddr {
     pub fn from_parts(
         node_id: PublicKey,
         relay_url: Option<RelayUrl>,
-        direct_addresses: Vec<SocketAddr>,
+        direct_addresses: impl IntoIterator<Item = SocketAddr>,
     ) -> Self {
         Self {
             node_id,

iroh-base/src/ticket/blob.rs

@@ -132,7 +132,7 @@ mod tests {
         let relay_url = None;
         BlobTicket {
             hash,
-            node: NodeAddr::from_parts(peer, relay_url, vec![addr]),
+            node: NodeAddr::from_parts(peer, relay_url, [addr]),
             format: BlobFormat::HashSeq,
         }
     }
@@ -163,7 +163,7 @@ mod tests {
                 .unwrap();
 
         let ticket = BlobTicket {
-            node: NodeAddr::from_parts(node_id, None, vec![]),
+            node: NodeAddr::from_parts(node_id, None, []),
             format: BlobFormat::Raw,
             hash,
         };

iroh-base/src/ticket/node.rs

@@ -128,7 +128,7 @@ mod tests {
         let addr = SocketAddr::from((Ipv4Addr::LOCALHOST, 1234));
         let relay_url = None;
         NodeTicket {
-            node: NodeAddr::from_parts(peer, relay_url, vec![addr]),
+            node: NodeAddr::from_parts(peer, relay_url, [addr]),
         }
     }
 
@@ -158,7 +158,7 @@ mod tests {
             node: NodeAddr::from_parts(
                 node_id,
                 Some("http://derp.me./".parse().unwrap()),
-                vec!["127.0.0.1:1024".parse().unwrap()],
+                ["127.0.0.1:1024".parse().unwrap()],
             ),
         };
         let base32 = base32::parse_vec(ticket.to_string().strip_prefix("node").unwrap()).unwrap();

iroh-cli/src/commands/blobs.rs

@@ -211,7 +211,7 @@ impl BlobCommands {
                                 address
                             } else {
                                 // use both the cli supplied ones and the ticket ones
-                                address.extend(info.direct_addresses.into_iter());
+                                address.extend(info.direct_addresses);
                                 address
                             };
 

iroh-docs/src/ticket.rs

@@ -87,7 +87,7 @@ mod tests {
 
         let ticket = DocTicket {
             capability: Capability::Read(namespace_id),
-            nodes: vec![NodeAddr::from_parts(node_id, None, vec![])],
+            nodes: vec![NodeAddr::from_parts(node_id, None, [])],
         };
         let base32 = base32::parse_vec(ticket.to_string().strip_prefix("doc").unwrap()).unwrap();
         let expected = parse_hexdump("

iroh-gossip/src/net.rs

@@ -805,11 +805,11 @@ impl Stream for TopicCommandStream {
     }
 }
 
-fn our_peer_data(endpoint: &Endpoint, direct_addresses: &[DirectAddr]) -> Result<PeerData> {
+fn our_peer_data(endpoint: &Endpoint, direct_addresses: &BTreeSet<DirectAddr>) -> Result<PeerData> {
     let addr = NodeAddr::from_parts(
         endpoint.node_id(),
         endpoint.home_relay(),
-        direct_addresses.iter().map(|x| x.addr).collect(),
+        direct_addresses.iter().map(|x| x.addr),
     );
     encode_peer_data(&addr.info)
 }

iroh-net/src/endpoint.rs

@@ -781,8 +781,11 @@ impl Endpoint {
             .await
             .ok_or(anyhow!("No IP endpoints found"))?;
         let relay = self.home_relay();
-        let addrs = addrs.into_iter().map(|x| x.addr).collect();
-        Ok(NodeAddr::from_parts(self.node_id(), relay, addrs))
+        Ok(NodeAddr::from_parts(
+            self.node_id(),
+            relay,
+            addrs.into_iter().map(|x| x.addr),
+        ))
     }
 
     /// Returns the [`RelayUrl`] of the Relay server used as home relay.