feat(iroh-base): Implement From & Into between NodeAddr and NodeTicket

[matheus23]

Description

Previously, when all you had was a NodeTicket, all you could get was a reference &NodeAddr from it to use with Endpoint::connect, which requires an owned NodeAddr.

This adds From impls for converting between NodeTicket and NodeAddr, so you can call endpoint.connect(ticket.into()).

Breaking Changes

None - only additions.

Notes & open questions

When I originally raised this, I thought of making the node_addr argument impl Into<NodeAddr>, but we have an instrument(remote = %node_addr.node_id.fmt_sort) annotation on that function. Due to ownership, it's hard to make that work with impl Into<NodeAddr>. As far as I can see there's three options:

• Make it node_addr: impl Into<NodeAddr> + Clone
• Split Endpoint::connect into two functions, a public, outer one with node_addr: impl Into<NodeAddr> and without an instrument annotation, and an inner private one with node_addr: NodeAddr, but with an instrument annotation.
• We just keep it as node_addr: NodeAddr with the instrument annotation and require users to call .into().

I found the third option to be the simplest. @flub please let me know if that sounds good.

Change checklist

• Self-review.
• Documentation updates following the style guide, if relevant.
• [ ] Tests if relevant. I honestly don't think we need tests for this.
• All breaking changes documented.

[github-actions]

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2717/docs/iroh/

Last updated: 2024-09-10T10:26:43Z

[flub]

I love adding docs, though this is a list with one item. Did you mean to add some more things?

[flub]

When I originally raised this, I thought of making the node_addr argument impl Into<NodeAddr>, but we have an instrument(remote = %node_addr.node_id.fmt_sort) annotation on that function. Due to ownership, it's hard to make that work with impl Into<NodeAddr>. As far as I can see there's three options:

* Make it `node_addr: impl Into<NodeAddr> + Clone`

* Split `Endpoint::connect` into two functions, a public, outer one with `node_addr: impl Into<NodeAddr>` and without an `instrument` annotation, and an inner private one with `node_addr: NodeAddr`, but with an `instrument` annotation.

* We just keep it as `node_addr: NodeAddr` with the `instrument` annotation and require users to call `.into()`.

I feel like instrumentation should not be affecting the API. Surely there's other ways of entering the right span in an async-aware way.

Having said that, requiring users to call .into() is pretty common and not that bad. I kind of like impl Into<...> on public APIs as well, but not sure we have a set pattern for it yet.

[dignifiedquire]

@flub can you take this PR over pleaes?