libp2p / specs

Technical specifications for the libp2p networking stack
https://libp2p.io
1.58k stars 275 forks source link

webrtc(private-to-private): leave the signalling stream open on successful connection #580

Closed achingbrain closed 1 year ago

achingbrain commented 1 year ago

After a successful connection we should leave the signalling stream open - this is to renegotiate ICE candidates if required.

Refs:

thomaseizinger commented 1 year ago

This means the underlying relayed connection also can't close, correct? Do we expect nodes to reestablish the signalling stream in that case?

achingbrain commented 1 year ago

This means the underlying relayed connection also can't close, correct?

Correct, yes.

Looking at the Web transports

Yes, I think this would only apply to WebRTC, that is, the negotiationneeded event doesn't seem to have an equivalent in the WebSockets or WebTransport APIs AFAICT.

Keeping a connection, especially a relayed one, seems like a pretty high price to pay for a somewhat obscure scenario. There's a lot of value in freeing up the relay to help other nodes on the network connect instead.

An alternative way to handle it would be to just open a new relayed connection if renegotiation is required? When the new incoming /webrtc-signaling stream is opened I guess you'd just check if you already have a RTCPeerConnection for that peer and use the incoming ICE candiates to renegotiate it rather than create a new one from scratch.

Or just do nothing and throw?

thomaseizinger commented 1 year ago

Keeping a connection, especially a relayed one, seems like a pretty high price to pay for a somewhat obscure scenario. There's a lot of value in freeing up the relay to help other nodes on the network connect instead.

An alternative way to handle it would be to just open a new relayed connection if renegotiation is required? When the new incoming /webrtc-signaling stream is opened I guess you'd just check if you already have a RTCPeerConnection for that peer and use the incoming ICE candiates to renegotiate it rather than create a new one from scratch.

Or just do nothing and throw?

I assume today's behaviour is that all streams on the current connection fail and the entire connection is marked as failed. That could happen for any reason so the application has to have some kind of re-connection scheme anyway, right?

Instead of leaving the stream open, it could be useful to specify that open a new signaling stream (which will likely imply making a new relayed connection).

I'd assume that browser nodes have an open reservation with at least one relay most of the time in order to allow new connections. Thus, establishing a new stream should be pretty quick.

achingbrain commented 1 year ago

I assume today's behaviour is that all streams on the current connection fail and the entire connection is marked as failed.

Right now, yes - if the connectionState changes to anything that isn't connecting or connected we treat it as failure and destroy the libp2p connection/any open streams on that connection.

so the application has to have some kind of re-connection scheme anyway, right?

In general yes, but WebRTC gives us the tools to attempt to handle this transparently at the transport layer.

I'd assume that browser nodes have an open reservation with at least one relay most of the time in order to allow new connections. Thus, establishing a new stream should be pretty quick.

Maybe, whatever disrupted the WebRTC session may have disrupted the relay connection too, plus if you were the dialler you may have closed the relay connection already.

Another use case here is mobile. If you're walking around running js-libp2p on react-native or whatever it would be pretty common to associate with a new cell tower which would likely involve a network change without (I think) much of a period of inaccessibility.

Again though, that may also disrupt your connection to the relay.

I think in the short term maybe we should just close the signalling stream and have the connection subject to the regular rules on connection culling, we can always revisit when we have more data.

achingbrain commented 1 year ago

I'm going to close this, we can revisit later if necessary.