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

fix(runtime update): wait until upgrade on chain #1321

Merged
merged 13 commits into from
Jan 8, 2024
61 changes: 52 additions & 9 deletions subxt/src/client/online_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// see LICENSE for license details.

use super::{OfflineClient, OfflineClientT};
use crate::config::Header;
use crate::custom_values::CustomValuesClient;
use crate::{
backend::{
Expand Down Expand Up @@ -437,17 +438,13 @@ impl<T: Config> RuntimeUpdaterStream<T> {
Err(err) => return Some(Err(err)),
};

let latest_block_ref = match self.client.backend().latest_finalized_block_ref().await {
Ok(block_ref) => block_ref,
Err(e) => return Some(Err(e)),
let at = match wait_for_runtime_upgrade(&self.client, &runtime_version).await {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit; maybe rename to wait_for_finalized_block_containing_update or something like that

Some(Ok(at)) => at,
Some(Err(err)) => return Some(Err(err)),
None => return None,
};
niklasad1 marked this conversation as resolved.
Show resolved Hide resolved

let metadata = match OnlineClient::fetch_metadata(
self.client.backend(),
latest_block_ref.hash(),
)
.await
{
let metadata = match OnlineClient::fetch_metadata(self.client.backend(), at.hash()).await {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we fetch the metadata from the backend here? I guess it is not part of the "LastRuntimeUpgrade" storage item for which we already got the scale_val below, that contains the spec_version?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep exactly; the storage item tells us that the runtime version has updated in this block (but doesn't contain the metadata too), and so once we see that a block is updated, we fetch the metadata at that block to use to update Subxt

Ok(metadata) => metadata,
Err(err) => return Some(Err(err)),
};
Expand Down Expand Up @@ -484,3 +481,49 @@ impl Update {
&self.metadata
}
}

/// Helper to wait until the runtime upgrade is applied on at finalized block.
async fn wait_for_runtime_upgrade<T: Config>(
client: &OnlineClient<T>,
runtime_version: &RuntimeVersion,
) -> Option<Result<T::Header, Error>> {
use scale_value::At;

let mut block_sub = match client.backend().stream_finalized_block_headers().await {
Ok(s) => s,
Err(err) => return Some(Err(err)),
};

let head = loop {
let (block, block_ref) = match block_sub.next().await? {
Ok(n) => n,
Err(err) => return Some(Err(err)),
};

let key: Vec<scale_value::Value> = vec![];
let addr = crate::dynamic::storage("System", "LastRuntimeUpgrade", key);

let chunk = match client.storage().at(block_ref).fetch(&addr).await {
Ok(Some(v)) => v,
Ok(None) => continue,
Err(e) => return Some(Err(e)),
};

let scale_val = match chunk.to_value() {
Ok(v) => v,
Err(e) => return Some(Err(e)),
};

let spec_version = scale_val
.at("spec_version")
.and_then(|v| v.as_u128())
.expect("specVersion should exist on RuntimeVersion; qed")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: since we're talking to a thing that could return any sort of nonsense it's probably best to return an actual error here

as u32;

if spec_version >= runtime_version.spec_version {
niklasad1 marked this conversation as resolved.
Show resolved Hide resolved
break block;
}
};

Some(Ok(head))
niklasad1 marked this conversation as resolved.
Show resolved Hide resolved
}
Loading