Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change the multi-stream connections API #2754

Closed
tomaka opened this issue Sep 15, 2022 · 2 comments · Fixed by #2758
Closed

Change the multi-stream connections API #2754

tomaka opened this issue Sep 15, 2022 · 2 comments · Fixed by #2758

Comments

@tomaka
Copy link
Contributor

tomaka commented Sep 15, 2022 •
edited
Loading

After trying to implement the WebRTC handshake, I've changed my mind on how to design the APIs.

Rather than providing a PeerId (the actual one that's been negotiated), the multi-stream connection opener now just says "this is a libp2p webrtc connection", and the negotiation is done entirely within smoldot.

In the same vain, single-stream connections should also get an extra parameter indicating something like "this needs multistream-select for yamux and noise". In the future, we can also imagine additionally indicating "this is websocket" or "this is websocket secure" and let smoldot do the WebSocket and TLS handshakes.

cc @melekes
cc #1712
@tomaka tomaka mentioned this issue Sep 15, 2022
Merged
@melekes
Copy link
Contributor

melekes commented Sep 15, 2022

Should we then rename 'multi-stream' to 'webrtc-multi-stream'?

@tomaka
Copy link
Contributor Author

tomaka commented Sep 15, 2022

I had instead in mind to add a parameter when creating a multistream connection indicating the type (in this case WebRTC)

mergify bot pushed a commit that referenced this issue Sep 15, 2022
@tomaka
Perform the multi-stream connections handshake inside smoldot (#2755)
047371f 
cc #2754

In terms of API, this removes the necessity to pass a `PeerId` when
creating a "multi-stream connection". The inner implementation now
performs the substream opening and Noise negotiation internally.
@tomaka tomaka mentioned this issue Sep 16, 2022
Merged
@mergify mergify bot closed this as completed in #2758 Sep 16, 2022
mergify bot added a commit that referenced this issue Sep 16, 2022
@tomaka @mergify
Provide the type of handshake when inserting a connection (#2758)
90fea16 
Close #2754

At the moment, we simply say "this is a single stream connection" and
smoldot negotiates Noise+Yamux on top of it, or we say "this is a
multistream connection" (without saying what kind) and smoldot
automatically negotiates WebRTC on top of it.

Instead of doing it this way, after this PR we now pass the type of
handshake as a parameter.

In addition, when we say "this is a WebRTC connection", we must now
provide the local and remote TLS certificate fingerprints. This is
necessary in order to properly perform the Noise handshake.

The main difficulty of this change was going through the Rust <-> JS FFI
layer with the two multihashes, as we can't know the length of a
multihash in advance.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Provide the type of handshake when inserting a connection tomaka/substrate-lite-1
2 participants
@melekes @tomaka