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

webrtc: specify multiaddresses on a browser-to-browser webrtc connection #579

Closed
sukunrt opened this issue Sep 21, 2023 · 5 comments
Closed

Comments

@sukunrt
Copy link
Member

sukunrt commented Sep 21, 2023

The spec currently doesn't say anything on what the multiaddress on an established /webrtc (browser to browser) connection should be.

Two options off the top of my head are:
1.
Initiator Side is /webrtc, with candidate pair from ICETransport if available.
Receiver Side, the address that was dialed: <realy-addr>/p2p-circuit/webrtc

For both sides,
We get the address candidate from the ICETransport(https://github.com/pion/ice/blob/master/candidatepair.go#L23) used to establish the connection and use those with a suffix /webrtc. I'm not sure if this is available in browsers.

Between the two, I think 1 is better. In this situation we have parity with other cases where the Receiver Side address is dialable in case of a disconnection. A problem here is there is probably a lot of code that assumes addresses with /p2p-circuit component are relayed addresses, e.g. https://github.com/vyzo/libp2p-flare-test/blob/085b6bf0f2beedbe6017b86bbf60ca6e64b28ea0/cmd/flarec/client.go#L531

@mxinden
Copy link
Member

mxinden commented Sep 27, 2023

As discussed out of band, I am in favor of (1).

Receiver Side, the address that was dialed: <realy-addr>/p2p-circuit/webrtc

That makes sense to me.

Initiator Side is /webrtc, with candidate pair from ICETransport if available.

Why not use relay-addr/p2p-circuit/webrtc here as well?

A problem here is there is probably a lot of code that assumes addresses with /p2p-circuit component are relayed addresses, e.g. https://github.com/vyzo/libp2p-flare-test/blob/085b6bf0f2beedbe6017b86bbf60ca6e64b28ea0/cmd/flarec/client.go#L531

This is unfortunate, but off the top of my head, the special handling is worth the complexity.

@achingbrain
Copy link
Member

The version with the ICE transport is interesting from an informational point of view but it's a bit weird in that it isn't dialable in and of itself.

I think the version with the relay-addr is probably the one to use since it's the address dialed to open the connection which seems a bit more predictable.

@sukunrt
Copy link
Member Author

sukunrt commented Oct 1, 2023

I'm now not in favour of 1 considering the argument given in #583
Consider the case of two non browser private nodes. After they've made a relayed connection the receiver will attempt to reverse the connection. Here the receiver needs to interpret the /webrtc address that it receives in identify as a relayed connection, skip it and proceed to DCUtR.

Because of the dialable /webrtc address's(<relay>/p2p-circuit/webrtc) interaction with DCUtR and smart dialing, we want to treat it as a relayed address.
But an established /webrtc connection is a direct connection and it's better to keep the address separate to not confuse it with the dialable but relayed address.

Doing 1 forces us to do make a definition like, if the webrtc address is on a connection it is direct, otherwise it is relayed.

@achingbrain
Copy link
Member

Would it be simpler to just add it to DCUtR as a special case?

As in change this line to attempt a unilateral upgrade if the incoming connection is either transient OR the remote address contains a p2p-circuit segment?

@sukunrt
Copy link
Member Author

sukunrt commented Oct 22, 2023

I've decided on doing the same thing as a circuit v2 connection:

remote end addr: <relay-addr>/p2p-circuit/webrtc
local end addr: <ip-addr>/udp/port/webrtc

@sukunrt sukunrt closed this as completed Oct 22, 2023
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

No branches or pull requests

3 participants