Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Remove the --unsafe-pruning CLI-argument (step 1) #10995

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0632f6b
sc-client-db: utils::open_database(...) — return OpenDbError so that …
Apr 29, 2022
154f04c
sc-client-db: utils::open_database(..) — accept the `create: bool` ar…
Apr 29, 2022
da678a8
sc-client-db: pruning — optional argument in the DatabaseSettings
Apr 29, 2022
ba3ce12
sc-state-db: Split `Error<E>` into separate `Error<E>` and `StateDbEr…
May 2, 2022
1294407
StateDb::open: choose the pruning-mode depending on the requested and…
May 2, 2022
6af69dc
sc-state-db: test for different combinations of stored and requested …
May 2, 2022
c0f34ed
CLI-argument: mark the unsafe-pruning as deprecated
May 2, 2022
1652a17
Fix tests
May 2, 2022
15d93a3
tests: do not specify --pruning when running the substrate over the e…
May 2, 2022
351fe04
fix types for benches
May 2, 2022
eb8cbcc
cargo fmt
May 3, 2022
d8a455a
Check whether the pruning-mode and sync-mode are compatible
May 4, 2022
f60188c
Merge remote-tracking branch 'origin/master' into rgafiyatullin-subst…
May 4, 2022
5eae200
cargo fmt
May 4, 2022
443c10f
Merge remote-tracking branch 'origin/master' into rgafiyatullin-subst…
May 4, 2022
3dfe1b9
parity-db: 0.3.11 -> 0.3.12
May 5, 2022
44c5792
sc-state-db: MetaDb::set_meta — a better doc-test
May 5, 2022
847d97f
cargo fmt
May 5, 2022
82320a2
make MetaDb read-only again!
May 5, 2022
e90167e
Merge pull request #1 from RGafiyatullin/rgafiyatullin-substrate-8103…
RGafiyatullin May 5, 2022
df30449
Remove the stray newline (and run the CI once again please)
May 5, 2022
fd1623d
Last nitpicks
May 5, 2022
b6fe998
A more comprehensive error message
May 5, 2022
8e0bc33
Merge remote-tracking branch 'origin/master' into rgafiyatullin-subst…
May 5, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bin/node/cli/benches/block_production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase {
database: DatabaseSource::RocksDb { path: root.join("db"), cache_size: 128 },
state_cache_size: 67108864,
state_cache_child_ratio: None,
state_pruning: PruningMode::ArchiveAll,
state_pruning: Some(PruningMode::ArchiveAll),
keep_blocks: KeepBlocks::All,
chain_spec: spec,
wasm_method: WasmExecutionMethod::Compiled,
Expand Down
2 changes: 1 addition & 1 deletion bin/node/cli/benches/transaction_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase {
database: DatabaseSource::RocksDb { path: root.join("db"), cache_size: 128 },
state_cache_size: 67108864,
state_cache_child_ratio: None,
state_pruning: PruningMode::ArchiveAll,
state_pruning: Some(PruningMode::ArchiveAll),
keep_blocks: KeepBlocks::All,
chain_spec: spec,
wasm_method: WasmExecutionMethod::Interpreted,
Expand Down
1 change: 0 additions & 1 deletion bin/node/cli/tests/benchmark_block_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ async fn benchmark_block_works() {
.args(["benchmark", "block", "--dev"])
.arg("-d")
.arg(base_dir.path())
.args(["--pruning", "archive"])
.args(["--from", "1", "--to", "1"])
.args(["--repeat", "1"])
.args(["--execution", "wasm", "--wasm-execution", "compiled"])
Expand Down
2 changes: 1 addition & 1 deletion bin/node/cli/tests/check_block_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ async fn check_block_works() {
common::run_node_for_a_while(base_path.path(), &["--dev", "--no-hardware-benchmarks"]).await;

let status = Command::new(cargo_bin("substrate"))
.args(&["check-block", "--dev", "--pruning", "archive", "-d"])
.args(&["check-block", "--dev", "-d"])
.arg(base_path.path())
.arg("1")
.status()
Expand Down
6 changes: 3 additions & 3 deletions bin/node/cli/tests/export_import_flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ impl<'a> ExportImportRevertExecutor<'a> {
// Adding "--binary" if need be.
let arguments: Vec<&str> = match format_opt {
FormatOpt::Binary => {
vec![&sub_command_str, "--dev", "--pruning", "archive", "--binary", "-d"]
vec![&sub_command_str, "--dev", "--binary", "-d"]
},
FormatOpt::Json => vec![&sub_command_str, "--dev", "--pruning", "archive", "-d"],
FormatOpt::Json => vec![&sub_command_str, "--dev", "-d"],
};

let tmp: TempDir;
Expand Down Expand Up @@ -161,7 +161,7 @@ impl<'a> ExportImportRevertExecutor<'a> {
/// Runs the `revert` command.
fn run_revert(&self) {
let output = Command::new(cargo_bin("substrate"))
.args(&["revert", "--dev", "--pruning", "archive", "-d"])
.args(&["revert", "--dev", "-d"])
.arg(&self.base_path.path())
.output()
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion bin/node/cli/tests/inspect_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ async fn inspect_works() {
common::run_node_for_a_while(base_path.path(), &["--dev", "--no-hardware-benchmarks"]).await;

let status = Command::new(cargo_bin("substrate"))
.args(&["inspect", "--dev", "--pruning", "archive", "-d"])
.args(&["inspect", "--dev", "-d"])
.arg(base_path.path())
.args(&["block", "1"])
.status()
Expand Down
2 changes: 1 addition & 1 deletion bin/node/testing/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ impl BenchDb {
let db_config = sc_client_db::DatabaseSettings {
state_cache_size: 16 * 1024 * 1024,
state_cache_child_ratio: Some((0, 100)),
state_pruning: PruningMode::ArchiveAll,
state_pruning: Some(PruningMode::ArchiveAll),
source: database_type.into_settings(dir.into()),
keep_blocks: sc_client_db::KeepBlocks::All,
};
Expand Down
3 changes: 3 additions & 0 deletions client/api/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,9 @@ pub trait Backend<Block: BlockT>: AuxStore + Send + Sync {
/// something that the import of a block would interfere with, e.g. importing
/// a new block or calculating the best head.
fn get_import_lock(&self) -> &RwLock<()>;

/// Tells whether the backend requires full-sync mode.
fn requires_full_sync(&self) -> bool;
}

/// Mark for all Backend implementations, that are making use of state data, stored locally.
Expand Down
3 changes: 3 additions & 0 deletions client/api/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ pub trait BlockBackend<Block: BlockT> {
fn has_indexed_transaction(&self, hash: &Block::Hash) -> sp_blockchain::Result<bool> {
Ok(self.indexed_transaction(hash)?.is_some())
}

/// Tells whether the current client configuration requires full-sync mode.
fn requires_full_sync(&self) -> bool;
}

/// Provide a list of potential uncle headers for a given block.
Expand Down
4 changes: 4 additions & 0 deletions client/api/src/in_mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,10 @@ where
fn get_import_lock(&self) -> &RwLock<()> {
&self.import_lock
}

fn requires_full_sync(&self) -> bool {
false
}
}

impl<Block: BlockT> backend::LocalBackend<Block> for Backend<Block> where Block::Hash: Ord {}
Expand Down
19 changes: 14 additions & 5 deletions client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
///
/// By default this is retrieved from `PruningMode` if it is available. Otherwise its
/// `PruningMode::default()`.
fn state_pruning(&self, unsafe_pruning: bool, role: &Role) -> Result<PruningMode> {
fn state_pruning(&self) -> Result<Option<PruningMode>> {
self.pruning_params()
.map(|x| x.state_pruning(unsafe_pruning, role))
.map(|x| x.state_pruning())
.unwrap_or_else(|| Ok(Default::default()))
}

Expand Down Expand Up @@ -494,8 +494,6 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
let telemetry_endpoints = self.telemetry_endpoints(&chain_spec)?;
let runtime_cache_size = self.runtime_cache_size()?;

let unsafe_pruning = self.import_params().map(|p| p.unsafe_pruning).unwrap_or(false);

Ok(Configuration {
impl_name: C::impl_name(),
impl_version: C::impl_version(),
Expand All @@ -516,7 +514,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
database: self.database_config(&config_dir, database_cache_size, database, &role)?,
state_cache_size: self.state_cache_size()?,
state_cache_child_ratio: self.state_cache_child_ratio()?,
state_pruning: self.state_pruning(unsafe_pruning, &role)?,
state_pruning: self.state_pruning()?,
keep_blocks: self.keep_blocks()?,
wasm_method: self.wasm_method()?,
wasm_runtime_overrides: self.wasm_runtime_overrides(),
Expand Down Expand Up @@ -643,6 +641,17 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
}
}

if self.import_params().map_or(false, |p| {
#[allow(deprecated)]
p.unsafe_pruning
}) {
// according to https://github.com/substrate/issues/8103;
warn!(
"WARNING: \"--unsafe-pruning\" CLI-flag is deprecated and has no effect. \
In future builds it will be removed, and providing this flag will lead to an error."
);
}

Ok(())
}
}
Expand Down
12 changes: 8 additions & 4 deletions client/cli/src/params/import_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,16 @@ pub struct ImportParams {
#[clap(flatten)]
pub database_params: DatabaseParams,

/// Force start with unsafe pruning settings.
/// THIS IS A DEPRECATED CLI-ARGUMENT.
///
/// 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.
/// It has been preserved in order to not break the compatibility with the existing scripts.
/// Enabling this option will lead to a runtime warning.
/// In future this option will be removed completely, thus specifying it will lead to a start
/// up error.
///
/// Details: <https://github.com/paritytech/substrate/issues/8103>
#[clap(long)]
#[deprecated = "According to https://github.com/paritytech/substrate/issues/8103"]
pub unsafe_pruning: bool,

/// Method for executing Wasm runtime code.
Expand Down
36 changes: 12 additions & 24 deletions client/cli/src/params/pruning_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

use crate::error;
use clap::Args;
use sc_service::{KeepBlocks, PruningMode, Role};
use sc_service::{KeepBlocks, PruningMode};

/// Parameters to define the pruning mode
#[derive(Debug, Clone, PartialEq, Args)]
Expand All @@ -39,29 +39,17 @@ pub struct PruningParams {

impl PruningParams {
/// Get the pruning value from the parameters
pub fn state_pruning(&self, unsafe_pruning: bool, role: &Role) -> error::Result<PruningMode> {
// 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.
Ok(match &self.pruning {
Some(ref s) if s == "archive" => PruningMode::ArchiveAll,
None if role.is_authority() => PruningMode::ArchiveAll,
None => PruningMode::default(),
Some(s) => {
if role.is_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(),
))
}

PruningMode::keep_blocks(s.parse().map_err(|_| {
error::Error::Input("Invalid pruning mode specified".to_string())
})?)
},
})
pub fn state_pruning(&self) -> error::Result<Option<PruningMode>> {
self.pruning
.as_ref()
.map(|s| match s.as_str() {
"archive" => Ok(PruningMode::ArchiveAll),
bc => bc
.parse()
.map_err(|_| error::Error::Input("Invalid pruning mode specified".to_string()))
.map(PruningMode::keep_blocks),
})
.transpose()
}

/// Get the block pruning value from the parameters
Expand Down
Loading