Add a IdentifyTransport #569
Conversation
|
|
||
| /// Implementation of `Future` that asks the remote of its `PeerId`. | ||
| // TODO: remove unneeded bounds | ||
| struct IdRetreiver<TMuxer> |
| endpoint: Endpoint, | ||
| } | ||
|
|
||
| enum IdRetreiverState<TMuxer> |
| IdRetreiverState::Finishing(public_key) => { | ||
| // Here is a tricky part: we need to get back the muxer in order to return | ||
| // it, but it is in an `Arc`. | ||
| let muxer = self.muxer.take() |
There was a problem hiding this comment.
This could be simplified by putting the muxer into the IdRetreiverState enum. There is no need for an Option then and consequently no take is required.
| let unwrapped = Arc::try_unwrap(muxer).unwrap_or_else(|_| { | ||
| panic!("we clone the Arc only to put it into substreams ; once in the \ | ||
| Finishing state, no substream or upgrade exists anymore ; \ | ||
| therefore there exists only one instance of the Arc ; qed") |
There was a problem hiding this comment.
Why not removing the Arc and using StreamMuxer::open_outbound and StreamMuxer::poll_outbound instead? (with TMuxer::Substream: AsyncRead + AsyncWrite ... )
There was a problem hiding this comment.
The "problem" is that we use the ConnectionUpgrade trait, and that takes ownership of the substream, which thus requires holding an Arc.
I guess it could be possible to do everything manually, but I'm not sure if the extra complexity is worth.
There was a problem hiding this comment.
I must be missing something because the change looks rather good to me: https://gist.github.com/twittner/3bbc474ba9599880fe063d8e7f82a47d
There was a problem hiding this comment.
That's not correct though. StreamMuxer::Substream is an opaque type that isn't supposed to be implementing any trait.
There was a problem hiding this comment.
Ah yes, I got carried away by just following compiler suggestions and forgot about the semantics of StreamMuxer, also because yamux streams do implement AsyncRead and AsyncWrite.
There was a problem hiding this comment.
However it does not feel right if the API leads to unwrap-or-else-panic idioms.
There was a problem hiding this comment.
Well, this is a special case because we are at the level before the node handler comes into play.
Normally the code that handles a protocol only needs a substream and not the whole muxer.
…_swarm * upstream/master: Add a IdentifyTransport (libp2p#569) Tests for HandledNode (libp2p#546) Some minor fixes reported by clippy (libp2p#600)
…e-handled_node_tasks * upstream/master: Add a IdentifyTransport (libp2p#569) Tests for HandledNode (libp2p#546) Some minor fixes reported by clippy (libp2p#600) Add a PeriodicIdentification protocol handler (libp2p#579) Add ProtocolsHandler trait (libp2p#573) Use paritytech/rust-secp256k1 (libp2p#598) Use websocket 0.21.0 (libp2p#597) Reexport multihash from the facade (libp2p#587) Add substrate to the list of projects using libp2p (libp2p#595) Remove spaces before semicolons (libp2p#591)
…lection_stream * upstream/master: Use vecdeque for fair polling (libp2p#610) Add back the Swarm (libp2p#613) Remove dependency on tokio_current_thread (libp2p#606) Use chashmap from crates (libp2p#615) Fix grammar in core/nodes etc. (libp2p#608) Inject event by value in ProtocolsHandler (libp2p#605) Add a PeriodicPingHandler and a PingListenHandler (libp2p#574) Fix stack overflow when printing a SubstreamRef (libp2p#599) Add a peer id generator (libp2p#583) eg. -> e.g.; ie. -> i.e. via repren (libp2p#592) Add a IdentifyTransport (libp2p#569) Tests for HandledNode (libp2p#546) Some minor fixes reported by clippy (libp2p#600) Add a PeriodicIdentification protocol handler (libp2p#579) Add ProtocolsHandler trait (libp2p#573) Use paritytech/rust-secp256k1 (libp2p#598) Use websocket 0.21.0 (libp2p#597) Reexport multihash from the facade (libp2p#587) Add substrate to the list of projects using libp2p (libp2p#595) Remove spaces before semicolons (libp2p#591)
Wraps around a
Transportthat yields aimpl StreamMuxer, queries thePeerIdfrom the remote, and yields a(PeerId, impl StreamMuxer).Replaces #481 and closes #460