feat(iroh): Accept custom protocols #2357
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
flub
reviewed
Jun 12, 2024
iroh/examples/custom-protocol.rs
Outdated
| } | ||
|
|
||
| impl<S: Store + fmt::Debug> Protocol for ExampleProto<S> { | ||
| fn as_arc_any(self: Arc<Self>) -> Arc<dyn Any + Send + Sync> { |
There was a problem hiding this comment.
into_arc_any is more correct i guess? as seems like a naming violation
There was a problem hiding this comment.
why does this have to exist at all? why not do this directly on the argument the node builder method accepts?
There was a problem hiding this comment.
I don't think there is a way to cast from dyn Protocol to dyn Any apart from having this as a trait method implemented on the concrete type.
let x: Arc<dyn Protocol> = protocols.get(alpn);
// this does not compile even if Protocol: Any
let x: Arc<dyn Any> = x;However what we can do, and what I did in the most recent commit, is move this into a separate trait which is auto-impled for all T: Send + Sync + 'static. I like this more, now users don't have to care about this at all and it is purely an implementation detail.
iroh/src/node.rs
Outdated
| pub fn get_protocol<P: Protocol>(&self, alpn: &[u8]) -> Option<Arc<P>> { | ||
| let protocols = self.protocols.read().unwrap(); | ||
| let protocol: Arc<dyn Protocol> = protocols.get(alpn)?.clone(); | ||
| let protocol_any: Arc<dyn Any + Send + Sync> = protocol.as_arc_any(); |
There was a problem hiding this comment.
why stored as Any if it is then downcast anyway?
4 tasks
Frando
force-pushed
the
custom-protocols-new
branch
from
7a032dc
to
ee043e5
Compare
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 19, 2024
## Description * feat(iroh-net): Allow to set the list of accepted ALPN protocols at runtime * refactor(iroh): Spawning a node can now be performed in two stages: First `Builder::build()` is called, which returns a Future that resolves to a new type `ProtocolBuilder`. The `ProtocolBuilder` is then spawned into the actual, running `Node`. If the intermediate step is not needed, `Builder::spawn` performs both in one call, therefore this change is not breaking. * feat(iroh): Allow to accept custom ALPN protocols in an Iroh node. Introduce a `ProtocolHandler` trait for accepting incoming connections and adds `ProtocolBuilder::accept` to register these handlers per ALPN. * refactor(iroh): Move towards more structured concurrency by spawning tasks into a `JoinSet` * refactor(iroh): Improve shutdown flow and perform more things concurently. originally based on #2357 but now directly to main ## Breaking Changes * `iroh_net::endpoint::make_server_config` takes `Arc<quinn::TransportConfig>` instead of `Option<quinn::TransportConfig>`. If you used the `None` case, replace with `quinn::TransportConfig::default()`. ## Notes & open questions <!-- Any notes, remarks or open questions you have to make about the PR. --> ## Change checklist - [x] Self-review. - [x] Documentation updates if relevant. - [ ] Tests if relevant. - [x] All breaking changes documented.
ppodolsky
pushed a commit
to izihawa/iroh
that referenced
this pull request
Jun 22, 2024
* feat(iroh-net): Allow to set the list of accepted ALPN protocols at runtime * refactor(iroh): Spawning a node can now be performed in two stages: First `Builder::build()` is called, which returns a Future that resolves to a new type `ProtocolBuilder`. The `ProtocolBuilder` is then spawned into the actual, running `Node`. If the intermediate step is not needed, `Builder::spawn` performs both in one call, therefore this change is not breaking. * feat(iroh): Allow to accept custom ALPN protocols in an Iroh node. Introduce a `ProtocolHandler` trait for accepting incoming connections and adds `ProtocolBuilder::accept` to register these handlers per ALPN. * refactor(iroh): Move towards more structured concurrency by spawning tasks into a `JoinSet` * refactor(iroh): Improve shutdown flow and perform more things concurently. originally based on n0-computer#2357 but now directly to main * `iroh_net::endpoint::make_server_config` takes `Arc<quinn::TransportConfig>` instead of `Option<quinn::TransportConfig>`. If you used the `None` case, replace with `quinn::TransportConfig::default()`. <!-- Any notes, remarks or open questions you have to make about the PR. --> - [x] Self-review. - [x] Documentation updates if relevant. - [ ] Tests if relevant. - [x] All breaking changes documented.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Allow to accept custom protocols in an iroh node.
See the included example which uses both a custom protocol and core iroh protocols.
supersedes #2284
Breaking Changes
todo
Notes & open questions
Change checklist