fix(webrtc): warp sync slow initialization and webrtc handshake bug#3256
Conversation
Initialize warp sync source finalized_block_height with best_block_number from gossip handshake instead of 0, so warp sync triggers immediately without waiting for a GrandPa neighbor packet.
gab8i
changed the title
dmitry-markin
left a comment
dmitry-markin
left a comment
There was a problem hiding this comment.
Substream waking in WebRTC looks good.
The idea behind optimistically targeting warp sync at the best block and not finalized one is also good, but we need to rename finalized_block_height to something like optimistic_finalized_block_height, or a better name, so future code doesn't rely on this value as actually being finalized.
Also, the comment at warp_sync.rs:768 likely needs updating and the corresponding logic checked for correctness.
| // Wait for either platform data or a coordinator write | ||
| // notification (e.g. after AcceptInNotifications queues a | ||
| // handshake response that needs to be flushed). | ||
| let write_listener = write_ready.listen(); |
There was a problem hiding this comment.
Should we move the write_ready.listen() outside the Box::pin future?
Currently, the write listener is registered once the the when_substreams_rw_ready is firstly polled.
This means we could arrive in a situation where .notify(usize::MAX); is called before registering the listener call.
For this to happen we need to:
- receive WakeUpReason::CoordinatorMessage(message)
message_sending.as_ref().as_pin_ref().is_none()should have some data to effectively take theNonepath and ignore thewhen_substreams_rw_ready.select_next_some()
Moving the listener above Box::pin solves this regardless of which states call when_substreams_rw_ready.poll_next and avoids the problem entirely
There was a problem hiding this comment.
You are totally right, it should definitely be moved out so that it is registered and the async block later on depends on the registered event listener!
This is to: - publish `smoldot-v3.3.0` npm. - publish crates `smoldot-light v1.3.0`, `smoldot v2.0.0`. ## Changes ### Added - Add `bitswap_unstable_stream` / `bitswap_unstable_unstream`: fetch many Bitswap CIDs in one subscription, receiving each block (or per-CID error) via `bitswap_unstable_streamEvent` notifications as it arrives. ([#3264](#3264)) ### Changed - Rename `bitswap_v1_get` to `bitswap_unstable_get` (`bitswap_v1_get` kept as a legacy alias). ([#3264](#3264)) - Discover peers via proper Kademlia walks, not only nodes with an open consensus stream, so smoldot keeps finding peers even when connected only to bootnodes with no free sync slots. ([#3284](#3284)) - Start warp sync without waiting for a GrandPa notification, seeding each source's finalized height from its announced best block; especially speeds up WebRTC, where data-channel setup adds latency. ([#3256](#3256)) ### Fixed - Fix a deadlock in the multistream-select task loop. ([#3256](#3256)) ## Release process Also updates `docs/RELEASING.md` with three fixes found while cutting this release: - **npm lockfiles:** discover every `file:`-linked project (`e2e-tests/js`, `e2e-tests/browser`, `benchmarks/js`) and refresh its lock, instead of hardcoding a list. - **Major crate cross:** also bump path-dep `version` strings (`full-node/`, `light-base/`, `wasm-node/rust/`) when a crate crosses a major (here `smoldot → 2.0.0`). - **Signing:** drop `--no-gpg-sign` so the release commit is signed. ## Lockfiles `benchmarks/Cargo.lock` also bumps `zombienet-sdk` 0.4.11 → 0.4.15 — stale on `main` (benchmarks isn't built `--locked` in CI), pulled current while resolving the version bump.
6 participants
The following PR just extract work already done within paritytech/smoldot-archive#2222.
The fixes are: