This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
try-runtime::fast-forward
#12896
Merged
paritytech-processbot
merged 23 commits into
paritytech:master
from
Cardinal-Cryptography:piomiko/try-runtime/forward
Feb 17, 2023
Merged
try-runtime::fast-forward
#12896
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
63ea95b
try-runtime::fast-forward
pmikolajczyk41 6cdf5e0
Revert un`pub`ing command's fields
pmikolajczyk41 4c56a1c
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 4641e1f
Handle storage change failure
pmikolajczyk41 87804f0
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 f8e4e28
Adjust Substrate node
pmikolajczyk41 576204d
Feature-gated imports
pmikolajczyk41 80e35f9
doc link
pmikolajczyk41 9ba596e
Feature-gated imports in node-template
pmikolajczyk41 8fb7ef3
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 785c608
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 b72c49d
Move trait, blanket implementation and auxiliary functions to a new m…
pmikolajczyk41 3d4a6d2
Distinguish between plain babe+timestamp and substrate enhanced info
pmikolajczyk41 620291c
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 dcdca6b
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 9bd97b9
Remove uncles inherents
pmikolajczyk41 30e3428
Missing argument
pmikolajczyk41 3b04222
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 5bbc309
Add doc comment about `blocktime_millis`
pmikolajczyk41 a3b28f2
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 cc6781c
Add licenses
pmikolajczyk41 4a41c76
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 638f224
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
292 changes: 292 additions & 0 deletions
292
utils/frame/try-runtime/cli/src/commands/fast_forward.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,292 @@ | ||
use crate::{ | ||
ggwpez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
build_executor, full_extensions, rpc_err_handler, state_machine_call, BlockT, LiveState, | ||
SharedParams, State, | ||
}; | ||
use parity_scale_codec::{Decode, Encode}; | ||
use sc_cli::Result; | ||
use sc_executor::{sp_wasm_interface::HostFunctions, WasmExecutor}; | ||
use serde::de::DeserializeOwned; | ||
use sp_core::H256; | ||
use sp_inherents::{InherentData, InherentDataProvider}; | ||
use sp_io::TestExternalities; | ||
use sp_runtime::{ | ||
traits::{Header, NumberFor, One}, | ||
Digest, DigestItem, | ||
}; | ||
use std::{fmt::Debug, str::FromStr}; | ||
use substrate_rpc_client::{ws_client, ChainApi}; | ||
|
||
/// Configurations of the [`Command::FastForward`]. | ||
#[derive(Debug, Clone, clap::Parser)] | ||
pub struct FastForwardCmd { | ||
/// How many blocks should be processed. If `None`, then blocks will be produced and processed | ||
/// in a loop. | ||
#[arg(long)] | ||
n_blocks: Option<u64>, | ||
|
||
/// The state type to use. | ||
#[command(subcommand)] | ||
state: State, | ||
|
||
/// The ws uri from which to fetch the block. | ||
/// | ||
/// If `state` is `Live`, this is ignored. Otherwise, it must not be empty. | ||
ggwpez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// todo: make use of clap's arg groups | ||
#[arg(long, value_parser = crate::parse::url)] | ||
block_ws_uri: Option<String>, | ||
|
||
/// Which try-state targets to execute when running this command. | ||
/// | ||
/// Expected values: | ||
/// - `all` | ||
/// - `none` | ||
/// - A comma separated list of pallets, as per pallet names in `construct_runtime!()` (e.g. | ||
/// `Staking, System`). | ||
/// - `rr-[x]` where `[x]` is a number. Then, the given number of pallets are checked in a | ||
/// round-robin fashion. | ||
#[arg(long, default_value = "all")] | ||
try_state: frame_try_runtime::TryStateSelect, | ||
} | ||
|
||
impl FastForwardCmd { | ||
fn block_ws_uri(&self) -> &str { | ||
match self.state { | ||
State::Live(LiveState { ref uri, .. }) => &uri, | ||
_ => self | ||
.block_ws_uri | ||
.as_ref() | ||
.expect("Either `--block-uri` must be provided, or state must be `live`"), | ||
} | ||
} | ||
} | ||
|
||
/// Something that can create inherent data providers and pre-runtime digest. | ||
/// | ||
/// It is possible for the caller to provide custom arguments to the callee by setting the | ||
/// `ExtraArgs` generic parameter. | ||
/// | ||
/// This module already provides some convenience implementation of this trait for closures. So, it | ||
/// should not be required to implement | ||
#[async_trait::async_trait] | ||
pub trait BlockBuildingInfoProvider<Block: BlockT, ExtraArgs = ()> { | ||
type InherentDataProviders: InherentDataProvider; | ||
|
||
async fn get_inherent_providers_and_pre_digest( | ||
&self, | ||
parent_hash: Block::Hash, | ||
extra_args: ExtraArgs, | ||
) -> Result<(Self::InherentDataProviders, Vec<DigestItem>)>; | ||
} | ||
|
||
#[async_trait::async_trait] | ||
impl<F, Block, IDP, ExtraArgs, Fut> BlockBuildingInfoProvider<Block, ExtraArgs> for F | ||
where | ||
Block: BlockT, | ||
F: Fn(Block::Hash, ExtraArgs) -> Fut + Sync + Send, | ||
Fut: std::future::Future<Output = Result<(IDP, Vec<DigestItem>)>> + Send + 'static, | ||
IDP: InherentDataProvider + 'static, | ||
ExtraArgs: Send + 'static, | ||
{ | ||
type InherentDataProviders = IDP; | ||
|
||
async fn get_inherent_providers_and_pre_digest( | ||
&self, | ||
parent: Block::Hash, | ||
extra_args: ExtraArgs, | ||
) -> Result<(Self::InherentDataProviders, Vec<DigestItem>)> { | ||
(*self)(parent, extra_args).await | ||
} | ||
} | ||
|
||
/// Read the block number corresponding to `hash` with an RPC call to `ws_uri`. | ||
async fn get_block_number<Block: BlockT>( | ||
hash: Block::Hash, | ||
ws_uri: &str, | ||
) -> Result<NumberFor<Block>> | ||
where | ||
Block::Header: DeserializeOwned, | ||
{ | ||
let rpc = ws_client(ws_uri).await?; | ||
Ok(ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, Some(hash)) | ||
kianenigma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.await | ||
.map_err(rpc_err_handler) | ||
.and_then(|maybe_header| maybe_header.ok_or("header_not_found").map(|h| *h.number()))?) | ||
} | ||
|
||
/// Call `method` with `data` and return the result. `externalities` will not change. | ||
async fn dry_run<T: Decode, Block: BlockT, HostFns: HostFunctions>( | ||
externalities: &TestExternalities, | ||
executor: &WasmExecutor<HostFns>, | ||
method: &'static str, | ||
data: &[u8], | ||
) -> Result<T> { | ||
let (_, result) = state_machine_call::<Block, HostFns>( | ||
externalities, | ||
executor, | ||
method, | ||
data, | ||
full_extensions(), | ||
)?; | ||
|
||
Ok(<T>::decode(&mut &*result)?) | ||
} | ||
|
||
/// Call `method` with `data` and actually save storage changes to `externalities`. | ||
async fn run<Block: BlockT, HostFns: HostFunctions>( | ||
externalities: &mut TestExternalities, | ||
executor: &WasmExecutor<HostFns>, | ||
method: &'static str, | ||
data: &[u8], | ||
) -> Result<()> { | ||
let (mut changes, _) = state_machine_call::<Block, HostFns>( | ||
externalities, | ||
executor, | ||
method, | ||
data, | ||
full_extensions(), | ||
)?; | ||
|
||
let storage_changes = changes | ||
.drain_storage_changes( | ||
&externalities.backend, | ||
&mut Default::default(), | ||
externalities.state_version, | ||
) | ||
.unwrap(); | ||
kianenigma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
externalities | ||
.backend | ||
.apply_transaction(storage_changes.transaction_storage_root, storage_changes.transaction); | ||
|
||
Ok(()) | ||
} | ||
|
||
/// Produce next empty block. | ||
async fn next_empty_block< | ||
Block: BlockT, | ||
HostFns: HostFunctions, | ||
BBIP: BlockBuildingInfoProvider<Block, Option<(InherentData, Digest)>>, | ||
>( | ||
externalities: &mut TestExternalities, | ||
executor: &WasmExecutor<HostFns>, | ||
parent_height: NumberFor<Block>, | ||
parent_hash: Block::Hash, | ||
block_building_info_provider: &Option<BBIP>, | ||
previous_block_building_info: Option<(InherentData, Digest)>, | ||
) -> Result<(Block, Option<(InherentData, Digest)>)> { | ||
let (maybe_inherent_data, pre_digest) = match &block_building_info_provider { | ||
None => (None, Default::default()), | ||
Some(bbip) => { | ||
let (inherent_data_provider, pre_digest) = bbip | ||
.get_inherent_providers_and_pre_digest(parent_hash, previous_block_building_info) | ||
.await?; | ||
let inherent_data = inherent_data_provider | ||
.create_inherent_data() | ||
.await | ||
.map_err(|e| sc_cli::Error::Input(format!("I don't know how to convert {e:?}")))?; | ||
|
||
(Some(inherent_data), Digest { logs: pre_digest }) | ||
}, | ||
}; | ||
|
||
let header = Block::Header::new( | ||
parent_height + One::one(), | ||
Default::default(), | ||
Default::default(), | ||
parent_hash, | ||
pre_digest.clone(), | ||
); | ||
let mut extrinsics = <Vec<Block::Extrinsic>>::new(); | ||
|
||
run::<Block, _>(externalities, executor, "Core_initialize_block", &header.encode()).await?; | ||
|
||
if let Some(ref inherent_data) = maybe_inherent_data { | ||
extrinsics = dry_run::<Vec<Block::Extrinsic>, Block, _>( | ||
externalities, | ||
executor, | ||
"BlockBuilder_inherent_extrinsics", | ||
&inherent_data.encode(), | ||
) | ||
.await?; | ||
} | ||
|
||
for xt in &extrinsics { | ||
run::<Block, _>(externalities, executor, "BlockBuilder_apply_extrinsic", &xt.encode()) | ||
.await?; | ||
} | ||
|
||
let header = dry_run::<Block::Header, Block, _>( | ||
externalities, | ||
executor, | ||
"BlockBuilder_finalize_block", | ||
&[0u8; 0], | ||
) | ||
.await?; | ||
|
||
run::<Block, _>(externalities, executor, "BlockBuilder_finalize_block", &[0u8; 0]).await?; | ||
|
||
Ok((Block::new(header, extrinsics), (maybe_inherent_data.map(|id| (id, pre_digest))))) | ||
} | ||
|
||
pub(crate) async fn fast_forward<Block, HostFns, BBIP>( | ||
shared: SharedParams, | ||
command: FastForwardCmd, | ||
block_building_info_provider: Option<BBIP>, | ||
) -> Result<()> | ||
where | ||
Block: BlockT<Hash = H256> + DeserializeOwned, | ||
Block::Hash: FromStr, | ||
Block::Header: DeserializeOwned, | ||
<Block::Hash as FromStr>::Err: Debug, | ||
NumberFor<Block>: FromStr, | ||
<NumberFor<Block> as FromStr>::Err: Debug, | ||
HostFns: HostFunctions, | ||
BBIP: BlockBuildingInfoProvider<Block, Option<(InherentData, Digest)>>, | ||
{ | ||
let executor = build_executor::<HostFns>(&shared); | ||
let ext = command.state.into_ext::<Block, HostFns>(&shared, &executor, None).await?; | ||
|
||
let mut last_block_hash = ext.block_hash; | ||
let mut last_block_number = | ||
get_block_number::<Block>(last_block_hash, command.block_ws_uri()).await?; | ||
let mut prev_block_building_info = None; | ||
|
||
let mut ext = ext.inner_ext; | ||
|
||
for _ in 1..=command.n_blocks.unwrap_or(u64::MAX) { | ||
// We are saving state before we overwrite it while producing new block. | ||
let backend = ext.as_backend(); | ||
|
||
log::info!("Producing new empty block at height {:?}", last_block_number + One::one()); | ||
|
||
let (next_block, new_block_building_info) = next_empty_block::<Block, HostFns, BBIP>( | ||
&mut ext, | ||
&executor, | ||
last_block_number, | ||
last_block_hash, | ||
&block_building_info_provider, | ||
prev_block_building_info, | ||
) | ||
.await?; | ||
|
||
log::info!("Produced a new block: {:?}", next_block.header()); | ||
|
||
// And now we restore previous state. | ||
ext.backend = backend; | ||
|
||
let state_root_check = true; | ||
let signature_check = true; | ||
let payload = | ||
(next_block.clone(), state_root_check, signature_check, command.try_state.clone()) | ||
.encode(); | ||
run::<Block, _>(&mut ext, &executor, "TryRuntime_execute_block", &payload).await?; | ||
|
||
log::info!("Executed the new block"); | ||
|
||
prev_block_building_info = new_block_building_info; | ||
last_block_hash = next_block.hash(); | ||
last_block_number += One::one(); | ||
} | ||
|
||
Ok(()) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Slightly unfortunate that chains implementing try-runtime have to add this snippet. Also, this kinda breaks #12975, any alternatives?
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 definitely agree that this complicates things, but I'm afraid that as long as we want to remain chain-agnostic, similar approach must be taken.
Firstly, please notice that all other subcommands of
try-runtime
were oblivious to the chain they were working with (as long it was exposing the proper runtime API calls). However, when it comes to block production (like infast-forward
), we are facing the process that is often unique to every chain (here: inherents and digests). I suppose that majority of the chains in the ecosystem will have exactly the same configuration here (timestamp + aura), but this won't be true everywhere.It can be also anticipated, that sooner or later we might enrich
try-runtime
with new functionalities that will have some chain-dependent components. This is why in [12975](#12975 I proposed splittingtry-runtime
tool into a part that contains pure (chain-agnostic) logic and a part where customization takes place. In this sense, I don't see using user snippet as breaking.Yes, the misunderstanding comes from my poor wording - by 'client side code' I was referring to the CLI part, i.e. the code that handles user's input to the
try-runtime
tool.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.
Okay, two thoughts for now:
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 find a client with different setup, I had to just complete this PR for
node-cli
(i.e. substrate node) - there we have Babe instead of Aura, uncles and storage proof inherents. Apart from this, you can check:polkadot_node_core_parachains_inherent
(https://github.com/paritytech/polkadot/blob/master/node/service/src/lib.rs#L1155), orsp_consensus_subspace::inherents
(https://github.com/subspace/subspace/blob/main/crates/subspace-service/src/lib.rs#L282)I don't want to push my approach, so if you see any alternative, I'm eager to implement it. Otherwise, if we stick to this way of customization (at least for now), I will create companion PR for Polkadot.
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 did not really check the code in here, but most of the stuff you are doing here is already provided for manual-seal:
substrate/client/consensus/manual-seal/src/consensus/aura.rs
Lines 75 to 90 in 4e38342
In general, manual-seal already provides functionality to "fast-forward" as well.
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.
while semantically
create_digest
is perfectly applicable in this PR, it requires a type that supportsAuxStore + ProvideRuntimeApi<B> + UsageProvider<B>
for Aura orAuxStore + HeaderBackend<B> + HeaderMetadata<B, Error = sp_blockchain::Error> + UsageProvider<B> + ProvideRuntimeApi<B>
for Babe, which goes far out of scope what we use in eithertry-runtime
itself or evencommands
modulesThere 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 mean the logic should be shared. I also need to rewrite sp-api that you can use it here instead of calling functions by their name.
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.
Okay, apologies for misunderstanding. Do you think that I should make some steps towards this logic sharing in this PR? If so, then what would be the good place to put common things in?
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.
Maybe for now you just copy the relevant code to
try-build
? And make it reusable for people? So that they don't always need to implement the logic?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 couldn't find any
try-build
, so I assumed that you meanttry-runtime
... I moved both trait and two most common (I guess) impementations to a new module withintry-runtime
. It cost a fewsp-*
dependencies, but indeed, ease of usage is way higher. Please let me know if this is what you suggested.