squash: joshy no rocks db (#2)

* Remove `testnets-common` (paritytech#2620)

`testnets-common` was introduced recently to start to separate testnet
parachain configurations from those of Polkadot and Kusama.

The `locks-review` and `polkadot-review` requirements are removed from
`parachains-common` in
paritytech#2564 and there are
[plans](paritytech#2564 (comment))
to move the Polkadot and Kusama contents of that package to the
fellowship, `testnets-common` is no longer needed.

This PR removes the crate and replaces uses of it in
`collectives-westend`, the only place it is currently used.

* cumulus-client-cli

* substrate-test-utils

* checkpoint

* remove outdated comment

* fmt

* polkadot-cli

* try-runtime

* bunch of cumulus stuff

* cumulus-test-client

* Mke cumulus test service use paritydb

* josh: disable db feature

---------

Co-authored-by: Dónal Murray <donal.murray@parity.io>
Co-authored-by: Joshy Orndorff <admin@joshyorndorff.com>

cumulus/client/cli/Cargo.toml

@@ -12,9 +12,9 @@ codec = { package = "parity-scale-codec", version = "3.0.0" }
 url = "2.4.0"
 
 # Substrate
-sc-cli = { path = "../../../substrate/client/cli" }
+sc-cli = { default-features = false, path = "../../../substrate/client/cli" }
 sc-client-api = { path = "../../../substrate/client/api" }
 sc-chain-spec = { path = "../../../substrate/client/chain-spec" }
-sc-service = { path = "../../../substrate/client/service" }
+sc-service = { default-features = false, path = "../../../substrate/client/service" }
 sp-core = { path = "../../../substrate/primitives/core" }
 sp-runtime = { path = "../../../substrate/primitives/runtime" }

cumulus/client/relay-chain-inprocess-interface/Cargo.toml

@@ -12,7 +12,7 @@ futures = "0.3.28"
 futures-timer = "3.0.2"
 
 # Substrate
-sc-cli = { path = "../../../substrate/client/cli" }
+sc-cli = { path = "../../../substrate/client/cli", default-features = false }
 sc-client-api = { path = "../../../substrate/client/api" }
 sc-telemetry = { path = "../../../substrate/client/telemetry" }
 sc-tracing = { path = "../../../substrate/client/tracing" }
@@ -25,7 +25,7 @@ sp-state-machine = { path = "../../../substrate/primitives/state-machine" }
 
 # Polkadot
 polkadot-cli = { path = "../../../polkadot/cli", default-features = false, features = ["cli"] }
-polkadot-service = { path = "../../../polkadot/node/service" }
+polkadot-service = { path = "../../../polkadot/node/service", default-features = false }
 
 # Cumulus
 cumulus-primitives-core = { path = "../../primitives/core" }

cumulus/client/relay-chain-minimal-node/Cargo.toml

@@ -26,8 +26,8 @@ polkadot-node-core-prospective-parachains = { path = "../../../polkadot/node/cor
 sc-authority-discovery = { path = "../../../substrate/client/authority-discovery" }
 sc-network = { path = "../../../substrate/client/network" }
 sc-network-common = { path = "../../../substrate/client/network/common" }
-sc-service = { path = "../../../substrate/client/service" }
 sc-client-api = { path = "../../../substrate/client/api" }
+sc-service = { path = "../../../substrate/client/service", default-features = false }
 substrate-prometheus-endpoint = { path = "../../../substrate/utils/prometheus" }
 sc-tracing = { path = "../../../substrate/client/tracing" }
 sc-utils = { path = "../../../substrate/client/utils" }

cumulus/client/relay-chain-rpc-interface/Cargo.toml

@@ -23,7 +23,7 @@ sp-runtime = { path = "../../../substrate/primitives/runtime" }
 sp-version = { path = "../../../substrate/primitives/version" }
 sc-client-api = { path = "../../../substrate/client/api" }
 sc-rpc-api = { path = "../../../substrate/client/rpc-api" }
-sc-service = { path = "../../../substrate/client/service" }
+sc-service = { path = "../../../substrate/client/service", default-features = false }
 
 tokio = { version = "1.32.0", features = ["sync"] }
 tokio-util = { version = "0.7.8", features = ["compat"] }

cumulus/client/service/Cargo.toml

@@ -14,7 +14,7 @@ sc-client-api = { path = "../../../substrate/client/api" }
 sc-consensus = { path = "../../../substrate/client/consensus/common" }
 sc-transaction-pool = { path = "../../../substrate/client/transaction-pool" }
 sc-rpc = { path = "../../../substrate/client/rpc" }
-sc-service = { path = "../../../substrate/client/service" }
+sc-service = { path = "../../../substrate/client/service", default-features = false }
 sc-sysinfo = { path = "../../../substrate/client/sysinfo" }
 sc-telemetry = { path = "../../../substrate/client/telemetry" }
 sc-network = { path = "../../../substrate/client/network" }
@@ -33,6 +33,8 @@ polkadot-primitives = { path = "../../../polkadot/primitives" }
 
 # Cumulus
 cumulus-client-cli = { path = "../cli" }
+
+# For some reason I can enable the collator here, even though it appears to leak rocks on its own...
 cumulus-client-collator = { path = "../collator" }
 cumulus-client-consensus-common = { path = "../consensus/common" }
 cumulus-client-pov-recovery = { path = "../pov-recovery" }

cumulus/test/client/Cargo.toml

@@ -9,7 +9,7 @@ publish = false
 codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] }
 
 # Substrate
-sc-service = { path = "../../../substrate/client/service" }
+sc-service = { path = "../../../substrate/client/service", default-features = false }
 sc-consensus = { path = "../../../substrate/client/consensus/common" }
 sc-block-builder = { path = "../../../substrate/client/block-builder" }
 sc-executor = { path = "../../../substrate/client/executor" }

cumulus/test/service/Cargo.toml

@@ -33,7 +33,7 @@ sc-client-api = { path = "../../../substrate/client/api" }
 sc-consensus = { path = "../../../substrate/client/consensus/common" }
 sc-executor = { path = "../../../substrate/client/executor" }
 sc-network = { path = "../../../substrate/client/network" }
-sc-service = { path = "../../../substrate/client/service" }
+sc-service = { path = "../../../substrate/client/service", default-features = false }
 sc-tracing = { path = "../../../substrate/client/tracing" }
 sc-transaction-pool = { path = "../../../substrate/client/transaction-pool" }
 sc-transaction-pool-api = { path = "../../../substrate/client/transaction-pool/api" }
@@ -50,16 +50,16 @@ sp-tracing = { path = "../../../substrate/primitives/tracing" }
 sp-timestamp = { path = "../../../substrate/primitives/timestamp" }
 sp-consensus = { path = "../../../substrate/primitives/consensus/common" }
 substrate-test-client = { path = "../../../substrate/test-utils/client" }
-sc-cli = { path = "../../../substrate/client/cli" }
+sc-cli = { path = "../../../substrate/client/cli", default-features = false }
 sc-block-builder = { path = "../../../substrate/client/block-builder" }
 sc-executor-wasmtime = { path = "../../../substrate/client/executor/wasmtime" }
 sc-executor-common = { path = "../../../substrate/client/executor/common" }
 
 # Polkadot
 polkadot-primitives = { path = "../../../polkadot/primitives" }
-polkadot-service = { path = "../../../polkadot/node/service" }
+polkadot-service = { path = "../../../polkadot/node/service", default-features = false }
 polkadot-test-service = { path = "../../../polkadot/node/test/service" }
-polkadot-cli = { path = "../../../polkadot/cli" }
+polkadot-cli = { path = "../../../polkadot/cli", default-features = false }
 polkadot-node-subsystem = { path = "../../../polkadot/node/subsystem" }
 polkadot-overseer = { path = "../../../polkadot/node/overseer" }
 
@@ -90,10 +90,10 @@ sp-authority-discovery = { path = "../../../substrate/primitives/authority-disco
 cumulus-test-client = { path = "../client" }
 
 # Polkadot dependencies
-polkadot-test-service = { path = "../../../polkadot/node/test/service" }
+polkadot-test-service = { path = "../../../polkadot/node/test/service", default-features = false }
 
 # Substrate dependencies
-sc-cli = { path = "../../../substrate/client/cli" }
+sc-cli = { path = "../../../substrate/client/cli", default-features = false }
 substrate-test-utils = { path = "../../../substrate/test-utils" }
 
 [features]

cumulus/test/service/src/lib.rs

@@ -784,7 +784,7 @@ pub fn node_config(
 		transaction_pool: Default::default(),
 		network: network_config,
 		keystore: KeystoreConfig::InMemory,
-		database: DatabaseSource::RocksDb { path: root.join("db"), cache_size: 128 },
+		database: DatabaseSource::ParityDb { path: root.join("db") },
 		trie_cache_maximum_size: Some(64 * 1024 * 1024),
 		state_pruning: Some(PruningMode::ArchiveAll),
 		blocks_pruning: BlocksPruning::KeepAll,

polkadot/cli/Cargo.toml

@@ -29,10 +29,10 @@ sp-core = { path = "../../substrate/primitives/core" }
 sp-io = { path = "../../substrate/primitives/io" }
 sp-keyring = { path = "../../substrate/primitives/keyring" }
 sp-maybe-compressed-blob = { path = "../../substrate/primitives/maybe-compressed-blob" }
-frame-benchmarking-cli = { path = "../../substrate/utils/frame/benchmarking-cli", optional = true }
-try-runtime-cli = { path = "../../substrate/utils/frame/try-runtime/cli", optional = true }
-sc-cli = { path = "../../substrate/client/cli", optional = true }
-sc-service = { path = "../../substrate/client/service", optional = true }
+frame-benchmarking-cli = { path = "../../substrate/utils/frame/benchmarking-cli", default-features = false, optional = true }
+try-runtime-cli = { path = "../../substrate/utils/frame/try-runtime/cli", default-features = false, optional = true }
+sc-cli = { path = "../../substrate/client/cli", default-features = false, optional = true }
+sc-service = { path = "../../substrate/client/service", default-features = false, optional = true }
 polkadot-node-metrics = { path = "../node/metrics" }
 polkadot-node-primitives = { path = "../node/primitives" }
 sc-tracing = { path = "../../substrate/client/tracing", optional = true }
@@ -44,8 +44,12 @@ sc-storage-monitor = { path = "../../substrate/client/storage-monitor" }
 substrate-build-script-utils = { path = "../../substrate/utils/build-script-utils" }
 
 [features]
-default = ["cli", "db", "full-node"]
-db = ["service/db"]
+default = [ "cli", "full-node" ]
+db = [
+	"sc-cli?/rocksdb",
+	"sc-service?/rocksdb",
+	"service?/db",
+]
 cli = [
 	"clap",
 	"frame-benchmarking-cli",

polkadot/node/metrics/Cargo.toml

@@ -13,8 +13,9 @@ gum = { package = "tracing-gum", path = "../gum" }
 
 metered = { package = "prioritized-metered-channel", version = "0.5.1", default-features = false, features = ["futures_channel"] }
 # Both `sc-service` and `sc-cli` are required by runtime metrics `logger_hook()`.
-sc-service = { path = "../../../substrate/client/service" }
-sc-cli = { path = "../../../substrate/client/cli" }
+# Regarding the above comment, should they be optional dependencies then?
+sc-service = { path = "../../../substrate/client/service", default-features = false }
+sc-cli = { path = "../../../substrate/client/cli", default-features = false }
 
 substrate-prometheus-endpoint = { path = "../../../substrate/utils/prometheus" }
 sc-tracing = { path = "../../../substrate/client/tracing" }
@@ -28,9 +29,8 @@ assert_cmd = "2.0.4"
 tempfile = "3.2.0"
 hyper = { version = "0.14.20", default-features = false, features = ["http1", "tcp"] }
 tokio = "1.24.2"
-polkadot-test-service = { path = "../test/service", features = ["runtime-metrics"] }
+polkadot-test-service = { path = "../test/service", features = ["runtime-metrics"], default-features = false }
 substrate-test-utils = { path = "../../../substrate/test-utils" }
-sc-service = { path = "../../../substrate/client/service" }
 sp-keyring = { path = "../../../substrate/primitives/keyring" }
 prometheus-parse = { version = "0.2.2" }
 

polkadot/node/service/Cargo.toml

@@ -18,7 +18,7 @@ sp-mmr-primitives = { path = "../../../substrate/primitives/merkle-mountain-rang
 sc-block-builder = { path = "../../../substrate/client/block-builder" }
 sc-chain-spec = { path = "../../../substrate/client/chain-spec" }
 sc-client-api = { path = "../../../substrate/client/api" }
-sc-client-db = { path = "../../../substrate/client/db" }
+sc-client-db = { default-features = false, path = "../../../substrate/client/db" }
 sc-consensus = { path = "../../../substrate/client/consensus/common" }
 sc-consensus-slots = { path = "../../../substrate/client/consensus/slots" }
 sc-executor = { path = "../../../substrate/client/executor" }
@@ -71,8 +71,8 @@ frame-system = { path = "../../../substrate/frame/system" }
 frame-system-rpc-runtime-api = { path = "../../../substrate/frame/system/rpc/runtime-api" }
 prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../../../substrate/utils/prometheus" }
 frame-support = { path = "../../../substrate/frame/support" }
-frame-benchmarking-cli = { path = "../../../substrate/utils/frame/benchmarking-cli" }
-frame-benchmarking = { path = "../../../substrate/frame/benchmarking" }
+frame-benchmarking-cli = { path = "../../../substrate/utils/frame/benchmarking-cli", default-features = false }
+frame-benchmarking = { path = "../../../substrate/frame/benchmarking", default-features = false }
 
 # External Crates
 async-trait = "0.1.57"
@@ -148,12 +148,11 @@ serial_test = "2.0.0"
 tempfile = "3.2"
 
 [features]
-default = ["db", "full-node"]
+default = [ "full-node" ]
 
-db = ["service/rocksdb"]
+db = [ "service/rocksdb", "sc-client-db/rocksdb", "frame-benchmarking-cli/rocksdb", "kvdb-rocksdb" ]
 
 full-node = [
-	"kvdb-rocksdb",
 	"parity-db",
 	"polkadot-approval-distribution",
 	"polkadot-availability-bitfield-distribution",

polkadot/node/service/src/lib.rs

@@ -337,6 +337,7 @@ impl IdentifyVariant for Box<dyn ChainSpec> {
 #[cfg(feature = "full-node")]
 pub fn open_database(db_source: &DatabaseSource) -> Result<Arc<dyn Database>, Error> {
 	let parachains_db = match db_source {
+		#[cfg(feature = "db")]
 		DatabaseSource::RocksDb { path, .. } => parachains_db::open_creating_rocksdb(
 			path.clone(),
 			parachains_db::CacheSizes::default(),
@@ -345,19 +346,33 @@ pub fn open_database(db_source: &DatabaseSource) -> Result<Arc<dyn Database>, Er
 			path.parent().ok_or(Error::DatabasePathRequired)?.into(),
 			parachains_db::CacheSizes::default(),
 		)?,
-		DatabaseSource::Auto { paritydb_path, rocksdb_path, .. } => {
+		DatabaseSource::Auto {
+			paritydb_path,
+			#[cfg(feature = "db")]
+			rocksdb_path,
+			..
+		} =>
 			if paritydb_path.is_dir() && paritydb_path.exists() {
 				parachains_db::open_creating_paritydb(
 					paritydb_path.parent().ok_or(Error::DatabasePathRequired)?.into(),
 					parachains_db::CacheSizes::default(),
 				)?
 			} else {
-				parachains_db::open_creating_rocksdb(
-					rocksdb_path.clone(),
-					parachains_db::CacheSizes::default(),
-				)?
-			}
-		},
+				#[cfg(feature = "db")]
+				{
+					parachains_db::open_creating_rocksdb(
+						rocksdb_path.clone(),
+						parachains_db::CacheSizes::default(),
+					)?
+				}
+				#[cfg(not(feature = "db"))]
+				{
+					parachains_db::open_creating_paritydb(
+						paritydb_path.parent().ok_or(Error::DatabasePathRequired)?.into(),
+						parachains_db::CacheSizes::default(),
+					)?
+				}
+			},
 		DatabaseSource::Custom { .. } => {
 			unimplemented!("No polkadot subsystem db for custom source.");
 		},
@@ -1414,6 +1429,7 @@ pub fn revert_backend(
 	Ok(())
 }
 
+#[cfg(feature = "full-node")]
 fn revert_chain_selection(db: Arc<dyn Database>, hash: Hash) -> sp_blockchain::Result<()> {
 	let config = chain_selection_subsystem::Config {
 		col_data: parachains_db::REAL_COLUMNS.col_chain_selection_data,
@@ -1428,6 +1444,7 @@ fn revert_chain_selection(db: Arc<dyn Database>, hash: Hash) -> sp_blockchain::R
 		.map_err(|err| sp_blockchain::Error::Backend(err.to_string()))
 }
 
+#[cfg(feature = "full-node")]
 fn revert_approval_voting(db: Arc<dyn Database>, hash: Hash) -> sp_blockchain::Result<()> {
 	let config = approval_voting_subsystem::Config {
 		col_approval_data: parachains_db::REAL_COLUMNS.col_approval_data,

polkadot/node/service/src/parachains_db/mod.rs

@@ -26,6 +26,10 @@ const LOG_TARGET: &str = "parachain::db";
 /// Column configuration per version.
 #[cfg(any(test, feature = "full-node"))]
 pub(crate) mod columns {
+	// This v0 module is only used in tests for parity_db; not in main code. But it is
+	// used in both tests and main code for rocks db. This feature gating makes sure there
+	// is never a warning, but perhaps there is a better way?
+	#[cfg(any(test, feature = "db"))]
 	pub mod v0 {
 		pub const NUM_COLUMNS: u32 = 3;
 	}
@@ -91,6 +95,7 @@ pub const REAL_COLUMNS: ColumnsConfig = ColumnsConfig {
 #[derive(PartialEq, Copy, Clone)]
 pub(crate) enum DatabaseKind {
 	ParityDB,
+	#[cfg(feature = "db")]
 	RocksDB,
 }
 
@@ -125,6 +130,7 @@ pub(crate) fn other_io_error(err: String) -> io::Error {
 
 /// Open the database on disk, creating it if it doesn't exist.
 #[cfg(feature = "full-node")]
+#[cfg(feature = "db")]
 pub fn open_creating_rocksdb(
 	root: PathBuf,
 	cache_sizes: CacheSizes,