fix(iroh-net)!: only call quinn_connect if a send addr is available

[Frando]

Description

A bug in the discovery flow assumed that if we had a mapped quic address for a node_id, then we had at least a relay URL or one direct address available for that node.

This meant there were instances in which discovery should have been launched before attempting to dial the node, but was never launched, leading to no UDP or relay address available for node errors.

Now we check if addresses are available for a node_id and launch discovery if we do not have any before we attempt to dial.

We also now take into account the "alive" status of any relay URL information we have on a node_id when determining if we need to run discovery for that node_id

tests

This also refactors the test DNS server and the test Pkarr relay server to use CleanupDropGuard for resource cleanup. It also moves the functions that launch those servers into the iroh-net::test_utils crate. This ended up being an unnecessary refactor (I ended up writing the test in the discovery crate anyway), but I left it in case we need to do other tests that rely on discovery outside of the discovery crate.

Notes & open questions

The above issue uncovered a more serious bug: the endpoint currently dies when it attempts to dial a node without any available address information because we return the no UDP or relay address available for node error. We should not do this. In that situation, we should let that connection timeout or launch discovery services inside the magicsocket. That bug (#2226) is not fixed in this PR.

Breaking changes

• Created new public struct RelayUrlInfo that combines the relay_url and additional information about the state of our connection to the remote node at this relay URL:

struct RelayUrlInfo {
    relay_url: RelayUrl,
    /// How long since there has been activity on this relay url
    last_alive: Option<Duration>,
    /// Latest latency information for this relay url
    latency: Option<Duration>,
}

• NodeInfo.relay_url (called ConnectionInfo outside of iroh-net) is now Option<RelayUrlInfo>, changed from Option<RelayUrl>

Change checklist

• Self-review.
• Documentation updates if relevant.
• Tests if relevant.

[ramfox]

We also need to ensure that when we get messages from the discovery service, that the messages contain non-empty address fields before returning: discovery.rs line 247. Check that the message contains addrs before adding it to the endpoint & notifying that we have had at least one message response.

[flub]

Maybe on some level the problem is that NodeAddr is allowed to be completely empty. We could try and change the type so that this is not possible. Of course this also means discovery is a problem, and maybe having discovery explicitly outside of the MagicEndpoint/MagicSock is not a bad option.

OTOH if NodeAddr has a single direct socket address in it that does not work, it would be accepted and the MagicSock would still simply time out. This case could be considered indistinguishable from an empty NodeAddr. This last criticism also applies to what is being tried here: if things align you get an early error, but it doesn't help the general case of the node simply not being reachable when tried.

So I kind of think that instead of adding the APIs here to attempt to figure out if a node is reachable that we should make MagicSock handle it more gracefully. Since however complex we make these APIs they'll never work (currently at least I think).

[ramfox]

Maybe on some level the problem is that NodeAddr is allowed to be completely empty.

But we now allow for users to dial with just the node_id, so we need to be able to allow for them to add an "empty" node addr.

I agree that this doesn't cover the entire solution: we should not fail when there are no available addrs inside the magicsock. At worst, the connection should timeout as you mentioned.

But that was a more complicated issue, the solution of which might mean moving Discovery management down into the magicsock & I wanted to make sure we had a fix for this particular logic bug (which only showed up when trying to join a document ticket) sooner.