From f3a690ca2db0f8e70825d19f123ea0497a1bfbab Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 10:45:41 +0100 Subject: [PATCH 01/11] Initial commit Forked at: 41bb2193a267805e2093a081bc3e2aaccc64283a Parent branch: origin/master From 9fe6f4f03fc71dfe2cabc2815676e1e54c587305 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 10:46:32 +0100 Subject: [PATCH 02/11] Increase killing grace period of CLI tests and display more info --- bin/node/cli/tests/common.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/bin/node/cli/tests/common.rs b/bin/node/cli/tests/common.rs index 682a30bcc0178..f25d4799a1516 100644 --- a/bin/node/cli/tests/common.rs +++ b/bin/node/cli/tests/common.rs @@ -27,13 +27,18 @@ use nix::unistd::Pid; /// /// 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 _ in 0..secs { + for i in 0..secs { match child.try_wait().unwrap() { - Some(status) => return Some(status), + Some(status) => { + if i > 5 { + eprintln!("Child process took {} seconds to exit gracefully", i); + } + return Some(status) + }, None => thread::sleep(Duration::from_secs(1)), } } - eprintln!("Took to long to exit. Killing..."); + eprintln!("Took too long to exit (> {} seconds). Killing...", secs); let _ = child.kill(); child.wait().unwrap(); @@ -60,5 +65,5 @@ pub fn run_command_for_a_while(base_path: &Path, dev: bool) { // 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()); + assert!(wait_for(&mut cmd, 120).map(|x| x.success()).unwrap_or_default()); } From 6d908bf71fa04324a865b588a1b3e6316f7ba75e Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 11:08:45 +0100 Subject: [PATCH 03/11] Use --dev everywhere possible --- bin/node/cli/tests/check_block_works.rs | 4 ++-- bin/node/cli/tests/common.rs | 2 +- bin/node/cli/tests/inspect_works.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/node/cli/tests/check_block_works.rs b/bin/node/cli/tests/check_block_works.rs index e4c93c9e88528..1b0528a587565 100644 --- a/bin/node/cli/tests/check_block_works.rs +++ b/bin/node/cli/tests/check_block_works.rs @@ -26,10 +26,10 @@ mod common; fn check_block_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_command_for_a_while(base_path.path(), false); + common::run_command_for_a_while(base_path.path(), true); let status = Command::new(cargo_bin("substrate")) - .args(&["check-block", "-d"]) + .args(&["check-block", "--dev", "--pruning", "archive", "-d"]) .arg(base_path.path()) .arg("1") .status() diff --git a/bin/node/cli/tests/common.rs b/bin/node/cli/tests/common.rs index f25d4799a1516..26e8cafa87f2a 100644 --- a/bin/node/cli/tests/common.rs +++ b/bin/node/cli/tests/common.rs @@ -50,7 +50,7 @@ pub fn run_command_for_a_while(base_path: &Path, dev: bool) { let mut cmd = Command::new(cargo_bin("substrate")); if dev { - cmd.arg("--dev"); + cmd.args(&["--dev", "--pruning", "archive"]); } let mut cmd = cmd diff --git a/bin/node/cli/tests/inspect_works.rs b/bin/node/cli/tests/inspect_works.rs index 0bd48c3693802..aafbb46f7be34 100644 --- a/bin/node/cli/tests/inspect_works.rs +++ b/bin/node/cli/tests/inspect_works.rs @@ -26,10 +26,10 @@ mod common; fn inspect_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_command_for_a_while(base_path.path(), false); + common::run_command_for_a_while(base_path.path(), true); let status = Command::new(cargo_bin("substrate")) - .args(&["inspect", "-d"]) + .args(&["inspect", "--dev", "--pruning", "archive", "-d"]) .arg(base_path.path()) .args(&["block", "1"]) .status() From c460c0ed7dc16b05fd296d6f10c862b0af89a412 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 11:31:29 +0100 Subject: [PATCH 04/11] Put pruning mode to its own params struct --- .../tests/import_export_and_revert_work.rs | 8 +-- client/cli/src/params/import_params.rs | 49 +++---------------- client/cli/src/params/mod.rs | 2 + 3 files changed, 14 insertions(+), 45 deletions(-) diff --git a/bin/node/cli/tests/import_export_and_revert_work.rs b/bin/node/cli/tests/import_export_and_revert_work.rs index e109aa279eb88..520ddb14c2aa1 100644 --- a/bin/node/cli/tests/import_export_and_revert_work.rs +++ b/bin/node/cli/tests/import_export_and_revert_work.rs @@ -27,10 +27,10 @@ fn import_export_and_revert_work() { let base_path = tempdir().expect("could not create a temp dir"); let exported_blocks = base_path.path().join("exported_blocks"); - common::run_command_for_a_while(base_path.path(), false); + common::run_command_for_a_while(base_path.path(), true); let status = Command::new(cargo_bin("substrate")) - .args(&["export-blocks", "-d"]) + .args(&["export-blocks", "--dev", "--pruning", "archive", "-d"]) .arg(base_path.path()) .arg(&exported_blocks) .status() @@ -43,7 +43,7 @@ fn import_export_and_revert_work() { let _ = fs::remove_dir_all(base_path.path().join("db")); let status = Command::new(cargo_bin("substrate")) - .args(&["import-blocks", "-d"]) + .args(&["import-blocks", "--dev", "--pruning", "archive", "-d"]) .arg(base_path.path()) .arg(&exported_blocks) .status() @@ -51,7 +51,7 @@ fn import_export_and_revert_work() { assert!(status.success()); let status = Command::new(cargo_bin("substrate")) - .args(&["revert", "-d"]) + .args(&["revert", "--dev", "--pruning", "archive", "-d"]) .arg(base_path.path()) .status() .unwrap(); diff --git a/client/cli/src/params/import_params.rs b/client/cli/src/params/import_params.rs index 98809a38ae446..d9843a13c216b 100644 --- a/client/cli/src/params/import_params.rs +++ b/client/cli/src/params/import_params.rs @@ -15,10 +15,7 @@ // along with Substrate. If not, see . use structopt::StructOpt; -use sc_service::{ - Configuration, RuntimeGenesis, - config::DatabaseConfig, PruningMode, -}; +use sc_service::{Configuration, RuntimeGenesis, config::DatabaseConfig}; use crate::error; use crate::arg_enums::{ @@ -26,25 +23,14 @@ use crate::arg_enums::{ DEFAULT_EXECUTION_IMPORT_BLOCK, DEFAULT_EXECUTION_OFFCHAIN_WORKER, DEFAULT_EXECUTION_OTHER, DEFAULT_EXECUTION_SYNCING }; +use crate::params::PruningParams; /// Parameters for block import. #[derive(Debug, StructOpt, Clone)] pub struct ImportParams { - /// Specify the state pruning mode, a number of blocks to keep or 'archive'. - /// - /// Default is to keep all block states if the node is running as a - /// validator (i.e. 'archive'), otherwise state is only kept for the last - /// 256 blocks. - #[structopt(long = "pruning", value_name = "PRUNING_MODE")] - pub pruning: Option, - - /// Force start with unsafe pruning settings. - /// - /// When running as a validator it is highly recommended to disable state - /// pruning (i.e. 'archive') which is the default. The node will refuse to - /// start as a validator if pruning is enabled unless this option is set. - #[structopt(long = "unsafe-pruning")] - pub unsafe_pruning: bool, + #[allow(missing_docs)] + #[structopt(flatten)] + pub pruning_params: PruningParams, /// Method for executing Wasm runtime code. #[structopt( @@ -87,7 +73,7 @@ impl ImportParams { /// Put block import CLI params into `config` object. pub fn update_config( &self, - config: &mut Configuration, + mut config: &mut Configuration, role: sc_service::Roles, is_dev: bool, ) -> error::Result<()> @@ -102,27 +88,7 @@ impl ImportParams { config.state_cache_size = self.state_cache_size; - // by default we disable pruning if the node is an authority (i.e. - // `ArchiveAll`), otherwise we keep state for the last 256 blocks. if the - // node is an authority and pruning is enabled explicitly, then we error - // unless `unsafe_pruning` is set. - config.pruning = match &self.pruning { - Some(ref s) if s == "archive" => PruningMode::ArchiveAll, - None if role == sc_service::Roles::AUTHORITY => PruningMode::ArchiveAll, - None => PruningMode::default(), - Some(s) => { - if role == sc_service::Roles::AUTHORITY && !self.unsafe_pruning { - return Err(error::Error::Input( - "Validators should run with state pruning disabled (i.e. archive). \ - You can ignore this check with `--unsafe-pruning`.".to_string() - )); - } - - PruningMode::keep_blocks(s.parse() - .map_err(|_| error::Error::Input("Invalid pruning mode specified".to_string()))? - ) - }, - }; + self.pruning_params.update_config(&mut config, role)?; config.wasm_method = self.wasm_method.into(); @@ -144,6 +110,7 @@ impl ImportParams { exec_all_or(exec.execution_offchain_worker, DEFAULT_EXECUTION_OFFCHAIN_WORKER), other: exec_all_or(exec.execution_other, DEFAULT_EXECUTION_OTHER), }; + Ok(()) } } diff --git a/client/cli/src/params/mod.rs b/client/cli/src/params/mod.rs index 75509afa42558..f684cab336423 100644 --- a/client/cli/src/params/mod.rs +++ b/client/cli/src/params/mod.rs @@ -19,6 +19,7 @@ mod transaction_pool_params; mod shared_params; mod node_key_params; mod network_configuration_params; +mod pruning_params; use std::str::FromStr; use std::fmt::Debug; @@ -28,6 +29,7 @@ pub use crate::params::transaction_pool_params::*; pub use crate::params::shared_params::*; pub use crate::params::node_key_params::*; pub use crate::params::network_configuration_params::*; +pub use crate::params::pruning_params::*; /// Wrapper type of `String` that holds an unsigned integer of arbitrary size, formatted as a decimal. #[derive(Debug, Clone)] From 59e6d3333ecde330b710ded1678f53a7e1d59d8c Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 11:35:07 +0100 Subject: [PATCH 05/11] Add pruning params to export-blocks command --- client/cli/src/commands/export_blocks_cmd.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/client/cli/src/commands/export_blocks_cmd.rs b/client/cli/src/commands/export_blocks_cmd.rs index 8db650ae8c88c..002ce6b8cf8a0 100644 --- a/client/cli/src/commands/export_blocks_cmd.rs +++ b/client/cli/src/commands/export_blocks_cmd.rs @@ -22,15 +22,14 @@ use log::info; use structopt::StructOpt; use sc_service::{ Configuration, ChainSpecExtension, RuntimeGenesis, ServiceBuilderCommand, ChainSpec, - config::DatabaseConfig, + config::DatabaseConfig, Roles, }; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use crate::error; use crate::VersionInfo; use crate::runtime::run_until_exit; -use crate::params::SharedParams; -use crate::params::BlockNumber; +use crate::params::{SharedParams, BlockNumber, PruningParams}; /// The `export-blocks` command used to export blocks. #[derive(Debug, StructOpt, Clone)] @@ -58,6 +57,10 @@ pub struct ExportBlocksCmd { #[allow(missing_docs)] #[structopt(flatten)] pub shared_params: SharedParams, + + #[allow(missing_docs)] + #[structopt(flatten)] + pub pruning_params: PruningParams, } impl ExportBlocksCmd { @@ -106,6 +109,7 @@ impl ExportBlocksCmd { F: FnOnce(&str) -> Result>, String>, { self.shared_params.update_config(&mut config, spec_factory, version)?; + self.pruning_params.update_config(&mut config, Roles::FULL)?; config.use_in_memory_keystore()?; Ok(()) From bb7abe14d80b41607045c025db7fb4c495e56928 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 11:35:48 +0100 Subject: [PATCH 06/11] Added missing file --- client/cli/src/params/pruning_params.rs | 77 +++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 client/cli/src/params/pruning_params.rs diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs new file mode 100644 index 0000000000000..aa70177d40f4c --- /dev/null +++ b/client/cli/src/params/pruning_params.rs @@ -0,0 +1,77 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +use structopt::StructOpt; +use sc_service::{Configuration, RuntimeGenesis, PruningMode}; + +use crate::error; + +/// Parameters to define the pruning mode +#[derive(Debug, StructOpt, Clone)] +pub struct PruningParams { + /// Specify the state pruning mode, a number of blocks to keep or 'archive'. + /// + /// Default is to keep all block states if the node is running as a + /// validator (i.e. 'archive'), otherwise state is only kept for the last + /// 256 blocks. + #[structopt(long = "pruning", value_name = "PRUNING_MODE")] + pub pruning: Option, + + /// Force start with unsafe pruning settings. + /// + /// When running as a validator it is highly recommended to disable state + /// pruning (i.e. 'archive') which is the default. The node will refuse to + /// start as a validator if pruning is enabled unless this option is set. + #[structopt(long = "unsafe-pruning")] + pub unsafe_pruning: bool, + +} + +impl PruningParams { + /// Put block pruning CLI params into `config` object. + pub fn update_config( + &self, + mut config: &mut Configuration, + role: sc_service::Roles, + ) -> error::Result<()> + where + G: RuntimeGenesis, + { + // by default we disable pruning if the node is an authority (i.e. + // `ArchiveAll`), otherwise we keep state for the last 256 blocks. if the + // node is an authority and pruning is enabled explicitly, then we error + // unless `unsafe_pruning` is set. + config.pruning = match &self.pruning { + Some(ref s) if s == "archive" => PruningMode::ArchiveAll, + None if role == sc_service::Roles::AUTHORITY => PruningMode::ArchiveAll, + None => PruningMode::default(), + Some(s) => { + if role == sc_service::Roles::AUTHORITY && !self.unsafe_pruning { + return Err(error::Error::Input( + "Validators should run with state pruning disabled (i.e. archive). \ + You can ignore this check with `--unsafe-pruning`.".to_string() + )); + } + + PruningMode::keep_blocks(s.parse() + .map_err(|_| error::Error::Input("Invalid pruning mode specified".to_string()))? + ) + }, + }; + + Ok(()) + } +} From 37cb258ae937e9db6489b71992bab30c124d0348 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 11:39:30 +0100 Subject: [PATCH 07/11] Removed not-dev mode in tests --- bin/node/cli/tests/check_block_works.rs | 2 +- bin/node/cli/tests/common.rs | 7 ++----- bin/node/cli/tests/import_export_and_revert_work.rs | 2 +- bin/node/cli/tests/inspect_works.rs | 2 +- bin/node/cli/tests/purge_chain_works.rs | 2 +- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/bin/node/cli/tests/check_block_works.rs b/bin/node/cli/tests/check_block_works.rs index 1b0528a587565..6b1d7bdcebe6e 100644 --- a/bin/node/cli/tests/check_block_works.rs +++ b/bin/node/cli/tests/check_block_works.rs @@ -26,7 +26,7 @@ mod common; fn check_block_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_command_for_a_while(base_path.path(), true); + common::run_command_for_a_while(base_path.path()); 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 26e8cafa87f2a..02b5fbc70ee08 100644 --- a/bin/node/cli/tests/common.rs +++ b/bin/node/cli/tests/common.rs @@ -46,14 +46,11 @@ pub fn wait_for(child: &mut Child, secs: usize) -> Option { } /// Run the node for a while (30 seconds) -pub fn run_command_for_a_while(base_path: &Path, dev: bool) { +pub fn run_command_for_a_while(base_path: &Path) { let mut cmd = Command::new(cargo_bin("substrate")); - if dev { - cmd.args(&["--dev", "--pruning", "archive"]); - } - let mut cmd = cmd + .args(&["--dev", "--pruning", "archive"]) .arg("-d") .arg(base_path) .spawn() diff --git a/bin/node/cli/tests/import_export_and_revert_work.rs b/bin/node/cli/tests/import_export_and_revert_work.rs index 520ddb14c2aa1..c13e398153fee 100644 --- a/bin/node/cli/tests/import_export_and_revert_work.rs +++ b/bin/node/cli/tests/import_export_and_revert_work.rs @@ -27,7 +27,7 @@ fn import_export_and_revert_work() { let base_path = tempdir().expect("could not create a temp dir"); let exported_blocks = base_path.path().join("exported_blocks"); - common::run_command_for_a_while(base_path.path(), true); + common::run_command_for_a_while(base_path.path()); let status = Command::new(cargo_bin("substrate")) .args(&["export-blocks", "--dev", "--pruning", "archive", "-d"]) diff --git a/bin/node/cli/tests/inspect_works.rs b/bin/node/cli/tests/inspect_works.rs index aafbb46f7be34..86f5e7d2c541c 100644 --- a/bin/node/cli/tests/inspect_works.rs +++ b/bin/node/cli/tests/inspect_works.rs @@ -26,7 +26,7 @@ mod common; fn inspect_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_command_for_a_while(base_path.path(), true); + common::run_command_for_a_while(base_path.path()); 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 42a5bc3ce143e..f50296cb0abac 100644 --- a/bin/node/cli/tests/purge_chain_works.rs +++ b/bin/node/cli/tests/purge_chain_works.rs @@ -25,7 +25,7 @@ mod common; fn purge_chain_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_command_for_a_while(base_path.path(), true); + common::run_command_for_a_while(base_path.path()); let status = Command::new(cargo_bin("substrate")) .args(&["purge-chain", "--dev", "-d"]) From a5ead659b840911d21df0b8c51e2cc391e2167e9 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 11:42:22 +0100 Subject: [PATCH 08/11] Add pruning mode to the revert command --- client/cli/src/commands/revert_cmd.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/client/cli/src/commands/revert_cmd.rs b/client/cli/src/commands/revert_cmd.rs index 9ab86986cdd07..a0ab023f3b79d 100644 --- a/client/cli/src/commands/revert_cmd.rs +++ b/client/cli/src/commands/revert_cmd.rs @@ -17,14 +17,13 @@ use std::fmt::Debug; use structopt::StructOpt; use sc_service::{ - Configuration, ChainSpecExtension, RuntimeGenesis, ServiceBuilderCommand, ChainSpec, + Configuration, ChainSpecExtension, RuntimeGenesis, ServiceBuilderCommand, ChainSpec, Roles, }; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use crate::error; use crate::VersionInfo; -use crate::params::BlockNumber; -use crate::params::SharedParams; +use crate::params::{BlockNumber, SharedParams, PruningParams}; /// The `revert` command used revert the chain to a previous state. #[derive(Debug, StructOpt, Clone)] @@ -36,6 +35,10 @@ pub struct RevertCmd { #[allow(missing_docs)] #[structopt(flatten)] pub shared_params: SharedParams, + + #[allow(missing_docs)] + #[structopt(flatten)] + pub pruning_params: PruningParams, } impl RevertCmd { @@ -72,6 +75,7 @@ impl RevertCmd { F: FnOnce(&str) -> Result>, String>, { self.shared_params.update_config(&mut config, spec_factory, version)?; + self.pruning_params.update_config(&mut config, Roles::FULL)?; config.use_in_memory_keystore()?; Ok(()) From 62e2ea7f183a741f78d635c8393b06490b73e7f2 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 11:49:49 +0100 Subject: [PATCH 09/11] Decrease killing grace period again --- bin/node/cli/tests/common.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/cli/tests/common.rs b/bin/node/cli/tests/common.rs index 02b5fbc70ee08..290f4963ebca2 100644 --- a/bin/node/cli/tests/common.rs +++ b/bin/node/cli/tests/common.rs @@ -62,5 +62,5 @@ pub fn run_command_for_a_while(base_path: &Path) { // Stop the process kill(Pid::from_raw(cmd.id().try_into().unwrap()), SIGINT).unwrap(); - assert!(wait_for(&mut cmd, 120).map(|x| x.success()).unwrap_or_default()); + assert!(wait_for(&mut cmd, 40).map(|x| x.success()).unwrap_or_default()); } From 2eaa86551b82ad94f2338c203816c94da14842ff Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 12:18:06 +0100 Subject: [PATCH 10/11] Move back unsafe_pruning to import_params --- client/cli/src/commands/export_blocks_cmd.rs | 2 +- client/cli/src/commands/revert_cmd.rs | 2 +- client/cli/src/params/import_params.rs | 10 +++++++++- client/cli/src/params/pruning_params.rs | 12 ++---------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/client/cli/src/commands/export_blocks_cmd.rs b/client/cli/src/commands/export_blocks_cmd.rs index 002ce6b8cf8a0..cdfa463c6d561 100644 --- a/client/cli/src/commands/export_blocks_cmd.rs +++ b/client/cli/src/commands/export_blocks_cmd.rs @@ -109,7 +109,7 @@ impl ExportBlocksCmd { F: FnOnce(&str) -> Result>, String>, { self.shared_params.update_config(&mut config, spec_factory, version)?; - self.pruning_params.update_config(&mut config, Roles::FULL)?; + self.pruning_params.update_config(&mut config, Roles::FULL, true)?; config.use_in_memory_keystore()?; Ok(()) diff --git a/client/cli/src/commands/revert_cmd.rs b/client/cli/src/commands/revert_cmd.rs index a0ab023f3b79d..f0c534898effd 100644 --- a/client/cli/src/commands/revert_cmd.rs +++ b/client/cli/src/commands/revert_cmd.rs @@ -75,7 +75,7 @@ impl RevertCmd { F: FnOnce(&str) -> Result>, String>, { self.shared_params.update_config(&mut config, spec_factory, version)?; - self.pruning_params.update_config(&mut config, Roles::FULL)?; + self.pruning_params.update_config(&mut config, Roles::FULL, true)?; config.use_in_memory_keystore()?; Ok(()) diff --git a/client/cli/src/params/import_params.rs b/client/cli/src/params/import_params.rs index d9843a13c216b..36c62bc979c5c 100644 --- a/client/cli/src/params/import_params.rs +++ b/client/cli/src/params/import_params.rs @@ -32,6 +32,14 @@ pub struct ImportParams { #[structopt(flatten)] pub pruning_params: PruningParams, + /// Force start with unsafe pruning settings. + /// + /// When running as a validator it is highly recommended to disable state + /// pruning (i.e. 'archive') which is the default. The node will refuse to + /// start as a validator if pruning is enabled unless this option is set. + #[structopt(long = "unsafe-pruning")] + pub unsafe_pruning: bool, + /// Method for executing Wasm runtime code. #[structopt( long = "wasm-execution", @@ -88,7 +96,7 @@ impl ImportParams { config.state_cache_size = self.state_cache_size; - self.pruning_params.update_config(&mut config, role)?; + self.pruning_params.update_config(&mut config, role, self.unsafe_pruning)?; config.wasm_method = self.wasm_method.into(); diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index aa70177d40f4c..ad64d757dcb61 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -29,15 +29,6 @@ pub struct PruningParams { /// 256 blocks. #[structopt(long = "pruning", value_name = "PRUNING_MODE")] pub pruning: Option, - - /// Force start with unsafe pruning settings. - /// - /// When running as a validator it is highly recommended to disable state - /// pruning (i.e. 'archive') which is the default. The node will refuse to - /// start as a validator if pruning is enabled unless this option is set. - #[structopt(long = "unsafe-pruning")] - pub unsafe_pruning: bool, - } impl PruningParams { @@ -46,6 +37,7 @@ impl PruningParams { &self, mut config: &mut Configuration, role: sc_service::Roles, + unsafe_pruning: bool, ) -> error::Result<()> where G: RuntimeGenesis, @@ -59,7 +51,7 @@ impl PruningParams { None if role == sc_service::Roles::AUTHORITY => PruningMode::ArchiveAll, None => PruningMode::default(), Some(s) => { - if role == sc_service::Roles::AUTHORITY && !self.unsafe_pruning { + if role == sc_service::Roles::AUTHORITY && !unsafe_pruning { return Err(error::Error::Input( "Validators should run with state pruning disabled (i.e. archive). \ You can ignore this check with `--unsafe-pruning`.".to_string() From 9bb423d507daa46f38d773e51d5f7292212d8876 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 25 Feb 2020 12:54:20 +0100 Subject: [PATCH 11/11] Applied proposed changes --- bin/node/cli/tests/check_block_works.rs | 2 +- bin/node/cli/tests/common.rs | 4 ++-- bin/node/cli/tests/import_export_and_revert_work.rs | 2 +- bin/node/cli/tests/inspect_works.rs | 2 +- bin/node/cli/tests/purge_chain_works.rs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bin/node/cli/tests/check_block_works.rs b/bin/node/cli/tests/check_block_works.rs index 6b1d7bdcebe6e..6bfb82a8bfafb 100644 --- a/bin/node/cli/tests/check_block_works.rs +++ b/bin/node/cli/tests/check_block_works.rs @@ -26,7 +26,7 @@ mod common; fn check_block_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_command_for_a_while(base_path.path()); + common::run_dev_node_for_a_while(base_path.path()); 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 290f4963ebca2..34e371195c16b 100644 --- a/bin/node/cli/tests/common.rs +++ b/bin/node/cli/tests/common.rs @@ -46,11 +46,11 @@ pub fn wait_for(child: &mut Child, secs: usize) -> Option { } /// Run the node for a while (30 seconds) -pub fn run_command_for_a_while(base_path: &Path) { +pub fn run_dev_node_for_a_while(base_path: &Path) { let mut cmd = Command::new(cargo_bin("substrate")); let mut cmd = cmd - .args(&["--dev", "--pruning", "archive"]) + .args(&["--dev"]) .arg("-d") .arg(base_path) .spawn() diff --git a/bin/node/cli/tests/import_export_and_revert_work.rs b/bin/node/cli/tests/import_export_and_revert_work.rs index c13e398153fee..131265e3b4ab9 100644 --- a/bin/node/cli/tests/import_export_and_revert_work.rs +++ b/bin/node/cli/tests/import_export_and_revert_work.rs @@ -27,7 +27,7 @@ fn import_export_and_revert_work() { let base_path = tempdir().expect("could not create a temp dir"); let exported_blocks = base_path.path().join("exported_blocks"); - common::run_command_for_a_while(base_path.path()); + common::run_dev_node_for_a_while(base_path.path()); let status = Command::new(cargo_bin("substrate")) .args(&["export-blocks", "--dev", "--pruning", "archive", "-d"]) diff --git a/bin/node/cli/tests/inspect_works.rs b/bin/node/cli/tests/inspect_works.rs index 86f5e7d2c541c..441b08ccf46da 100644 --- a/bin/node/cli/tests/inspect_works.rs +++ b/bin/node/cli/tests/inspect_works.rs @@ -26,7 +26,7 @@ mod common; fn inspect_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_command_for_a_while(base_path.path()); + common::run_dev_node_for_a_while(base_path.path()); 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 f50296cb0abac..020259d0c595a 100644 --- a/bin/node/cli/tests/purge_chain_works.rs +++ b/bin/node/cli/tests/purge_chain_works.rs @@ -25,7 +25,7 @@ mod common; fn purge_chain_works() { let base_path = tempdir().expect("could not create a temp dir"); - common::run_command_for_a_while(base_path.path()); + common::run_dev_node_for_a_while(base_path.path()); let status = Command::new(cargo_bin("substrate")) .args(&["purge-chain", "--dev", "-d"])