Fix purge-chain and print DB info on startup (#5840)

* purge-chain accepts --db option

* print DB info on startup

* Small refactoring

* Added back &self

* Add DatabaseParams for PurgeChain, ImportParams and ExportBlocks

* Don't force default value

* Remove unused fields

* Update client/cli/src/commands/export_blocks_cmd.rs

* Fix stuff

Co-authored-by: Cecile Tonglet <cecile@parity.io>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

substrate/client/cli/src/commands/export_blocks_cmd.rs

@@ -15,7 +15,7 @@
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
 use crate::error;
-use crate::params::{BlockNumber, PruningParams, SharedParams};
+use crate::params::{BlockNumber, DatabaseParams, PruningParams, SharedParams};
 use crate::CliConfiguration;
 use log::info;
 use sc_service::{
@@ -48,7 +48,7 @@ pub struct ExportBlocksCmd {
 	pub to: Option<BlockNumber>,
 
 	/// Use binary output rather than JSON.
-	#[structopt(long = "binary", value_name = "BOOL", parse(try_from_str), default_value("false"))]
+	#[structopt(long)]
 	pub binary: bool,
 
 	#[allow(missing_docs)]
@@ -58,6 +58,10 @@ pub struct ExportBlocksCmd {
 	#[allow(missing_docs)]
 	#[structopt(flatten)]
 	pub pruning_params: PruningParams,
+
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub database_params: DatabaseParams,
 }
 
 impl ExportBlocksCmd {
@@ -103,4 +107,8 @@ impl CliConfiguration for ExportBlocksCmd {
 	fn pruning_params(&self) -> Option<&PruningParams> {
 		Some(&self.pruning_params)
 	}
+
+	fn database_params(&self) -> Option<&DatabaseParams> {
+		Some(&self.database_params)
+	}
 }

substrate/client/cli/src/commands/mod.rs

@@ -150,6 +150,12 @@ macro_rules! substrate_cli_subcommands {
 				}
 			}
 
+			fn database_params(&self) -> Option<&$crate::DatabaseParams> {
+				match self {
+					$($enum::$variant(cmd) => cmd.database_params()),*
+				}
+			}
+
 			fn base_path(&self) -> $crate::Result<::std::option::Option<::std::path::PathBuf>> {
 				match self {
 					$($enum::$variant(cmd) => cmd.base_path()),*

substrate/client/cli/src/commands/purge_chain_cmd.rs

@@ -15,9 +15,9 @@
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
 use crate::error;
-use crate::params::SharedParams;
+use crate::params::{DatabaseParams, SharedParams};
 use crate::CliConfiguration;
-use sc_service::{config::DatabaseConfig, Configuration};
+use sc_service::Configuration;
 use std::fmt::Debug;
 use std::fs;
 use std::io::{self, Write};
@@ -33,18 +33,19 @@ pub struct PurgeChainCmd {
 	#[allow(missing_docs)]
 	#[structopt(flatten)]
 	pub shared_params: SharedParams,
+
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub database_params: DatabaseParams,
 }
 
 impl PurgeChainCmd {
 	/// Run the purge command
 	pub fn run(&self, config: Configuration) -> error::Result<()> {
-		let db_path = match &config.database {
-			DatabaseConfig::RocksDb { path, .. } => path,
-			_ => {
-				eprintln!("Cannot purge custom database implementation");
-				return Ok(());
-			}
-		};
+		let db_path = config.database.path()
+			.ok_or_else(||
+				error::Error::Input("Cannot purge custom database implementation".into())
+		)?;
 
 		if !self.yes {
 			print!("Are you sure to remove {:?}? [y/N]: ", &db_path);
@@ -81,4 +82,8 @@ impl CliConfiguration for PurgeChainCmd {
 	fn shared_params(&self) -> &SharedParams {
 		&self.shared_params
 	}
+
+	fn database_params(&self) -> Option<&DatabaseParams> {
+		Some(&self.database_params)
+	}
 }

substrate/client/cli/src/config.rs

@@ -16,20 +16,20 @@
 
 //! Configuration trait for a CLI based on substrate
 
+use crate::arg_enums::Database;
 use crate::error::Result;
 use crate::{
-	init_logger, ImportParams, KeystoreParams, NetworkParams, NodeKeyParams,
+	init_logger, DatabaseParams, ImportParams, KeystoreParams, NetworkParams, NodeKeyParams,
 	OffchainWorkerParams, PruningParams, SharedParams, SubstrateCli,
 };
-use crate::arg_enums::Database;
 use app_dirs::{AppDataType, AppInfo};
 use names::{Generator, Name};
-use sc_service::config::{
-	WasmExecutionMethod, Role, OffchainWorkerConfig,
-	Configuration, DatabaseConfig, ExtTransport, KeystoreConfig, NetworkConfiguration,
-	NodeKeyConfig, PrometheusConfig, PruningMode, TelemetryEndpoints, TransactionPoolOptions, TaskType
-};
 use sc_client_api::execution_extensions::ExecutionStrategies;
+use sc_service::config::{
+	Configuration, DatabaseConfig, ExtTransport, KeystoreConfig, NetworkConfiguration,
+	NodeKeyConfig, OffchainWorkerConfig, PrometheusConfig, PruningMode, Role, TaskType,
+	TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod,
+};
 use sc_service::{ChainSpec, TracingReceiver};
 use std::future::Future;
 use std::net::SocketAddr;
@@ -75,8 +75,12 @@ pub trait CliConfiguration: Sized {
 
 	/// Get the NodeKeyParams for this object
 	fn node_key_params(&self) -> Option<&NodeKeyParams> {
-		self.network_params()
-			.map(|x| &x.node_key_params)
+		self.network_params().map(|x| &x.node_key_params)
+	}
+
+	/// Get the DatabaseParams for this object
+	fn database_params(&self) -> Option<&DatabaseParams> {
+		self.import_params().map(|x| &x.database_params)
 	}
 
 	/// Get the base path of the configuration (if any)
@@ -152,33 +156,39 @@ pub trait CliConfiguration: Sized {
 
 	/// Get the database cache size.
 	///
-	/// By default this is retrieved from `ImportParams` if it is available. Otherwise its `None`.
+	/// By default this is retrieved from `DatabaseParams` if it is available. Otherwise its `None`.
 	fn database_cache_size(&self) -> Result<Option<usize>> {
-		Ok(self.import_params()
+		Ok(self.database_params()
 			.map(|x| x.database_cache_size())
 			.unwrap_or(Default::default()))
 	}
 
 	/// Get the database backend variant.
 	///
-	/// By default this is retrieved from `ImportParams` if it is available. Otherwise its `None`.
+	/// By default this is retrieved from `DatabaseParams` if it is available. Otherwise its `None`.
 	fn database(&self) -> Result<Option<Database>> {
-		Ok(self.import_params().map(|x| x.database()))
+		Ok(self.database_params().and_then(|x| x.database()))
 	}
 
-	/// Get the database configuration.
-	///
-	/// By default this is retrieved from `SharedParams`
-	fn database_config(&self,
+	/// Get the database configuration object for the parameters provided
+	fn database_config(
+		&self,
 		base_path: &PathBuf,
 		cache_size: usize,
 		database: Database,
 	) -> Result<DatabaseConfig> {
-		Ok(self.shared_params().database_config(
-			base_path,
-			cache_size,
-			database,
-		))
+		Ok(match database {
+			Database::RocksDb => DatabaseConfig::RocksDb {
+				path: base_path.join("db"),
+				cache_size,
+			},
+			Database::SubDb => DatabaseConfig::SubDb {
+				path: base_path.join("subdb"),
+			},
+			Database::ParityDb => DatabaseConfig::ParityDb {
+				path: base_path.join("paritydb"),
+			},
+		})
 	}
 
 	/// Get the state cache size.
@@ -313,7 +323,7 @@ pub trait CliConfiguration: Sized {
 	fn offchain_worker(&self, role: &Role) -> Result<OffchainWorkerConfig> {
 		self.offchain_worker_params()
 			.map(|x| x.offchain_worker(role))
-			.unwrap_or_else(|| { Ok(OffchainWorkerConfig::default()) })
+			.unwrap_or_else(|| Ok(OffchainWorkerConfig::default()))
 	}
 
 	/// Returns `Ok(true)` if authoring should be forced

substrate/client/cli/src/params/database_params.rs

@@ -0,0 +1,47 @@
+// Copyright 2018-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 <http://www.gnu.org/licenses/>.
+
+use crate::arg_enums::Database;
+use structopt::StructOpt;
+
+/// Parameters for block import.
+#[derive(Debug, StructOpt, Clone)]
+pub struct DatabaseParams {
+	/// Select database backend to use.
+	#[structopt(
+		long,
+		alias = "db",
+		value_name = "DB",
+		case_insensitive = true,
+	)]
+	pub database: Option<Database>,
+
+	/// Limit the memory the database cache can use.
+	#[structopt(long = "db-cache", value_name = "MiB")]
+	pub database_cache_size: Option<usize>,
+}
+
+impl DatabaseParams {
+	/// Limit the memory the database cache can use.
+	pub fn database(&self) -> Option<Database> {
+		self.database
+	}
+
+	/// Limit the memory the database cache can use.
+	pub fn database_cache_size(&self) -> Option<usize> {
+		self.database_cache_size
+	}
+}

substrate/client/cli/src/params/import_params.rs

@@ -15,14 +15,13 @@
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
 use crate::arg_enums::{
-	ExecutionStrategy, TracingReceiver, WasmExecutionMethod, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION,
-	DEFAULT_EXECUTION_IMPORT_BLOCK, DEFAULT_EXECUTION_OFFCHAIN_WORKER, DEFAULT_EXECUTION_OTHER,
-	DEFAULT_EXECUTION_SYNCING, Database,
+	ExecutionStrategy, TracingReceiver, WasmExecutionMethod,
+	DEFAULT_EXECUTION_BLOCK_CONSTRUCTION, DEFAULT_EXECUTION_IMPORT_BLOCK,
+	DEFAULT_EXECUTION_OFFCHAIN_WORKER, DEFAULT_EXECUTION_OTHER, DEFAULT_EXECUTION_SYNCING,
 };
+use crate::params::DatabaseParams;
 use crate::params::PruningParams;
-use crate::Result;
 use sc_client_api::execution_extensions::ExecutionStrategies;
-use sc_service::{PruningMode, Role};
 use structopt::StructOpt;
 
 /// Parameters for block import.
@@ -32,6 +31,10 @@ pub struct ImportParams {
 	#[structopt(flatten)]
 	pub pruning_params: PruningParams,
 
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub database_params: DatabaseParams,
+
 	/// Force start with unsafe pruning settings.
 	///
 	/// When running as a validator it is highly recommended to disable state
@@ -54,22 +57,12 @@ pub struct ImportParams {
 	#[structopt(flatten)]
 	pub execution_strategies: ExecutionStrategiesParams,
 
-	/// Select database backend to use.
-	#[structopt(
-		long = "database",
-		alias = "db",
-		value_name = "DB",
-		case_insensitive = true,
-		default_value = "RocksDb"
-	)]
-	pub database: Database,
-
-	/// Limit the memory the database cache can use.
-	#[structopt(long = "db-cache", value_name = "MiB")]
-	pub database_cache_size: Option<usize>,
-
 	/// Specify the state cache size.
-	#[structopt(long = "state-cache-size", value_name = "Bytes", default_value = "67108864")]
+	#[structopt(
+		long = "state-cache-size",
+		value_name = "Bytes",
+		default_value = "67108864"
+	)]
 	pub state_cache_size: usize,
 
 	/// Comma separated list of targets for tracing.
@@ -132,21 +125,6 @@ impl ImportParams {
 			other: exec_all_or(exec.execution_other, DEFAULT_EXECUTION_OTHER),
 		}
 	}
-
-	/// Get the pruning mode from the parameters
-	pub fn pruning(&self, unsafe_pruning: bool, role: &Role) -> Result<PruningMode> {
-		self.pruning_params.pruning(unsafe_pruning, role)
-	}
-
-	/// Limit the memory the database cache can use.
-	pub fn database_cache_size(&self) -> Option<usize> {
-		self.database_cache_size
-	}
-
-	/// Limit the memory the database cache can use.
-	pub fn database(&self) -> Database {
-		self.database
-	}
 }
 
 /// Execution strategies parameters.

substrate/client/cli/src/params/mod.rs

@@ -14,18 +14,20 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
+mod database_params;
 mod import_params;
 mod keystore_params;
 mod network_params;
 mod node_key_params;
+mod offchain_worker_params;
 mod pruning_params;
 mod shared_params;
 mod transaction_pool_params;
-mod offchain_worker_params;
 
 use std::fmt::Debug;
 use std::str::FromStr;
 
+pub use crate::params::database_params::*;
 pub use crate::params::import_params::*;
 pub use crate::params::keystore_params::*;
 pub use crate::params::network_params::*;

substrate/client/cli/src/params/shared_params.rs

@@ -14,10 +14,8 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
-use sc_service::config::DatabaseConfig;
 use std::path::PathBuf;
 use structopt::StructOpt;
-use crate::arg_enums::Database;
 
 /// Shared parameters used by all `CoreParams`.
 #[derive(Debug, StructOpt, Clone)]
@@ -72,27 +70,6 @@ impl SharedParams {
 		}
 	}
 
-	/// Get the database configuration object for the parameters provided
-	pub fn database_config(
-		&self,
-		base_path: &PathBuf,
-		cache_size: usize,
-		database: Database,
-	) -> DatabaseConfig {
-		match database {
-			Database::RocksDb => DatabaseConfig::RocksDb {
-				path: base_path.join("db"),
-				cache_size,
-			},
-			Database::SubDb => DatabaseConfig::SubDb {
-				path: base_path.join("subdb"),
-			},
-			Database::ParityDb => DatabaseConfig::ParityDb {
-				path: base_path.join("paritydb"),
-			},
-		}
-	}
-
 	/// Get the filters for the logging
 	pub fn log_filters(&self) -> Option<String> {
 		self.log.clone()

substrate/client/cli/src/runner.rs

@@ -164,6 +164,10 @@ impl<C: SubstrateCli> Runner<C> {
 		info!("📋 Chain specification: {}", self.config.chain_spec.name());
 		info!("🏷  Node name: {}", self.config.network.node_name);
 		info!("👤 Role: {}", self.config.display_role());
+		info!("💾 Database: {} at {}",
+			self.config.database,
+			self.config.database.path().map_or_else(|| "<unknown>".to_owned(), |p| p.display().to_string())
+		);
 		info!("⛓  Native runtime: {}", runtime_version);
 
 		match self.config.role {