Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

feat: add WebRTC transport #12529

Open
wants to merge 79 commits into
base: master
Choose a base branch
from

Conversation

melekes
Copy link
Contributor

@melekes melekes commented Oct 19, 2022

Refs paritytech/smoldot#1712
Impl libp2p/rust-libp2p#2622
polkadot companion: paritytech/polkadot#6529
cumulus companion: paritytech/cumulus#2076
Closes paritytech/polkadot-sdk#547

  • In dev mode the node will advertise and accept /webrtc connections;
  • The initial plan was to enable it for non-validators as well, but ultimately we've decided to postpone this until the majority of nodes & downstream users (parachains) upgrade to libp2p-identify 0.41.1 (see feat: add WebRTC transport #12529 (comment));
  • The transport will generate and store the certificate, which is required for WebRTC identity, in
    base dir. In the future, when a new version of ring library is released, the certificate will be
    deterministically derived from the node's peer ID.

@melekes melekes added A3-in_progress Pull request is in progress. No review needed at this stage. C3-medium PR touches the given topic and has a medium impact on builders. labels Oct 19, 2022
@melekes melekes self-assigned this Oct 19, 2022
client/cli/src/params/network_params.rs Outdated Show resolved Hide resolved
client/cli/src/params/network_params.rs Outdated Show resolved Hide resolved
@tomaka

This comment was marked as resolved.

@tomaka tomaka added B5-clientnoteworthy and removed C3-medium PR touches the given topic and has a medium impact on builders. labels Oct 19, 2022
melekes added a commit to melekes/rust-libp2p that referenced this pull request Oct 20, 2022
by analogue with `id_keys`.

See paritytech/substrate#12529 (comment)

Both peer's identity and certificate will need to be stored somewhere (disk) and reused across
restarts. Since this is a dominant use-case, I've switched `Config` API to accept a certificate and
opted out of generating one completely.
melekes added a commit to melekes/substrate that referenced this pull request Nov 17, 2022
Cargo.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@tomaka tomaka left a comment

Choose a reason for hiding this comment

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

It's still unclear to me what to do w.r.t. the bandwidth
Everything would be way easier if webrtc-rs simply kept track of how many bytes have been sent and received in total, which would let us calculate the bandwidth, it really doesn't seem like a difficult change to me

client/network/src/config.rs Outdated Show resolved Hide resolved
client/network/src/config.rs Outdated Show resolved Hide resolved
client/network/src/service.rs Outdated Show resolved Hide resolved
client/network/src/discovery.rs Outdated Show resolved Hide resolved
@melekes
Copy link
Contributor Author

melekes commented Nov 22, 2022

sorry had to force push to do #12529 (comment)

@tomaka
Copy link
Contributor

tomaka commented Nov 22, 2022

sorry had to force push to do #12529 (comment)

It doesn't matter now, but you didn't have to do that. We squash commits anyway, so whether you're based on top of another branch doesn't matter. The point was just to merge this in two times: first the libp2p changes, then the WebRTC changes. This way, we can easily bisect in case a bug is introduced.

@melekes
Copy link
Contributor Author

melekes commented Nov 22, 2022

so whether you're based on top of another branch doesn't matter.

well it matters for me because the amount of conflicts reduces dramatically when you rebase 1 commit vs 22 😄

@seunlanlege
Copy link
Contributor

Bump

@stale stale bot removed the A3-stale label Jul 21, 2023
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/infrastructure-treasury-funding/3507/3

@melekes melekes requested review from a team July 25, 2023 14:51
@melekes melekes changed the title [client/network] Add WebRTC transport feat: add WebRTC transport Jul 25, 2023
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3257797

@melekes
Copy link
Contributor Author

melekes commented Jul 26, 2023

zombienet test failed again, but with different error this time:

2023-07-25 16:48:35.169  INFO tokio-runtime-worker libp2p_kad::handler: Remote 12D3KooWQCkBm1BYtkHpocxCwMgR8yjitEeHGx8spzcDLGt2gkBm no longer supports our kademlia protocol

2023-07-25 16:48:35.169 TRACE tokio-runtime-worker sub-libp2p: Ignoring self-reported address /ip4/10.24.122.238/udp/30333/webrtc-direct/certhash/uEiAvF2IuPZJuKkdb5E-gSM_AmumLi3P5ZR_fvZGPE3vJEA from 12D3KooWQCkBm1BYtkHpocxCwMgR8yjitEeHGx8spzcDLGt2gkBm as remote node is not part of the Kademlia DHT supported by the local node.

2023-07-25 16:48:37.548 TRACE tokio-runtime-worker sub-libp2p: Libp2p => Query for 0020cc78cfbc884fd5668ebb8f2948752f2dbc3a4c665b1b678f6190ec7208cbff72 yielded 0 results    
2023-07-25 16:48:37.548 DEBUG tokio-runtime-worker sub-libp2p: Libp2p => Random Kademlia query has yielded empty results

not yet sure what is happening, but it looks like Kademlia does not provide dave with addresses to connect to...

```
[2023-07-26 09:13:50] error: failed to select a version for `regex`.
[2023-07-26 09:13:50]     ... required by package `webrtc v0.8.0`
[2023-07-26 09:13:50]     ... which satisfies dependency `webrtc = "^0.8.0"` of package `libp2p-webrtc v0.6.0-alpha`
[2023-07-26 09:13:50]     ... which satisfies dependency `libp2p-webrtc = "^0.6.0-alpha"` of package `sc-network v0.10.0-dev (/builds/parity/mirrors/substrate/client/network)`
[2023-07-26 09:13:50]     ... which satisfies git dependency `sc-network` (locked to 0.10.0-dev) of package `polkadot-availability-distribution v0.9.43 (/builds/parity/mirrors/substrate/companions/polkadot/node/network/availability-distribution)`
[2023-07-26 09:13:50]     ... which satisfies path dependency `polkadot-availability-distribution` (locked to 0.9.43) of package `polkadot-service v0.9.43 (/builds/parity/mirrors/substrate/companions/polkadot/node/service)`
[2023-07-26 09:13:50]     ... which satisfies path dependency `service` (locked to 0.9.43) of package `polkadot-cli v0.9.43 (/builds/parity/mirrors/substrate/companions/polkadot/cli)`
[2023-07-26 09:13:50]     ... which satisfies path dependency `polkadot-cli` (locked to 0.9.43) of package `polkadot v0.9.43 (/builds/parity/mirrors/substrate/companions/polkadot)`
[2023-07-26 09:13:50] versions that meet the requirements `^1.7.1` are: 1.9.1, 1.9.0, 1.8.4, 1.8.3, 1.8.2, 1.8.1, 1.8.0, 1.7.3, 1.7.2, 1.7.1
[2023-07-26 09:13:50]
[2023-07-26 09:13:50] all possible versions conflict with previously selected packages.
[2023-07-26 09:13:50]
[2023-07-26 09:13:50]   previously selected package `regex v1.6.0`
[2023-07-26 09:13:50]     ... which satisfies dependency `regex = "^1.1"` (locked to 1.6.0) of package `Inflector v0.11.4`
[2023-07-26 09:13:50]     ... which satisfies dependency `Inflector = "^0.11.4"` (locked to 0.11.4) of package `frame-benchmarking-cli v4.0.0-dev (/builds/parity/mirrors/substrate/utils/frame/benchmarking-cli)`
[2023-07-26 09:13:50]     ... which satisfies git dependency `frame-benchmarking-cli` (locked to 4.0.0-dev) of package `polkadot-cli v0.9.43 (/builds/parity/mirrors/substrate/companions/polkadot/cli)`
[2023-07-26 09:13:50]     ... which satisfies path dependency `polkadot-cli` (locked to 0.9.43) of package `polkadot v0.9.43 (/builds/parity/mirrors/substrate/companions/polkadot)`
[2023-07-26 09:13:50]
[2023-07-26 09:13:50] failed to select a version for `regex` which could resolve this conflict
```
@melekes melekes requested a review from koute as a code owner July 26, 2023 11:26
@mxinden
Copy link
Contributor

mxinden commented Jul 26, 2023

zombienet test failed again, but with different error this time:

2023-07-25 16:48:35.169  INFO tokio-runtime-worker libp2p_kad::handler: Remote 12D3KooWQCkBm1BYtkHpocxCwMgR8yjitEeHGx8spzcDLGt2gkBm no longer supports our kademlia protocol

2023-07-25 16:48:35.169 TRACE tokio-runtime-worker sub-libp2p: Ignoring self-reported address /ip4/10.24.122.238/udp/30333/webrtc-direct/certhash/uEiAvF2IuPZJuKkdb5E-gSM_AmumLi3P5ZR_fvZGPE3vJEA from 12D3KooWQCkBm1BYtkHpocxCwMgR8yjitEeHGx8spzcDLGt2gkBm as remote node is not part of the Kademlia DHT supported by the local node.

2023-07-25 16:48:37.548 TRACE tokio-runtime-worker sub-libp2p: Libp2p => Query for 0020cc78cfbc884fd5668ebb8f2948752f2dbc3a4c665b1b678f6190ec7208cbff72 yielded 0 results    
2023-07-25 16:48:37.548 DEBUG tokio-runtime-worker sub-libp2p: Libp2p => Random Kademlia query has yielded empty results

not yet sure what is happening, but it looks like Kademlia does not provide dave with addresses to connect to...

Just an idea: are you either calling Kademlia::set_mode(Mode::Server) or are you providing an external address @melekes?

@melekes
Copy link
Contributor Author

melekes commented Jul 26, 2023

Just an idea: are you either calling Kademlia::set_mode(Mode::Server) or are you providing an external address @melekes?

@mxinden neither? I was hoping that the node will discover it's external address via bootnode and switch to server mode automatically.

@melekes
Copy link
Contributor Author

melekes commented Aug 2, 2023

2023-07-31 09:04:31 Substream 10786 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10788 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10790 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10792 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10794 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10796 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10798 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10800 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10802 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10804 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10806 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10808 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10810 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10812 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10814 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10816 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10818 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10820 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10822 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10824 dropped without graceful close, sending Reset
2023-07-31 09:04:31 Substream 10826 dropped without graceful close, sending Reset

Not sure why Charlie needs 10000 substreams unless there's a bug somewhere.

Alice & Bob writing to non-existing streams:

2023-07-31 09:05:15.895 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.895 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.896 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)
2023-07-31 09:05:15.897 ERROR tokio-runtime-worker webrtc_sctp::association::association_internal: [] stream 104 not found)

Dave fails to open ~2k substreams due to gone receiver:

rg "Can't send data channel 0: send failed because receiver is gone" | wc -l
    2387

@altonen
Copy link
Contributor

altonen commented Aug 2, 2023

@melekes are these problems reproducible with just libp2p or is Substrate somehow causing these issues?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T0-node This PR/Issue is related to the topic “node”.
Projects
Status: Backlog 🗒
Development

Successfully merging this pull request may close these issues.

Roadmap for allowing browser-embedded nodes to connect to Substrate networks