Skip to content
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

Subscribe to Runtime upgrades for proper extrinsic construction #513

Merged
merged 38 commits into from
May 11, 2022

Conversation

lexnv
Copy link
Collaborator

@lexnv lexnv commented Apr 20, 2022

There are cases when the subxt Client is connected to a substrate node
that performs a Runtime update (forkless update).

The RuntimeVersion is populated when the Client first connects to a node, but it was never
updated to keep in sync with the node. The spec_version and transaction_version of the
RuntimeVersion are included in the transaction parameters. Having the values out of sync means
that the subxt Client can no longer submit any extrinsics.

This PR exposes the ability to subscribe to the runtime updates via subscribe_runtime_version and
receive notifications when the node performs certain updates.

Customers can easily subscribe to runtime updates utilizing a few lines of code

    let update_client = api.client.updates();
    tokio::spawn(async move {
        let result = update_client.perform_runtime_updates().await;
        println!("Runtime update failed with result={:?}", result);
    });

To keep subxt agnostic of the async runtime preferred by the customer, and to simplify the
code needed for providing the updates, the runtime update is handled entirely by the customer.
Furthermore, when a runtime update is detected the customer has the ability to set on the
subxt Client the new RuntimeVersion and Metadata which would allow for further extrinsics to be submitted.

An example to illustrate the runtime update is introduced:

> RUST_LOG=debug cargo run --example subscribe_runtime_updates                                                            

Balance transfer extrinsic submitted: 0xce5c…003a
[2022-05-02T16:31:00Z INFO  subxt::updates] Performing runtime update from 100 to 101
[2022-05-02T16:31:00Z DEBUG subxt::updates] Performing metadata update
[2022-05-02T16:31:00Z DEBUG subxt::updates] Runtime update completed

Balance transfer extrinsic submitted: 0x444e…a06c
[2022-05-02T16:31:54Z INFO  subxt::updates] Performing runtime update from 101 to 102
[2022-05-02T16:31:54Z DEBUG subxt::updates] Performing metadata update
[2022-05-02T16:31:54Z DEBUG subxt::updates] Runtime update completed

Balance transfer extrinsic submitted: 0xca0e…df24

Performing the same operations of updating the node, but without updating subxt Client itself:

Balance transfer extrinsic submitted: 0xfbef…0230
Balance transfer extrinsic submitted: 0x6790…a08f
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Rpc(Call(Custom { code: 1010, message: "Invalid Transaction", data: Some(RawValue("Transaction has a bad signature")) }))', examples/examples/subscribe_runtime_updates.rs:55:14

Next Steps

  • Synchronize the client metadata with the runtime metadata to detect changes using Static Metadata Validation #478 (however it would be counterproductive to do so until the static metadata PR is merged due to conflicts).

Closes #509.

lexnv added 3 commits April 20, 2022 14:10
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>
@lexnv lexnv requested review from ascjones and jsdw April 20, 2022 14:39
subxt/src/client.rs Outdated Show resolved Hide resolved
Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

I found the API a little hard to use for end-users.

Basically, I was kind of expecting subxt to spawn a separate task that does the runtime subscription for the user and update the runtime version under the hood. Is that too opinionated?

After the a runtime upgrade the subxt client is mostly useless anyway (e.g. the spec version gets bumped)

Otherwise, the code looks good

@jsdw
Copy link
Collaborator

jsdw commented Apr 28, 2022

This is an interesting point! I was also expecting subxt to spawn something under the hood and do this transparently, but as @lexnv points out, subxt is currently runtime agnostic, which is handy, and it would have to give that up to spawn tasks.

Let's have a think about this and try to find the simplest API possible to update the runtime version appropriately that keeps subxt agnostic of the runtime used.

Now that the metadata validation PR is in, I'd hope that after a runtime upgrade subxt is still mostly using the correct types, and as long as we make sure to pull new metadata from the node on a runtime update, we can continue to properly validate calls.

I note that we are pulling in Tokio from jsonrpsee at the mo, but thinking ahead to things like supporting wasm, (which would no longer bring that in) means I would still bve reluctant to committing to it here, for the sake of future simplicity!

@jsdw
Copy link
Collaborator

jsdw commented Apr 28, 2022

A random stab at what an API could look like:

// spawn a background task to keep subxt up to date w.r.t node runtime updates.
tokio::spawn(async move {
    api.client.perform_automatic_runtime_updates().await // awaits forever.
});

// use the api however you like in the meantime, and know it'll be kept uptodate in the background...
api.tx().balances()...

How this would work roughly is that the background update task would subscribe to runtime updates, and when it receives one, do something like:

  • acquire a lock on the (async) mutex containing runtime + metadata,
  • go and download the metadata
  • update metadata + runtime version
  • unlock the mutex.

The idea is that as soon as we know an update has happened, block any pending requests until we have new runtime + metadata and can send them with the correct versions and validate them against the correct metadata. It'll require a bit of care to do properly (need to handle errors while locked and such).

lexnv added 11 commits May 2, 2022 14:09
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>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv
Copy link
Collaborator Author

lexnv commented May 2, 2022

This revision adds an alternative to those described above.

Unfortunately, the client cannot be moved inside the tokio task and reutilized further.
Neither can a reference to the API/client be made available inside the task as it requires a 'static lifetime.

Having usability in mind as a first step, an appropriate solution to this would be exposing a wrapper of the
subxt::Client similar to the StorageClient. The wrapper would need to take care of the updates:

  1. subscribing to events and fetching metadata
  2. as well as updating the main client RuntimeVersion and Metadata (not just the frame_metadata::Metadata, but the one utilized by subxt::Client for caching).

For the first case, the Client's rpc can be cloned and moved inside the tokio task.
For the latter, RuntimeVersion and Metadata need to be protected by Arc<RwLock>s.

This basically would translate into the following interaction with the API

    let update_client = api.client.updates();
    tokio::spawn(async move {
        update_client.perform_runtime_updates().await;
    });
    
    // Use API further for the main use-case
    api.tx().balances()...

lexnv added 8 commits May 2, 2022 20:00
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>
lexnv added 10 commits May 4, 2022 19:23
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>
Revert "Fix test deadlock"

This reverts commit 4a79933.

Revert "Update examples and integration-tests"

This reverts commit 5423f6e.

Revert "Remove unused dependencies"

This reverts commit e8ecbab.

Revert "codegen: Use async API to handle async locking"

This reverts commit ced4646.

Revert "subxt: Block executor while decoding dynamic events"

This reverts commit 8b3ba4a.

Revert "subxt: Switch to async::Mutex"

This reverts commit f5bde9b.
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>
codegen/src/api/constants.rs Outdated Show resolved Hide resolved
subxt/src/updates.rs Outdated Show resolved Hide resolved
@jsdw
Copy link
Collaborator

jsdw commented May 9, 2022

Left a couple of very minor comments, but otherwise I think this is great stuff, and am happy to see it merge :)

lexnv and others added 6 commits May 9, 2022 19:02
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: James Wilson <james@jsdw.me>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

Looks great,

I like the improved API

Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

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

Awesome stuff :)

@jsdw jsdw merged commit 115073a into master May 11, 2022
@jsdw jsdw deleted the 509_runtime_upgrades branch May 11, 2022 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gracefully handle runtime upgrades while connected to a node
3 participants