From 472a40d5aaaeaba74daa976eed9d5d9793e802ea Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 9 Nov 2023 16:36:55 +0400 Subject: [PATCH 1/7] execute correct block hash --- core/src/commands/execute_block.rs | 80 +++++++++++----------------- core/src/commands/offchain_worker.rs | 39 +++++++++----- core/src/state.rs | 62 +++++++++++++++++++-- core/tests/execute_block.rs | 17 +++--- 4 files changed, 125 insertions(+), 73 deletions(-) diff --git a/core/src/commands/execute_block.rs b/core/src/commands/execute_block.rs index b36afcbaa67..ceb66133d75 100644 --- a/core/src/commands/execute_block.rs +++ b/core/src/commands/execute_block.rs @@ -19,7 +19,6 @@ use std::{fmt::Debug, str::FromStr}; use parity_scale_codec::Encode; use sc_executor::sp_wasm_interface::HostFunctions; -use sp_rpc::{list::ListOrValue, number::NumberOrHex}; use sp_runtime::{ generic::SignedBlock, traits::{Block as BlockT, Header as HeaderT, NumberFor}, @@ -27,7 +26,7 @@ use sp_runtime::{ use substrate_rpc_client::{ws_client, ChainApi}; use crate::{ - build_executor, full_extensions, rpc_err_handler, + build_executor, full_extensions, hash_of, rpc_err_handler, state::{LiveState, State}, state_machine_call_with_proof, SharedParams, LOG_TARGET, }; @@ -95,26 +94,39 @@ where HostFns: HostFunctions, { let executor = build_executor::(&shared); - let ext = command - .state - .to_ext::(&shared, &executor, None, true, false) - .await?; - - // get the block number associated with this block. let block_ws_uri = command.block_ws_uri(); let rpc = ws_client(&block_ws_uri).await?; - let next_hash = next_hash_of::(&rpc, ext.block_hash).await?; - log::info!(target: LOG_TARGET, "fetching next block: {:?} ", next_hash); + let live_state = match command.state { + State::Live(live_state) => live_state, + _ => { + unreachable!("execute block currently only supports Live state") + } + }; + + // The block we want to *execute* at is the block passed by the user + dbg!(live_state.at.clone()); + let execute_at = live_state + .at + .clone() + .map(|s| hash_of::(s.as_str())) + .transpose()?; - let block = ChainApi::<(), Block::Hash, Block::Header, SignedBlock>::block( - &rpc, - Some(next_hash), - ) - .await - .map_err(rpc_err_handler)? - .expect("header exists, block should also exist; qed") - .block; + // let prev_block_live_state = prev_block_live_state::(&rpc, &live_state).await?; + let prev_block_live_state = live_state.to_prev_block_live_state::().await?; + + // Get state for the prev block + let ext = State::Live(prev_block_live_state) + .to_ext::(&shared, &executor, None, true, false) + .await?; + + // Execute the desired block on top of it + let block = + ChainApi::<(), Block::Hash, Block::Header, SignedBlock>::block(&rpc, execute_at) + .await + .map_err(rpc_err_handler)? + .expect("header exists, block should also exist; qed") + .block; // A digest item gets added when the runtime is processing the block, so we need to pop // the last one to be consistent with what a gossiped block would contain. @@ -144,35 +156,3 @@ where Ok(()) } - -pub(crate) async fn next_hash_of( - rpc: &substrate_rpc_client::WsClient, - hash: Block::Hash, -) -> sc_cli::Result -where - Block: BlockT + serde::de::DeserializeOwned, - Block::Header: serde::de::DeserializeOwned, -{ - let number = ChainApi::<(), Block::Hash, Block::Header, ()>::header(rpc, Some(hash)) - .await - .map_err(rpc_err_handler) - .and_then(|maybe_header| maybe_header.ok_or("header_not_found").map(|h| *h.number()))?; - - let next = number + sp_runtime::traits::One::one(); - - let next_hash = match ChainApi::<(), Block::Hash, Block::Header, ()>::block_hash( - rpc, - Some(ListOrValue::Value(NumberOrHex::Number( - next.try_into() - .map_err(|_| "failed to convert number to block number")?, - ))), - ) - .await - .map_err(rpc_err_handler)? - { - ListOrValue::Value(t) => t.expect("value passed in; value comes out; qed"), - _ => unreachable!(), - }; - - Ok(next_hash) -} diff --git a/core/src/commands/offchain_worker.rs b/core/src/commands/offchain_worker.rs index 2b3daeca83c..56f6aae802a 100644 --- a/core/src/commands/offchain_worker.rs +++ b/core/src/commands/offchain_worker.rs @@ -23,9 +23,7 @@ use sp_runtime::traits::{Block as BlockT, NumberFor}; use substrate_rpc_client::{ws_client, ChainApi}; use crate::{ - build_executor, - commands::execute_block::next_hash_of, - full_extensions, parse, rpc_err_handler, + build_executor, full_extensions, hash_of, parse, rpc_err_handler, state::{LiveState, State}, state_machine_call, SharedParams, LOG_TARGET, }; @@ -74,20 +72,33 @@ where as FromStr>::Err: Debug, HostFns: HostFunctions, { - let executor = build_executor(&shared); - // we first build the externalities with the remote code. - let ext = command - .state - .to_ext::(&shared, &executor, None, true, false) - .await?; + let executor = build_executor::(&shared); + let block_ws_uri = command.header_ws_uri(); + let rpc = ws_client(&block_ws_uri).await?; + + let live_state = match command.state { + State::Live(live_state) => live_state, + _ => { + unreachable!("execute block currently only supports Live state") + } + }; - let header_ws_uri = command.header_ws_uri(); + // The block we want to *execute* at is the block passed by the user + let execute_at = live_state + .at + .clone() + .map(|s| hash_of::(s.as_str())) + .transpose()?; - let rpc = ws_client(&header_ws_uri).await?; - let next_hash = next_hash_of::(&rpc, ext.block_hash).await?; - log::info!(target: LOG_TARGET, "fetching next header: {:?} ", next_hash); + // let prev_block_live_state = prev_block_live_state::(&rpc, &live_state).await?; + let prev_block_live_state = live_state.to_prev_block_live_state::().await?; + + // Get state for the prev block + let ext = State::Live(prev_block_live_state) + .to_ext::(&shared, &executor, None, true, false) + .await?; - let header = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, Some(next_hash)) + let header = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, execute_at) .await .map_err(rpc_err_handler) .map(|maybe_header| maybe_header.ok_or("Header does not exist"))??; diff --git a/core/src/state.rs b/core/src/state.rs index aab58bc4aaa..adc6e330027 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -23,17 +23,19 @@ use frame_remote_externalities::{ use parity_scale_codec::Decode; use sc_cli::RuntimeVersion; use sc_executor::{sp_wasm_interface::HostFunctions, WasmExecutor}; -use sp_api::HashT; +use sp_api::{HashT, HeaderT}; use sp_core::{ hexdisplay::HexDisplay, storage::well_known_keys, traits::ReadRuntimeVersion, twox_128, }; +use sp_rpc::{list::ListOrValue, number::NumberOrHex}; use sp_runtime::{ - traits::{BlakeTwo256, Block as BlockT}, + traits::{BlakeTwo256, Block as BlockT, CheckedSub, One}, DeserializeOwned, }; +use substrate_rpc_client::{ws_client, ChainApi}; use crate::{ - ensure_try_runtime, hash_of, parse, + ensure_try_runtime, hash_of, parse, rpc_err_handler, shared_parameters::{Runtime, SharedParams}, LOG_TARGET, }; @@ -80,6 +82,60 @@ pub struct LiveState { pub child_tree: bool, } +impl LiveState { + /// Converts this `LiveState` into a `LiveState` for the previous block. + /// + /// Useful for opertations like when you want to execute a block, but also need the state of the + /// block *before* it. + pub async fn to_prev_block_live_state(self) -> sc_cli::Result + where + ::Err: Debug, + { + // We want to execute the block `at`, therefore need the state of the block *before* it. + dbg!(self.at.clone()); + let at = self + .at + .clone() + .map(|s| hash_of::(s.as_str())) + .transpose()?; + + // Get the block number requested by the user, or the current block number if they + // didn't specify one. + let rpc = ws_client(&self.uri).await?; + let number = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, at) + .await + .map_err(rpc_err_handler) + .and_then(|maybe_header| maybe_header.ok_or("header_not_found").map(|h| *h.number()))?; + + // Get the previous number. + let prev_number = number + .checked_sub(&One::one()) + .expect("cannot get block number below 0"); + + // Get the previous block hash + let previous_hash = match ChainApi::<(), Block::Hash, Block::Header, ()>::block_hash( + &rpc, + Some(ListOrValue::Value(NumberOrHex::Number( + prev_number + .try_into() + .map_err(|_| "failed to convert number to block number")?, + ))), + ) + .await + .map_err(rpc_err_handler)? + { + ListOrValue::Value(t) => t.expect("tried to fetch a block that doesn't exist"), + _ => unreachable!(), + }; + + dbg!(hex::encode(&previous_hash)); + Ok(LiveState { + at: Some(hex::encode(previous_hash)), + ..self + }) + } +} + /// The source of runtime *state* to use. #[derive(Debug, Clone, clap::Subcommand)] pub enum State { diff --git a/core/tests/execute_block.rs b/core/tests/execute_block.rs index 1a3173db32f..93d9b85143d 100644 --- a/core/tests/execute_block.rs +++ b/core/tests/execute_block.rs @@ -60,23 +60,28 @@ async fn execute_block_works() { .unwrap() } - let block_number = 1; + let block_number = 3; let block_hash = common::block_hash(block_number, &ws_url).await.unwrap(); // Try to execute the block. let mut block_execution = execute_block(&ws_url, block_hash); // The execute-block command is actually executing the next block. - let expected_output = format!( - r#".*Block #{} successfully executed"#, - block_number.saturating_add(1) - ); + let expected_output = format!(r#".*Block #{} successfully executed"#, block_number); let re = Regex::new(expected_output.as_str()).unwrap(); let matched = common::wait_for_stream_pattern_match(block_execution.stderr.take().unwrap(), re).await; - // Assert that the block-execution process has executed a block. + // Assert that the block-execution process has executed the expected block. assert!(matched.is_ok()); + + // Assert that the block-execution exited succesfully + assert!(block_execution + .wait_with_output() + .await + .unwrap() + .status + .success()); }) .await } From 42b3da7b1a3fd271e3ed9da90f17df53edc33467 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 9 Nov 2023 16:48:48 +0400 Subject: [PATCH 2/7] Remove unnecessary debug statement in execute_block.rs --- core/src/commands/execute_block.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/commands/execute_block.rs b/core/src/commands/execute_block.rs index b6d425b8f22..1c31cfe1b01 100644 --- a/core/src/commands/execute_block.rs +++ b/core/src/commands/execute_block.rs @@ -105,7 +105,6 @@ where }; // The block we want to *execute* at is the block passed by the user - dbg!(live_state.at.clone()); let execute_at = live_state .at .clone() From 30b34bd5ed71bffbe305bb5f688ba3a06a9743a0 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Thu, 9 Nov 2023 16:49:38 +0400 Subject: [PATCH 3/7] Remove commented out code in execute_block.rs and offchain_worker.rs --- core/src/commands/execute_block.rs | 1 - core/src/commands/offchain_worker.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/core/src/commands/execute_block.rs b/core/src/commands/execute_block.rs index 1c31cfe1b01..b12df45670a 100644 --- a/core/src/commands/execute_block.rs +++ b/core/src/commands/execute_block.rs @@ -111,7 +111,6 @@ where .map(|s| hash_of::(s.as_str())) .transpose()?; - // let prev_block_live_state = prev_block_live_state::(&rpc, &live_state).await?; let prev_block_live_state = live_state.to_prev_block_live_state::().await?; // Get state for the prev block diff --git a/core/src/commands/offchain_worker.rs b/core/src/commands/offchain_worker.rs index c499f5e3b33..2b7f097dfd5 100644 --- a/core/src/commands/offchain_worker.rs +++ b/core/src/commands/offchain_worker.rs @@ -90,7 +90,6 @@ where .map(|s| hash_of::(s.as_str())) .transpose()?; - // let prev_block_live_state = prev_block_live_state::(&rpc, &live_state).await?; let prev_block_live_state = live_state.to_prev_block_live_state::().await?; // Get state for the prev block From 6380dc846af802770f2cc77079784191e42e84e9 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Fri, 10 Nov 2023 09:52:17 +0400 Subject: [PATCH 4/7] Move at conversion to dedicated method --- core/src/commands/execute_block.rs | 8 ++------ core/src/commands/offchain_worker.rs | 8 ++------ core/src/state.rs | 17 ++++++++++++----- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/core/src/commands/execute_block.rs b/core/src/commands/execute_block.rs index b12df45670a..36fd9a06324 100644 --- a/core/src/commands/execute_block.rs +++ b/core/src/commands/execute_block.rs @@ -26,7 +26,7 @@ use sp_runtime::{ use substrate_rpc_client::{ws_client, ChainApi}; use crate::{ - build_executor, full_extensions, hash_of, rpc_err_handler, + build_executor, full_extensions, rpc_err_handler, state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck}, state_machine_call_with_proof, SharedParams, LOG_TARGET, }; @@ -105,11 +105,7 @@ where }; // The block we want to *execute* at is the block passed by the user - let execute_at = live_state - .at - .clone() - .map(|s| hash_of::(s.as_str())) - .transpose()?; + let execute_at = live_state.at::()?; let prev_block_live_state = live_state.to_prev_block_live_state::().await?; diff --git a/core/src/commands/offchain_worker.rs b/core/src/commands/offchain_worker.rs index 2b7f097dfd5..499af1dbe1f 100644 --- a/core/src/commands/offchain_worker.rs +++ b/core/src/commands/offchain_worker.rs @@ -23,7 +23,7 @@ use sp_runtime::traits::{Block as BlockT, NumberFor}; use substrate_rpc_client::{ws_client, ChainApi}; use crate::{ - build_executor, full_extensions, hash_of, parse, rpc_err_handler, + build_executor, full_extensions, parse, rpc_err_handler, state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck}, state_machine_call, SharedParams, LOG_TARGET, }; @@ -84,11 +84,7 @@ where }; // The block we want to *execute* at is the block passed by the user - let execute_at = live_state - .at - .clone() - .map(|s| hash_of::(s.as_str())) - .transpose()?; + let execute_at = live_state.at::()?; let prev_block_live_state = live_state.to_prev_block_live_state::().await?; diff --git a/core/src/state.rs b/core/src/state.rs index 8666df5a1dd..f3b75287333 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -83,6 +83,17 @@ pub struct LiveState { } impl LiveState { + /// Return the `at` block hash as a `Hash`, if it exists. + pub fn at(&self) -> sc_cli::Result::Hash>> + where + ::Err: Debug, + { + self.at + .clone() + .map(|s| hash_of::(s.as_str())) + .transpose() + } + /// Converts this `LiveState` into a `LiveState` for the previous block. /// /// Useful for opertations like when you want to execute a block, but also need the state of the @@ -92,11 +103,7 @@ impl LiveState { ::Err: Debug, { // We want to execute the block `at`, therefore need the state of the block *before* it. - let at = self - .at - .clone() - .map(|s| hash_of::(s.as_str())) - .transpose()?; + let at = self.at::()?; // Get the block number requested by the user, or the current block number if they // didn't specify one. From 2984e1ce9cc14a4802a9627bc0dc3492ab81de21 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Fri, 10 Nov 2023 10:33:05 +0400 Subject: [PATCH 5/7] simpler way to get previous hash --- core/src/state.rs | 32 +++++++------------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/core/src/state.rs b/core/src/state.rs index f3b75287333..646951b3be3 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -27,9 +27,8 @@ use sp_api::{HashT, HeaderT}; use sp_core::{ hexdisplay::HexDisplay, storage::well_known_keys, traits::ReadRuntimeVersion, twox_128, }; -use sp_rpc::{list::ListOrValue, number::NumberOrHex}; use sp_runtime::{ - traits::{BlakeTwo256, Block as BlockT, CheckedSub, One}, + traits::{BlakeTwo256, Block as BlockT}, DeserializeOwned, }; use substrate_rpc_client::{ws_client, ChainApi}; @@ -108,31 +107,14 @@ impl LiveState { // Get the block number requested by the user, or the current block number if they // didn't specify one. let rpc = ws_client(&self.uri).await?; - let number = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, at) + let previous_hash = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, at) .await .map_err(rpc_err_handler) - .and_then(|maybe_header| maybe_header.ok_or("header_not_found").map(|h| *h.number()))?; - - // Get the previous number. - let prev_number = number - .checked_sub(&One::one()) - .expect("cannot get block number below 0"); - - // Get the previous block hash - let previous_hash = match ChainApi::<(), Block::Hash, Block::Header, ()>::block_hash( - &rpc, - Some(ListOrValue::Value(NumberOrHex::Number( - prev_number - .try_into() - .map_err(|_| "failed to convert number to block number")?, - ))), - ) - .await - .map_err(rpc_err_handler)? - { - ListOrValue::Value(t) => t.expect("tried to fetch a block that doesn't exist"), - _ => unreachable!(), - }; + .and_then(|maybe_header| { + maybe_header + .ok_or("header_not_found") + .map(|h| *h.parent_hash()) + })?; Ok(LiveState { at: Some(hex::encode(previous_hash)), From 95ced0934ef9210964bf2640f24c34af6df9f3fd Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Fri, 10 Nov 2023 11:46:31 +0400 Subject: [PATCH 6/7] bump version --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cfd788c4a6c..a6f954f1ef2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11262,7 +11262,7 @@ checksum = "3528ecfd12c466c6f163363caf2d02a71161dd5e1cc6ae7b34207ea2d42d81ed" [[package]] name = "try-runtime-cli" -version = "0.4.0" +version = "0.4.1" dependencies = [ "clap", "env_logger", @@ -11314,7 +11314,7 @@ dependencies = [ [[package]] name = "try-runtime-core" -version = "0.4.0" +version = "0.4.1" dependencies = [ "assert_cmd", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 772eccdd395..152172982f4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ members = [ ] [workspace.package] -version = "0.4.0" +version = "0.4.1" authors = ["Parity Technologies "] description = "Substrate's programmatic testing framework." edition = "2021" From 967ef09b54aab0e06d5629a8353a8ea2423e6da1 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Fri, 10 Nov 2023 11:48:57 +0400 Subject: [PATCH 7/7] Revert "bump version" This reverts commit 95ced0934ef9210964bf2640f24c34af6df9f3fd. --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a6f954f1ef2..cfd788c4a6c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11262,7 +11262,7 @@ checksum = "3528ecfd12c466c6f163363caf2d02a71161dd5e1cc6ae7b34207ea2d42d81ed" [[package]] name = "try-runtime-cli" -version = "0.4.1" +version = "0.4.0" dependencies = [ "clap", "env_logger", @@ -11314,7 +11314,7 @@ dependencies = [ [[package]] name = "try-runtime-core" -version = "0.4.1" +version = "0.4.0" dependencies = [ "assert_cmd", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 152172982f4..772eccdd395 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ members = [ ] [workspace.package] -version = "0.4.1" +version = "0.4.0" authors = ["Parity Technologies "] description = "Substrate's programmatic testing framework." edition = "2021"