-
Notifications
You must be signed in to change notification settings - Fork 254
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
light-client: Add experimental light-client support #965
Conversation
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Been dreaming about this for a long time 🚀 IIRC @rphmeier also expressed an interest in this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good,
How to deal with the git dependency
is a blocker from my side otherwise really cool to have support for this in subxt
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Nice, looking forward to trying this out. Is it also intended to work in-browser with Wasm compilation as well as native executables? |
let result = self.client.json_rpc_request(request, self.chain_id); | ||
if let Err(err) = result { | ||
tracing::warn!( | ||
target: LOG_TARGET, | ||
"Cannot send RPC request to lightclient {:?}", | ||
err.to_string() | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just want to point out that this can realistically happen. If a TooBusy
error is returned, you're supposed to back-pressure the requests sender.
I understand that from your point of view it's not very convenient, so I've opened smol-dot/smoldot#804 to clarify this thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify: there will always be an error here. But maybe I can provide some configuration option that guarantees a minimum buffer size, so that you have a runtime guarantee that it won't happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I was a bit confused this morning since I was working on something else in parallel.
What you should do is pass max_pending_requests: u32::max_value()
and max_subscriptions: u32::max_value()
when adding the chain. There's no need to make these parameters configurable.
These parameters exist only to make sure that the memory usage of smoldot is bounded in situations where the JSON-RPC client is potentially malicious and can spam you with requests. This isn't relevant here, since you are the JSON-RPC client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great PR, looks good to me
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good work, I like the multiplexing over the background task a lot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few remaining nits and small things, but in general this is amazing stuff! Clean, well documented and tested. Good job!!
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
* rpc/types: Decode `SubstrateTxStatus` for substrate and smoldot Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * lightclient: Add light client Error Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * lightclient: Add background task to manage RPC responses Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * lightclient: Implement the light client RPC in subxt Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * subxt: Expose light client under experimental feature-flag Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * artifacts: Add development chain spec for local nodes Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Update cargo lock Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * examples: Add light client example Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Update sp-* crates and smoldot to use git with branch / rev Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Apply cargo fmt Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Fix clippy Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Import hashmap entry Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * lightclient: Fetch spec only if jsonrpsee feature is enabled Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Update subxt/src/rpc/lightclient/background.rs Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com> * Fix typo Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * artifacts: Update dev chain spec Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * types: Handle storage replies from chainHead_storage Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * artifacts: Add polkadot spec Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * lightclient: Handle RPC error responses Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * examples: Tx basic with light client for local nodes Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * example: Light client coprehensive example for live chains Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * examples: Remove prior light client example Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * feature: Rename experimental to unstable Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * book: Add light client section Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * testing: Fix clippy Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * lightclient: Ignore validated events Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust tests for light-clients and normal clients Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * testing: Keep lightclient variant Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Remove support for chainHead_storage for light client Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Update light client to point to crates.io Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Update sp-crates from crates.io Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Replace Atomic with u64 Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Add LightClientBuilder Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust chainspec with provided bootnodes Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Add potential_relay_chains to light client builder Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Move the light-client to the background task Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust tracing logs Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Update book and example Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Apply cargo fmt Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Remove dev_spec.json artifact Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Examples fix duplicate Cargo.toml Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Use tracing_subscriber crate Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Fix clippy for different features Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Add comment about bootNodes Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Add comment about tracing-sub dependency Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Run integration-tests with light-client Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Feature guard some incompatible tests Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * ci: Enable light-client tests under feature flag Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * ci: Fix git step name Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust flags for testing Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust warnings Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Rename feature flag jsonrpsee-ws to jsonrpsee Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Fix cargo check Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * ci: Run tests on just 2 threads Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Move light-client to subxt/src/client Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust LightClientBuilder Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Use ws_url to construct light client for testing Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Refactor background Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Address feedback Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Remove polkadot.spec and trim sub_id Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Wait for substrate to produce block before connecting light client Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust builder and tests Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Apply fmt Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * ci: Use release for light client testing Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Add single test for light-client Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Wait for more blocks Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Use polkadot endpoint for testing Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust cargo check Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * examples: Remove light client chain connection example Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust cargo.toml section for the old example Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust background task to use usize for subscription Id Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Build bootnodes with serde_json::Value directly Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Make channel between subxt user and subxt background unbounded Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Update subxt/src/client/lightclient/builder.rs Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com> * Switch to smoldot 0.6.0 from 0.5.0 Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Move testing to `full_client` and `light_client` higher modules Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Remove subscriptionID type Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Remove subxt/integration-testing feature flag Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust wait_for_blocks documentation Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Adjust utils import for testing Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> * Remove into_iter from builder construction Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> --------- Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
This PR exposes the
LightClient
that uses smoldot to connect to chains.Smoldot exposes a single non-clonable object that exposes all responses of all methods and all subscriptions via
a
next()
method call. Because of this, a background task is spawned to multiplex the responses backto the appropriate submitters.
Smoldot generates some extra events while submitting an extrinsic, this is related to the new RPC Spec V2.
As a result of that, spurious events emitted by
transaction_unstable_watchEvent
are ignored.The
SubstrateTxStatus
needed to be changed as well to support both lower camelCase and upper CamelCase.When using the smoldot from crates-io, the implementation panics at:
However, using the latest from the smoldot repo does not reproduce the issue.
The PR exposes 2 ways to construct the Light Client:
The former version is working as expected, while the latter seems to cause an overflow issue in smoldot
Testing Done
Initially using smoldot to target a local node caused the client to hang with
GrandPa warp sync
warnings.Using the latest branch, I was able to start a node as normal:
./polkadot-6dc9e84dde2 --dev --alice --validator
cargo run --example tx_basic_light_client
Available functionality
The following functionality is tested both on a local node, as well as on the polkadot live chain:
chainHead
RPC methodKnown limitations
value: ["0xhex", "0xhex"]
shape, where subxt expects from substrate chainsresult: "0xhex"
value: "0xhex"
shape, where subxt expects from substrate chainsresult: "0xhex"
(this is handled internally)Closes: #962