From 03b1297736c6f6c33ba03934c4248891afd49c86 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 7 Feb 2024 17:37:42 +0100 Subject: [PATCH 01/11] Only require Hash instead of Block Signed-off-by: Oliver Tale-Yazdi --- substrate/bin/node/cli/src/command.rs | 3 +- substrate/client/db/src/bench.rs | 84 +++++++++---------- substrate/client/db/src/lib.rs | 51 ++++++----- .../benchmarking-cli/src/pallet/command.rs | 18 +++- .../benchmarking-cli/src/storage/write.rs | 6 +- 5 files changed, 88 insertions(+), 74 deletions(-) diff --git a/substrate/bin/node/cli/src/command.rs b/substrate/bin/node/cli/src/command.rs index dc28705c2aea9..dbe729ec802c0 100644 --- a/substrate/bin/node/cli/src/command.rs +++ b/substrate/bin/node/cli/src/command.rs @@ -28,6 +28,7 @@ use node_primitives::Block; use sc_cli::{Result, SubstrateCli}; use sc_service::PartialComponents; use sp_keyring::Sr25519Keyring; +use sp_runtime::traits::HashingFor; use std::sync::Arc; @@ -106,7 +107,7 @@ pub fn run() -> Result<()> { ) } - cmd.run::(config) + cmd.run_with_hasher::, sp_statement_store::runtime_api::HostFunctions>(config) }, BenchmarkCmd::Block(cmd) => { // ensure that we keep the task manager alive diff --git a/substrate/client/db/src/bench.rs b/substrate/client/db/src/bench.rs index 03ad4817b53bc..2d5bd0dca8593 100644 --- a/substrate/client/db/src/bench.rs +++ b/substrate/client/db/src/bench.rs @@ -19,7 +19,7 @@ //! State backend that's useful for benchmarking use crate::{DbState, DbStateBuilder}; -use hash_db::{Hasher, Prefix}; +use hash_db::{Hasher as DbHasher, Prefix}; use kvdb::{DBTransaction, KeyValueDB}; use linked_hash_map::LinkedHashMap; use parking_lot::Mutex; @@ -27,10 +27,7 @@ use sp_core::{ hexdisplay::HexDisplay, storage::{ChildInfo, TrackedStorageKey}, }; -use sp_runtime::{ - traits::{Block as BlockT, HashingFor}, - StateVersion, Storage, -}; +use sp_runtime::{traits::Hash, StateVersion, Storage}; use sp_state_machine::{ backend::Backend as StateBackend, BackendTransaction, ChildStorageCollection, DBValue, IterArgs, StorageCollection, StorageIterator, StorageKey, StorageValue, @@ -45,16 +42,16 @@ use std::{ sync::Arc, }; -type State = DbState; +type State = DbState; -struct StorageDb { +struct StorageDb { db: Arc, - _block: std::marker::PhantomData, + _phantom: std::marker::PhantomData, } -impl sp_state_machine::Storage> for StorageDb { - fn get(&self, key: &Block::Hash, prefix: Prefix) -> Result, String> { - let prefixed_key = prefixed_key::>(key, prefix); +impl sp_state_machine::Storage for StorageDb { + fn get(&self, key: &Hasher::Output, prefix: Prefix) -> Result, String> { + let prefixed_key = prefixed_key::(key, prefix); self.db .get(0, &prefixed_key) .map_err(|e| format!("Database backend error: {:?}", e)) @@ -75,29 +72,29 @@ struct KeyTracker { } /// State that manages the backend database reference. Allows runtime to control the database. -pub struct BenchmarkingState { - root: Cell, - genesis_root: B::Hash, - state: RefCell>>, +pub struct BenchmarkingState { + root: Cell, + genesis_root: Hasher::Output, + state: RefCell>>, db: Cell>>, genesis: HashMap, (Vec, i32)>, record: Cell>>, key_tracker: Arc>, whitelist: RefCell>, - proof_recorder: Option>>, - proof_recorder_root: Cell, - shared_trie_cache: SharedTrieCache>, + proof_recorder: Option>, + proof_recorder_root: Cell, + shared_trie_cache: SharedTrieCache, } /// A raw iterator over the `BenchmarkingState`. -pub struct RawIter { - inner: as StateBackend>>::RawIter, +pub struct RawIter { + inner: as StateBackend>::RawIter, child_trie: Option>, key_tracker: Arc>, } -impl StorageIterator> for RawIter { - type Backend = BenchmarkingState; +impl StorageIterator for RawIter { + type Backend = BenchmarkingState; type Error = String; fn next_key(&mut self, backend: &Self::Backend) -> Option> { @@ -128,7 +125,7 @@ impl StorageIterator> for RawIter { } } -impl BenchmarkingState { +impl BenchmarkingState { /// Create a new instance that creates a database in a temporary dir. pub fn new( genesis: Storage, @@ -137,9 +134,9 @@ impl BenchmarkingState { enable_tracking: bool, ) -> Result { let state_version = sp_runtime::StateVersion::default(); - let mut root = B::Hash::default(); - let mut mdb = MemoryDB::>::default(); - sp_trie::trie_types::TrieDBMutBuilderV1::>::new(&mut mdb, &mut root).build(); + let mut root = Default::default(); + let mut mdb = MemoryDB::::default(); + sp_trie::trie_types::TrieDBMutBuilderV1::::new(&mut mdb, &mut root).build(); let mut state = BenchmarkingState { state: RefCell::new(None), @@ -169,7 +166,7 @@ impl BenchmarkingState { child_content.data.iter().map(|(k, v)| (k.as_ref(), Some(v.as_ref()))), ) }); - let (root, transaction): (B::Hash, _) = + let (root, transaction): (Hasher::Output, _) = state.state.borrow().as_ref().unwrap().full_storage_root( genesis.top.iter().map(|(k, v)| (k.as_ref(), Some(v.as_ref()))), child_delta, @@ -193,9 +190,9 @@ impl BenchmarkingState { recorder.reset(); self.proof_recorder_root.set(self.root.get()); } - let storage_db = Arc::new(StorageDb:: { db, _block: Default::default() }); + let storage_db = Arc::new(StorageDb:: { db, _phantom: Default::default() }); *self.state.borrow_mut() = Some( - DbStateBuilder::::new(storage_db, self.root.get()) + DbStateBuilder::::new(storage_db, self.root.get()) .with_optional_recorder(self.proof_recorder.clone()) .with_cache(self.shared_trie_cache.local_cache()) .build(), @@ -341,17 +338,17 @@ fn state_err() -> String { "State is not open".into() } -impl StateBackend> for BenchmarkingState { - type Error = as StateBackend>>::Error; - type TrieBackendStorage = as StateBackend>>::TrieBackendStorage; - type RawIter = RawIter; +impl StateBackend for BenchmarkingState { + type Error = as StateBackend>::Error; + type TrieBackendStorage = as StateBackend>::TrieBackendStorage; + type RawIter = RawIter; fn storage(&self, key: &[u8]) -> Result>, Self::Error> { self.add_read_key(None, key); self.state.borrow().as_ref().ok_or_else(state_err)?.storage(key) } - fn storage_hash(&self, key: &[u8]) -> Result, Self::Error> { + fn storage_hash(&self, key: &[u8]) -> Result, Self::Error> { self.add_read_key(None, key); self.state.borrow().as_ref().ok_or_else(state_err)?.storage_hash(key) } @@ -373,7 +370,7 @@ impl StateBackend> for BenchmarkingState { &self, child_info: &ChildInfo, key: &[u8], - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { self.add_read_key(Some(child_info.storage_key()), key); self.state .borrow() @@ -385,7 +382,7 @@ impl StateBackend> for BenchmarkingState { fn closest_merkle_value( &self, key: &[u8], - ) -> Result>, Self::Error> { + ) -> Result>, Self::Error> { self.add_read_key(None, key); self.state.borrow().as_ref().ok_or_else(state_err)?.closest_merkle_value(key) } @@ -394,7 +391,7 @@ impl StateBackend> for BenchmarkingState { &self, child_info: &ChildInfo, key: &[u8], - ) -> Result>, Self::Error> { + ) -> Result>, Self::Error> { self.add_read_key(None, key); self.state .borrow() @@ -443,7 +440,7 @@ impl StateBackend> for BenchmarkingState { &self, delta: impl Iterator)>, state_version: StateVersion, - ) -> (B::Hash, BackendTransaction>) { + ) -> (Hasher::Output, BackendTransaction) { self.state .borrow() .as_ref() @@ -455,7 +452,7 @@ impl StateBackend> for BenchmarkingState { child_info: &ChildInfo, delta: impl Iterator)>, state_version: StateVersion, - ) -> (B::Hash, bool, BackendTransaction>) { + ) -> (Hasher::Output, bool, BackendTransaction) { self.state .borrow() .as_ref() @@ -479,8 +476,8 @@ impl StateBackend> for BenchmarkingState { fn commit( &self, - storage_root: as Hasher>::Out, - mut transaction: BackendTransaction>, + storage_root: ::Out, + mut transaction: BackendTransaction, main_storage_changes: StorageCollection, child_storage_changes: ChildStorageCollection, ) -> Result<(), Self::Error> { @@ -634,8 +631,7 @@ impl StateBackend> for BenchmarkingState { log::debug!(target: "benchmark", "Some proof size: {}", &proof_size); proof_size } else { - if let Some(size) = proof.encoded_compact_size::>(proof_recorder_root) - { + if let Some(size) = proof.encoded_compact_size::(proof_recorder_root) { size as u32 } else if proof_recorder_root == self.root.get() { log::debug!(target: "benchmark", "No changes - no proof"); @@ -654,7 +650,7 @@ impl StateBackend> for BenchmarkingState { } } -impl std::fmt::Debug for BenchmarkingState { +impl std::fmt::Debug for BenchmarkingState { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "Bench DB") } diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index 2d8622d5f12dc..870b4150b9df7 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -101,14 +101,11 @@ pub use bench::BenchmarkingState; const CACHE_HEADERS: usize = 8; /// DB-backed patricia trie state, transaction type is an overlay of changes to commit. -pub type DbState = - sp_state_machine::TrieBackend>>, HashingFor>; +pub type DbState = sp_state_machine::TrieBackend>, H>; /// Builder for [`DbState`]. -pub type DbStateBuilder = sp_state_machine::TrieBackendBuilder< - Arc>>, - HashingFor, ->; +pub type DbStateBuilder = + sp_state_machine::TrieBackendBuilder>, Hasher>; /// Length of a [`DbHash`]. const DB_HASH_LEN: usize = 32; @@ -135,13 +132,17 @@ enum DbExtrinsic { /// It makes sure that the hash we are using stays pinned in storage /// until this structure is dropped. pub struct RefTrackingState { - state: DbState, + state: DbState>, storage: Arc>, parent_hash: Option, } impl RefTrackingState { - fn new(state: DbState, storage: Arc>, parent_hash: Option) -> Self { + fn new( + state: DbState>, + storage: Arc>, + parent_hash: Option, + ) -> Self { RefTrackingState { state, parent_hash, storage } } } @@ -162,12 +163,12 @@ impl std::fmt::Debug for RefTrackingState { /// A raw iterator over the `RefTrackingState`. pub struct RawIter { - inner: as StateBackend>>::RawIter, + inner: > as StateBackend>>::RawIter, } impl StorageIterator> for RawIter { type Backend = RefTrackingState; - type Error = as StateBackend>>::Error; + type Error = > as StateBackend>>::Error; fn next_key(&mut self, backend: &Self::Backend) -> Option> { self.inner.next_key(&backend.state) @@ -186,8 +187,9 @@ impl StorageIterator> for RawIter { } impl StateBackend> for RefTrackingState { - type Error = as StateBackend>>::Error; - type TrieBackendStorage = as StateBackend>>::TrieBackendStorage; + type Error = > as StateBackend>>::Error; + type TrieBackendStorage = + > as StateBackend>>::TrieBackendStorage; type RawIter = RawIter; fn storage(&self, key: &[u8]) -> Result>, Self::Error> { @@ -284,7 +286,8 @@ impl StateBackend> for RefTrackingState { } impl AsTrieBackend> for RefTrackingState { - type TrieBackendStorage = as StateBackend>>::TrieBackendStorage; + type TrieBackendStorage = + > as StateBackend>>::TrieBackendStorage; fn as_trie_backend( &self, @@ -1936,7 +1939,7 @@ impl Backend { fn empty_state(&self) -> RecordStatsState, Block> { let root = EmptyStorage::::new().0; // Empty trie - let db_state = DbStateBuilder::::new(self.storage.clone(), root) + let db_state = DbStateBuilder::>::new(self.storage.clone(), root) .with_optional_cache(self.shared_trie_cache.as_ref().map(|c| c.local_cache())) .build(); let state = RefTrackingState::new(db_state, self.storage.clone(), None); @@ -2428,9 +2431,12 @@ impl sc_client_api::backend::Backend for Backend { if hash == self.blockchain.meta.read().genesis_hash { if let Some(genesis_state) = &*self.genesis_state.read() { let root = genesis_state.root; - let db_state = DbStateBuilder::::new(genesis_state.clone(), root) - .with_optional_cache(self.shared_trie_cache.as_ref().map(|c| c.local_cache())) - .build(); + let db_state = + DbStateBuilder::>::new(genesis_state.clone(), root) + .with_optional_cache( + self.shared_trie_cache.as_ref().map(|c| c.local_cache()), + ) + .build(); let state = RefTrackingState::new(db_state, self.storage.clone(), None); return Ok(RecordStatsState::new(state, None, self.state_usage.clone())) @@ -2449,11 +2455,12 @@ impl sc_client_api::backend::Backend for Backend { self.storage.state_db.pin(&hash, hdr.number.saturated_into::(), hint) { let root = hdr.state_root; - let db_state = DbStateBuilder::::new(self.storage.clone(), root) - .with_optional_cache( - self.shared_trie_cache.as_ref().map(|c| c.local_cache()), - ) - .build(); + let db_state = + DbStateBuilder::>::new(self.storage.clone(), root) + .with_optional_cache( + self.shared_trie_cache.as_ref().map(|c| c.local_cache()), + ) + .build(); let state = RefTrackingState::new(db_state, self.storage.clone(), Some(hash)); Ok(RecordStatsState::new(state, Some(hash), self.state_usage.clone())) } else { diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 5c76ca68e85f5..3ccf1c8306dc8 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -37,7 +37,7 @@ use sp_core::{ }; use sp_externalities::Extensions; use sp_keystore::{testing::MemoryKeystore, KeystoreExt}; -use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; +use sp_runtime::traits::{Block as BlockT, Hash, HashingFor}; use sp_state_machine::StateMachine; use std::{collections::HashMap, fmt::Debug, fs, str::FromStr, time}; @@ -141,10 +141,20 @@ not created by a node that was compiled with the flag"; impl PalletCmd { /// Runs the command and benchmarks the chain. + #[deprecated( + note = "`run` will have a breaking change after July 2024. Use `run_with_hasher` instead." + )] pub fn run(&self, config: Configuration) -> Result<()> where BB: BlockT + Debug, - <<::Header as HeaderT>::Number as std::str::FromStr>::Err: std::fmt::Debug, + ExtraHostFunctions: sp_wasm_interface::HostFunctions, + { + self.run_with_hasher::, ExtraHostFunctions>(config) + } + + pub fn run_with_hasher(&self, config: Configuration) -> Result<()> + where + Hasher: Hash, ExtraHostFunctions: sp_wasm_interface::HostFunctions, { let _d = self.execution.as_ref().map(|exec| { @@ -199,7 +209,7 @@ impl PalletCmd { let genesis_storage = spec.build_storage()?; let mut changes = Default::default(); let cache_size = Some(self.database_cache_size as usize); - let state_with_tracking = BenchmarkingState::::new( + let state_with_tracking = BenchmarkingState::::new( genesis_storage.clone(), cache_size, // Record proof size @@ -207,7 +217,7 @@ impl PalletCmd { // Enable storage tracking true, )?; - let state_without_tracking = BenchmarkingState::::new( + let state_without_tracking = BenchmarkingState::::new( genesis_storage, cache_size, // Do not record proof size diff --git a/substrate/utils/frame/benchmarking-cli/src/storage/write.rs b/substrate/utils/frame/benchmarking-cli/src/storage/write.rs index 65941497bda41..4fa56b6e818f8 100644 --- a/substrate/utils/frame/benchmarking-cli/src/storage/write.rs +++ b/substrate/utils/frame/benchmarking-cli/src/storage/write.rs @@ -57,7 +57,7 @@ impl StorageCmd { let best_hash = client.usage_info().chain.best_hash; let header = client.header(best_hash)?.ok_or("Header not found")?; let original_root = *header.state_root(); - let trie = DbStateBuilder::::new(storage.clone(), original_root).build(); + let trie = DbStateBuilder::>::new(storage.clone(), original_root).build(); info!("Preparing keys from block {}", best_hash); // Load all KV pairs and randomly shuffle them. @@ -189,7 +189,7 @@ fn convert_tx( /// if `child_info` exist then it means this is a child tree key fn measure_write( db: Arc>, - trie: &DbState, + trie: &DbState>, key: Vec, new_v: Vec, version: StateVersion, @@ -220,7 +220,7 @@ fn measure_write( /// if `child_info` exist then it means this is a child tree key fn check_new_value( db: Arc>, - trie: &DbState, + trie: &DbState>, key: &Vec, new_v: &Vec, version: StateVersion, From ef7e19ec1e43975d24a03aa1e6946be4153c6398 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 7 Feb 2024 17:47:49 +0100 Subject: [PATCH 02/11] Add prdoc Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_3244.prdoc | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 prdoc/pr_3244.prdoc diff --git a/prdoc/pr_3244.prdoc b/prdoc/pr_3244.prdoc new file mode 100644 index 0000000000000..8906626449c26 --- /dev/null +++ b/prdoc/pr_3244.prdoc @@ -0,0 +1,10 @@ +title: "Make the `benchmark pallet`` command only require a Hash" + +doc: + - audience: Node Dev + description: | + Currently the `benchmark pallet` command requires a `Block` type, whole only using its hash. Now this is changed to only require the hash. Some refactoring in DB crates was needed to achieve this. + +crates: + - name: sc-client-db + - name: frame-benchmarking-cli From ff75e90a201e5440d4c828f259d3dcb662e30a3e Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 7 Feb 2024 18:09:25 +0100 Subject: [PATCH 03/11] Fix compile Signed-off-by: Oliver Tale-Yazdi --- polkadot/cli/src/command.rs | 2 +- substrate/client/db/src/bench.rs | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/polkadot/cli/src/command.rs b/polkadot/cli/src/command.rs index 7319f28a2e2aa..e1bc9a25716be 100644 --- a/polkadot/cli/src/command.rs +++ b/polkadot/cli/src/command.rs @@ -451,7 +451,7 @@ pub fn run() -> Result<()> { if cfg!(feature = "runtime-benchmarks") { runner.sync_run(|config| { - cmd.run::(config) + cmd.run_with_hasher::, ()>(config) .map_err(|e| Error::SubstrateCli(e)) }) } else { diff --git a/substrate/client/db/src/bench.rs b/substrate/client/db/src/bench.rs index 2d5bd0dca8593..f064611ba3ce3 100644 --- a/substrate/client/db/src/bench.rs +++ b/substrate/client/db/src/bench.rs @@ -659,6 +659,7 @@ impl std::fmt::Debug for BenchmarkingState { #[cfg(test)] mod test { use crate::bench::BenchmarkingState; + use sp_runtime::traits::HashingFor; use sp_state_machine::backend::Backend as _; fn hex(hex: &str) -> Vec { @@ -677,7 +678,8 @@ mod test { ..sp_runtime::Storage::default() }; let bench_state = - BenchmarkingState::::new(storage, None, false, true).unwrap(); + BenchmarkingState::>::new(storage, None, false, true) + .unwrap(); assert_eq!(bench_state.read_write_count(), (0, 0, 0, 0)); assert_eq!(bench_state.keys(Default::default()).unwrap().count(), 1); @@ -686,9 +688,13 @@ mod test { #[test] fn read_to_main_and_child_tries() { - let bench_state = - BenchmarkingState::::new(Default::default(), None, false, true) - .unwrap(); + let bench_state = BenchmarkingState::>::new( + Default::default(), + None, + false, + true, + ) + .unwrap(); for _ in 0..2 { let child1 = sp_core::storage::ChildInfo::new_default(b"child1"); From f3a9b6f43a7529ab424079089a99603b63321465 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 7 Feb 2024 19:36:20 +0100 Subject: [PATCH 04/11] Just remove the old run function Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 1 + cumulus/parachain-template/node/src/command.rs | 2 +- cumulus/polkadot-parachain/src/command.rs | 2 +- polkadot/cli/Cargo.toml | 8 +++++++- polkadot/cli/src/command.rs | 2 +- substrate/bin/node-template/node/src/command.rs | 2 +- substrate/bin/node/cli/src/command.rs | 2 +- .../benchmarking-cli/src/pallet/command.rs | 17 +++-------------- 8 files changed, 16 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27a1efc38098c..7f1ed686e84c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12130,6 +12130,7 @@ dependencies = [ "sp-io", "sp-keyring", "sp-maybe-compressed-blob", + "sp-runtime", "substrate-build-script-utils", "thiserror", "try-runtime-cli", diff --git a/cumulus/parachain-template/node/src/command.rs b/cumulus/parachain-template/node/src/command.rs index 6ddb68a359a78..72b3ab7bb4b94 100644 --- a/cumulus/parachain-template/node/src/command.rs +++ b/cumulus/parachain-template/node/src/command.rs @@ -183,7 +183,7 @@ pub fn run() -> Result<()> { match cmd { BenchmarkCmd::Pallet(cmd) => if cfg!(feature = "runtime-benchmarks") { - runner.sync_run(|config| cmd.run::(config)) + runner.sync_run(|config| cmd.run::, ()>(config)) } else { Err("Benchmarking wasn't enabled when building the node. \ You can enable it with `--features runtime-benchmarks`." diff --git a/cumulus/polkadot-parachain/src/command.rs b/cumulus/polkadot-parachain/src/command.rs index 19d067068f48f..3be232ce69890 100644 --- a/cumulus/polkadot-parachain/src/command.rs +++ b/cumulus/polkadot-parachain/src/command.rs @@ -584,7 +584,7 @@ pub fn run() -> Result<()> { match cmd { BenchmarkCmd::Pallet(cmd) => if cfg!(feature = "runtime-benchmarks") { - runner.sync_run(|config| cmd.run::(config)) + runner.sync_run(|config| cmd.run::, ()>(config)) } else { Err("Benchmarking wasn't enabled when building the node. \ You can enable it with `--features runtime-benchmarks`." diff --git a/polkadot/cli/Cargo.toml b/polkadot/cli/Cargo.toml index 3186e31f39e80..54583e9136d90 100644 --- a/polkadot/cli/Cargo.toml +++ b/polkadot/cli/Cargo.toml @@ -42,6 +42,7 @@ sc-tracing = { path = "../../substrate/client/tracing", optional = true } sc-sysinfo = { path = "../../substrate/client/sysinfo" } sc-executor = { path = "../../substrate/client/executor" } sc-storage-monitor = { path = "../../substrate/client/storage-monitor" } +sp-runtime = { path = "../../substrate/primitives/runtime" } [build-dependencies] substrate-build-script-utils = { path = "../../substrate/utils/build-script-utils" } @@ -63,9 +64,14 @@ runtime-benchmarks = [ "polkadot-node-metrics/runtime-benchmarks", "sc-service?/runtime-benchmarks", "service/runtime-benchmarks", + "sp-runtime/runtime-benchmarks" ] full-node = ["service/full-node"] -try-runtime = ["service/try-runtime", "try-runtime-cli/try-runtime"] +try-runtime = [ + "service/try-runtime", + "try-runtime-cli/try-runtime", + "sp-runtime/try-runtime" +] fast-runtime = ["service/fast-runtime"] pyroscope = ["pyro", "pyroscope_pprofrs"] diff --git a/polkadot/cli/src/command.rs b/polkadot/cli/src/command.rs index e1bc9a25716be..f71891ecde34c 100644 --- a/polkadot/cli/src/command.rs +++ b/polkadot/cli/src/command.rs @@ -451,7 +451,7 @@ pub fn run() -> Result<()> { if cfg!(feature = "runtime-benchmarks") { runner.sync_run(|config| { - cmd.run_with_hasher::, ()>(config) + cmd.run::, ()>(config) .map_err(|e| Error::SubstrateCli(e)) }) } else { diff --git a/substrate/bin/node-template/node/src/command.rs b/substrate/bin/node-template/node/src/command.rs index a25157693cd43..3778df6642294 100644 --- a/substrate/bin/node-template/node/src/command.rs +++ b/substrate/bin/node-template/node/src/command.rs @@ -117,7 +117,7 @@ pub fn run() -> sc_cli::Result<()> { ) } - cmd.run::(config) + cmd.run::, ()>(config) }, BenchmarkCmd::Block(cmd) => { let PartialComponents { client, .. } = service::new_partial(&config)?; diff --git a/substrate/bin/node/cli/src/command.rs b/substrate/bin/node/cli/src/command.rs index dbe729ec802c0..9645173202866 100644 --- a/substrate/bin/node/cli/src/command.rs +++ b/substrate/bin/node/cli/src/command.rs @@ -107,7 +107,7 @@ pub fn run() -> Result<()> { ) } - cmd.run_with_hasher::, sp_statement_store::runtime_api::HostFunctions>(config) + cmd.run::, sp_statement_store::runtime_api::HostFunctions>(config) }, BenchmarkCmd::Block(cmd) => { // ensure that we keep the task manager alive diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 3ccf1c8306dc8..dce49db15f7d4 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -37,7 +37,7 @@ use sp_core::{ }; use sp_externalities::Extensions; use sp_keystore::{testing::MemoryKeystore, KeystoreExt}; -use sp_runtime::traits::{Block as BlockT, Hash, HashingFor}; +use sp_runtime::traits::Hash; use sp_state_machine::StateMachine; use std::{collections::HashMap, fmt::Debug, fs, str::FromStr, time}; @@ -140,19 +140,8 @@ This could mean that you either did not build the node correctly with the \ not created by a node that was compiled with the flag"; impl PalletCmd { - /// Runs the command and benchmarks the chain. - #[deprecated( - note = "`run` will have a breaking change after July 2024. Use `run_with_hasher` instead." - )] - pub fn run(&self, config: Configuration) -> Result<()> - where - BB: BlockT + Debug, - ExtraHostFunctions: sp_wasm_interface::HostFunctions, - { - self.run_with_hasher::, ExtraHostFunctions>(config) - } - - pub fn run_with_hasher(&self, config: Configuration) -> Result<()> + /// Runs the command and benchmarks a pallet. + pub fn run(&self, config: Configuration) -> Result<()> where Hasher: Hash, ExtraHostFunctions: sp_wasm_interface::HostFunctions, From 6d28c70ab53a59297e050d25980e3db1c04f697a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 8 Feb 2024 16:41:05 +0100 Subject: [PATCH 05/11] Update substrate/client/db/src/bench.rs Co-authored-by: cheme --- substrate/client/db/src/bench.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/client/db/src/bench.rs b/substrate/client/db/src/bench.rs index f064611ba3ce3..750397fa69901 100644 --- a/substrate/client/db/src/bench.rs +++ b/substrate/client/db/src/bench.rs @@ -46,7 +46,7 @@ type State = DbState; struct StorageDb { db: Arc, - _phantom: std::marker::PhantomData, + _phantom: std::marker::PhantomData, } impl sp_state_machine::Storage for StorageDb { From 7f903a0578b85b3b1906162e19e139a2888a8b5d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 8 Feb 2024 17:00:16 +0100 Subject: [PATCH 06/11] Update substrate/client/db/src/bench.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- substrate/client/db/src/bench.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/client/db/src/bench.rs b/substrate/client/db/src/bench.rs index 750397fa69901..32503cf63c0ad 100644 --- a/substrate/client/db/src/bench.rs +++ b/substrate/client/db/src/bench.rs @@ -44,7 +44,7 @@ use std::{ type State = DbState; -struct StorageDb { +struct StorageDb { db: Arc, _phantom: std::marker::PhantomData, } From 4eba3ee357c5529115231756e32e0019ecb99288 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 8 Feb 2024 17:00:21 +0100 Subject: [PATCH 07/11] Update prdoc/pr_3244.prdoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- prdoc/pr_3244.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_3244.prdoc b/prdoc/pr_3244.prdoc index 8906626449c26..68a5a5143e669 100644 --- a/prdoc/pr_3244.prdoc +++ b/prdoc/pr_3244.prdoc @@ -1,4 +1,4 @@ -title: "Make the `benchmark pallet`` command only require a Hash" +title: "Make the `benchmark pallet`` command only require a Hasher" doc: - audience: Node Dev From b78a9836ed5dc62399499763ec6193ae97179c7d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 8 Feb 2024 17:04:42 +0100 Subject: [PATCH 08/11] prdoc: mention patch Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_3244.prdoc | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/prdoc/pr_3244.prdoc b/prdoc/pr_3244.prdoc index 68a5a5143e669..35fc6a2573a2d 100644 --- a/prdoc/pr_3244.prdoc +++ b/prdoc/pr_3244.prdoc @@ -3,7 +3,15 @@ title: "Make the `benchmark pallet`` command only require a Hasher" doc: - audience: Node Dev description: | - Currently the `benchmark pallet` command requires a `Block` type, whole only using its hash. Now this is changed to only require the hash. Some refactoring in DB crates was needed to achieve this. + Currently the `benchmark pallet` command requires a `Block` type, whole only using its hash. + Now this is changed to only require the hash. This means to use `HashingFor` in the + spot where `Block` was required. + Example patch for your node with `cmd` being `BenchmarkCmd::Pallet(cmd)`: + ```patch + - cmd.run::(config) + + cmd.run::, ()>(config) + ``` + crates: - name: sc-client-db From 465e3bd2cc3f306bce56e51a8bfc967c3d29d2f0 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 8 Feb 2024 17:05:12 +0100 Subject: [PATCH 09/11] prdoc: hash -> hasher Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_3244.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_3244.prdoc b/prdoc/pr_3244.prdoc index 35fc6a2573a2d..a55d689ee6429 100644 --- a/prdoc/pr_3244.prdoc +++ b/prdoc/pr_3244.prdoc @@ -3,7 +3,7 @@ title: "Make the `benchmark pallet`` command only require a Hasher" doc: - audience: Node Dev description: | - Currently the `benchmark pallet` command requires a `Block` type, whole only using its hash. + Currently the `benchmark pallet` command requires a `Block` type, whole only using its hasher. Now this is changed to only require the hash. This means to use `HashingFor` in the spot where `Block` was required. Example patch for your node with `cmd` being `BenchmarkCmd::Pallet(cmd)`: From 9f4ba8cb167d05f568a233c5a0ee44137547ab14 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 8 Feb 2024 17:05:43 +0100 Subject: [PATCH 10/11] toml formatting Signed-off-by: Oliver Tale-Yazdi --- polkadot/cli/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polkadot/cli/Cargo.toml b/polkadot/cli/Cargo.toml index 54583e9136d90..75afa2d199f40 100644 --- a/polkadot/cli/Cargo.toml +++ b/polkadot/cli/Cargo.toml @@ -64,13 +64,13 @@ runtime-benchmarks = [ "polkadot-node-metrics/runtime-benchmarks", "sc-service?/runtime-benchmarks", "service/runtime-benchmarks", - "sp-runtime/runtime-benchmarks" + "sp-runtime/runtime-benchmarks", ] full-node = ["service/full-node"] try-runtime = [ "service/try-runtime", + "sp-runtime/try-runtime", "try-runtime-cli/try-runtime", - "sp-runtime/try-runtime" ] fast-runtime = ["service/fast-runtime"] pyroscope = ["pyro", "pyroscope_pprofrs"] From 4c6fdc5bb0041e43465276755d6f34667f9b3c43 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 8 Feb 2024 17:07:06 +0100 Subject: [PATCH 11/11] typos Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_3244.prdoc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/prdoc/pr_3244.prdoc b/prdoc/pr_3244.prdoc index a55d689ee6429..3d851c8afe0d2 100644 --- a/prdoc/pr_3244.prdoc +++ b/prdoc/pr_3244.prdoc @@ -1,18 +1,18 @@ -title: "Make the `benchmark pallet`` command only require a Hasher" +title: "Make the `benchmark pallet` command only require a Hasher" doc: - audience: Node Dev description: | - Currently the `benchmark pallet` command requires a `Block` type, whole only using its hasher. - Now this is changed to only require the hash. This means to use `HashingFor` in the - spot where `Block` was required. + Currently the `benchmark pallet` command requires a `Block` type, while only using its hasher. + Now this is changed to only require the hasher. This means to use `HashingFor` in the + place where `Block` was required. Example patch for your node with `cmd` being `BenchmarkCmd::Pallet(cmd)`: + ```patch - cmd.run::(config) + cmd.run::, ()>(config) ``` - crates: - name: sc-client-db - name: frame-benchmarking-cli