From 368903f7aa9ef652bf8157d476dc13cb36e3affc Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 19 Aug 2020 18:15:50 +0200 Subject: [PATCH] Dynamic Benchmarking DB Whitelist (#6815) * Add `get_whitelist` api * add whitelisted caller * Whitelist caller * remove caller 0 * initial piping of origin (not actual value yet) * remove attempt to pass origin around * Add whitelist for `DidUpdate` storage on `pallet_timestamp` * fix traits * only add to whitelist if !contains * PassBy not implemented error * Whitelist read/writes explicitly per key * update docs * reduce trait constraint * copy pasta * Apply suggestions from code review Co-authored-by: Guillaume Thiolliere Co-authored-by: Alexander Popiak * rename functions @apopiak * missed some renaming * enable doc tests * Update docs Co-authored-by: Guillaume Thiolliere Co-authored-by: Alexander Popiak --- Cargo.lock | 4 + bin/node/runtime/src/lib.rs | 18 +- client/db/src/bench.rs | 37 +-- frame/balances/src/benchmarking.rs | 8 +- frame/benchmarking/Cargo.toml | 4 + frame/benchmarking/src/lib.rs | 293 ++++++++++++++-------- frame/benchmarking/src/tests.rs | 12 +- frame/benchmarking/src/utils.rs | 47 +++- frame/collective/src/benchmarking.rs | 14 +- frame/indices/src/benchmarking.rs | 10 +- frame/proxy/src/benchmarking.rs | 18 +- frame/staking/src/benchmarking.rs | 20 +- frame/system/benchmarking/src/lib.rs | 8 +- frame/timestamp/src/benchmarking.rs | 14 +- frame/treasury/src/benchmarking.rs | 22 +- frame/utility/src/benchmarking.rs | 7 +- frame/vesting/src/benchmarking.rs | 12 +- primitives/externalities/src/lib.rs | 11 +- primitives/runtime-interface/Cargo.toml | 1 + primitives/runtime-interface/src/impls.rs | 4 + primitives/state-machine/src/backend.rs | 14 +- primitives/state-machine/src/basic.rs | 8 +- primitives/state-machine/src/ext.rs | 8 +- primitives/state-machine/src/read_only.rs | 8 +- primitives/storage/Cargo.toml | 3 +- primitives/storage/src/lib.rs | 21 ++ 26 files changed, 421 insertions(+), 205 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ec5af8aca4ecf..c80c0557443a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1559,6 +1559,7 @@ version = "2.0.0-rc5" dependencies = [ "frame-support", "frame-system", + "hex-literal", "linregress", "parity-scale-codec", "paste", @@ -1567,6 +1568,7 @@ dependencies = [ "sp-runtime", "sp-runtime-interface", "sp-std", + "sp-storage", ] [[package]] @@ -8047,6 +8049,7 @@ dependencies = [ "sp-runtime-interface-test-wasm", "sp-state-machine", "sp-std", + "sp-storage", "sp-tracing", "sp-wasm-interface", "static_assertions", @@ -8176,6 +8179,7 @@ name = "sp-storage" version = "2.0.0-rc5" dependencies = [ "impl-serde 0.2.3", + "parity-scale-codec", "ref-cast", "serde", "sp-debug-derive", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index aa0ddfc61a7bb..9d19f20c5e19a 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1130,7 +1130,7 @@ impl_runtime_apis! { repeat: u32, extra: bool, ) -> Result, sp_runtime::RuntimeString> { - use frame_benchmarking::{Benchmarking, BenchmarkBatch, add_benchmark}; + use frame_benchmarking::{Benchmarking, BenchmarkBatch, add_benchmark, TrackedStorageKey}; // Trying to add benchmarks directly to the Session Pallet caused cyclic dependency issues. // To get around that, we separated the Session benchmarks into its own crate, which is why // we need these two lines below. @@ -1142,21 +1142,19 @@ impl_runtime_apis! { impl pallet_offences_benchmarking::Trait for Runtime {} impl frame_system_benchmarking::Trait for Runtime {} - let whitelist: Vec> = vec![ + let whitelist: Vec = vec![ // Block Number - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec(), + hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(), // Total Issuance - hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec(), + hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec().into(), // Execution Phase - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec(), + hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec().into(), // Event Count - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec(), + hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec().into(), // System Events - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7").to_vec(), - // Caller 0 Account - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da946c154ffd9992e395af90b5b13cc6f295c77033fce8a9045824a6690bbf99c6db269502f0a8d1d2a008542d5690a0749").to_vec(), + hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7").to_vec().into(), // Treasury Account - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec(), + hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into(), ]; let mut batches = Vec::::new(); diff --git a/client/db/src/bench.rs b/client/db/src/bench.rs index c3bed3e24f617..93b8048529fc4 100644 --- a/client/db/src/bench.rs +++ b/client/db/src/bench.rs @@ -24,7 +24,10 @@ use std::collections::HashMap; use hash_db::{Prefix, Hasher}; use sp_trie::{MemoryDB, prefixed_key}; -use sp_core::{storage::ChildInfo, hexdisplay::HexDisplay}; +use sp_core::{ + storage::{ChildInfo, TrackedStorageKey}, + hexdisplay::HexDisplay +}; use sp_runtime::traits::{Block as BlockT, HashFor}; use sp_runtime::Storage; use sp_state_machine::{DBValue, backend::Backend as StateBackend, StorageCollection}; @@ -95,7 +98,7 @@ pub struct BenchmarkingState { shared_cache: SharedCache, // shared cache is always empty key_tracker: RefCell, KeyTracker>>, read_write_tracker: RefCell, - whitelist: RefCell>>, + whitelist: RefCell>, } impl BenchmarkingState { @@ -155,15 +158,14 @@ impl BenchmarkingState { fn add_whitelist_to_tracker(&self) { let mut key_tracker = self.key_tracker.borrow_mut(); - let whitelisted = KeyTracker { - has_been_read: true, - has_been_written: true, - }; - let whitelist = self.whitelist.borrow(); whitelist.iter().for_each(|key| { - key_tracker.insert(key.to_vec(), whitelisted); + let whitelisted = KeyTracker { + has_been_read: key.has_been_read, + has_been_written: key.has_been_written, + }; + key_tracker.insert(key.key.clone(), whitelisted); }); } @@ -181,18 +183,21 @@ impl BenchmarkingState { let maybe_tracker = key_tracker.get(key); - let has_been_read = KeyTracker { - has_been_read: true, - has_been_written: false, - }; - match maybe_tracker { None => { + let has_been_read = KeyTracker { + has_been_read: true, + has_been_written: false, + }; key_tracker.insert(key.to_vec(), has_been_read); read_write_tracker.add_read(); }, Some(tracker) => { if !tracker.has_been_read { + let has_been_read = KeyTracker { + has_been_read: true, + has_been_written: tracker.has_been_written, + }; key_tracker.insert(key.to_vec(), has_been_read); read_write_tracker.add_read(); } else { @@ -426,7 +431,11 @@ impl StateBackend> for BenchmarkingState { self.wipe_tracker() } - fn set_whitelist(&self, new: Vec>) { + fn get_whitelist(&self) -> Vec { + self.whitelist.borrow().to_vec() + } + + fn set_whitelist(&self, new: Vec) { *self.whitelist.borrow_mut() = new; } diff --git a/frame/balances/src/benchmarking.rs b/frame/balances/src/benchmarking.rs index 73547fe814aa2..21f43c7c63640 100644 --- a/frame/balances/src/benchmarking.rs +++ b/frame/balances/src/benchmarking.rs @@ -22,7 +22,7 @@ use super::*; use frame_system::RawOrigin; -use frame_benchmarking::{benchmarks, account}; +use frame_benchmarking::{benchmarks, account, whitelisted_caller}; use sp_runtime::traits::Bounded; use crate::Module as Balances; @@ -40,7 +40,7 @@ benchmarks! { // * Transfer will create the recipient account. transfer { let existential_deposit = T::ExistentialDeposit::get(); - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); // Give some multiple of the existential deposit + creation fee + transfer fee let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); @@ -60,7 +60,7 @@ benchmarks! { // * Both accounts exist and will continue to exist. #[extra] transfer_best_case { - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); let recipient: T::AccountId = account("recipient", 0, SEED); let recipient_lookup: ::Source = T::Lookup::unlookup(recipient.clone()); @@ -80,7 +80,7 @@ benchmarks! { // Benchmark `transfer_keep_alive` with the worst possible condition: // * The recipient account is created. transfer_keep_alive { - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); let recipient: T::AccountId = account("recipient", 0, SEED); let recipient_lookup: ::Source = T::Lookup::unlookup(recipient.clone()); diff --git a/frame/benchmarking/Cargo.toml b/frame/benchmarking/Cargo.toml index 37dcd85b5985f..917988a825fad 100644 --- a/frame/benchmarking/Cargo.toml +++ b/frame/benchmarking/Cargo.toml @@ -20,9 +20,13 @@ sp-runtime-interface = { version = "2.0.0-rc5", path = "../../primitives/runtime sp-runtime = { version = "2.0.0-rc5", path = "../../primitives/runtime", default-features = false } sp-std = { version = "2.0.0-rc5", path = "../../primitives/std", default-features = false } sp-io = { version = "2.0.0-rc5", path = "../../primitives/io", default-features = false } +sp-storage = { version = "2.0.0-rc5", path = "../../primitives/storage", default-features = false } frame-support = { version = "2.0.0-rc5", default-features = false, path = "../support" } frame-system = { version = "2.0.0-rc5", default-features = false, path = "../system" } +[dev-dependencies] +hex-literal = "0.2.1" + [features] default = [ "std" ] std = [ diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index 7ef274f25b157..cebdcbcfecd25 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -32,6 +32,7 @@ pub use sp_io::storage::root as storage_root; pub use sp_runtime::traits::Zero; pub use frame_support; pub use paste; +pub use sp_storage::TrackedStorageKey; /// Construct pallet benchmarks for weighing dispatchables. /// @@ -418,156 +419,220 @@ macro_rules! benchmarks_iter { #[doc(hidden)] macro_rules! benchmark_backend { // parsing arms - ( { $( $instance:ident )? } $name:ident { - $( $where_clause:tt )* - } { - $( $common:tt )* - } { - $( PRE { $( $pre_parsed:tt )* } )* - } { $eval:block } { + ( + { $( $instance:ident )? } + $name:ident + { $( $where_clause:tt )* } + { $( $common:tt )* } + { $( PRE { $( $pre_parsed:tt )* } )* } + { $eval:block } + { let $pre_id:tt : $pre_ty:ty = $pre_ex:expr; $( $rest:tt )* - } $postcode:block) => { + } + $postcode:block + ) => { $crate::benchmark_backend! { - { $( $instance)? } $name { $( $where_clause )* } { $( $common )* } { + { $( $instance)? } + $name + { $( $where_clause )* } + { $( $common )* } + { $( PRE { $( $pre_parsed )* } )* PRE { $pre_id , $pre_ty , $pre_ex } - } { $eval } { $( $rest )* } $postcode + } + { $eval } + { $( $rest )* } + $postcode } }; - ( { $( $instance:ident )? } $name:ident { - $( $where_clause:tt )* - } { - $( $common:tt )* - } { - $( $parsed:tt )* - } { $eval:block } { - let $param:ident in ( $param_from:expr ) .. $param_to:expr => $param_instancer:expr; - $( $rest:tt )* - } $postcode:block) => { + ( + { $( $instance:ident )? } + $name:ident + { $( $where_clause:tt )* } + { $( $common:tt )* } + { $( $parsed:tt )* } + { $eval:block } + { + let $param:ident in ( $param_from:expr ) .. $param_to:expr => $param_instancer:expr; + $( $rest:tt )* + } + $postcode:block + ) => { $crate::benchmark_backend! { - { $( $instance)? } $name { $( $where_clause )* } { $( $common )* } { + { $( $instance)? } + $name + { $( $where_clause )* } + { $( $common )* } + { $( $parsed )* PARAM { $param , $param_from , $param_to , $param_instancer } - } { $eval } { $( $rest )* } $postcode + } + { $eval } + { $( $rest )* } + $postcode } }; // mutation arm to look after defaulting to a common param - ( { $( $instance:ident )? } $name:ident { - $( $where_clause:tt )* - } { - $( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )* - } { - $( $parsed:tt )* - } { $eval:block } { - let $param:ident in ...; - $( $rest:tt )* - } $postcode:block) => { + ( + { $( $instance:ident )? } + $name:ident + { $( $where_clause:tt )* } + { $( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )* } + { $( $parsed:tt )* } + { $eval:block } + { + let $param:ident in ...; + $( $rest:tt )* + } + $postcode:block + ) => { $crate::benchmark_backend! { - { $( $instance)? } $name { $( $where_clause )* } { - $( { $common , $common_from , $common_to , $common_instancer } )* - } { - $( $parsed )* - } { $eval } { + { $( $instance)? } + $name + { $( $where_clause )* } + { $( { $common , $common_from , $common_to , $common_instancer } )* } + { $( $parsed )* } + { $eval } + { let $param in ({ $( let $common = $common_from; )* $param }) .. ({ $( let $common = $common_to; )* $param }) => ({ $( let $common = || -> Result<(), &'static str> { $common_instancer ; Ok(()) }; )* $param()? }); $( $rest )* - } $postcode + } + $postcode } }; // mutation arm to look after defaulting only the range to common param - ( { $( $instance:ident )? } $name:ident { - $( $where_clause:tt )* - } { - $( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )* - } { - $( $parsed:tt )* - } { $eval:block } { - let $param:ident in _ .. _ => $param_instancer:expr ; - $( $rest:tt )* - } $postcode:block) => { + ( + { $( $instance:ident )? } + $name:ident + { $( $where_clause:tt )* } + { $( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )* } + { $( $parsed:tt )* } + { $eval:block } + { + let $param:ident in _ .. _ => $param_instancer:expr ; + $( $rest:tt )* + } + $postcode:block + ) => { $crate::benchmark_backend! { - { $( $instance)? } $name { $( $where_clause )* } { - $( { $common , $common_from , $common_to , $common_instancer } )* - } { - $( $parsed )* - } { $eval } { + { $( $instance)? } + $name + { $( $where_clause )* } + { $( { $common , $common_from , $common_to , $common_instancer } )* } + { $( $parsed )* } + { $eval } + { let $param in ({ $( let $common = $common_from; )* $param }) .. ({ $( let $common = $common_to; )* $param }) => $param_instancer ; $( $rest )* - } $postcode + } + $postcode } }; // mutation arm to look after a single tt for param_from. - ( { $( $instance:ident )? } $name:ident { - $( $where_clause:tt )* - } { - $( $common:tt )* - } { - $( $parsed:tt )* - } { $eval:block } { - let $param:ident in $param_from:tt .. $param_to:expr => $param_instancer:expr ; - $( $rest:tt )* - } $postcode:block) => { + ( + { $( $instance:ident )? } + $name:ident + { $( $where_clause:tt )* } + { $( $common:tt )* } + { $( $parsed:tt )* } + { $eval:block } + { + let $param:ident in $param_from:tt .. $param_to:expr => $param_instancer:expr ; + $( $rest:tt )* + } + $postcode:block + ) => { $crate::benchmark_backend! { { $( $instance)? } - $name { $( $where_clause )* } { $( $common )* } { $( $parsed )* } { $eval } { + $name + { $( $where_clause )* } + { $( $common )* } + { $( $parsed )* } + { $eval } + { let $param in ( $param_from ) .. $param_to => $param_instancer; $( $rest )* - } $postcode + } + $postcode } }; // mutation arm to look after the default tail of `=> ()` - ( { $( $instance:ident )? } $name:ident { - $( $where_clause:tt )* - } { - $( $common:tt )* - } { - $( $parsed:tt )* - } { $eval:block } { - let $param:ident in $param_from:tt .. $param_to:expr; - $( $rest:tt )* - } $postcode:block) => { + ( + { $( $instance:ident )? } + $name:ident + { $( $where_clause:tt )* } + { $( $common:tt )* } + { $( $parsed:tt )* } + { $eval:block } + { + let $param:ident in $param_from:tt .. $param_to:expr; + $( $rest:tt )* + } + $postcode:block + ) => { $crate::benchmark_backend! { { $( $instance)? } - $name { $( $where_clause )* } { $( $common )* } { $( $parsed )* } { $eval } { + $name + { $( $where_clause )* } + { $( $common )* } + { $( $parsed )* } + { $eval } + { let $param in $param_from .. $param_to => (); $( $rest )* - } $postcode + } + $postcode } }; // mutation arm to look after `let _ =` - ( { $( $instance:ident )? } $name:ident { - $( $where_clause:tt )* - } { - $( $common:tt )* - } { - $( $parsed:tt )* - } { $eval:block } { - let $pre_id:tt = $pre_ex:expr; - $( $rest:tt )* - } $postcode:block) => { + ( + { $( $instance:ident )? } + $name:ident + { $( $where_clause:tt )* } + { $( $common:tt )* } + { $( $parsed:tt )* } + { $eval:block } + { + let $pre_id:tt = $pre_ex:expr; + $( $rest:tt )* + } + $postcode:block + ) => { $crate::benchmark_backend! { { $( $instance)? } - $name { $( $where_clause )* } { $( $common )* } { $( $parsed )* } { $eval } { + $name + { $( $where_clause )* } + { $( $common )* } + { $( $parsed )* } + { $eval } + { let $pre_id : _ = $pre_ex; $( $rest )* - } $postcode + } + $postcode } }; // actioning arm - ( { $( $instance:ident )? } $name:ident { - $( $where_clause:tt )* - } { - $( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )* - } { - $( PRE { $pre_id:tt , $pre_ty:ty , $pre_ex:expr } )* - $( PARAM { $param:ident , $param_from:expr , $param_to:expr , $param_instancer:expr } )* - } { $eval:block } { $( $post:tt )* } $postcode:block) => { + ( + { $( $instance:ident )? } + $name:ident + { $( $where_clause:tt )* } + { $( { $common:ident , $common_from:tt , $common_to:expr , $common_instancer:expr } )* } + { + $( PRE { $pre_id:tt , $pre_ty:ty , $pre_ex:expr } )* + $( PARAM { $param:ident , $param_from:expr , $param_to:expr , $param_instancer:expr } )* + } + { $eval:block } + { $( $post:tt )* } + $postcode:block + ) => { #[allow(non_camel_case_types)] struct $name; #[allow(unused_variables)] @@ -726,7 +791,7 @@ macro_rules! impl_benchmark { highest_range_values: &[u32], steps: &[u32], repeat: u32, - whitelist: &[Vec] + whitelist: &[$crate::TrackedStorageKey] ) -> Result, &'static str> { // Map the input to the selected benchmark. let extrinsic = sp_std::str::from_utf8(extrinsic) @@ -736,8 +801,14 @@ macro_rules! impl_benchmark { _ => return Err("Could not find extrinsic."), }; - // Add whitelist to DB - $crate::benchmarking::set_whitelist(whitelist.to_vec()); + // Add whitelist to DB including whitelisted caller + let mut whitelist = whitelist.to_vec(); + let whitelisted_caller_key = + as frame_support::storage::StorageMap<_,_>>::hashed_key_for( + $crate::whitelisted_caller::() + ); + whitelist.push(whitelisted_caller_key.into()); + $crate::benchmarking::set_whitelist(whitelist); // Warm up the DB $crate::benchmarking::commit_db(); @@ -947,19 +1018,25 @@ macro_rules! impl_benchmark_test { /// let params = (&pallet, &benchmark, &lowest_range_values, &highest_range_values, &steps, repeat, &whitelist); /// ``` /// -/// The `whitelist` is a `Vec>` of storage keys that you would like to skip for DB tracking. For example: +/// The `whitelist` is a parameter you pass to control the DB read/write tracking. +/// We use a vector of [TrackedStorageKey](./struct.TrackedStorageKey.html), which is a simple struct used to set +/// if a key has been read or written to. /// -/// ```ignore -/// let whitelist: Vec> = vec![ +/// For values that should be skipped entirely, we can just pass `key.into()`. For example: +/// +/// ``` +/// use frame_benchmarking::TrackedStorageKey; +/// let whitelist: Vec = vec![ /// // Block Number -/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec(), +/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(), /// // Total Issuance -/// hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec(), +/// hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec().into(), /// // Execution Phase -/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec(), +/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec().into(), /// // Event Count -/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec(), +/// hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec().into(), /// ]; +/// ``` /// /// Then define a mutable local variable to hold your `BenchmarkBatch` object: /// diff --git a/frame/benchmarking/src/tests.rs b/frame/benchmarking/src/tests.rs index 6a4dc7eee4ee7..127645d430564 100644 --- a/frame/benchmarking/src/tests.rs +++ b/frame/benchmarking/src/tests.rs @@ -20,7 +20,6 @@ #![cfg(test)] use super::*; -use codec::Decode; use sp_std::prelude::*; use sp_runtime::{traits::{BlakeTwo256, IdentityLookup}, testing::{H256, Header}}; use frame_support::{ @@ -64,12 +63,10 @@ pub trait OtherTrait { type OtherEvent; } -pub trait Trait: OtherTrait where Self::OtherEvent: Into { +pub trait Trait: frame_system::Trait + OtherTrait + where Self::OtherEvent: Into<::Event> +{ type Event; - type BlockNumber; - type AccountId: 'static + Default + Decode; - type Origin: From> + - Into, Self::Origin>>; } #[derive(Clone, Eq, PartialEq)] @@ -105,9 +102,6 @@ impl frame_system::Trait for Test { impl Trait for Test { type Event = (); - type BlockNumber = u32; - type Origin = Origin; - type AccountId = u64; } impl OtherTrait for Test { diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index 5a2bd55ff79aa..8c25f035802fc 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -21,6 +21,7 @@ use codec::{Encode, Decode}; use sp_std::{vec::Vec, prelude::Box}; use sp_io::hashing::blake2_256; use sp_runtime::RuntimeString; +use sp_storage::TrackedStorageKey; /// An alphabet of possible parameters to use for benchmarking. #[derive(Encode, Decode, Clone, Copy, PartialEq, Debug)] @@ -101,19 +102,52 @@ pub trait Benchmarking { self.commit() } - /// Get the read/write count + /// Get the read/write count. fn read_write_count(&self) -> (u32, u32, u32, u32) { self.read_write_count() } - /// Reset the read/write count + /// Reset the read/write count. fn reset_read_write_count(&mut self) { self.reset_read_write_count() } - fn set_whitelist(&mut self, new: Vec>) { + /// Get the DB whitelist. + fn get_whitelist(&self) -> Vec { + self.get_whitelist() + } + + /// Set the DB whitelist. + fn set_whitelist(&mut self, new: Vec) { self.set_whitelist(new) } + + // Add a new item to the DB whitelist. + fn add_to_whitelist(&mut self, add: TrackedStorageKey) { + let mut whitelist = self.get_whitelist(); + match whitelist.iter_mut().find(|x| x.key == add.key) { + // If we already have this key in the whitelist, update to be the most constrained value. + Some(item) => { + *item = TrackedStorageKey { + key: add.key, + has_been_read: item.has_been_read || add.has_been_read, + has_been_written: item.has_been_written || add.has_been_written, + } + }, + // If the key does not exist, add it. + None => { + whitelist.push(add); + } + } + self.set_whitelist(whitelist); + } + + // Remove an item from the DB whitelist. + fn remove_from_whitelist(&mut self, remove: Vec) { + let mut whitelist = self.get_whitelist(); + whitelist.retain(|x| x.key != remove); + self.set_whitelist(whitelist); + } } /// The pallet benchmarking trait. @@ -141,7 +175,7 @@ pub trait Benchmarking { highest_range_values: &[u32], steps: &[u32], repeat: u32, - whitelist: &[Vec] + whitelist: &[TrackedStorageKey] ) -> Result, &'static str>; } @@ -165,3 +199,8 @@ pub fn account(name: &'static str, index: u32, seed let entropy = (name, index, seed).using_encoded(blake2_256); AccountId::decode(&mut &entropy[..]).unwrap_or_default() } + +/// This caller account is automatically whitelisted for DB reads/writes by the benchmarking macro. +pub fn whitelisted_caller() -> AccountId { + account::("whitelisted_caller", 0, 0) +} diff --git a/frame/collective/src/benchmarking.rs b/frame/collective/src/benchmarking.rs index b9558d8c8ce1c..2c777fadc4cc4 100644 --- a/frame/collective/src/benchmarking.rs +++ b/frame/collective/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; use frame_system::RawOrigin as SystemOrigin; use frame_system::EventRecord; -use frame_benchmarking::{benchmarks_instance, account}; +use frame_benchmarking::{benchmarks_instance, account, whitelisted_caller}; use sp_runtime::traits::Bounded; use sp_std::mem::size_of; @@ -123,7 +123,7 @@ benchmarks_instance! { members.push(member); } - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); Collective::::set_members(SystemOrigin::Root.into(), members, None, MAX_MEMBERS)?; @@ -153,7 +153,7 @@ benchmarks_instance! { members.push(member); } - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); Collective::::set_members(SystemOrigin::Root.into(), members, None, MAX_MEMBERS)?; @@ -184,7 +184,7 @@ benchmarks_instance! { let member = account("member", i, SEED); members.push(member); } - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); Collective::::set_members(SystemOrigin::Root.into(), members, None, MAX_MEMBERS)?; @@ -377,7 +377,7 @@ benchmarks_instance! { let member = account("member", i, SEED); members.push(member); } - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); Collective::::set_members(SystemOrigin::Root.into(), members.clone(), None, MAX_MEMBERS)?; @@ -458,7 +458,7 @@ benchmarks_instance! { let member = account("member", i, SEED); members.push(member); } - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); Collective::::set_members( SystemOrigin::Root.into(), @@ -530,7 +530,7 @@ benchmarks_instance! { let member = account("member", i, SEED); members.push(member); } - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); members.push(caller.clone()); Collective::::set_members( SystemOrigin::Root.into(), diff --git a/frame/indices/src/benchmarking.rs b/frame/indices/src/benchmarking.rs index a6b543bb43f4a..e8465c44cdc16 100644 --- a/frame/indices/src/benchmarking.rs +++ b/frame/indices/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; use frame_system::RawOrigin; -use frame_benchmarking::{benchmarks, account}; +use frame_benchmarking::{benchmarks, account, whitelisted_caller}; use sp_runtime::traits::Bounded; use crate::Module as Indices; @@ -35,7 +35,7 @@ benchmarks! { // Index being claimed let i in 0 .. 1000; let account_index = T::AccountIndex::from(i); - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); }: _(RawOrigin::Signed(caller.clone()), account_index) verify { @@ -47,7 +47,7 @@ benchmarks! { let i in 0 .. 1000; let account_index = T::AccountIndex::from(i); // Setup accounts - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); let recipient: T::AccountId = account("recipient", i, SEED); T::Currency::make_free_balance_be(&recipient, BalanceOf::::max_value()); @@ -63,7 +63,7 @@ benchmarks! { let i in 0 .. 1000; let account_index = T::AccountIndex::from(i); // Setup accounts - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); // Claim the index Indices::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; @@ -93,7 +93,7 @@ benchmarks! { let i in 0 .. 1000; let account_index = T::AccountIndex::from(i); // Setup accounts - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); // Claim the index Indices::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; diff --git a/frame/proxy/src/benchmarking.rs b/frame/proxy/src/benchmarking.rs index 3cbe517dfd7da..f68a2c3a4cd86 100644 --- a/frame/proxy/src/benchmarking.rs +++ b/frame/proxy/src/benchmarking.rs @@ -21,14 +21,14 @@ use super::*; use frame_system::RawOrigin; -use frame_benchmarking::{benchmarks, account}; +use frame_benchmarking::{benchmarks, account, whitelisted_caller}; use sp_runtime::traits::Bounded; use crate::Module as Proxy; const SEED: u32 = 0; fn add_proxies(n: u32, maybe_who: Option) -> Result<(), &'static str> { - let caller = maybe_who.unwrap_or_else(|| account("caller", 0, SEED)); + let caller = maybe_who.unwrap_or_else(|| whitelisted_caller()); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); for i in 0..n { Proxy::::add_proxy( @@ -50,35 +50,35 @@ benchmarks! { // In this case the caller is the "target" proxy let caller: T::AccountId = account("target", p - 1, SEED); // ... and "real" is the traditional caller. This is not a typo. - let real: T::AccountId = account("caller", 0, SEED); + let real: T::AccountId = whitelisted_caller(); let call: ::Call = frame_system::Call::::remark(vec![]).into(); }: _(RawOrigin::Signed(caller), real, Some(T::ProxyType::default()), Box::new(call)) add_proxy { let p in ...; - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); }: _(RawOrigin::Signed(caller), account("target", T::MaxProxies::get().into(), SEED), T::ProxyType::default()) remove_proxy { let p in ...; - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); }: _(RawOrigin::Signed(caller), account("target", 0, SEED), T::ProxyType::default()) remove_proxies { let p in ...; - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); }: _(RawOrigin::Signed(caller)) anonymous { let p in ...; - }: _(RawOrigin::Signed(account("caller", 0, SEED)), T::ProxyType::default(), 0) + }: _(RawOrigin::Signed(whitelisted_caller()), T::ProxyType::default(), 0) kill_anonymous { let p in 0 .. (T::MaxProxies::get() - 2).into(); - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - Module::::anonymous(RawOrigin::Signed(account("caller", 0, SEED)).into(), T::ProxyType::default(), 0)?; + Module::::anonymous(RawOrigin::Signed(whitelisted_caller()).into(), T::ProxyType::default(), 0)?; let height = system::Module::::block_number(); let ext_index = system::Module::::extrinsic_index().unwrap_or(0); let anon = Module::::anonymous_account(&caller, &T::ProxyType::default(), 0, None); diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index aab92ef4ce5a1..77eecb2ef04d5 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -23,7 +23,7 @@ use testing_utils::*; use sp_runtime::traits::One; use frame_system::RawOrigin; -pub use frame_benchmarking::{benchmarks, account}; +pub use frame_benchmarking::{benchmarks, account, whitelisted_caller}; const SEED: u32 = 0; const MAX_SPANS: u32 = 100; const MAX_VALIDATORS: u32 = 1000; @@ -280,7 +280,7 @@ benchmarks! { let validator = create_validator_with_nominators::(n, T::MaxNominatorRewardedPerValidator::get() as u32, true)?; let current_era = CurrentEra::get().unwrap(); - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); let balance_before = T::Currency::free_balance(&validator); }: _(RawOrigin::Signed(caller), validator.clone(), current_era) verify { @@ -294,7 +294,7 @@ benchmarks! { let validator = create_validator_with_nominators::(n, T::MaxNominatorRewardedPerValidator::get() as u32, false)?; let current_era = CurrentEra::get().unwrap(); - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); let balance_before = T::Currency::free_balance(&validator); }: payout_stakers(RawOrigin::Signed(caller), validator.clone(), current_era) verify { @@ -419,7 +419,7 @@ benchmarks! { let total_payout = T::Currency::minimum_balance() * 1000.into(); >::insert(current_era, total_payout); - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); }: { for arg in payout_calls_arg { >::payout_stakers(RawOrigin::Signed(caller.clone()).into(), arg.0, arg.1)?; @@ -471,6 +471,10 @@ benchmarks! { let era = >::current_era().unwrap_or(0); let caller: T::AccountId = account("caller", n, SEED); + + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); }: { let result = >::submit_election_solution( RawOrigin::Signed(caller.clone()).into(), @@ -532,6 +536,10 @@ benchmarks! { let era = >::current_era().unwrap_or(0); let caller: T::AccountId = account("caller", n, SEED); + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); + // submit a very bad solution on-chain { // this is needed to fool the chain to accept this solution. @@ -584,6 +592,10 @@ benchmarks! { let caller: T::AccountId = account("caller", n, SEED); let era = >::current_era().unwrap_or(0); + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); + // submit a seq-phragmen with all the good stuff on chain. { let (winners, compact, score, size) = get_seq_phragmen_solution::(true); diff --git a/frame/system/benchmarking/src/lib.rs b/frame/system/benchmarking/src/lib.rs index a3e7797996ac9..653d9536f1797 100644 --- a/frame/system/benchmarking/src/lib.rs +++ b/frame/system/benchmarking/src/lib.rs @@ -24,15 +24,13 @@ use sp_std::vec; use sp_std::prelude::*; use sp_core::{ChangesTrieConfiguration, storage::well_known_keys}; use sp_runtime::traits::Hash; -use frame_benchmarking::{benchmarks, account}; +use frame_benchmarking::{benchmarks, whitelisted_caller}; use frame_support::traits::Get; use frame_support::storage::{self, StorageMap}; use frame_system::{Module as System, Call, RawOrigin, DigestItemOf, AccountInfo}; mod mock; -const SEED: u32 = 0; - pub struct Module(System); pub trait Trait: frame_system::Trait {} @@ -42,7 +40,7 @@ benchmarks! { remark { let b in 0 .. T::MaximumBlockLength::get(); let remark_message = vec![1; b as usize]; - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); }: _(RawOrigin::Signed(caller), remark_message) set_heap_pages { @@ -139,7 +137,7 @@ benchmarks! { } suicide { - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); let account_info = AccountInfo:: { nonce: 1337.into(), refcount: 0, diff --git a/frame/timestamp/src/benchmarking.rs b/frame/timestamp/src/benchmarking.rs index 9b1c976229e66..1cd0f15ca01b9 100644 --- a/frame/timestamp/src/benchmarking.rs +++ b/frame/timestamp/src/benchmarking.rs @@ -23,7 +23,7 @@ use super::*; use sp_std::prelude::*; use frame_system::RawOrigin; use frame_support::{ensure, traits::OnFinalize}; -use frame_benchmarking::benchmarks; +use frame_benchmarking::{benchmarks, TrackedStorageKey}; use crate::Module as Timestamp; @@ -34,8 +34,14 @@ benchmarks! { set { let t in 1 .. MAX_TIME; + // Ignore write to `DidUpdate` since it transient. + let did_update_key = crate::DidUpdate::hashed_key().to_vec(); + frame_benchmarking::benchmarking::add_to_whitelist(TrackedStorageKey { + key: did_update_key, + has_been_read: false, + has_been_written: true, + }); }: _(RawOrigin::None, t.into()) - verify { ensure!(Timestamp::::now() == t.into(), "Time was not set."); } @@ -44,8 +50,10 @@ benchmarks! { let t in 1 .. MAX_TIME; Timestamp::::set(RawOrigin::None.into(), t.into())?; ensure!(DidUpdate::exists(), "Time was not set."); + // Ignore read/write to `DidUpdate` since it is transient. + let did_update_key = crate::DidUpdate::hashed_key().to_vec(); + frame_benchmarking::benchmarking::add_to_whitelist(did_update_key.into()); }: { Timestamp::::on_finalize(t.into()); } - verify { ensure!(!DidUpdate::exists(), "Time was not removed."); } diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 8dddf3581aef2..295326e1639ad 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -22,7 +22,7 @@ use super::*; use frame_system::RawOrigin; -use frame_benchmarking::{benchmarks, account}; +use frame_benchmarking::{benchmarks, account, whitelisted_caller}; use frame_support::traits::OnInitialize; use crate::Module as Treasury; @@ -45,7 +45,7 @@ fn setup_proposal(u: u32) -> ( // Create the pre-requisite information needed to create a `report_awesome`. fn setup_awesome(length: u32) -> (T::AccountId, Vec, T::AccountId) { - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); let value = T::TipReportDepositBase::get() + T::TipReportDepositPerByte::get() * length.into() + T::Currency::minimum_balance(); @@ -116,6 +116,9 @@ benchmarks! { propose_spend { let u in 0 .. 1000; let (caller, value, beneficiary_lookup) = setup_proposal::(u); + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); }: _(RawOrigin::Signed(caller), value, beneficiary_lookup) reject_proposal { @@ -143,6 +146,9 @@ benchmarks! { report_awesome { let r in 0 .. MAX_BYTES; let (caller, reason, awesome_person) = setup_awesome::(r); + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); }: _(RawOrigin::Signed(caller), reason, awesome_person) retract_tip { @@ -155,6 +161,9 @@ benchmarks! { )?; let reason_hash = T::Hashing::hash(&reason[..]); let hash = T::Hashing::hash_of(&(&reason_hash, &awesome_person)); + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); }: _(RawOrigin::Signed(caller), hash) tip_new { @@ -162,6 +171,9 @@ benchmarks! { let t in 1 .. MAX_TIPPERS; let (caller, reason, beneficiary, value) = setup_tip::(r, t)?; + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); }: _(RawOrigin::Signed(caller), reason, beneficiary, value) tip { @@ -179,6 +191,9 @@ benchmarks! { ensure!(Tips::::contains_key(hash), "tip does not exist"); create_tips::(t - 1, hash.clone(), value)?; let caller = account("member", t - 1, SEED); + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); }: _(RawOrigin::Signed(caller), hash, value) close_tip { @@ -206,6 +221,9 @@ benchmarks! { create_tips::(t, hash.clone(), value)?; let caller = account("caller", t, SEED); + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); }: _(RawOrigin::Signed(caller), hash) on_initialize { diff --git a/frame/utility/src/benchmarking.rs b/frame/utility/src/benchmarking.rs index 155a279807afc..8ca0e216f2887 100644 --- a/frame/utility/src/benchmarking.rs +++ b/frame/utility/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; use frame_system::{RawOrigin, EventRecord}; -use frame_benchmarking::{benchmarks, account}; +use frame_benchmarking::{benchmarks, account, whitelisted_caller}; const SEED: u32 = 0; @@ -43,7 +43,7 @@ benchmarks! { let call = frame_system::Call::remark(vec![]).into(); calls.push(call); } - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); }: _(RawOrigin::Signed(caller), calls) verify { assert_last_event::(Event::BatchCompleted.into()) @@ -53,6 +53,9 @@ benchmarks! { let u in 0 .. 1000; let caller = account("caller", u, SEED); let call = Box::new(frame_system::Call::remark(vec![]).into()); + // Whitelist caller account from further DB operations. + let caller_key = frame_system::Account::::hashed_key_for(&caller); + frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); }: _(RawOrigin::Signed(caller), u as u16, call) } diff --git a/frame/vesting/src/benchmarking.rs b/frame/vesting/src/benchmarking.rs index 24cdc28c97f20..974289aac3218 100644 --- a/frame/vesting/src/benchmarking.rs +++ b/frame/vesting/src/benchmarking.rs @@ -22,7 +22,7 @@ use super::*; use frame_system::{RawOrigin, Module as System}; -use frame_benchmarking::{benchmarks, account}; +use frame_benchmarking::{benchmarks, account, whitelisted_caller}; use sp_runtime::traits::Bounded; use crate::Module as Vesting; @@ -64,7 +64,7 @@ benchmarks! { vest_locked { let l in 0 .. MAX_LOCKS; - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); add_locks::(&caller, l as u8); add_vesting_schedule::(&caller)?; @@ -88,7 +88,7 @@ benchmarks! { vest_unlocked { let l in 0 .. MAX_LOCKS; - let caller = account("caller", 0, SEED); + let caller = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); add_locks::(&caller, l as u8); add_vesting_schedule::(&caller)?; @@ -125,7 +125,7 @@ benchmarks! { "Vesting schedule not added", ); - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); }: vest_other(RawOrigin::Signed(caller.clone()), other_lookup) verify { // Nothing happened since everything is still vested. @@ -152,7 +152,7 @@ benchmarks! { "Vesting schedule still active", ); - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); }: vest_other(RawOrigin::Signed(caller.clone()), other_lookup) verify { // Vesting schedule is removed! @@ -166,7 +166,7 @@ benchmarks! { vested_transfer { let l in 0 .. MAX_LOCKS; - let caller: T::AccountId = account("caller", 0, SEED); + let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); let target: T::AccountId = account("target", 0, SEED); let target_lookup: ::Source = T::Lookup::unlookup(target.clone()); diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs index 8e141867195b7..01570e0bfadd3 100644 --- a/primitives/externalities/src/lib.rs +++ b/primitives/externalities/src/lib.rs @@ -25,7 +25,7 @@ use std::any::{Any, TypeId}; -use sp_storage::ChildInfo; +use sp_storage::{ChildInfo, TrackedStorageKey}; pub use scope_limited::{set_and_run_with_externalities, with_externalities}; pub use extensions::{Extension, Extensions, ExtensionStore}; @@ -248,12 +248,19 @@ pub trait Externalities: ExtensionStore { /// Resets read/write count for the benchmarking process. fn reset_read_write_count(&mut self); + /// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + /// Benchmarking related functionality and shouldn't be used anywhere else! + /// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + /// + /// Gets the current DB tracking whitelist. + fn get_whitelist(&self) -> Vec; + /// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! /// Benchmarking related functionality and shouldn't be used anywhere else! /// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! /// /// Adds new storage keys to the DB tracking whitelist. - fn set_whitelist(&mut self, new: Vec>); + fn set_whitelist(&mut self, new: Vec); } /// Extension for the [`Externalities`] trait. diff --git a/primitives/runtime-interface/Cargo.toml b/primitives/runtime-interface/Cargo.toml index 16d5a14e889b1..f16000bff4924 100644 --- a/primitives/runtime-interface/Cargo.toml +++ b/primitives/runtime-interface/Cargo.toml @@ -21,6 +21,7 @@ sp-externalities = { version = "0.8.0-rc5", optional = true, path = "../external codec = { package = "parity-scale-codec", version = "1.3.1", default-features = false } static_assertions = "1.0.0" primitive-types = { version = "0.7.0", default-features = false } +sp-storage = { version = "2.0.0-rc5", default-features = false, path = "../storage" } [dev-dependencies] sp-runtime-interface-test-wasm = { version = "2.0.0-rc5", path = "test-wasm" } diff --git a/primitives/runtime-interface/src/impls.rs b/primitives/runtime-interface/src/impls.rs index 259d3517f001d..da57cf086beef 100644 --- a/primitives/runtime-interface/src/impls.rs +++ b/primitives/runtime-interface/src/impls.rs @@ -537,3 +537,7 @@ impl PassBy for sp_wasm_interface::ValueType { impl PassBy for sp_wasm_interface::Value { type PassBy = Codec; } + +impl PassBy for sp_storage::TrackedStorageKey { + type PassBy = Codec; +} diff --git a/primitives/state-machine/src/backend.rs b/primitives/state-machine/src/backend.rs index 9ec03c4d1e249..cfff2c6fc6967 100644 --- a/primitives/state-machine/src/backend.rs +++ b/primitives/state-machine/src/backend.rs @@ -19,7 +19,10 @@ use hash_db::Hasher; use codec::{Decode, Encode}; -use sp_core::{traits::RuntimeCode, storage::{ChildInfo, well_known_keys}}; +use sp_core::{ + traits::RuntimeCode, + storage::{ChildInfo, well_known_keys, TrackedStorageKey} +}; use crate::{ trie_backend::TrieBackend, trie_backend_essence::TrieBackendStorage, @@ -226,10 +229,13 @@ pub trait Backend: std::fmt::Debug { unimplemented!() } - /// Update the whitelist for tracking db reads/writes - fn set_whitelist(&self, _: Vec>) { - unimplemented!() + /// Get the whitelist for tracking db reads/writes + fn get_whitelist(&self) -> Vec { + Default::default() } + + /// Update the whitelist for tracking db reads/writes + fn set_whitelist(&self, _: Vec) {} } impl<'a, T: Backend, H: Hasher> Backend for &'a T { diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index 3ddf79dbd9127..3db7a54750a02 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -27,7 +27,7 @@ use sp_trie::trie_types::Layout; use sp_core::{ storage::{ well_known_keys::is_child_storage_key, Storage, - ChildInfo, StorageChild, + ChildInfo, StorageChild, TrackedStorageKey, }, traits::Externalities, Blake2Hasher, }; @@ -325,7 +325,11 @@ impl Externalities for BasicExternalities { unimplemented!("reset_read_write_count is not supported in Basic") } - fn set_whitelist(&mut self, _: Vec>) { + fn get_whitelist(&self) -> Vec { + unimplemented!("get_whitelist is not supported in Basic") + } + + fn set_whitelist(&mut self, _: Vec) { unimplemented!("set_whitelist is not supported in Basic") } } diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index d7d4bc145eb06..e57636b300a5f 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -26,7 +26,7 @@ use crate::{ use hash_db::Hasher; use sp_core::{ offchain::storage::OffchainOverlayedChanges, - storage::{well_known_keys::is_child_storage_key, ChildInfo}, + storage::{well_known_keys::is_child_storage_key, ChildInfo, TrackedStorageKey}, traits::Externalities, hexdisplay::HexDisplay, }; use sp_trie::{trie_types::Layout, empty_child_trie_root}; @@ -609,7 +609,11 @@ where self.backend.reset_read_write_count() } - fn set_whitelist(&mut self, new: Vec>) { + fn get_whitelist(&self) -> Vec { + self.backend.get_whitelist() + } + + fn set_whitelist(&mut self, new: Vec) { self.backend.set_whitelist(new) } } diff --git a/primitives/state-machine/src/read_only.rs b/primitives/state-machine/src/read_only.rs index b8a35ced1eb0c..99023ec772ec3 100644 --- a/primitives/state-machine/src/read_only.rs +++ b/primitives/state-machine/src/read_only.rs @@ -24,7 +24,7 @@ use std::{ use crate::{Backend, StorageKey, StorageValue}; use hash_db::Hasher; use sp_core::{ - storage::ChildInfo, + storage::{ChildInfo, TrackedStorageKey}, traits::Externalities, Blake2Hasher, }; use codec::Encode; @@ -194,7 +194,11 @@ impl<'a, H: Hasher, B: 'a + Backend> Externalities for ReadOnlyExternalities< unimplemented!("reset_read_write_count is not supported in ReadOnlyExternalities") } - fn set_whitelist(&mut self, _: Vec>) { + fn get_whitelist(&self) -> Vec { + unimplemented!("get_whitelist is not supported in ReadOnlyExternalities") + } + + fn set_whitelist(&mut self, _: Vec) { unimplemented!("set_whitelist is not supported in ReadOnlyExternalities") } } diff --git a/primitives/storage/Cargo.toml b/primitives/storage/Cargo.toml index cb7f2daa50e83..46d76fd7d2832 100644 --- a/primitives/storage/Cargo.toml +++ b/primitives/storage/Cargo.toml @@ -18,7 +18,8 @@ serde = { version = "1.0.101", optional = true, features = ["derive"] } impl-serde = { version = "0.2.3", optional = true } ref-cast = "1.0.0" sp-debug-derive = { version = "2.0.0-rc5", path = "../debug-derive" } +codec = { package = "parity-scale-codec", version = "1.3.1", default-features = false, features = ["derive"] } [features] default = [ "std" ] -std = [ "sp-std/std", "serde", "impl-serde" ] +std = [ "sp-std/std", "serde", "impl-serde", "codec/std" ] diff --git a/primitives/storage/src/lib.rs b/primitives/storage/src/lib.rs index 073d80291c13e..b253733e7b29e 100644 --- a/primitives/storage/src/lib.rs +++ b/primitives/storage/src/lib.rs @@ -25,6 +25,7 @@ use sp_debug_derive::RuntimeDebug; use sp_std::{vec::Vec, ops::{Deref, DerefMut}}; use ref_cast::RefCast; +use codec::{Encode, Decode}; /// Storage key. #[derive(PartialEq, Eq, RuntimeDebug)] @@ -34,6 +35,26 @@ pub struct StorageKey( pub Vec, ); +/// Storage key with read/write tracking information. +#[derive(PartialEq, Eq, RuntimeDebug, Clone, Encode, Decode)] +#[cfg_attr(feature = "std", derive(Hash, PartialOrd, Ord))] +pub struct TrackedStorageKey { + pub key: Vec, + pub has_been_read: bool, + pub has_been_written: bool, +} + +// Easily convert a key to a `TrackedStorageKey` that has been read and written to. +impl From> for TrackedStorageKey { + fn from(key: Vec) -> Self { + Self { + key: key, + has_been_read: true, + has_been_written: true, + } + } +} + /// Storage key of a child trie, it contains the prefix to the key. #[derive(PartialEq, Eq, RuntimeDebug)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize, Hash, PartialOrd, Ord, Clone))]