From 7ece64545c21be7513d0e8a753f4566ce69e5f76 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 1 Dec 2022 12:42:09 +0000 Subject: [PATCH 01/12] cli: Improve pruning documentation Signed-off-by: Alexandru Vasile --- client/cli/src/params/pruning_params.rs | 29 ++++++++++++++++--------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index 2da1de919771c..aa9edbd9c83b8 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -23,21 +23,30 @@ use sc_service::{BlocksPruning, PruningMode}; /// Parameters to define the pruning mode #[derive(Debug, Clone, PartialEq, Args)] pub struct PruningParams { - /// Specify the state pruning mode, a number of blocks to keep or 'archive'. + /// Specify the state pruning mode. /// - /// Default is to keep only the last 256 blocks, - /// otherwise, the state can be kept for all of the blocks (i.e 'archive'), - /// or for all of the canonical blocks (i.e 'archive-canonical'). + /// This mode specifies when the block's state (ie, storage) + /// should be pruned (ie, removed) from the database. + /// + /// Options available: + /// 'archive' Keep the state of all blocks. + /// 'archive-canonical' Keep only the state of finalized (canonical) blocks. + /// [number] Keep the state of the last number of finalized (canonical) blocks. + /// + /// The default option is to keep the last 256 blocks (number == 256). #[arg(alias = "pruning", long, value_name = "PRUNING_MODE")] pub state_pruning: Option, - /// Specify the blocks pruning mode, a number of blocks to keep or 'archive'. + /// Specify the blocks pruning mode. + /// + /// This mode specifies when the block's body (including justifications) + /// should be pruned (ie, removed) from the database. /// - /// Default is to keep all finalized blocks. - /// otherwise, all blocks can be kept (i.e 'archive'), - /// or for all canonical blocks (i.e 'archive-canonical'), - /// or for the last N blocks (i.e a number). + /// Options available: + /// 'archive' Keep all blocks. + /// 'archive-canonical' Keep only finalized (canonical) blocks. + /// [number] Keep the last number of finalized (canonical) blocks. /// - /// NOTE: only finalized blocks are subject for removal! + /// The default option is 'archive-canonical'. #[arg(alias = "keep-blocks", long, value_name = "COUNT")] pub blocks_pruning: Option, } From fdbc9a09fb8b4eb84be4667996e3b9f72a506187 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Thu, 1 Dec 2022 13:15:13 +0000 Subject: [PATCH 02/12] cli: Keep `finalized` notation and remove `canonical` one --- client/cli/src/params/pruning_params.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index aa9edbd9c83b8..a62f4c11e4585 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -30,8 +30,8 @@ pub struct PruningParams { /// /// Options available: /// 'archive' Keep the state of all blocks. - /// 'archive-canonical' Keep only the state of finalized (canonical) blocks. - /// [number] Keep the state of the last number of finalized (canonical) blocks. + /// 'archive-canonical' Keep only the state of finalized blocks. + /// [number] Keep the state of the last number of finalized blocks. /// /// The default option is to keep the last 256 blocks (number == 256). #[arg(alias = "pruning", long, value_name = "PRUNING_MODE")] @@ -43,8 +43,8 @@ pub struct PruningParams { /// /// Options available: /// 'archive' Keep all blocks. - /// 'archive-canonical' Keep only finalized (canonical) blocks. - /// [number] Keep the last number of finalized (canonical) blocks. + /// 'archive-canonical' Keep only finalized blocks. + /// [number] Keep the last number of finalized blocks. /// /// The default option is 'archive-canonical'. #[arg(alias = "keep-blocks", long, value_name = "COUNT")] From 804aff6d4ab7875f7cda94b9c9451dddcb8869c1 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Thu, 1 Dec 2022 13:48:35 +0000 Subject: [PATCH 03/12] cli: Fix cargo doc --- client/cli/src/params/pruning_params.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index a62f4c11e4585..a22ee2b2c398f 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -31,7 +31,7 @@ pub struct PruningParams { /// Options available: /// 'archive' Keep the state of all blocks. /// 'archive-canonical' Keep only the state of finalized blocks. - /// [number] Keep the state of the last number of finalized blocks. + /// number Keep the state of the last number of finalized blocks. /// /// The default option is to keep the last 256 blocks (number == 256). #[arg(alias = "pruning", long, value_name = "PRUNING_MODE")] @@ -44,7 +44,7 @@ pub struct PruningParams { /// Options available: /// 'archive' Keep all blocks. /// 'archive-canonical' Keep only finalized blocks. - /// [number] Keep the last number of finalized blocks. + /// number Keep the last number of finalized blocks. /// /// The default option is 'archive-canonical'. #[arg(alias = "keep-blocks", long, value_name = "COUNT")] From eb579bc538f0f6aba8a378b988a5a6bf5ea6ddfd Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 5 Dec 2022 13:29:32 +0000 Subject: [PATCH 04/12] cli: `PruningModeClap` IR enum Signed-off-by: Alexandru Vasile --- client/cli/src/params/pruning_params.rs | 48 ++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index a22ee2b2c398f..e167124c3cdae 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -17,7 +17,7 @@ // along with this program. If not, see . use crate::error; -use clap::Args; +use clap::{builder::PossibleValue, Args}; use sc_service::{BlocksPruning, PruningMode}; /// Parameters to define the pruning mode @@ -86,3 +86,49 @@ impl PruningParams { } } } + +/// Specifies the pruning mode of the database. +/// +/// This specifies when the block's data (either state via `--state-pruning` +/// or body via `--blocks-pruning`) should be pruned (ie, removed) from +/// the database. +#[derive(Clone)] +enum PruningModeClap { + /// Keep the data of all blocks. + Archive, + /// Keep only the data of finalized blocks. + ArchiveCanonical, + /// Keep the data of the last number of finalized blocks. + Custom(u32), +} + +impl clap::ValueEnum for PruningModeClap { + fn value_variants<'a>() -> &'a [Self] { + &[ + Self::Archive, + Self::ArchiveCanonical, + // NOTE: skip non-unit variants. + ] + } + + fn to_possible_value(&self) -> Option { + Some(match self { + Self::Archive => PossibleValue::new("archive").help("Keep the data of all blocks"), + Self::ArchiveCanonical => PossibleValue::new("archive-canonical") + .help("Keep only the data of finalized blocks"), + Self::Custom(_) => PossibleValue::new("a number") + .help("Keep the data of the last number of finalized blocks"), + }) + } + + fn from_str(input: &str, _ignore_case: bool) -> Result { + match input { + "archive" => Ok(Self::Archive), + "archive-canonical" => Ok(Self::ArchiveCanonical), + bc => bc + .parse() + .map_err(|_| "Invalid pruning mode specified".to_string()) + .map(Self::Custom), + } + } +} From 7eb7956609c24b441bbe0fa95a23d41738d4007f Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 5 Dec 2022 14:05:59 +0000 Subject: [PATCH 05/12] cli: Convert PruningModeClap into pruning modes Signed-off-by: Alexandru Vasile --- client/cli/src/params/pruning_params.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index e167124c3cdae..84ae75b3e5190 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -92,7 +92,7 @@ impl PruningParams { /// This specifies when the block's data (either state via `--state-pruning` /// or body via `--blocks-pruning`) should be pruned (ie, removed) from /// the database. -#[derive(Clone)] +#[derive(Debug, Clone, PartialEq)] enum PruningModeClap { /// Keep the data of all blocks. Archive, @@ -132,3 +132,23 @@ impl clap::ValueEnum for PruningModeClap { } } } + +impl Into for PruningModeClap { + fn into(self) -> PruningMode { + match self { + PruningModeClap::Archive => PruningMode::ArchiveAll, + PruningModeClap::ArchiveCanonical => PruningMode::ArchiveCanonical, + PruningModeClap::Custom(n) => PruningMode::blocks_pruning(n), + } + } +} + +impl Into for PruningModeClap { + fn into(self) -> BlocksPruning { + match self { + PruningModeClap::Archive => BlocksPruning::KeepAll, + PruningModeClap::ArchiveCanonical => BlocksPruning::KeepFinalized, + PruningModeClap::Custom(n) => BlocksPruning::Some(n), + } + } +} From 8da7e10fa05336320979f13d7703a28a50cd5f81 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 5 Dec 2022 14:12:11 +0000 Subject: [PATCH 06/12] cli: Use `PruningModeClap` Signed-off-by: Alexandru Vasile --- client/cli/src/params/pruning_params.rs | 54 ++++--------------------- 1 file changed, 8 insertions(+), 46 deletions(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index 84ae75b3e5190..cacc8f40e996e 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -27,63 +27,25 @@ pub struct PruningParams { /// /// This mode specifies when the block's state (ie, storage) /// should be pruned (ie, removed) from the database. - /// - /// Options available: - /// 'archive' Keep the state of all blocks. - /// 'archive-canonical' Keep only the state of finalized blocks. - /// number Keep the state of the last number of finalized blocks. - /// - /// The default option is to keep the last 256 blocks (number == 256). - #[arg(alias = "pruning", long, value_name = "PRUNING_MODE")] - pub state_pruning: Option, + #[arg(alias = "pruning", long, value_name = "PRUNING_MODE", default_value = "256")] + pub state_pruning: PruningModeClap, /// Specify the blocks pruning mode. /// /// This mode specifies when the block's body (including justifications) /// should be pruned (ie, removed) from the database. - /// - /// Options available: - /// 'archive' Keep all blocks. - /// 'archive-canonical' Keep only finalized blocks. - /// number Keep the last number of finalized blocks. - /// - /// The default option is 'archive-canonical'. - #[arg(alias = "keep-blocks", long, value_name = "COUNT")] - pub blocks_pruning: Option, + #[arg(alias = "keep-blocks", long, value_name = "COUNT", default_value = "archive-canonical")] + pub blocks_pruning: PruningModeClap, } impl PruningParams { /// Get the pruning value from the parameters pub fn state_pruning(&self) -> error::Result> { - self.state_pruning - .as_ref() - .map(|s| match s.as_str() { - "archive" => Ok(PruningMode::ArchiveAll), - "archive-canonical" => Ok(PruningMode::ArchiveCanonical), - bc => bc - .parse() - .map_err(|_| { - error::Error::Input("Invalid state pruning mode specified".to_string()) - }) - .map(PruningMode::blocks_pruning), - }) - .transpose() + Ok(Some(self.state_pruning.into())) } /// Get the block pruning value from the parameters pub fn blocks_pruning(&self) -> error::Result { - match self.blocks_pruning.as_ref() { - Some(bp) => match bp.as_str() { - "archive" => Ok(BlocksPruning::KeepAll), - "archive-canonical" => Ok(BlocksPruning::KeepFinalized), - bc => bc - .parse() - .map_err(|_| { - error::Error::Input("Invalid blocks pruning mode specified".to_string()) - }) - .map(BlocksPruning::Some), - }, - None => Ok(BlocksPruning::KeepFinalized), - } + Ok(self.blocks_pruning.into()) } } @@ -92,8 +54,8 @@ impl PruningParams { /// This specifies when the block's data (either state via `--state-pruning` /// or body via `--blocks-pruning`) should be pruned (ie, removed) from /// the database. -#[derive(Debug, Clone, PartialEq)] -enum PruningModeClap { +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum PruningModeClap { /// Keep the data of all blocks. Archive, /// Keep only the data of finalized blocks. From 9cf6f5ecb8e96eaac508ccd535fd28a06c29ba4b Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 5 Dec 2022 14:13:23 +0000 Subject: [PATCH 07/12] cli: Rename to `DatabasePruningMode` Signed-off-by: Alexandru Vasile --- client/cli/src/params/pruning_params.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index cacc8f40e996e..69c89ca9901ef 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -28,13 +28,13 @@ pub struct PruningParams { /// This mode specifies when the block's state (ie, storage) /// should be pruned (ie, removed) from the database. #[arg(alias = "pruning", long, value_name = "PRUNING_MODE", default_value = "256")] - pub state_pruning: PruningModeClap, + pub state_pruning: DatabasePruningMode, /// Specify the blocks pruning mode. /// /// This mode specifies when the block's body (including justifications) /// should be pruned (ie, removed) from the database. #[arg(alias = "keep-blocks", long, value_name = "COUNT", default_value = "archive-canonical")] - pub blocks_pruning: PruningModeClap, + pub blocks_pruning: DatabasePruningMode, } impl PruningParams { @@ -55,7 +55,7 @@ impl PruningParams { /// or body via `--blocks-pruning`) should be pruned (ie, removed) from /// the database. #[derive(Debug, Clone, Copy, PartialEq)] -pub enum PruningModeClap { +pub enum DatabasePruningMode { /// Keep the data of all blocks. Archive, /// Keep only the data of finalized blocks. @@ -64,7 +64,7 @@ pub enum PruningModeClap { Custom(u32), } -impl clap::ValueEnum for PruningModeClap { +impl clap::ValueEnum for DatabasePruningMode { fn value_variants<'a>() -> &'a [Self] { &[ Self::Archive, @@ -95,22 +95,22 @@ impl clap::ValueEnum for PruningModeClap { } } -impl Into for PruningModeClap { +impl Into for DatabasePruningMode { fn into(self) -> PruningMode { match self { - PruningModeClap::Archive => PruningMode::ArchiveAll, - PruningModeClap::ArchiveCanonical => PruningMode::ArchiveCanonical, - PruningModeClap::Custom(n) => PruningMode::blocks_pruning(n), + DatabasePruningMode::Archive => PruningMode::ArchiveAll, + DatabasePruningMode::ArchiveCanonical => PruningMode::ArchiveCanonical, + DatabasePruningMode::Custom(n) => PruningMode::blocks_pruning(n), } } } -impl Into for PruningModeClap { +impl Into for DatabasePruningMode { fn into(self) -> BlocksPruning { match self { - PruningModeClap::Archive => BlocksPruning::KeepAll, - PruningModeClap::ArchiveCanonical => BlocksPruning::KeepFinalized, - PruningModeClap::Custom(n) => BlocksPruning::Some(n), + DatabasePruningMode::Archive => BlocksPruning::KeepAll, + DatabasePruningMode::ArchiveCanonical => BlocksPruning::KeepFinalized, + DatabasePruningMode::Custom(n) => BlocksPruning::Some(n), } } } From 9379757e924b2b9f22525ce328c3e33184a40a8e Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 5 Dec 2022 14:48:32 +0000 Subject: [PATCH 08/12] cli: Implement `FromStr` instead of `clap::ValueEnum` Signed-off-by: Alexandru Vasile --- client/cli/src/params/pruning_params.rs | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index 69c89ca9901ef..91868e163e57e 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -17,7 +17,7 @@ // along with this program. If not, see . use crate::error; -use clap::{builder::PossibleValue, Args}; +use clap::Args; use sc_service::{BlocksPruning, PruningMode}; /// Parameters to define the pruning mode @@ -64,26 +64,10 @@ pub enum DatabasePruningMode { Custom(u32), } -impl clap::ValueEnum for DatabasePruningMode { - fn value_variants<'a>() -> &'a [Self] { - &[ - Self::Archive, - Self::ArchiveCanonical, - // NOTE: skip non-unit variants. - ] - } - - fn to_possible_value(&self) -> Option { - Some(match self { - Self::Archive => PossibleValue::new("archive").help("Keep the data of all blocks"), - Self::ArchiveCanonical => PossibleValue::new("archive-canonical") - .help("Keep only the data of finalized blocks"), - Self::Custom(_) => PossibleValue::new("a number") - .help("Keep the data of the last number of finalized blocks"), - }) - } +impl std::str::FromStr for DatabasePruningMode { + type Err = String; - fn from_str(input: &str, _ignore_case: bool) -> Result { + fn from_str(input: &str) -> Result { match input { "archive" => Ok(Self::Archive), "archive-canonical" => Ok(Self::ArchiveCanonical), From 535662996eb3dd8d7ca6efa653afa0a3ee57a0f7 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Thu, 8 Dec 2022 17:31:54 +0200 Subject: [PATCH 09/12] Update client/cli/src/params/pruning_params.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- client/cli/src/params/pruning_params.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index 91868e163e57e..f1a6105d93d37 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -33,7 +33,7 @@ pub struct PruningParams { /// /// This mode specifies when the block's body (including justifications) /// should be pruned (ie, removed) from the database. - #[arg(alias = "keep-blocks", long, value_name = "COUNT", default_value = "archive-canonical")] + #[arg(alias = "keep-blocks", long, value_name = "PRUNING_MODE", default_value = "archive-canonical")] pub blocks_pruning: DatabasePruningMode, } From f25b4916b460d65ddf3e65ddcf2229fe67abb1c0 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 8 Dec 2022 16:34:44 +0000 Subject: [PATCH 10/12] Fix clippy Signed-off-by: Alexandru Vasile --- client/cli/src/params/pruning_params.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index f1a6105d93d37..9df7f681ccd14 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -33,7 +33,12 @@ pub struct PruningParams { /// /// This mode specifies when the block's body (including justifications) /// should be pruned (ie, removed) from the database. - #[arg(alias = "keep-blocks", long, value_name = "PRUNING_MODE", default_value = "archive-canonical")] + #[arg( + alias = "keep-blocks", + long, + value_name = "PRUNING_MODE", + default_value = "archive-canonical" + )] pub blocks_pruning: DatabasePruningMode, } From e3ffe3e4507ddaf89f8e4097f61a33dd8ef14dd9 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Thu, 8 Dec 2022 18:44:09 +0000 Subject: [PATCH 11/12] cli: Add option documentation back Signed-off-by: Alexandru Vasile --- client/cli/src/params/pruning_params.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index 9df7f681ccd14..3a3e5432da5f9 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -27,12 +27,26 @@ pub struct PruningParams { /// /// This mode specifies when the block's state (ie, storage) /// should be pruned (ie, removed) from the database. + /// + /// Possible values: + /// 'archive' Keep the state of all blocks. + /// 'archive-canonical' Keep only the state of finalized blocks. + /// number Keep the state of the last number of finalized blocks. + /// + /// The default option is to keep the last 256 blocks (number == 256). #[arg(alias = "pruning", long, value_name = "PRUNING_MODE", default_value = "256")] pub state_pruning: DatabasePruningMode, /// Specify the blocks pruning mode. /// /// This mode specifies when the block's body (including justifications) /// should be pruned (ie, removed) from the database. + /// + /// Possible values: + /// 'archive' Keep all blocks. + /// 'archive-canonical' Keep only finalized blocks. + /// number Keep the last number of finalized blocks. + /// + /// The default option is 'archive-canonical'. #[arg( alias = "keep-blocks", long, From aee88445f09e2aff3b6ed7cdd2dd12daafcd2519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 8 Dec 2022 20:46:36 +0100 Subject: [PATCH 12/12] Apply suggestions from code review --- client/cli/src/params/pruning_params.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/client/cli/src/params/pruning_params.rs b/client/cli/src/params/pruning_params.rs index 3a3e5432da5f9..7e50f53d7169a 100644 --- a/client/cli/src/params/pruning_params.rs +++ b/client/cli/src/params/pruning_params.rs @@ -32,8 +32,6 @@ pub struct PruningParams { /// 'archive' Keep the state of all blocks. /// 'archive-canonical' Keep only the state of finalized blocks. /// number Keep the state of the last number of finalized blocks. - /// - /// The default option is to keep the last 256 blocks (number == 256). #[arg(alias = "pruning", long, value_name = "PRUNING_MODE", default_value = "256")] pub state_pruning: DatabasePruningMode, /// Specify the blocks pruning mode. @@ -44,9 +42,7 @@ pub struct PruningParams { /// Possible values: /// 'archive' Keep all blocks. /// 'archive-canonical' Keep only finalized blocks. - /// number Keep the last number of finalized blocks. - /// - /// The default option is 'archive-canonical'. + /// number Keep the last `number` of finalized blocks. #[arg( alias = "keep-blocks", long,