libp2p / rust-libp2p

The Rust Implementation of the libp2p networking stack.
https://libp2p.io
MIT License
4.61k stars 960 forks source link

Switch from `webrtc-rs` to `str0m` #3659

Open thomaseizinger opened 1 year ago

thomaseizinger commented 1 year ago

Description

The webrtc-rs implementation has a few fundamental issues due to its design. async callbacks require locks and prevent the idiomatic use of &mut. This results in problems such as https://github.com/webrtc-rs/webrtc/issues/413 which currently prevent us from upgrading to the latest version: https://github.com/libp2p/rust-libp2p/pull/3552

The new kid on the block is str0m, a sans IO implementation of the webrtc stack: https://github.com/algesten/str0m

In the tests, it looks like they already support data channels, meaning we might be able to replace webrtc with it.

Motivation

Related issues:

Are you planning to do it yourself in a pull request?

No.

thomaseizinger commented 1 year ago

cc @melekes

Depending on how many problems you run into once you deploy libp2p-webrtc, we might want to consider devoting some time to try this out instead of patching webrtc-rs.

thomaseizinger commented 1 year ago

@mxinden Any chance we can get this on the roadmap?

thomaseizinger commented 1 year ago

@mxinden Any chance we can get this on the roadmap?

If we want to do https://github.com/libp2p/specs/issues/544 (which I think is a really good idea), I believe this issue is a blocker.

drHuangMHT commented 1 year ago

Related: #3138 with upstream issue on webrtc #104.

mxinden commented 1 year ago

@mxinden Any chance we can get this on the roadmap?

Can you create a pull request @thomaseizinger? Allows us to discuss priorities with the other items.

thomaseizinger commented 1 year ago

I've opened an issue for the failed SDP parsing which is where we hit a roadblock when @mxinden and I last explored what it would take to switch: https://github.com/algesten/str0m/issues/164

thomaseizinger commented 1 year ago

The playground @mxinden and I have been using is here: https://github.com/thomaseizinger/str0m/tree/libp2p-webrtc-playground

Will keep exploring on the side.

thomaseizinger commented 1 year ago

The playground @mxinden and I have been using is here: thomaseizinger/str0m@libp2p-webrtc-playground

Will keep exploring on the side.

I've tried #164 out and with an additional small change to the SDP, this now parses correctly. I've pushed it to the branch above. The new output is now:

2023-05-12T07:47:36.455616Z  INFO str0m::ice::agent: Set remote credentials: IceCreds { ufrag: "libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b:libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b", pass: "libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b:libp2p+webrtc+v1/b01d95e45dee6a3f4e77bb87081eb97b" }
2023-05-12T07:47:36.455660Z  INFO str0m: DTLS setup is: false
2023-05-12T07:47:36.455720Z  INFO str0m::sctp: Uninited => AwaitRemoteAssociation
2023-05-12T07:47:36.455760Z DEBUG str0m::channel: Allocate channel id: ChannelId(0)
2023-05-12T07:47:36.455801Z TRACE str0m::ice::agent: Enqueueing event: IceRestart(IceCreds { ufrag: "77hg", pass: "ADebVBR6nBhkiq5R8a1iSa" })
2023-05-12T07:47:36.455829Z  INFO str0m::ice::agent: State change (new connection): New -> Checking
2023-05-12T07:47:36.455845Z TRACE str0m::ice::agent: Enqueueing event: IceConnectionStateChange(Checking)
2023-05-12T07:47:36.455864Z TRACE str0m::ice::agent: Stop timeout since ice-lite do no checks
2023-05-12T07:47:36.455871Z TRACE str0m::sctp: Handle timeout: Instant { tv_sec: 4949, tv_nsec: 489601509 }
2023-05-12T07:47:36.455882Z DEBUG str0m::channel: Associate stream id ChannelId(0) => 1
2023-05-12T07:47:36.455902Z TRACE str0m::ice::agent: Poll event: Some(IceRestart(IceCreds { ufrag: "77hg", pass: "ADebVBR6nBhkiq5R8a1iSa" }))
2023-05-12T07:47:36.455911Z TRACE str0m::ice::agent: Poll event: Some(IceConnectionStateChange(Checking))
2023-05-12T07:47:36.455918Z DEBUG str0m: IceConnectionStateChange(Checking)

This hang there which might be due to what is mentioned in https://github.com/algesten/str0m/issues/164#issuecomment-1535841284.

thomaseizinger commented 1 year ago

New tracking issue on the str0m side: https://github.com/algesten/str0m/issues/166

thomaseizinger commented 1 year ago

@altonen Did I understand you in https://github.com/algesten/str0m/issues/166 correctly that you are working on this? Mind sharing a draft-PR? I am very excited what this looks like :)

altonen commented 1 year ago

I have a small experiment going on. It's not production-ready and still very rough around the edges (including WebRTC). If this turns out to be just a waste of time, I'll upstream the str0m-based WebRTC implementation to libp2p.

thomaseizinger commented 1 year ago

I have a small experiment going on. It's not production-ready and still very rough around the edges (including WebRTC). If this turns out to be just a waste of time, I'll upstream the str0m-based WebRTC implementation to libp2p.

Looks great! Thanks for sharing :)

In case somebody picks this up, I am sure your WebRTC implementation is a good reference point.

Btw: we are working on a similar builder API: https://github.com/libp2p/rust-libp2p/pull/4120

altonen commented 1 year ago

Cheers, I hope the implementation is of small help to you if I don't make the PR myself at some point.

thomaseizinger commented 1 year ago

FTR, the main implementation is here: https://github.com/altonen/litep2p/blob/master/src/transport/webrtc

DougAnderson444 commented 6 months ago

I'm going to see if I can get this integrated into rust-libp2p

DougAnderson444 commented 3 months ago

Just an update from my May comment, I did start a branch on this but was taking much longer than I expected and I had to stop in order to switch back to my other priorities.

If anyone wants to reference my branch (I'm not saying it's good... I'm just saying it's there 😆) the link is: https://github.com/DougAnderson444/rust-libp2p/tree/webrtc-str0m/transports/webrtc-str0m

That's there as reference in case anyone wants to have a look before they take a crack at this. But also feel free to completely ignore it!