refactor(iroh-router)!: change accept to take an AsRef<[u8]>

[rklaehn]

Description

Change RouterBuilder::accept to take an AsRef<[u8]> instead of a Vec. Also change iroh node accept.

Breaking Changes

• RouterBuilder::accept takes impl AsRef<[u8]>. Existing code should still work!
• Node::accept takes impl AsRef<[u8]>. Existing code should still work!

Notes & open questions

Question: since we are making the API more pleasant at the expense of a bit of inefficiency, should we also change the second parameter to take a &Arc instead of an Arc?

So

fn accept(alpn: impl AsRef<[u8]>, proto: &Arc<dyn ProtocolHandler>)

In the various examples I found that you very frequently need the protocol handler again after accept-ing it. So this saves one .clone() in many cases, at the expense of an arc clone, which we basically treat as free anyway.

Change checklist

• Self-review.
• Documentation updates following the style guide, if relevant.
• Tests if relevant.
• All breaking changes documented.

[github-actions]

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

Last updated: 2024-11-26T09:50:05Z

[github-actions]

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: ee87dc1

[flub]

Nice, I like this.

[flub]

Question: since we are making the API more pleasant at the expense of a bit of inefficiency, should we also change the second parameter to take a &Arc instead of an Arc?

So

fn accept(alpn: impl AsRef<[u8]>, proto: &Arc<dyn ProtocolHandler>)

In the various examples I found that you very frequently need the protocol handler again after accept-ing it. So this saves one .clone() in many cases, at the expense of an arc clone, which we basically treat as free anyway.

I find &Arc<dyn ProtocolHandler> a very weird signature though. That's purely an emotional response and not a technical one though.

[rklaehn]

Question: since we are making the API more pleasant at the expense of a bit of inefficiency, should we also change the second parameter to take a &Arc instead of an Arc?
So

fn accept(alpn: impl AsRef<[u8]>, proto: &Arc<dyn ProtocolHandler>)

In the various examples I found that you very frequently need the protocol handler again after accept-ing it. So this saves one .clone() in many cases, at the expense of an arc clone, which we basically treat as free anyway.

I find &Arc<dyn ProtocolHandler> a very weird signature though. That's purely an emotional response and not a technical one though.

Yeah, I share it. On the other hand compared to some atrocities involving Pin it looks almost simple. Possibly a type alias or even a newtype might be the way to go.