From c2ebff64e0acf30b9317a63523945948d1a366a9 Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Thu, 18 Mar 2021 16:04:10 +0100 Subject: [PATCH 01/18] make builder generic to allow using different hash types --- Cargo.lock | 3 + utils/frame/remote-externalities/Cargo.toml | 4 ++ utils/frame/remote-externalities/src/lib.rs | 71 +++++++++++++-------- 3 files changed, 52 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ada75934c79b7..c945ed9accd5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6580,6 +6580,7 @@ version = "0.9.0" dependencies = [ "async-std", "env_logger 0.8.3", + "frame-support", "hex-literal", "jsonrpsee-http-client", "jsonrpsee-proc-macros", @@ -6588,6 +6589,8 @@ dependencies = [ "parity-scale-codec", "sp-core", "sp-io", + "sp-runtime", + "sp-std", ] [[package]] diff --git a/utils/frame/remote-externalities/Cargo.toml b/utils/frame/remote-externalities/Cargo.toml index de90933e17978..ab2cf305957a7 100644 --- a/utils/frame/remote-externalities/Cargo.toml +++ b/utils/frame/remote-externalities/Cargo.toml @@ -25,6 +25,10 @@ codec = { package = "parity-scale-codec", version = "2.0.0" } sp-io = { version = "3.0.0", path = "../../../primitives/io" } sp-core = { version = "3.0.0", path = "../../../primitives/core" } +sp-runtime = { version = "3.0.0", path = "../../../primitives/runtime" } +sp-std = { version = "3.0.0", path = "../../../primitives/std" } + +frame-support = { version = "3.0.0", path = "../../../frame/support" } [dev-dependencies] async-std = { version = "1.6.5", features = ["attributes"] } diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 8211274c46298..705b8633532f2 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -103,10 +103,11 @@ use std::{ fs, + fmt::Debug, path::{Path, PathBuf}, }; use log::*; -use sp_core::{hashing::twox_128}; +use sp_core::hashing::twox_128; pub use sp_io::TestExternalities; use sp_core::{ hexdisplay::HexDisplay, @@ -115,15 +116,30 @@ use sp_core::{ use codec::{Encode, Decode}; use jsonrpsee_http_client::{HttpClient, HttpConfig}; +use sp_runtime::traits::{ + CheckEqual, + SimpleBitOps, Member, MaybeDisplay, + MaybeSerializeDeserialize, MaybeMallocSizeOf, +}; +use frame_support::Parameter; + +// TODO: Make KeyPair generic type KeyPair = (StorageKey, StorageData); -type Hash = sp_core::H256; -// TODO: make these two generic. + +// Helper trait to reduce code duplication +pub trait RuntimeHash: Parameter + Member + MaybeSerializeDeserialize + Debug + MaybeDisplay + SimpleBitOps + Ord + + Default + Copy + CheckEqual + sp_std::hash::Hash + AsRef<[u8]> + AsMut<[u8]> + MaybeMallocSizeOf {} + +// implement the helper trait for each type, +// that already implements all traits the helper trait is built from +impl + AsMut<[u8]> + MaybeMallocSizeOf> RuntimeHash for T {} const LOG_TARGET: &str = "remote-ext"; const TARGET: &str = "http://localhost:9933"; jsonrpsee_proc_macros::rpc_client_api! { - RpcApi { + RpcApi { #[rpc(method = "state_getPairs", positional_params)] fn storage_pairs(prefix: StorageKey, hash: Option) -> Vec<(StorageKey, StorageData)>; #[rpc(method = "chain_getFinalizedHead")] @@ -133,9 +149,9 @@ jsonrpsee_proc_macros::rpc_client_api! { /// The execution mode. #[derive(Clone)] -pub enum Mode { +pub enum Mode { /// Online. - Online(OnlineConfig), + Online(OnlineConfig), /// Offline. Uses a cached file and needs not any client config. Offline(OfflineConfig), } @@ -153,7 +169,7 @@ pub struct OfflineConfig { /// /// A cache config may be present and will be written to in that case. #[derive(Clone)] -pub struct OnlineConfig { +pub struct OnlineConfig { /// The HTTP uri to use. pub uri: String, /// The block number at which to connect. Will be latest finalized head if not provided. @@ -164,13 +180,13 @@ pub struct OnlineConfig { pub modules: Vec, } -impl Default for OnlineConfig { +impl Default for OnlineConfig { fn default() -> Self { Self { uri: TARGET.to_owned(), at: None, cache: None, modules: Default::default() } } } -impl OnlineConfig { +impl OnlineConfig { /// Return a new http rpc client. fn rpc(&self) -> HttpClient { HttpClient::new(&self.uri, HttpConfig { max_request_body_size: u32::MAX }) @@ -202,30 +218,32 @@ impl CacheConfig { } /// Builder for remote-externalities. -pub struct Builder { +pub struct Builder { inject: Vec, - mode: Mode, + mode: Mode, + _hash: std::marker::PhantomData, } -impl Default for Builder { +impl Default for Builder { fn default() -> Self { Self { inject: Default::default(), - mode: Mode::Online(OnlineConfig::default()) + mode: Mode::Online(OnlineConfig::default()), + _hash: Default::default(), } } } // Mode methods -impl Builder { - fn as_online(&self) -> &OnlineConfig { +impl Builder { + fn as_online(&self) -> &OnlineConfig { match &self.mode { Mode::Online(config) => &config, _ => panic!("Unexpected mode: Online"), } } - fn as_online_mut(&mut self) -> &mut OnlineConfig { + fn as_online_mut(&mut self) -> &mut OnlineConfig { match &mut self.mode { Mode::Online(config) => config, _ => panic!("Unexpected mode: Online"), @@ -234,10 +252,10 @@ impl Builder { } // RPC methods -impl Builder { +impl Builder { async fn rpc_get_head(&self) -> Result { trace!(target: LOG_TARGET, "rpc: finalized_head"); - RpcApi::finalized_head(&self.as_online().rpc()).await.map_err(|e| { + RpcApi::::finalized_head(&self.as_online().rpc()).await.map_err(|e| { error!("Error = {:?}", e); "rpc finalized_head failed." }) @@ -252,7 +270,7 @@ impl Builder { at: Hash, ) -> Result, &'static str> { trace!(target: LOG_TARGET, "rpc: storage_pairs: {:?} / {:?}", prefix, at); - RpcApi::storage_pairs(&self.as_online().rpc(), prefix, Some(at)).await.map_err(|e| { + RpcApi::::storage_pairs(&self.as_online().rpc(), prefix, Some(at)).await.map_err(|e| { error!("Error = {:?}", e); "rpc storage_pairs failed" }) @@ -260,7 +278,7 @@ impl Builder { } // Internal methods -impl Builder { +impl Builder { /// Save the given data as cache. fn save_cache(&self, data: &[KeyPair], path: &Path) -> Result<(), &'static str> { info!(target: LOG_TARGET, "writing to cache file {:?}", path); @@ -341,7 +359,7 @@ impl Builder { } // Public methods -impl Builder { +impl Builder { /// Create a new builder. pub fn new() -> Self { Default::default() @@ -356,7 +374,7 @@ impl Builder { } /// Configure a cache to be used. - pub fn mode(mut self, mode: Mode) -> Self { + pub fn mode(mut self, mode: Mode) -> Self { self.mode = mode; self } @@ -379,6 +397,7 @@ impl Builder { #[cfg(test)] mod tests { use super::*; + type Hash = sp_core::H256; fn init_logger() { let _ = env_logger::Builder::from_default_env() @@ -390,7 +409,7 @@ mod tests { #[async_std::test] async fn can_build_one_pallet() { init_logger(); - Builder::new() + Builder::::new() .mode(Mode::Online(OnlineConfig { modules: vec!["Proxy".into()], ..Default::default() @@ -404,7 +423,7 @@ mod tests { #[async_std::test] async fn can_load_cache() { init_logger(); - Builder::new() + Builder::::new() .mode(Mode::Offline(OfflineConfig { cache: CacheConfig { name: "proxy_test".into(), ..Default::default() }, })) @@ -417,7 +436,7 @@ mod tests { #[async_std::test] async fn can_create_cache() { init_logger(); - Builder::new() + Builder::::new() .mode(Mode::Online(OnlineConfig { cache: Some(CacheConfig { name: "test_cache_to_remove.bin".into(), @@ -447,6 +466,6 @@ mod tests { #[async_std::test] async fn can_build_all() { init_logger(); - Builder::new().build().await.unwrap().execute_with(|| {}); + Builder::::new().build().await.unwrap().execute_with(|| {}); } } From a66b0c890f4a759914f322379a12d2ed1f5326e1 Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Thu, 18 Mar 2021 16:27:28 +0100 Subject: [PATCH 02/18] expose "cache", "block_number" and "modules" as cli options for live state --- utils/frame/try-runtime/cli/src/lib.rs | 99 +++++++++++++++++++------- 1 file changed, 74 insertions(+), 25 deletions(-) diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 4ab38692a5cfa..495a4a52db65d 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -18,7 +18,7 @@ //! `Structopt`-ready struct for `try-runtime`. use parity_scale_codec::Decode; -use std::{fmt::Debug, str::FromStr}; +use std::{fmt::Debug, path::PathBuf, str::FromStr}; use sc_service::Configuration; use sc_cli::{CliConfiguration, ExecutionStrategy, WasmExecutionMethod}; use sc_executor::NativeExecutor; @@ -27,6 +27,9 @@ use sp_state_machine::StateMachine; use sp_runtime::traits::{Block as BlockT, NumberFor}; use sp_core::storage::{StorageData, StorageKey, well_known_keys}; +// TODO: allow usage of different hashes in cli +pub type Hash = sp_core::H256; + /// Various commands to try out the new runtime, over configurable states. /// /// For now this only assumes running the `on_runtime_upgrade` hooks. @@ -37,10 +40,6 @@ pub struct TryRuntimeCmd { #[structopt(flatten)] pub shared_params: sc_cli::SharedParams, - /// The state to use to run the migration. Should be a valid FILE or HTTP URI. - #[structopt(short, long, default_value = "http://localhost:9933")] - pub state: State, - /// The execution strategy that should be used for benchmarks #[structopt( long = "execution", @@ -60,31 +59,75 @@ pub struct TryRuntimeCmd { default_value = "Interpreted" )] pub wasm_method: WasmExecutionMethod, + + /// The state to use to run the migration. + #[structopt(subcommand)] + pub state: State, } + /// The state to use for a migration dry-run. -#[derive(Debug)] +#[derive(Debug, structopt::StructOpt)] pub enum State { - /// A snapshot. Inner value is a file path. - Snap(String), + /// Use a snapshot as state to run the migration. + Snap { + #[structopt(flatten)] + cache_params: CacheParams, + }, + + /// Use a live chain to run the migration. + Live { + /// An optional cache file to WRITE to. Not cached if set to `None`. + #[structopt(short, long)] + cache_file: Option, + + /// The block number at which to connect. Will be latest finalized head if not provided. + #[structopt(short, long)] + block_number: Option, + + /// The modules to scrape. If empty, entire chain state will be scraped. + #[structopt(short, long, require_delimiter = true)] + modules: Option>, + + /// The url to connect to. + #[structopt(default_value = "http://localhost:9933", parse(try_from_str = parse_url))] + url: String, + }, +} + +fn parse_url(s: &str) -> Result { + match s.get(..7) { + // could use Url crate as well, but lets keep it simple for now. + Some("http://") => Ok(s.to_string()), + _ => Err("not a valid url"), + } +} - /// A live chain. Inner value is the HTTP uri. - Live(String), +#[derive(Debug, structopt::StructOpt)] +pub struct CacheParams { + /// The directory of the snapshot. + #[structopt(short, long, default_value = ".")] + directory: String, + + /// The file name of the snapshot. + #[structopt(default_value = "CACHE")] + file_name: String, } -impl FromStr for State { +impl FromStr for CacheParams { type Err = &'static str; fn from_str(s: &str) -> Result { - match s.get(..7) { - // could use Url crate as well, but lets keep it simple for now. - Some("http://") => Ok(State::Live(s.to_string())), - Some("file://") => s - .split("//") - .collect::>() - .get(1) - .map(|s| State::Snap(s.to_string())) - .ok_or("invalid file URI"), - _ => Err("invalid format. Must be a valid HTTP or File URI"), + let p: PathBuf = s.parse().map_err(|_| "invalid path")?; + let parent = p.parent(); + let file_name = p.file_name(); + + match file_name { + Some(file_name) => Ok(Self { + // TODO: maybe don't use to_string_lossy here + directory: parent.map(|p| p.to_string_lossy().into()).unwrap_or(".".to_string()), + file_name: file_name.to_string_lossy().into() + }), + None => Err("invalid path"), } } } @@ -123,11 +166,17 @@ impl TryRuntimeCmd { let ext = { use remote_externalities::{Builder, Mode, CacheConfig, OfflineConfig, OnlineConfig}; let builder = match &self.state { - State::Snap(file_path) => Builder::new().mode(Mode::Offline(OfflineConfig { - cache: CacheConfig { name: file_path.into(), ..Default::default() }, + State::Snap { cache_params: CacheParams { file_name, directory } } => Builder::::new().mode(Mode::Offline(OfflineConfig { + cache: CacheConfig { name: file_name.into(), directory: directory.into(), ..Default::default() }, })), - State::Live(http_uri) => Builder::new().mode(Mode::Online(OnlineConfig { - uri: http_uri.into(), + State::Live { url, cache_file, block_number, modules } => Builder::::new().mode(Mode::Online(OnlineConfig { + uri: url.into(), + cache: cache_file.as_ref().map(|c| CacheConfig { + name: c.file_name.clone(), + directory: c.directory.clone(), + }), + modules: modules.clone().unwrap_or(Vec::new()), + at: block_number.to_owned(), ..Default::default() })), }; From b9b73598c6e1772770f8b9def424feed04d16f67 Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Thu, 18 Mar 2021 23:31:42 +0100 Subject: [PATCH 03/18] Change Builder to be generic over Block instead of Hash add rpc method to get hash from block number allow passing of block numbers and hashes --- Cargo.lock | 2 - utils/frame/remote-externalities/Cargo.toml | 3 - utils/frame/remote-externalities/src/lib.rs | 95 +++++++++++---------- utils/frame/try-runtime/cli/src/lib.rs | 30 ++++--- 4 files changed, 72 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c945ed9accd5f..3f0dc58f21ce5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6580,7 +6580,6 @@ version = "0.9.0" dependencies = [ "async-std", "env_logger 0.8.3", - "frame-support", "hex-literal", "jsonrpsee-http-client", "jsonrpsee-proc-macros", @@ -6590,7 +6589,6 @@ dependencies = [ "sp-core", "sp-io", "sp-runtime", - "sp-std", ] [[package]] diff --git a/utils/frame/remote-externalities/Cargo.toml b/utils/frame/remote-externalities/Cargo.toml index ab2cf305957a7..71cf7ae66f94f 100644 --- a/utils/frame/remote-externalities/Cargo.toml +++ b/utils/frame/remote-externalities/Cargo.toml @@ -26,9 +26,6 @@ codec = { package = "parity-scale-codec", version = "2.0.0" } sp-io = { version = "3.0.0", path = "../../../primitives/io" } sp-core = { version = "3.0.0", path = "../../../primitives/core" } sp-runtime = { version = "3.0.0", path = "../../../primitives/runtime" } -sp-std = { version = "3.0.0", path = "../../../primitives/std" } - -frame-support = { version = "3.0.0", path = "../../../frame/support" } [dev-dependencies] async-std = { version = "1.6.5", features = ["attributes"] } diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 705b8633532f2..89482c0830b3d 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -103,7 +103,6 @@ use std::{ fs, - fmt::Debug, path::{Path, PathBuf}, }; use log::*; @@ -116,42 +115,35 @@ use sp_core::{ use codec::{Encode, Decode}; use jsonrpsee_http_client::{HttpClient, HttpConfig}; -use sp_runtime::traits::{ - CheckEqual, - SimpleBitOps, Member, MaybeDisplay, - MaybeSerializeDeserialize, MaybeMallocSizeOf, +use sp_runtime::{ + generic::BlockId, + traits::{ + Block as BlockT, NumberFor, + } }; -use frame_support::Parameter; // TODO: Make KeyPair generic type KeyPair = (StorageKey, StorageData); -// Helper trait to reduce code duplication -pub trait RuntimeHash: Parameter + Member + MaybeSerializeDeserialize + Debug + MaybeDisplay + SimpleBitOps + Ord - + Default + Copy + CheckEqual + sp_std::hash::Hash + AsRef<[u8]> + AsMut<[u8]> + MaybeMallocSizeOf {} - -// implement the helper trait for each type, -// that already implements all traits the helper trait is built from -impl + AsMut<[u8]> + MaybeMallocSizeOf> RuntimeHash for T {} - const LOG_TARGET: &str = "remote-ext"; const TARGET: &str = "http://localhost:9933"; jsonrpsee_proc_macros::rpc_client_api! { - RpcApi { + RpcApi { #[rpc(method = "state_getPairs", positional_params)] - fn storage_pairs(prefix: StorageKey, hash: Option) -> Vec<(StorageKey, StorageData)>; + fn storage_pairs(prefix: StorageKey, hash: Option) -> Vec<(StorageKey, StorageData)>; #[rpc(method = "chain_getFinalizedHead")] - fn finalized_head() -> Hash; + fn finalized_head() -> B::Hash; + #[rpc(method = "chain_getBlockHash")] + fn block_hash(number: NumberFor) -> B::Hash; } } /// The execution mode. #[derive(Clone)] -pub enum Mode { +pub enum Mode { /// Online. - Online(OnlineConfig), + Online(OnlineConfig), /// Offline. Uses a cached file and needs not any client config. Offline(OfflineConfig), } @@ -169,24 +161,24 @@ pub struct OfflineConfig { /// /// A cache config may be present and will be written to in that case. #[derive(Clone)] -pub struct OnlineConfig { +pub struct OnlineConfig { /// The HTTP uri to use. pub uri: String, /// The block number at which to connect. Will be latest finalized head if not provided. - pub at: Option, + pub at: Option>, /// An optional cache file to WRITE to, not for reading. Not cached if set to `None`. pub cache: Option, /// The modules to scrape. If empty, entire chain state will be scraped. pub modules: Vec, } -impl Default for OnlineConfig { +impl Default for OnlineConfig { fn default() -> Self { Self { uri: TARGET.to_owned(), at: None, cache: None, modules: Default::default() } } } -impl OnlineConfig { +impl OnlineConfig { /// Return a new http rpc client. fn rpc(&self) -> HttpClient { HttpClient::new(&self.uri, HttpConfig { max_request_body_size: u32::MAX }) @@ -218,32 +210,30 @@ impl CacheConfig { } /// Builder for remote-externalities. -pub struct Builder { +pub struct Builder { inject: Vec, - mode: Mode, - _hash: std::marker::PhantomData, + mode: Mode, } -impl Default for Builder { +impl Default for Builder { fn default() -> Self { Self { inject: Default::default(), mode: Mode::Online(OnlineConfig::default()), - _hash: Default::default(), } } } // Mode methods -impl Builder { - fn as_online(&self) -> &OnlineConfig { +impl Builder { + fn as_online(&self) -> &OnlineConfig { match &self.mode { Mode::Online(config) => &config, _ => panic!("Unexpected mode: Online"), } } - fn as_online_mut(&mut self) -> &mut OnlineConfig { + fn as_online_mut(&mut self) -> &mut OnlineConfig { match &mut self.mode { Mode::Online(config) => config, _ => panic!("Unexpected mode: Online"), @@ -252,10 +242,10 @@ impl Builder { } // RPC methods -impl Builder { - async fn rpc_get_head(&self) -> Result { +impl Builder { + async fn rpc_get_head(&self) -> Result { trace!(target: LOG_TARGET, "rpc: finalized_head"); - RpcApi::::finalized_head(&self.as_online().rpc()).await.map_err(|e| { + RpcApi::::finalized_head(&self.as_online().rpc()).await.map_err(|e| { error!("Error = {:?}", e); "rpc finalized_head failed." }) @@ -267,18 +257,30 @@ impl Builder { async fn rpc_get_pairs( &self, prefix: StorageKey, - at: Hash, + at: B::Hash, ) -> Result, &'static str> { trace!(target: LOG_TARGET, "rpc: storage_pairs: {:?} / {:?}", prefix, at); - RpcApi::::storage_pairs(&self.as_online().rpc(), prefix, Some(at)).await.map_err(|e| { + RpcApi::::storage_pairs(&self.as_online().rpc(), prefix, Some(at)).await.map_err(|e| { error!("Error = {:?}", e); "rpc storage_pairs failed" }) } + + /// Relay the request to `chain_getBlockHash` rpc endpoint. + async fn rpc_get_hash( + &self, + number: NumberFor, + ) -> Result { + trace!(target: LOG_TARGET, "rpc: block_hash: {:?}", number); + RpcApi::::block_hash(&self.as_online().rpc(), number).await.map_err(|e| { + error!("Error = {:?}", e); + "rpc block_hash failed" + }) + } } // Internal methods -impl Builder { +impl Builder { /// Save the given data as cache. fn save_cache(&self, data: &[KeyPair], path: &Path) -> Result<(), &'static str> { info!(target: LOG_TARGET, "writing to cache file {:?}", path); @@ -293,6 +295,13 @@ impl Builder { Decode::decode(&mut &*bytes).map_err(|_| "decode failed") } + async fn block_id_to_hash(&self, block_id: BlockId) -> Result { + Ok(match block_id { + BlockId::Hash(hash) => hash, + BlockId::Number(number) => self.rpc_get_hash(number).await?, + }) + } + /// Build `Self` from a network node denoted by `uri`. async fn load_remote(&self) -> Result, &'static str> { let config = self.as_online(); @@ -307,7 +316,7 @@ impl Builder { let mut filtered_kv = vec![]; for f in config.modules.iter() { let hashed_prefix = StorageKey(twox_128(f.as_bytes()).to_vec()); - let module_kv = self.rpc_get_pairs(hashed_prefix.clone(), at).await?; + let module_kv = self.rpc_get_pairs(hashed_prefix.clone(), self.block_id_to_hash(at).await?).await?; info!( target: LOG_TARGET, "downloaded data for module {} (count: {} / prefix: {:?}).", @@ -320,7 +329,7 @@ impl Builder { filtered_kv } else { info!(target: LOG_TARGET, "downloading data for all modules."); - self.rpc_get_pairs(StorageKey(vec![]), at).await?.into_iter().collect::>() + self.rpc_get_pairs(StorageKey(vec![]), self.block_id_to_hash(at).await?).await?.into_iter().collect::>() }; Ok(keys_and_values) @@ -330,7 +339,7 @@ impl Builder { info!(target: LOG_TARGET, "initializing remote client to {:?}", self.as_online().uri); if self.as_online().at.is_none() { let at = self.rpc_get_head().await?; - self.as_online_mut().at = Some(at); + self.as_online_mut().at = Some(BlockId::Hash(at)); } Ok(()) } @@ -359,7 +368,7 @@ impl Builder { } // Public methods -impl Builder { +impl Builder { /// Create a new builder. pub fn new() -> Self { Default::default() @@ -374,7 +383,7 @@ impl Builder { } /// Configure a cache to be used. - pub fn mode(mut self, mode: Mode) -> Self { + pub fn mode(mut self, mode: Mode) -> Self { self.mode = mode; self } diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 495a4a52db65d..6770154f0f0cc 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -20,16 +20,13 @@ use parity_scale_codec::Decode; use std::{fmt::Debug, path::PathBuf, str::FromStr}; use sc_service::Configuration; -use sc_cli::{CliConfiguration, ExecutionStrategy, WasmExecutionMethod}; +use sc_cli::{BlockNumberOrHash, CliConfiguration, ExecutionStrategy, WasmExecutionMethod}; use sc_executor::NativeExecutor; use sc_service::NativeExecutionDispatch; use sp_state_machine::StateMachine; use sp_runtime::traits::{Block as BlockT, NumberFor}; use sp_core::storage::{StorageData, StorageKey, well_known_keys}; -// TODO: allow usage of different hashes in cli -pub type Hash = sp_core::H256; - /// Various commands to try out the new runtime, over configurable states. /// /// For now this only assumes running the `on_runtime_upgrade` hooks. @@ -65,7 +62,6 @@ pub struct TryRuntimeCmd { pub state: State, } - /// The state to use for a migration dry-run. #[derive(Debug, structopt::StructOpt)] pub enum State { @@ -82,8 +78,8 @@ pub enum State { cache_file: Option, /// The block number at which to connect. Will be latest finalized head if not provided. - #[structopt(short, long)] - block_number: Option, + #[structopt(short, long, multiple = false)] + block_number: Option, /// The modules to scrape. If empty, entire chain state will be scraped. #[structopt(short, long, require_delimiter = true)] @@ -136,6 +132,10 @@ impl TryRuntimeCmd { pub async fn run(&self, config: Configuration) -> sc_cli::Result<()> where B: BlockT, + B::Hash: FromStr, + ::Err: std::fmt::Debug, + NumberFor: FromStr, + as FromStr>::Err: std::fmt::Debug, ExecDispatch: NativeExecutionDispatch + 'static, { let spec = config.chain_spec; @@ -166,17 +166,27 @@ impl TryRuntimeCmd { let ext = { use remote_externalities::{Builder, Mode, CacheConfig, OfflineConfig, OnlineConfig}; let builder = match &self.state { - State::Snap { cache_params: CacheParams { file_name, directory } } => Builder::::new().mode(Mode::Offline(OfflineConfig { + State::Snap { + cache_params: CacheParams { file_name, directory } + } => Builder::::new().mode(Mode::Offline(OfflineConfig { cache: CacheConfig { name: file_name.into(), directory: directory.into(), ..Default::default() }, })), - State::Live { url, cache_file, block_number, modules } => Builder::::new().mode(Mode::Online(OnlineConfig { + State::Live { + url, + cache_file, + block_number, + modules + } => Builder::::new().mode(Mode::Online(OnlineConfig { uri: url.into(), cache: cache_file.as_ref().map(|c| CacheConfig { name: c.file_name.clone(), directory: c.directory.clone(), }), modules: modules.clone().unwrap_or(Vec::new()), - at: block_number.to_owned(), + at: match block_number { + Some(b) => Some(b.parse::()?), + None => None, + }, ..Default::default() })), }; From d0bd084ad79d2e734589e81dc584b68025c53b5e Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Fri, 19 Mar 2021 09:05:56 +0100 Subject: [PATCH 04/18] fix live tests --- Cargo.lock | 2 +- utils/frame/remote-externalities/Cargo.toml | 2 +- utils/frame/remote-externalities/src/lib.rs | 20 +++++++++++--------- utils/frame/try-runtime/cli/src/lib.rs | 8 ++++---- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3f0dc58f21ce5..d4daa64207c69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6578,7 +6578,6 @@ dependencies = [ name = "remote-externalities" version = "0.9.0" dependencies = [ - "async-std", "env_logger 0.8.3", "hex-literal", "jsonrpsee-http-client", @@ -6589,6 +6588,7 @@ dependencies = [ "sp-core", "sp-io", "sp-runtime", + "tokio 0.2.25", ] [[package]] diff --git a/utils/frame/remote-externalities/Cargo.toml b/utils/frame/remote-externalities/Cargo.toml index 71cf7ae66f94f..b8bee6380006a 100644 --- a/utils/frame/remote-externalities/Cargo.toml +++ b/utils/frame/remote-externalities/Cargo.toml @@ -28,7 +28,7 @@ sp-core = { version = "3.0.0", path = "../../../primitives/core" } sp-runtime = { version = "3.0.0", path = "../../../primitives/runtime" } [dev-dependencies] -async-std = { version = "1.6.5", features = ["attributes"] } +tokio = { version = "0.2", features = ["macros"] } [features] remote-test = [] diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 89482c0830b3d..2c832a691904c 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -406,7 +406,9 @@ impl Builder { #[cfg(test)] mod tests { use super::*; - type Hash = sp_core::H256; + pub type Header = sp_runtime::generic::Header; + pub type Block = sp_runtime::generic::Block; + pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; fn init_logger() { let _ = env_logger::Builder::from_default_env() @@ -415,10 +417,10 @@ mod tests { .try_init(); } - #[async_std::test] + #[tokio::test] async fn can_build_one_pallet() { init_logger(); - Builder::::new() + Builder::::new() .mode(Mode::Online(OnlineConfig { modules: vec!["Proxy".into()], ..Default::default() @@ -429,10 +431,10 @@ mod tests { .execute_with(|| {}); } - #[async_std::test] + #[tokio::test] async fn can_load_cache() { init_logger(); - Builder::::new() + Builder::::new() .mode(Mode::Offline(OfflineConfig { cache: CacheConfig { name: "proxy_test".into(), ..Default::default() }, })) @@ -442,10 +444,10 @@ mod tests { .execute_with(|| {}); } - #[async_std::test] + #[tokio::test] async fn can_create_cache() { init_logger(); - Builder::::new() + Builder::::new() .mode(Mode::Online(OnlineConfig { cache: Some(CacheConfig { name: "test_cache_to_remove.bin".into(), @@ -472,9 +474,9 @@ mod tests { } } - #[async_std::test] + #[tokio::test] async fn can_build_all() { init_logger(); - Builder::::new().build().await.unwrap().execute_with(|| {}); + Builder::::new().build().await.unwrap().execute_with(|| {}); } } diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 6770154f0f0cc..f654c0734f0a7 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -77,9 +77,9 @@ pub enum State { #[structopt(short, long)] cache_file: Option, - /// The block number at which to connect. Will be latest finalized head if not provided. + /// The block number at which to connect. Can be either a number or a hash (staring with `0x`) Will be latest finalized head if not provided. #[structopt(short, long, multiple = false)] - block_number: Option, + block_at: Option, /// The modules to scrape. If empty, entire chain state will be scraped. #[structopt(short, long, require_delimiter = true)] @@ -174,7 +174,7 @@ impl TryRuntimeCmd { State::Live { url, cache_file, - block_number, + block_at, modules } => Builder::::new().mode(Mode::Online(OnlineConfig { uri: url.into(), @@ -183,7 +183,7 @@ impl TryRuntimeCmd { directory: c.directory.clone(), }), modules: modules.clone().unwrap_or(Vec::new()), - at: match block_number { + at: match block_at { Some(b) => Some(b.parse::()?), None => None, }, From b2ee46131d074721389cd8c4d610ee25a1974b19 Mon Sep 17 00:00:00 2001 From: Steve Biedermann Date: Fri, 19 Mar 2021 10:11:48 +0100 Subject: [PATCH 05/18] fix formatting in utils/frame/remote-externalities/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- utils/frame/remote-externalities/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 2c832a691904c..7a8236a185b53 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -117,9 +117,7 @@ use jsonrpsee_http_client::{HttpClient, HttpConfig}; use sp_runtime::{ generic::BlockId, - traits::{ - Block as BlockT, NumberFor, - } + traits::{Block as BlockT, NumberFor} }; // TODO: Make KeyPair generic From 1ca4673b0b430f709d3c87408b119ca8f4d5e4ca Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Fri, 19 Mar 2021 10:51:52 +0100 Subject: [PATCH 06/18] change cli to only accept block hashes break up lines that were too long use starts_with instead of match s.get use unwrap_or_default instead of unwrap_or(Vec::new()) --- utils/frame/remote-externalities/src/lib.rs | 34 +++------------- utils/frame/try-runtime/cli/src/lib.rs | 43 ++++++++++++++++----- 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 7a8236a185b53..80f6af31a3a0c 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -115,10 +115,7 @@ use sp_core::{ use codec::{Encode, Decode}; use jsonrpsee_http_client::{HttpClient, HttpConfig}; -use sp_runtime::{ - generic::BlockId, - traits::{Block as BlockT, NumberFor} -}; +use sp_runtime::traits::Block as BlockT; // TODO: Make KeyPair generic type KeyPair = (StorageKey, StorageData); @@ -132,8 +129,6 @@ jsonrpsee_proc_macros::rpc_client_api! { fn storage_pairs(prefix: StorageKey, hash: Option) -> Vec<(StorageKey, StorageData)>; #[rpc(method = "chain_getFinalizedHead")] fn finalized_head() -> B::Hash; - #[rpc(method = "chain_getBlockHash")] - fn block_hash(number: NumberFor) -> B::Hash; } } @@ -163,7 +158,7 @@ pub struct OnlineConfig { /// The HTTP uri to use. pub uri: String, /// The block number at which to connect. Will be latest finalized head if not provided. - pub at: Option>, + pub at: Option, /// An optional cache file to WRITE to, not for reading. Not cached if set to `None`. pub cache: Option, /// The modules to scrape. If empty, entire chain state will be scraped. @@ -263,18 +258,6 @@ impl Builder { "rpc storage_pairs failed" }) } - - /// Relay the request to `chain_getBlockHash` rpc endpoint. - async fn rpc_get_hash( - &self, - number: NumberFor, - ) -> Result { - trace!(target: LOG_TARGET, "rpc: block_hash: {:?}", number); - RpcApi::::block_hash(&self.as_online().rpc(), number).await.map_err(|e| { - error!("Error = {:?}", e); - "rpc block_hash failed" - }) - } } // Internal methods @@ -293,13 +276,6 @@ impl Builder { Decode::decode(&mut &*bytes).map_err(|_| "decode failed") } - async fn block_id_to_hash(&self, block_id: BlockId) -> Result { - Ok(match block_id { - BlockId::Hash(hash) => hash, - BlockId::Number(number) => self.rpc_get_hash(number).await?, - }) - } - /// Build `Self` from a network node denoted by `uri`. async fn load_remote(&self) -> Result, &'static str> { let config = self.as_online(); @@ -314,7 +290,7 @@ impl Builder { let mut filtered_kv = vec![]; for f in config.modules.iter() { let hashed_prefix = StorageKey(twox_128(f.as_bytes()).to_vec()); - let module_kv = self.rpc_get_pairs(hashed_prefix.clone(), self.block_id_to_hash(at).await?).await?; + let module_kv = self.rpc_get_pairs(hashed_prefix.clone(), at).await?; info!( target: LOG_TARGET, "downloaded data for module {} (count: {} / prefix: {:?}).", @@ -327,7 +303,7 @@ impl Builder { filtered_kv } else { info!(target: LOG_TARGET, "downloading data for all modules."); - self.rpc_get_pairs(StorageKey(vec![]), self.block_id_to_hash(at).await?).await?.into_iter().collect::>() + self.rpc_get_pairs(StorageKey(vec![]), at).await?.into_iter().collect::>() }; Ok(keys_and_values) @@ -337,7 +313,7 @@ impl Builder { info!(target: LOG_TARGET, "initializing remote client to {:?}", self.as_online().uri); if self.as_online().at.is_none() { let at = self.rpc_get_head().await?; - self.as_online_mut().at = Some(BlockId::Hash(at)); + self.as_online_mut().at = Some(at); } Ok(()) } diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index f654c0734f0a7..e59285484a132 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -20,7 +20,7 @@ use parity_scale_codec::Decode; use std::{fmt::Debug, path::PathBuf, str::FromStr}; use sc_service::Configuration; -use sc_cli::{BlockNumberOrHash, CliConfiguration, ExecutionStrategy, WasmExecutionMethod}; +use sc_cli::{CliConfiguration, ExecutionStrategy, WasmExecutionMethod}; use sc_executor::NativeExecutor; use sc_service::NativeExecutionDispatch; use sp_state_machine::StateMachine; @@ -77,9 +77,10 @@ pub enum State { #[structopt(short, long)] cache_file: Option, - /// The block number at which to connect. Can be either a number or a hash (staring with `0x`) Will be latest finalized head if not provided. - #[structopt(short, long, multiple = false)] - block_at: Option, + /// The block hash at which to connect. + /// Will be latest finalized head if not provided. + #[structopt(short, long, multiple = false, parse(try_from_str = parse_hash))] + block_at: Option, /// The modules to scrape. If empty, entire chain state will be scraped. #[structopt(short, long, require_delimiter = true)] @@ -91,11 +92,29 @@ pub enum State { }, } +fn parse_hash(block_number: &str) -> Result { + let block_number = if block_number.starts_with("0x") { + &block_number[2..] + } else { + block_number + }; + + if let Some(pos) = block_number.chars().position(|c| !c.is_ascii_hexdigit()) { + Err(format!( + "Expected block hash, found illegal hex character at position: {}", + 2 + pos, + )) + } else { + Ok(block_number.into()) + } +} + fn parse_url(s: &str) -> Result { - match s.get(..7) { + if s.starts_with("http://") { // could use Url crate as well, but lets keep it simple for now. - Some("http://") => Ok(s.to_string()), - _ => Err("not a valid url"), + Ok(s.to_string()) + } else { + Err("not a valid url") } } @@ -169,7 +188,11 @@ impl TryRuntimeCmd { State::Snap { cache_params: CacheParams { file_name, directory } } => Builder::::new().mode(Mode::Offline(OfflineConfig { - cache: CacheConfig { name: file_name.into(), directory: directory.into(), ..Default::default() }, + cache: CacheConfig { + name: file_name.into(), + directory: directory.into(), + ..Default::default() + }, })), State::Live { url, @@ -182,9 +205,9 @@ impl TryRuntimeCmd { name: c.file_name.clone(), directory: c.directory.clone(), }), - modules: modules.clone().unwrap_or(Vec::new()), + modules: modules.clone().unwrap_or_default(), at: match block_at { - Some(b) => Some(b.parse::()?), + Some(b) => Some(b.parse().map_err(|e| format!("Could not parse hash: {:?}", e))?), None => None, }, ..Default::default() From 5f24ddba79cf9a01a89c7216a94bf6660c2c6963 Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Fri, 19 Mar 2021 10:55:51 +0100 Subject: [PATCH 07/18] improve error message --- utils/frame/try-runtime/cli/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index e59285484a132..01963078a79b1 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -114,7 +114,7 @@ fn parse_url(s: &str) -> Result { // could use Url crate as well, but lets keep it simple for now. Ok(s.to_string()) } else { - Err("not a valid url") + Err("not a valid HTTP url: must start with 'http://'") } } From 17f4f7cf9fe00be71344c9538015fce7b72977fe Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Fri, 19 Mar 2021 10:58:30 +0100 Subject: [PATCH 08/18] fix indentation --- utils/frame/remote-externalities/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 80f6af31a3a0c..aed257b8ec5e2 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -241,7 +241,7 @@ impl Builder { RpcApi::::finalized_head(&self.as_online().rpc()).await.map_err(|e| { error!("Error = {:?}", e); "rpc finalized_head failed." - }) + }) } /// Relay the request to `state_getPairs` rpc endpoint. @@ -256,7 +256,7 @@ impl Builder { RpcApi::::storage_pairs(&self.as_online().rpc(), prefix, Some(at)).await.map_err(|e| { error!("Error = {:?}", e); "rpc storage_pairs failed" - }) + }) } } From 8f66336b333fbe4e2bb8b7f5a3045ba11b621af2 Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Fri, 19 Mar 2021 11:13:50 +0100 Subject: [PATCH 09/18] replace Block with sp_runtime::testing::Block --- utils/frame/remote-externalities/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index aed257b8ec5e2..6b4f511a38687 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -380,9 +380,9 @@ impl Builder { #[cfg(test)] mod tests { use super::*; - pub type Header = sp_runtime::generic::Header; - pub type Block = sp_runtime::generic::Block; - pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; + use sp_runtime::testing::{H256 as Hash, Block as RawBlock, ExtrinsicWrapper}; + + type Block = RawBlock>; fn init_logger() { let _ = env_logger::Builder::from_default_env() From b14b3f0a994c2a807fa9d92cf144080fbaf6f626 Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Fri, 19 Mar 2021 14:10:42 +0100 Subject: [PATCH 10/18] Move cache test out of remote-test feature tests Add cache file (contains only "Proxy" module) for local test --- utils/frame/remote-externalities/src/lib.rs | 36 +++++++++++------- .../remote-externalities/test_data/proxy_test | Bin 0 -> 39 bytes 2 files changed, 23 insertions(+), 13 deletions(-) create mode 100644 utils/frame/remote-externalities/test_data/proxy_test diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 6b4f511a38687..012e50c978d89 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -376,41 +376,51 @@ impl Builder { } } -#[cfg(feature = "remote-test")] #[cfg(test)] -mod tests { - use super::*; - use sp_runtime::testing::{H256 as Hash, Block as RawBlock, ExtrinsicWrapper}; +mod test_prelude { + pub use super::*; + pub use sp_runtime::testing::{H256 as Hash, Block as RawBlock, ExtrinsicWrapper}; - type Block = RawBlock>; + pub type Block = RawBlock>; - fn init_logger() { + pub fn init_logger() { let _ = env_logger::Builder::from_default_env() .format_module_path(false) .format_level(true) .try_init(); } +} + +#[cfg(test)] +mod tests { + use super::test_prelude::*; #[tokio::test] - async fn can_build_one_pallet() { + async fn can_load_cache() { init_logger(); Builder::::new() - .mode(Mode::Online(OnlineConfig { - modules: vec!["Proxy".into()], - ..Default::default() + .mode(Mode::Offline(OfflineConfig { + cache: CacheConfig { name: "test_data/proxy_test".into(), ..Default::default() }, })) .build() .await .unwrap() .execute_with(|| {}); } +} + +#[cfg(feature = "remote-test")] +#[cfg(test)] +mod remote_tests { + use super::test_prelude::*; #[tokio::test] - async fn can_load_cache() { + async fn can_build_one_pallet() { init_logger(); Builder::::new() - .mode(Mode::Offline(OfflineConfig { - cache: CacheConfig { name: "proxy_test".into(), ..Default::default() }, + .mode(Mode::Online(OnlineConfig { + modules: vec!["Proxy".into()], + ..Default::default() })) .build() .await diff --git a/utils/frame/remote-externalities/test_data/proxy_test b/utils/frame/remote-externalities/test_data/proxy_test new file mode 100644 index 0000000000000000000000000000000000000000..548ce9cdba4f157e3ff0018d314f3fecfbed9f8f GIT binary patch literal 39 vcmZQ+kl?)D>{e98_qB(Af%W>u%I&?*zKN<^Se*X}{?*hKULwHEz`y_iDLxHx literal 0 HcmV?d00001 From f37239474539df42bc67674e23bd8b1d241e11f9 Mon Sep 17 00:00:00 2001 From: Steve Biedermann Date: Sat, 20 Mar 2021 08:55:22 +0100 Subject: [PATCH 11/18] simplify match expression to and_then Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- utils/frame/try-runtime/cli/src/lib.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 01963078a79b1..953f41248964b 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -136,13 +136,10 @@ impl FromStr for CacheParams { let parent = p.parent(); let file_name = p.file_name(); - match file_name { - Some(file_name) => Ok(Self { - // TODO: maybe don't use to_string_lossy here - directory: parent.map(|p| p.to_string_lossy().into()).unwrap_or(".".to_string()), - file_name: file_name.to_string_lossy().into() - }), - None => Err("invalid path"), + file_name.and_then(|file_name| Self { + directory: parent.map(|p| p.to_string_lossy().into()).unwrap_or(".".to_string()), + file_name: file_name.to_string_lossy().into() + }).ok_or("invalid path") } } } From 9aabea819715b310d5506cc30576f177920bffbd Mon Sep 17 00:00:00 2001 From: Steve Biedermann Date: Sat, 20 Mar 2021 09:04:08 +0100 Subject: [PATCH 12/18] Combine the two cfg attributes into one Co-authored-by: David --- utils/frame/remote-externalities/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 012e50c978d89..e0978a76a07af 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -409,8 +409,7 @@ mod tests { } } -#[cfg(feature = "remote-test")] -#[cfg(test)] +#[cfg(all(test, feature = "remote-test"))] mod remote_tests { use super::test_prelude::*; From 2fd4e685646851a5b09ebeeb18136b4f6ba63b1f Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Sat, 20 Mar 2021 09:06:06 +0100 Subject: [PATCH 13/18] Restrict visibility of test_prelude use statements to crate level --- utils/frame/remote-externalities/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index e0978a76a07af..9a1ef1d3bb3c2 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -378,12 +378,12 @@ impl Builder { #[cfg(test)] mod test_prelude { - pub use super::*; - pub use sp_runtime::testing::{H256 as Hash, Block as RawBlock, ExtrinsicWrapper}; + pub(crate) use super::*; + pub(crate) use sp_runtime::testing::{H256 as Hash, Block as RawBlock, ExtrinsicWrapper}; - pub type Block = RawBlock>; + pub(crate) type Block = RawBlock>; - pub fn init_logger() { + pub(crate) fn init_logger() { let _ = env_logger::Builder::from_default_env() .format_module_path(false) .format_level(true) From 4d884d908297097a2a491d35170adfa83e225807 Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Sat, 20 Mar 2021 09:13:59 +0100 Subject: [PATCH 14/18] Fix usage of and_then --- utils/frame/try-runtime/cli/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 953f41248964b..d9b7d16a93659 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -136,11 +136,10 @@ impl FromStr for CacheParams { let parent = p.parent(); let file_name = p.file_name(); - file_name.and_then(|file_name| Self { + file_name.and_then(|file_name| Some(Self { directory: parent.map(|p| p.to_string_lossy().into()).unwrap_or(".".to_string()), file_name: file_name.to_string_lossy().into() - }).ok_or("invalid path") - } + })).ok_or("invalid path") } } @@ -188,7 +187,6 @@ impl TryRuntimeCmd { cache: CacheConfig { name: file_name.into(), directory: directory.into(), - ..Default::default() }, })), State::Live { From 9fe7be560e5cc6dc4e05e015f4ab601931c446c4 Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Sat, 20 Mar 2021 09:34:07 +0100 Subject: [PATCH 15/18] Rename cache to snapshot --- utils/frame/remote-externalities/src/lib.rs | 71 +++++++++++---------- utils/frame/try-runtime/cli/src/lib.rs | 35 +++++----- 2 files changed, 56 insertions(+), 50 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 9a1ef1d3bb3c2..6a1a7a2430621 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -18,7 +18,7 @@ //! # Remote Externalities //! //! An equivalent of `sp_io::TestExternalities` that can load its state from a remote substrate -//! based chain, or a local cache file. +//! based chain, or a local snapshot file. //! //! #### Runtime to Test Against //! @@ -137,37 +137,37 @@ jsonrpsee_proc_macros::rpc_client_api! { pub enum Mode { /// Online. Online(OnlineConfig), - /// Offline. Uses a cached file and needs not any client config. + /// Offline. Uses a snapshot file and needs not any client config. Offline(OfflineConfig), } /// configuration of the online execution. /// -/// A cache config must be present. +/// A snapshot config must be present. #[derive(Clone)] pub struct OfflineConfig { - /// The configuration of the cache file to use. It must be present. - pub cache: CacheConfig, + /// The configuration of the snapshot file to use. It must be present. + pub snapshot: SnapshotConfig, } /// Configuration of the online execution. /// -/// A cache config may be present and will be written to in that case. +/// A snapshot config may be present and will be written to in that case. #[derive(Clone)] pub struct OnlineConfig { /// The HTTP uri to use. pub uri: String, /// The block number at which to connect. Will be latest finalized head if not provided. pub at: Option, - /// An optional cache file to WRITE to, not for reading. Not cached if set to `None`. - pub cache: Option, + /// An optional snapshot file to WRITE to, not for reading. Not written if set to `None`. + pub snapshot: Option, /// The modules to scrape. If empty, entire chain state will be scraped. pub modules: Vec, } impl Default for OnlineConfig { fn default() -> Self { - Self { uri: TARGET.to_owned(), at: None, cache: None, modules: Default::default() } + Self { uri: TARGET.to_owned(), at: None, snapshot: None, modules: Default::default() } } } @@ -179,9 +179,9 @@ impl OnlineConfig { } } -/// Configuration of the cache. +/// Configuration of the snapshot. #[derive(Clone)] -pub struct CacheConfig { +pub struct SnapshotConfig { // TODO: I could mix these two into one filed, but I think separate is better bc one can be // configurable while one not. /// File name. @@ -190,13 +190,13 @@ pub struct CacheConfig { pub directory: String, } -impl Default for CacheConfig { +impl Default for SnapshotConfig { fn default() -> Self { - Self { name: "CACHE".into(), directory: ".".into() } + Self { name: "SNAPSHOT".into(), directory: ".".into() } } } -impl CacheConfig { +impl SnapshotConfig { fn path(&self) -> PathBuf { Path::new(&self.directory).join(self.name.clone()) } @@ -262,16 +262,16 @@ impl Builder { // Internal methods impl Builder { - /// Save the given data as cache. - fn save_cache(&self, data: &[KeyPair], path: &Path) -> Result<(), &'static str> { - info!(target: LOG_TARGET, "writing to cache file {:?}", path); + /// Save the given data as snapshot. + fn save_snapshot(&self, data: &[KeyPair], path: &Path) -> Result<(), &'static str> { + info!(target: LOG_TARGET, "writing to snapshot file {:?}", path); fs::write(path, data.encode()).map_err(|_| "fs::write failed.")?; Ok(()) } - /// initialize `Self` from cache. Panics if the file does not exist. - fn load_cache(&self, path: &Path) -> Result, &'static str> { - info!(target: LOG_TARGET, "scraping keypairs from cache {:?}", path,); + /// initialize `Self` from snapshot. Panics if the file does not exist. + fn load_snapshot(&self, path: &Path) -> Result, &'static str> { + info!(target: LOG_TARGET, "scraping keypairs from snapshot {:?}", path,); let bytes = fs::read(path).map_err(|_| "fs::read failed.")?; Decode::decode(&mut &*bytes).map_err(|_| "decode failed") } @@ -320,12 +320,12 @@ impl Builder { async fn pre_build(mut self) -> Result, &'static str> { let mut base_kv = match self.mode.clone() { - Mode::Offline(config) => self.load_cache(&config.cache.path())?, + Mode::Offline(config) => self.load_snapshot(&config.snapshot.path())?, Mode::Online(config) => { self.init_remote_client().await?; let kp = self.load_remote().await?; - if let Some(c) = config.cache { - self.save_cache(&kp, &c.path())?; + if let Some(c) = config.snapshot { + self.save_snapshot(&kp, &c.path())?; } kp } @@ -356,7 +356,7 @@ impl Builder { self } - /// Configure a cache to be used. + /// Configure a snapshot to be used. pub fn mode(mut self, mode: Mode) -> Self { self.mode = mode; self @@ -396,15 +396,15 @@ mod tests { use super::test_prelude::*; #[tokio::test] - async fn can_load_cache() { + async fn can_load_snapshot() { init_logger(); Builder::::new() .mode(Mode::Offline(OfflineConfig { - cache: CacheConfig { name: "test_data/proxy_test".into(), ..Default::default() }, + snapshot: SnapshotConfig { name: "test_data/proxy_test".into(), ..Default::default() }, })) .build() .await - .unwrap() + .expect("Can't read snapshot file") .execute_with(|| {}); } } @@ -423,27 +423,28 @@ mod remote_tests { })) .build() .await - .unwrap() + .expect("Can't reach the remote node. Is it running?") .execute_with(|| {}); } #[tokio::test] - async fn can_create_cache() { + async fn can_create_snapshot() { init_logger(); Builder::::new() .mode(Mode::Online(OnlineConfig { - cache: Some(CacheConfig { - name: "test_cache_to_remove.bin".into(), + snapshot: Some(SnapshotConfig { + name: "test_snapshot_to_remove.bin".into(), ..Default::default() }), ..Default::default() })) .build() .await + .expect("Can't reach the remote node. Is it running?") .unwrap() .execute_with(|| {}); - let to_delete = std::fs::read_dir(CacheConfig::default().directory) + let to_delete = std::fs::read_dir(SnapshotConfig::default().directory) .unwrap() .into_iter() .map(|d| d.unwrap()) @@ -460,6 +461,10 @@ mod remote_tests { #[tokio::test] async fn can_build_all() { init_logger(); - Builder::::new().build().await.unwrap().execute_with(|| {}); + Builder::::new() + .build() + .await + .expect("Can't reach the remote node. Is it running?") + .execute_with(|| {}); } } diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index d9b7d16a93659..77af97d3d60f0 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -68,14 +68,14 @@ pub enum State { /// Use a snapshot as state to run the migration. Snap { #[structopt(flatten)] - cache_params: CacheParams, + snapshot_path: SnapshotPath, }, /// Use a live chain to run the migration. Live { - /// An optional cache file to WRITE to. Not cached if set to `None`. + /// An optional snapshot file to WRITE to. Not written if set to `None`. #[structopt(short, long)] - cache_file: Option, + snapshot_path: Option, /// The block hash at which to connect. /// Will be latest finalized head if not provided. @@ -119,17 +119,17 @@ fn parse_url(s: &str) -> Result { } #[derive(Debug, structopt::StructOpt)] -pub struct CacheParams { +pub struct SnapshotPath { /// The directory of the snapshot. #[structopt(short, long, default_value = ".")] directory: String, /// The file name of the snapshot. - #[structopt(default_value = "CACHE")] + #[structopt(default_value = "SNAPSHOT")] file_name: String, } -impl FromStr for CacheParams { +impl FromStr for SnapshotPath { type Err = &'static str; fn from_str(s: &str) -> Result { let p: PathBuf = s.parse().map_err(|_| "invalid path")?; @@ -179,24 +179,25 @@ impl TryRuntimeCmd { ); let ext = { - use remote_externalities::{Builder, Mode, CacheConfig, OfflineConfig, OnlineConfig}; + use remote_externalities::{Builder, Mode, SnapshotConfig, OfflineConfig, OnlineConfig}; let builder = match &self.state { - State::Snap { - cache_params: CacheParams { file_name, directory } - } => Builder::::new().mode(Mode::Offline(OfflineConfig { - cache: CacheConfig { - name: file_name.into(), - directory: directory.into(), - }, - })), + State::Snap { snapshot_path } => { + let SnapshotPath { directory, file_name } = snapshot_path; + Builder::::new().mode(Mode::Offline(OfflineConfig { + snapshot: SnapshotConfig { + name: file_name.into(), + directory: directory.into(), + }, + })) + }, State::Live { url, - cache_file, + snapshot_path, block_at, modules } => Builder::::new().mode(Mode::Online(OnlineConfig { uri: url.into(), - cache: cache_file.as_ref().map(|c| CacheConfig { + snapshot: snapshot_path.as_ref().map(|c| SnapshotConfig { name: c.file_name.clone(), directory: c.directory.clone(), }), From 2f0032627e3aa3a404c57696d951b6e30bf5beba Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Sat, 20 Mar 2021 10:01:08 +0100 Subject: [PATCH 16/18] Remove fully qualified path for Debug --- utils/frame/try-runtime/cli/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index 77af97d3d60f0..d7c0996e8a694 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -148,9 +148,9 @@ impl TryRuntimeCmd { where B: BlockT, B::Hash: FromStr, - ::Err: std::fmt::Debug, + ::Err: Debug, NumberFor: FromStr, - as FromStr>::Err: std::fmt::Debug, + as FromStr>::Err: Debug, ExecDispatch: NativeExecutionDispatch + 'static, { let spec = config.chain_spec; From f957ef92b456417706118d18d06b4adbea281471 Mon Sep 17 00:00:00 2001 From: hardliner66 Date: Sat, 20 Mar 2021 10:16:00 +0100 Subject: [PATCH 17/18] Refine naming. snapshot to state_snapshot --- utils/frame/remote-externalities/src/lib.rs | 50 ++++++++++----------- utils/frame/try-runtime/cli/src/lib.rs | 12 ++--- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 6a1a7a2430621..91b4ac1cdd1fb 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -18,7 +18,7 @@ //! # Remote Externalities //! //! An equivalent of `sp_io::TestExternalities` that can load its state from a remote substrate -//! based chain, or a local snapshot file. +//! based chain, or a local state snapshot file. //! //! #### Runtime to Test Against //! @@ -137,37 +137,37 @@ jsonrpsee_proc_macros::rpc_client_api! { pub enum Mode { /// Online. Online(OnlineConfig), - /// Offline. Uses a snapshot file and needs not any client config. + /// Offline. Uses a state snapshot file and needs not any client config. Offline(OfflineConfig), } /// configuration of the online execution. /// -/// A snapshot config must be present. +/// A state snapshot config must be present. #[derive(Clone)] pub struct OfflineConfig { - /// The configuration of the snapshot file to use. It must be present. - pub snapshot: SnapshotConfig, + /// The configuration of the state snapshot file to use. It must be present. + pub state_snapshot: SnapshotConfig, } /// Configuration of the online execution. /// -/// A snapshot config may be present and will be written to in that case. +/// A state snapshot config may be present and will be written to in that case. #[derive(Clone)] pub struct OnlineConfig { /// The HTTP uri to use. pub uri: String, /// The block number at which to connect. Will be latest finalized head if not provided. pub at: Option, - /// An optional snapshot file to WRITE to, not for reading. Not written if set to `None`. - pub snapshot: Option, + /// An optional state snapshot file to WRITE to, not for reading. Not written if set to `None`. + pub state_snapshot: Option, /// The modules to scrape. If empty, entire chain state will be scraped. pub modules: Vec, } impl Default for OnlineConfig { fn default() -> Self { - Self { uri: TARGET.to_owned(), at: None, snapshot: None, modules: Default::default() } + Self { uri: TARGET.to_owned(), at: None, state_snapshot: None, modules: Default::default() } } } @@ -179,7 +179,7 @@ impl OnlineConfig { } } -/// Configuration of the snapshot. +/// Configuration of the state snapshot. #[derive(Clone)] pub struct SnapshotConfig { // TODO: I could mix these two into one filed, but I think separate is better bc one can be @@ -262,16 +262,16 @@ impl Builder { // Internal methods impl Builder { - /// Save the given data as snapshot. - fn save_snapshot(&self, data: &[KeyPair], path: &Path) -> Result<(), &'static str> { - info!(target: LOG_TARGET, "writing to snapshot file {:?}", path); + /// Save the given data as state snapshot. + fn save_state_snapshot(&self, data: &[KeyPair], path: &Path) -> Result<(), &'static str> { + info!(target: LOG_TARGET, "writing to state snapshot file {:?}", path); fs::write(path, data.encode()).map_err(|_| "fs::write failed.")?; Ok(()) } - /// initialize `Self` from snapshot. Panics if the file does not exist. - fn load_snapshot(&self, path: &Path) -> Result, &'static str> { - info!(target: LOG_TARGET, "scraping keypairs from snapshot {:?}", path,); + /// initialize `Self` from state snapshot. Panics if the file does not exist. + fn load_state_snapshot(&self, path: &Path) -> Result, &'static str> { + info!(target: LOG_TARGET, "scraping keypairs from state snapshot {:?}", path,); let bytes = fs::read(path).map_err(|_| "fs::read failed.")?; Decode::decode(&mut &*bytes).map_err(|_| "decode failed") } @@ -320,12 +320,12 @@ impl Builder { async fn pre_build(mut self) -> Result, &'static str> { let mut base_kv = match self.mode.clone() { - Mode::Offline(config) => self.load_snapshot(&config.snapshot.path())?, + Mode::Offline(config) => self.load_state_snapshot(&config.state_snapshot.path())?, Mode::Online(config) => { self.init_remote_client().await?; let kp = self.load_remote().await?; - if let Some(c) = config.snapshot { - self.save_snapshot(&kp, &c.path())?; + if let Some(c) = config.state_snapshot { + self.save_state_snapshot(&kp, &c.path())?; } kp } @@ -356,7 +356,7 @@ impl Builder { self } - /// Configure a snapshot to be used. + /// Configure a state snapshot to be used. pub fn mode(mut self, mode: Mode) -> Self { self.mode = mode; self @@ -396,15 +396,15 @@ mod tests { use super::test_prelude::*; #[tokio::test] - async fn can_load_snapshot() { + async fn can_load_state_snapshot() { init_logger(); Builder::::new() .mode(Mode::Offline(OfflineConfig { - snapshot: SnapshotConfig { name: "test_data/proxy_test".into(), ..Default::default() }, + state_snapshot: SnapshotConfig { name: "test_data/proxy_test".into(), ..Default::default() }, })) .build() .await - .expect("Can't read snapshot file") + .expect("Can't read state snapshot file") .execute_with(|| {}); } } @@ -428,11 +428,11 @@ mod remote_tests { } #[tokio::test] - async fn can_create_snapshot() { + async fn can_create_state_snapshot() { init_logger(); Builder::::new() .mode(Mode::Online(OnlineConfig { - snapshot: Some(SnapshotConfig { + state_snapshot: Some(SnapshotConfig { name: "test_snapshot_to_remove.bin".into(), ..Default::default() }), diff --git a/utils/frame/try-runtime/cli/src/lib.rs b/utils/frame/try-runtime/cli/src/lib.rs index d7c0996e8a694..ff8c5c08ec5b7 100644 --- a/utils/frame/try-runtime/cli/src/lib.rs +++ b/utils/frame/try-runtime/cli/src/lib.rs @@ -65,7 +65,7 @@ pub struct TryRuntimeCmd { /// The state to use for a migration dry-run. #[derive(Debug, structopt::StructOpt)] pub enum State { - /// Use a snapshot as state to run the migration. + /// Use a state snapshot as state to run the migration. Snap { #[structopt(flatten)] snapshot_path: SnapshotPath, @@ -73,7 +73,7 @@ pub enum State { /// Use a live chain to run the migration. Live { - /// An optional snapshot file to WRITE to. Not written if set to `None`. + /// An optional state snapshot file to WRITE to. Not written if set to `None`. #[structopt(short, long)] snapshot_path: Option, @@ -120,11 +120,11 @@ fn parse_url(s: &str) -> Result { #[derive(Debug, structopt::StructOpt)] pub struct SnapshotPath { - /// The directory of the snapshot. + /// The directory of the state snapshot. #[structopt(short, long, default_value = ".")] directory: String, - /// The file name of the snapshot. + /// The file name of the state snapshot. #[structopt(default_value = "SNAPSHOT")] file_name: String, } @@ -184,7 +184,7 @@ impl TryRuntimeCmd { State::Snap { snapshot_path } => { let SnapshotPath { directory, file_name } = snapshot_path; Builder::::new().mode(Mode::Offline(OfflineConfig { - snapshot: SnapshotConfig { + state_snapshot: SnapshotConfig { name: file_name.into(), directory: directory.into(), }, @@ -197,7 +197,7 @@ impl TryRuntimeCmd { modules } => Builder::::new().mode(Mode::Online(OnlineConfig { uri: url.into(), - snapshot: snapshot_path.as_ref().map(|c| SnapshotConfig { + state_snapshot: snapshot_path.as_ref().map(|c| SnapshotConfig { name: c.file_name.clone(), directory: c.directory.clone(), }), From ef4f87d8400e3420fd4aa9c9014b91dcc96106f7 Mon Sep 17 00:00:00 2001 From: Steve Biedermann Date: Sun, 21 Mar 2021 13:45:32 +0100 Subject: [PATCH 18/18] Remove unnecessary comment Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- utils/frame/remote-externalities/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/frame/remote-externalities/src/lib.rs b/utils/frame/remote-externalities/src/lib.rs index 91b4ac1cdd1fb..40e2fe656b8e9 100644 --- a/utils/frame/remote-externalities/src/lib.rs +++ b/utils/frame/remote-externalities/src/lib.rs @@ -117,7 +117,6 @@ use jsonrpsee_http_client::{HttpClient, HttpConfig}; use sp_runtime::traits::Block as BlockT; -// TODO: Make KeyPair generic type KeyPair = (StorageKey, StorageData); const LOG_TARGET: &str = "remote-ext";