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.

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

add webrtc #131

Merged
merged 1 commit into from
Jul 17, 2022
Merged

add webrtc #131

merged 1 commit into from
Jul 17, 2022

Conversation

ckousik
Copy link
Contributor

@ckousik ckousik commented Jul 12, 2022

Add protocol code point for webrtc.
multiformats/go-multiaddr#179
libp2p/specs#412

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks @ckousik.

As far as I can tell webrtc and certhash is still missing in https://github.com/multiformats/multicodec/blob/master/table.csv. (Each multiaddr component is also a multicodec.) Would you mind filing a pull request there as well?

Once that is in, we can merge here.

@@ -33,5 +33,6 @@ code, size, name, comment
277, 0, p2p-stardust,
275, 0, p2p-webrtc-star,
276, 0, p2p-webrtc-direct,
280, 0, webrtc, ICE-lite webrtc transport
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly "ICE-lite" can only be used as a WebRTC server. In other words a browser can not do "ICE-lite", i.e. uses different UDP ports for each connection and does not know its port upfront and thus goes through the whole ICE process. Please correct me if I am wrong.

While I think the "ICE-lite webrtc transport" description is fine for now, I would hope for browsers to be able to re-use the same protocol component (/webrtc) in the browser-to-browser use-case, thus we might change the description later on.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lite implementation is only appropriate for devices that will always be connected to
the public Internet and have a public IP address at which it can receive packets from any
correspondent. ICE will not function when a lite implementation is placed behind a NAT
(RFC8445).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just following up here, the initial hope, namely to reuse /webrtc for browser-to-browser (now called webrtc-w3c) is not possible. See libp2p/specs#497 which uses /webrtc-w3c instead.

@mxinden
Copy link
Member

mxinden commented Jul 12, 2022

//CC @melekes (no actions required)

@melekes
Copy link

melekes commented Jul 13, 2022

opened multiformats/multicodec#279 and multiformats/multicodec#280

melekes added a commit to melekes/multicodec that referenced this pull request Jul 13, 2022
melekes added a commit to melekes/rust-multiaddr that referenced this pull request Jul 13, 2022
mxinden pushed a commit to multiformats/multicodec that referenced this pull request Jul 17, 2022
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opened multiformats/multicodec#279 and multiformats/multicodec#280

With these merged, let's proceed here. Thanks @ckousik and @melekes.

@mxinden mxinden merged commit e992254 into multiformats:master Jul 17, 2022
mergify bot pushed a commit to paritytech/smoldot that referenced this pull request Sep 8, 2022
Update for multiformats/multiaddr#130 and
multiformats/multiaddr#131

Even though I disagree with these protocols, I guess the way forward is
to conform.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants