diff --git a/Cargo.lock b/Cargo.lock index 9f740f4de4f59..5c48ffdb78219 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4512,6 +4512,7 @@ dependencies = [ "frame-system", "futures 0.3.16", "hex-literal", + "jsonrpsee-ws-client", "log 0.4.14", "nix", "node-executor", @@ -4525,6 +4526,7 @@ dependencies = [ "platforms", "rand 0.7.3", "regex", + "remote-externalities", "sc-authority-discovery", "sc-basic-authorship", "sc-chain-spec", @@ -4569,7 +4571,9 @@ dependencies = [ "substrate-build-script-utils", "substrate-frame-cli", "tempfile", + "tokio", "try-runtime-cli", + "wait-timeout", ] [[package]] @@ -10478,9 +10482,9 @@ checksum = "cda74da7e1a664f795bb1f8a87ec406fb89a02522cf6e50620d016add6dbbf5c" [[package]] name = "tokio" -version = "1.10.0" +version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01cf844b23c6131f624accf65ce0e4e9956a8bb329400ea5bcc26ae3a5c20b0b" +checksum = "c2c2416fdedca8443ae44b4527de1ea633af61d8f7169ffa6e72c5b53d24efcc" dependencies = [ "autocfg 1.0.1", "bytes 1.0.1", diff --git a/bin/node-template/node/src/cli.rs b/bin/node-template/node/src/cli.rs index 8b551051c1b19..8ed1d35ba5f92 100644 --- a/bin/node-template/node/src/cli.rs +++ b/bin/node-template/node/src/cli.rs @@ -35,7 +35,7 @@ pub enum Subcommand { /// Revert the chain to a previous state. Revert(sc_cli::RevertCmd), - /// The custom benchmark subcommmand benchmarking runtime pallets. + /// The custom benchmark subcommand benchmarking runtime pallets. #[structopt(name = "benchmark", about = "Benchmark runtime pallets.")] Benchmark(frame_benchmarking_cli::BenchmarkCmd), } diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index 6a12af4b278b7..83e32167bf232 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -124,6 +124,10 @@ regex = "1" platforms = "1.1" async-std = { version = "1.6.5", features = ["attributes"] } soketto = "0.4.2" +jsonrpsee-ws-client = { version = "0.3.0", default-features = false, features = ["tokio1"] } +tokio = { version = "1.10", features = ["macros", "time"] } +wait-timeout = "0.2" +remote-externalities = { path = "../../../utils/frame/remote-externalities" } [build-dependencies] structopt = { version = "0.3.8", optional = true } @@ -135,7 +139,7 @@ try-runtime-cli = { version = "0.10.0-dev", optional = true, path = "../../../ut sc-cli = { version = "0.10.0-dev", path = "../../../client/cli", optional = true } [features] -default = [ "cli" ] +default = ["cli"] cli = [ "node-executor/wasmi-errno", "node-inspect", diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 352e007a891ba..5e727afa304bb 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -265,7 +265,7 @@ pub fn testnet_genesis( .map(|x| &x.0) .chain(initial_nominators.iter()) .for_each(|x| { - if !endowed_accounts.contains(&x) { + if !endowed_accounts.contains(x) { endowed_accounts.push(x.clone()) } }); diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index acc7df5b1e5a3..b1a3bd4722597 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -86,7 +86,7 @@ pub fn new_partial( let (client, backend, keystore_container, task_manager) = sc_service::new_full_parts::( - &config, + config, telemetry.as_ref().map(|(_, telemetry)| telemetry.handle()), executor, )?; @@ -277,7 +277,7 @@ pub fn new_full_base( let _rpc_handlers = sc_service::spawn_tasks(sc_service::SpawnTasksParams { config, - backend: backend.clone(), + backend, client: client.clone(), keystore: keystore_container.sync_keystore(), network: network.clone(), @@ -507,7 +507,7 @@ pub fn new_light_base( babe_block_import, Some(Box::new(justification_import)), client.clone(), - select_chain.clone(), + select_chain, move |_, ()| async move { let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); diff --git a/bin/node/cli/tests/check_block_works.rs b/bin/node/cli/tests/check_block_works.rs index 707fd217e33e8..216bcc6d9fc13 100644 --- a/bin/node/cli/tests/check_block_works.rs +++ b/bin/node/cli/tests/check_block_works.rs @@ -24,11 +24,11 @@ use tempfile::tempdir; pub mod common; -#[test] -fn check_block_works() { +#[tokio::test] +async fn check_block_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_node_for_a_while(base_path.path(), &["--dev"]); + common::run_node_for_a_while(base_path.path(), &["--dev"]).await; let status = Command::new(cargo_bin("substrate")) .args(&["check-block", "--dev", "--pruning", "archive", "-d"]) diff --git a/bin/node/cli/tests/common.rs b/bin/node/cli/tests/common.rs index 54b9c749bf1de..85effc858e155 100644 --- a/bin/node/cli/tests/common.rs +++ b/bin/node/cli/tests/common.rs @@ -23,58 +23,115 @@ use nix::{ sys::signal::{kill, Signal::SIGINT}, unistd::Pid, }; +use node_primitives::Block; +use remote_externalities::rpc_api; use std::{ convert::TryInto, + ops::{Deref, DerefMut}, path::Path, process::{Child, Command, ExitStatus}, - thread, time::Duration, }; +use tokio::time::timeout; + +static LOCALHOST_WS: &str = "ws://127.0.0.1:9944/"; /// Wait for the given `child` the given number of `secs`. /// /// Returns the `Some(exit status)` or `None` if the process did not finish in the given time. -pub fn wait_for(child: &mut Child, secs: usize) -> Option { - for i in 0..secs { - match child.try_wait().unwrap() { - Some(status) => { - if i > 5 { - eprintln!("Child process took {} seconds to exit gracefully", i); - } - return Some(status) - }, - None => thread::sleep(Duration::from_secs(1)), +pub fn wait_for(child: &mut Child, secs: u64) -> Result { + let result = wait_timeout::ChildExt::wait_timeout(child, Duration::from_secs(5.min(secs))) + .map_err(|_| ())?; + if let Some(exit_status) = result { + Ok(exit_status) + } else { + if secs > 5 { + eprintln!("Child process taking over 5 seconds to exit gracefully"); + let result = wait_timeout::ChildExt::wait_timeout(child, Duration::from_secs(secs - 5)) + .map_err(|_| ())?; + if let Some(exit_status) = result { + return Ok(exit_status) + } } + eprintln!("Took too long to exit (> {} seconds). Killing...", secs); + let _ = child.kill(); + child.wait().unwrap(); + Err(()) } - eprintln!("Took too long to exit (> {} seconds). Killing...", secs); - let _ = child.kill(); - child.wait().unwrap(); +} + +/// Wait for at least n blocks to be finalized within a specified time. +pub async fn wait_n_finalized_blocks( + n: usize, + timeout_secs: u64, +) -> Result<(), tokio::time::error::Elapsed> { + timeout(Duration::from_secs(timeout_secs), wait_n_finalized_blocks_from(n, LOCALHOST_WS)).await +} + +/// Wait for at least n blocks to be finalized from a specified node +pub async fn wait_n_finalized_blocks_from(n: usize, url: &str) { + let mut built_blocks = std::collections::HashSet::new(); + let mut interval = tokio::time::interval(Duration::from_secs(2)); - None + loop { + if let Ok(block) = rpc_api::get_finalized_head::(url.to_string()).await { + built_blocks.insert(block); + if built_blocks.len() > n { + break + } + }; + interval.tick().await; + } } -/// Run the node for a while (30 seconds) -pub fn run_node_for_a_while(base_path: &Path, args: &[&str]) { +/// Run the node for a while (3 blocks) +pub async fn run_node_for_a_while(base_path: &Path, args: &[&str]) { let mut cmd = Command::new(cargo_bin("substrate")); - let mut cmd = cmd.args(args).arg("-d").arg(base_path).spawn().unwrap(); + let mut child = KillChildOnDrop(cmd.args(args).arg("-d").arg(base_path).spawn().unwrap()); // Let it produce some blocks. - thread::sleep(Duration::from_secs(30)); - assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running"); + let _ = wait_n_finalized_blocks(3, 30).await; + + assert!(child.try_wait().unwrap().is_none(), "the process should still be running"); // Stop the process - kill(Pid::from_raw(cmd.id().try_into().unwrap()), SIGINT).unwrap(); - assert!(wait_for(&mut cmd, 40).map(|x| x.success()).unwrap_or_default()); + kill(Pid::from_raw(child.id().try_into().unwrap()), SIGINT).unwrap(); + assert!(wait_for(&mut child, 40).map(|x| x.success()).unwrap()); } /// Run the node asserting that it fails with an error pub fn run_node_assert_fail(base_path: &Path, args: &[&str]) { let mut cmd = Command::new(cargo_bin("substrate")); - let mut cmd = cmd.args(args).arg("-d").arg(base_path).spawn().unwrap(); + let mut child = KillChildOnDrop(cmd.args(args).arg("-d").arg(base_path).spawn().unwrap()); - // Let it produce some blocks. - thread::sleep(Duration::from_secs(10)); - assert!(cmd.try_wait().unwrap().is_some(), "the process should not be running anymore"); + // Let it produce some blocks, but it should die within 10 seconds. + assert_ne!( + wait_timeout::ChildExt::wait_timeout(&mut *child, Duration::from_secs(10)).unwrap(), + None, + "the process should not be running anymore" + ); +} + +pub struct KillChildOnDrop(pub Child); + +impl Drop for KillChildOnDrop { + fn drop(&mut self) { + let _ = self.0.kill(); + } +} + +impl Deref for KillChildOnDrop { + type Target = Child; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for KillChildOnDrop { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } } diff --git a/bin/node/cli/tests/database_role_subdir_migration.rs b/bin/node/cli/tests/database_role_subdir_migration.rs index 516908111ae72..9338d8a8e4f43 100644 --- a/bin/node/cli/tests/database_role_subdir_migration.rs +++ b/bin/node/cli/tests/database_role_subdir_migration.rs @@ -25,9 +25,9 @@ use tempfile::tempdir; pub mod common; -#[test] +#[tokio::test] #[cfg(unix)] -fn database_role_subdir_migration() { +async fn database_role_subdir_migration() { type Block = RawBlock>; let base_path = tempdir().expect("could not create a temp dir"); @@ -62,7 +62,8 @@ fn database_role_subdir_migration() { "44445", "--no-prometheus", ], - ); + ) + .await; // check if the database dir had been migrated assert!(!path.join("db_version").exists()); diff --git a/bin/node/cli/tests/export_import_flow.rs b/bin/node/cli/tests/export_import_flow.rs index 7cbaa152699b4..937f03b8e5dae 100644 --- a/bin/node/cli/tests/export_import_flow.rs +++ b/bin/node/cli/tests/export_import_flow.rs @@ -182,13 +182,13 @@ impl<'a> ExportImportRevertExecutor<'a> { } } -#[test] -fn export_import_revert() { +#[tokio::test] +async fn export_import_revert() { let base_path = tempdir().expect("could not create a temp dir"); let exported_blocks_file = base_path.path().join("exported_blocks"); let db_path = base_path.path().join("db"); - common::run_node_for_a_while(base_path.path(), &["--dev"]); + common::run_node_for_a_while(base_path.path(), &["--dev"]).await; let mut executor = ExportImportRevertExecutor::new(&base_path, &exported_blocks_file, &db_path); diff --git a/bin/node/cli/tests/inspect_works.rs b/bin/node/cli/tests/inspect_works.rs index 2a89801547a4b..6f980d2acbfcb 100644 --- a/bin/node/cli/tests/inspect_works.rs +++ b/bin/node/cli/tests/inspect_works.rs @@ -24,11 +24,11 @@ use tempfile::tempdir; pub mod common; -#[test] -fn inspect_works() { +#[tokio::test] +async fn inspect_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_node_for_a_while(base_path.path(), &["--dev"]); + common::run_node_for_a_while(base_path.path(), &["--dev"]).await; let status = Command::new(cargo_bin("substrate")) .args(&["inspect", "--dev", "--pruning", "archive", "-d"]) diff --git a/bin/node/cli/tests/purge_chain_works.rs b/bin/node/cli/tests/purge_chain_works.rs index 0f16a51e5d0a4..8a8601c863d95 100644 --- a/bin/node/cli/tests/purge_chain_works.rs +++ b/bin/node/cli/tests/purge_chain_works.rs @@ -22,12 +22,12 @@ use tempfile::tempdir; pub mod common; -#[test] +#[tokio::test] #[cfg(unix)] -fn purge_chain_works() { +async fn purge_chain_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_node_for_a_while(base_path.path(), &["--dev"]); + common::run_node_for_a_while(base_path.path(), &["--dev"]).await; let status = Command::new(cargo_bin("substrate")) .args(&["purge-chain", "--dev", "-d"]) diff --git a/bin/node/cli/tests/running_the_node_and_interrupt.rs b/bin/node/cli/tests/running_the_node_and_interrupt.rs index 03a1826f2f080..fc5094c2d722f 100644 --- a/bin/node/cli/tests/running_the_node_and_interrupt.rs +++ b/bin/node/cli/tests/running_the_node_and_interrupt.rs @@ -17,7 +17,6 @@ // along with this program. If not, see . #![cfg(unix)] - use assert_cmd::cargo::cargo_bin; use nix::{ sys::signal::{ @@ -26,67 +25,43 @@ use nix::{ }, unistd::Pid, }; -use sc_service::Deref; use std::{ convert::TryInto, - ops::DerefMut, process::{Child, Command}, - thread, - time::Duration, }; use tempfile::tempdir; pub mod common; -#[test] -fn running_the_node_works_and_can_be_interrupted() { - fn run_command_and_kill(signal: Signal) { +#[tokio::test] +async fn running_the_node_works_and_can_be_interrupted() { + async fn run_command_and_kill(signal: Signal) { let base_path = tempdir().expect("could not create a temp dir"); - let mut cmd = Command::new(cargo_bin("substrate")) - .args(&["--dev", "-d"]) - .arg(base_path.path()) - .spawn() - .unwrap(); + let mut cmd = common::KillChildOnDrop( + Command::new(cargo_bin("substrate")) + .args(&["--dev", "-d"]) + .arg(base_path.path()) + .spawn() + .unwrap(), + ); - thread::sleep(Duration::from_secs(20)); + common::wait_n_finalized_blocks(3, 30).await.unwrap(); assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running"); kill(Pid::from_raw(cmd.id().try_into().unwrap()), signal).unwrap(); assert_eq!( common::wait_for(&mut cmd, 30).map(|x| x.success()), - Some(true), + Ok(true), "the process must exit gracefully after signal {}", signal, ); } - run_command_and_kill(SIGINT); - run_command_and_kill(SIGTERM); -} - -struct KillChildOnDrop(Child); - -impl Drop for KillChildOnDrop { - fn drop(&mut self) { - let _ = self.0.kill(); - } -} - -impl Deref for KillChildOnDrop { - type Target = Child; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl DerefMut for KillChildOnDrop { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 - } + run_command_and_kill(SIGINT).await; + run_command_and_kill(SIGTERM).await; } -#[test] -fn running_two_nodes_with_the_same_ws_port_should_work() { +#[tokio::test] +async fn running_two_nodes_with_the_same_ws_port_should_work() { fn start_node() -> Child { Command::new(cargo_bin("substrate")) .args(&["--dev", "--tmp", "--ws-port=45789"]) @@ -94,10 +69,10 @@ fn running_two_nodes_with_the_same_ws_port_should_work() { .unwrap() } - let mut first_node = KillChildOnDrop(start_node()); - let mut second_node = KillChildOnDrop(start_node()); + let mut first_node = common::KillChildOnDrop(start_node()); + let mut second_node = common::KillChildOnDrop(start_node()); - thread::sleep(Duration::from_secs(30)); + let _ = common::wait_n_finalized_blocks(3, 30).await; assert!(first_node.try_wait().unwrap().is_none(), "The first node should still be running"); assert!(second_node.try_wait().unwrap().is_none(), "The second node should still be running"); @@ -107,12 +82,12 @@ fn running_two_nodes_with_the_same_ws_port_should_work() { assert_eq!( common::wait_for(&mut first_node, 30).map(|x| x.success()), - Some(true), + Ok(true), "The first node must exit gracefully", ); assert_eq!( common::wait_for(&mut second_node, 30).map(|x| x.success()), - Some(true), + Ok(true), "The second node must exit gracefully", ); } diff --git a/bin/node/cli/tests/temp_base_path_works.rs b/bin/node/cli/tests/temp_base_path_works.rs index c107740b9b0a5..5d8e6c9ec4539 100644 --- a/bin/node/cli/tests/temp_base_path_works.rs +++ b/bin/node/cli/tests/temp_base_path_works.rs @@ -29,37 +29,34 @@ use std::{ io::Read, path::PathBuf, process::{Command, Stdio}, - thread, - time::Duration, }; pub mod common; -#[test] -fn temp_base_path_works() { +#[tokio::test] +async fn temp_base_path_works() { let mut cmd = Command::new(cargo_bin("substrate")); - - let mut cmd = cmd - .args(&["--dev", "--tmp"]) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .unwrap(); + let mut child = common::KillChildOnDrop( + cmd.args(&["--dev", "--tmp"]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(), + ); // Let it produce some blocks. - thread::sleep(Duration::from_secs(30)); - assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running"); + common::wait_n_finalized_blocks(3, 30).await.unwrap(); + assert!(child.try_wait().unwrap().is_none(), "the process should still be running"); // Stop the process - kill(Pid::from_raw(cmd.id().try_into().unwrap()), SIGINT).unwrap(); - assert!(common::wait_for(&mut cmd, 40).map(|x| x.success()).unwrap_or_default()); + kill(Pid::from_raw(child.id().try_into().unwrap()), SIGINT).unwrap(); + assert!(common::wait_for(&mut child, 40).map(|x| x.success()).unwrap_or_default()); // Ensure the database has been deleted let mut stderr = String::new(); - cmd.stderr.unwrap().read_to_string(&mut stderr).unwrap(); + child.stderr.as_mut().unwrap().read_to_string(&mut stderr).unwrap(); let re = Regex::new(r"Database: .+ at (\S+)").unwrap(); - let db_path = - PathBuf::from(re.captures(stderr.as_str()).unwrap().get(1).unwrap().as_str().to_string()); + let db_path = PathBuf::from(re.captures(stderr.as_str()).unwrap().get(1).unwrap().as_str()); assert!(!db_path.exists()); } diff --git a/client/executor/src/native_executor.rs b/client/executor/src/native_executor.rs index 77b1ec7abf4f2..979bcc072c88b 100644 --- a/client/executor/src/native_executor.rs +++ b/client/executor/src/native_executor.rs @@ -189,7 +189,7 @@ impl WasmExecutor { /// Perform a call into the given runtime. /// - /// The runtime is passed as a [`RuntimeBlob`]. The runtime will be isntantiated with the + /// The runtime is passed as a [`RuntimeBlob`]. The runtime will be instantiated with the /// parameters this `WasmExecutor` was initialized with. /// /// In case of problems with during creation of the runtime or instantation, a `Err` is @@ -247,7 +247,7 @@ impl sp_core::traits::ReadRuntimeVersion for WasmExecutor { } // If the blob didn't have embedded runtime version section, we fallback to the legacy - // way of fetching the verison: i.e. instantiating the given instance and calling + // way of fetching the version: i.e. instantiating the given instance and calling // `Core_version` on it. self.uncached_call(