diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index e28a3bb2adb9d..fd82b92cae64c 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -514,18 +514,8 @@ impl_runtime_apis! { impl frame_system_benchmarking::Config for Runtime {} impl baseline::Config for Runtime {} - let whitelist: Vec = vec![ - // Block Number - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(), - // Total Issuance - hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec().into(), - // Execution Phase - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec().into(), - // Event Count - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec().into(), - // System Events - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7").to_vec().into(), - ]; + use frame_support::traits::WhitelistedStorageKeys; + let whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); let mut batches = Vec::::new(); let params = (&config, &whitelist); @@ -556,3 +546,40 @@ impl_runtime_apis! { } } } + +#[cfg(test)] +mod tests { + use super::*; + use frame_support::traits::WhitelistedStorageKeys; + use sp_core::hexdisplay::HexDisplay; + use std::collections::HashSet; + + #[test] + fn check_whitelist() { + let whitelist: HashSet = AllPalletsWithSystem::whitelisted_storage_keys() + .iter() + .map(|e| HexDisplay::from(&e.key).to_string()) + .collect(); + + // Block Number + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac") + ); + // Total Issuance + assert!( + whitelist.contains("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80") + ); + // Execution Phase + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a") + ); + // Event Count + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850") + ); + // System Events + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7") + ); + } +} diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 5aa488f328a5e..c6e733502f416 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2142,22 +2142,14 @@ impl_runtime_apis! { impl baseline::Config for Runtime {} impl pallet_nomination_pools_benchmarking::Config for Runtime {} - let whitelist: Vec = vec![ - // Block Number - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(), - // Total Issuance - hex_literal::hex!("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80").to_vec().into(), - // Execution Phase - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a").to_vec().into(), - // Event Count - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850").to_vec().into(), - // System Events - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7").to_vec().into(), - // System BlockWeight - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef734abf5cb34d6244378cddbf18e849d96").to_vec().into(), - // Treasury Account - hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into(), - ]; + use frame_support::traits::WhitelistedStorageKeys; + let mut whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); + + // Treasury Account + // TODO: this is manual for now, someday we might be able to use a + // macro for this particular key + let treasury_key = frame_system::Account::::hashed_key_for(Treasury::account_id()); + whitelist.push(treasury_key.to_vec().into()); let mut batches = Vec::::new(); let params = (&config, &whitelist); @@ -2171,8 +2163,44 @@ impl_runtime_apis! { mod tests { use super::*; use frame_election_provider_support::NposSolution; + use frame_support::traits::WhitelistedStorageKeys; use frame_system::offchain::CreateSignedTransaction; + use sp_core::hexdisplay::HexDisplay; use sp_runtime::UpperOf; + use std::collections::HashSet; + + #[test] + fn check_whitelist() { + let whitelist: HashSet = AllPalletsWithSystem::whitelisted_storage_keys() + .iter() + .map(|e| HexDisplay::from(&e.key).to_string()) + .collect(); + + // Block Number + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac") + ); + // Total Issuance + assert!( + whitelist.contains("c2261276cc9d1f8598ea4b6a74b15c2f57c875e4cff74148e4628f264b974c80") + ); + // Execution Phase + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef7ff553b5a9862a516939d82b3d3d8661a") + ); + // Event Count + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef70a98fdbe9ce6c55837576c60c7af3850") + ); + // System Events + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef780d41e5e16056765bc8461851072c9d7") + ); + // System BlockWeight + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef734abf5cb34d6244378cddbf18e849d96") + ); + } #[test] fn validate_transaction_submitter_bounds() { diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 0a45350366449..aceb92172582f 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -492,6 +492,7 @@ pub mod pallet { /// The total units issued in the system. #[pallet::storage] #[pallet::getter(fn total_issuance)] + #[pallet::whitelist_storage] pub type TotalIssuance, I: 'static = ()> = StorageValue<_, T::Balance, ValueQuery>; /// The Balances pallet example of storing the balance of an account. diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index f0fb6bacedffb..e5941a33fee13 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -166,6 +166,24 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { quote::quote! { #frame_support::traits::StorageVersion::default() } }; + let whitelisted_storage_idents: Vec = def + .storages + .iter() + .filter_map(|s| s.whitelisted.then_some(s.ident.clone())) + .collect(); + + let whitelisted_storage_keys_impl = quote::quote![ + use #frame_support::traits::{StorageInfoTrait, TrackedStorageKey, WhitelistedStorageKeys}; + impl<#type_impl_gen> WhitelistedStorageKeys for #pallet_ident<#type_use_gen> #storages_where_clauses { + fn whitelisted_storage_keys() -> #frame_support::sp_std::vec::Vec { + use #frame_support::sp_std::vec; + vec![#( + TrackedStorageKey::new(#whitelisted_storage_idents::<#type_use_gen>::hashed_key().to_vec()) + ),*] + } + } + ]; + quote::quote_spanned!(def.pallet_struct.attr_span => #pallet_error_metadata @@ -253,5 +271,6 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { } #storage_info + #whitelisted_storage_keys_impl ) } diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index f0e1353774910..321c4dd5d4914 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -28,6 +28,7 @@ mod keyword { syn::custom_keyword!(getter); syn::custom_keyword!(storage_prefix); syn::custom_keyword!(unbounded); + syn::custom_keyword!(whitelist_storage); syn::custom_keyword!(OptionQuery); syn::custom_keyword!(ResultQuery); syn::custom_keyword!(ValueQuery); @@ -37,16 +38,21 @@ mod keyword { /// * `#[pallet::getter(fn dummy)]` /// * `#[pallet::storage_prefix = "CustomName"]` /// * `#[pallet::unbounded]` +/// * `#[pallet::whitelist_storage] pub enum PalletStorageAttr { Getter(syn::Ident, proc_macro2::Span), StorageName(syn::LitStr, proc_macro2::Span), Unbounded(proc_macro2::Span), + WhitelistStorage(proc_macro2::Span), } impl PalletStorageAttr { fn attr_span(&self) -> proc_macro2::Span { match self { - Self::Getter(_, span) | Self::StorageName(_, span) | Self::Unbounded(span) => *span, + Self::Getter(_, span) | + Self::StorageName(_, span) | + Self::Unbounded(span) | + Self::WhitelistStorage(span) => *span, } } } @@ -84,6 +90,9 @@ impl syn::parse::Parse for PalletStorageAttr { content.parse::()?; Ok(Self::Unbounded(attr_span)) + } else if lookahead.peek(keyword::whitelist_storage) { + content.parse::()?; + Ok(Self::WhitelistStorage(attr_span)) } else { Err(lookahead.error()) } @@ -94,6 +103,7 @@ struct PalletStorageAttrInfo { getter: Option, rename_as: Option, unbounded: bool, + whitelisted: bool, } impl PalletStorageAttrInfo { @@ -101,12 +111,14 @@ impl PalletStorageAttrInfo { let mut getter = None; let mut rename_as = None; let mut unbounded = false; + let mut whitelisted = false; for attr in attrs { match attr { PalletStorageAttr::Getter(ident, ..) if getter.is_none() => getter = Some(ident), PalletStorageAttr::StorageName(name, ..) if rename_as.is_none() => rename_as = Some(name), PalletStorageAttr::Unbounded(..) if !unbounded => unbounded = true, + PalletStorageAttr::WhitelistStorage(..) if !whitelisted => whitelisted = true, attr => return Err(syn::Error::new( attr.attr_span(), @@ -115,7 +127,7 @@ impl PalletStorageAttrInfo { } } - Ok(PalletStorageAttrInfo { getter, rename_as, unbounded }) + Ok(PalletStorageAttrInfo { getter, rename_as, unbounded, whitelisted }) } } @@ -171,6 +183,8 @@ pub struct StorageDef { pub named_generics: Option, /// If the value stored in this storage is unbounded. pub unbounded: bool, + /// Whether or not reads to this storage key will be ignored by benchmarking + pub whitelisted: bool, } /// The parsed generic from the @@ -672,7 +686,7 @@ impl StorageDef { }; let attrs: Vec = helper::take_item_pallet_attrs(&mut item.attrs)?; - let PalletStorageAttrInfo { getter, rename_as, unbounded } = + let PalletStorageAttrInfo { getter, rename_as, unbounded, whitelisted } = PalletStorageAttrInfo::from_attrs(attrs)?; let cfg_attrs = helper::get_item_cfg_attrs(&item.attrs); @@ -814,6 +828,7 @@ impl StorageDef { cfg_attrs, named_generics, unbounded, + whitelisted, }) } } diff --git a/frame/support/procedural/src/storage/mod.rs b/frame/support/procedural/src/storage/mod.rs index 756653f7ba85d..e8e2d7529cb3f 100644 --- a/frame/support/procedural/src/storage/mod.rs +++ b/frame/support/procedural/src/storage/mod.rs @@ -32,6 +32,7 @@ pub(crate) use instance_trait::INHERENT_INSTANCE_NAME; use frame_support_procedural_tools::{ generate_crate_access, generate_hidden_includes, syn_ext as ext, }; + use quote::quote; /// All information contained in input of decl_storage diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 55ded6da9c5a1..b8b4635862c38 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1855,6 +1855,21 @@ pub mod pallet_prelude { /// pub(super) type MyStorage = StorageValue; /// ``` /// +/// The optional attribute `#[pallet::whitelist_storage]` will declare the +/// storage as whitelisted from benchmarking. Doing so will exclude reads of +/// that value's storage key from counting towards weight calculations during +/// benchmarking. +/// +/// This attribute should only be attached to storages that are known to be +/// read/used in every block. This will result in a more accurate benchmarking weight. +/// +/// ### Example +/// ```ignore +/// #[pallet::storage] +/// #[pallet::whitelist_storage] +/// pub(super) type Number = StorageValue<_, T::BlockNumber, ValueQuery>; +/// ``` +/// /// All the `cfg` attributes are automatically copied to the items generated for the storage, /// i.e. the getter, storage prefix, and the metadata element etc. /// diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index d4f0e73557c77..16ac7929831b4 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -89,6 +89,7 @@ pub mod schedule; mod storage; pub use storage::{ Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, + TrackedStorageKey, WhitelistedStorageKeys, }; mod dispatch; diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index d40d82c28e87e..24653d1899836 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -17,7 +17,9 @@ //! Traits for encoding data related to pallet's storage items. +use crate::sp_std::collections::btree_set::BTreeSet; use impl_trait_for_tuples::impl_for_tuples; +pub use sp_core::storage::TrackedStorageKey; use sp_std::prelude::*; /// An instance of a pallet in the storage. @@ -90,3 +92,29 @@ impl StorageInfoTrait for Tuple { pub trait PartialStorageInfoTrait { fn partial_storage_info() -> Vec; } + +/// Allows a pallet to specify storage keys to whitelist during benchmarking. +/// This means those keys will be excluded from the benchmarking performance +/// calculation. +pub trait WhitelistedStorageKeys { + /// Returns a [`Vec`] indicating the storage keys that + /// should be whitelisted during benchmarking. This means that those keys + /// will be excluded from the benchmarking performance calculation. + fn whitelisted_storage_keys() -> Vec; +} + +#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] +#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] +#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] +impl WhitelistedStorageKeys for Tuple { + fn whitelisted_storage_keys() -> Vec { + // de-duplicate the storage keys + let mut combined_keys: BTreeSet = BTreeSet::new(); + for_tuples!( #( + for storage_key in Tuple::whitelisted_storage_keys() { + combined_keys.insert(storage_key); + } + )* ); + combined_keys.into_iter().collect::>() + } +} diff --git a/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr b/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr index 6313bd691f943..80c6526bbf888 100644 --- a/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr +++ b/frame/support/test/tests/pallet_ui/storage_invalid_attribute.stderr @@ -1,4 +1,4 @@ -error: expected one of: `getter`, `storage_prefix`, `unbounded` +error: expected one of: `getter`, `storage_prefix`, `unbounded`, `whitelist_storage` --> $DIR/storage_invalid_attribute.rs:16:12 | 16 | #[pallet::generate_store(pub trait Store)] diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index ba076d963bda7..fdb2a2644020c 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -562,6 +562,7 @@ pub mod pallet { /// The current weight for the block. #[pallet::storage] + #[pallet::whitelist_storage] #[pallet::getter(fn block_weight)] pub(super) type BlockWeight = StorageValue<_, ConsumedWeight, ValueQuery>; @@ -584,6 +585,7 @@ pub mod pallet { /// The current block number being processed. Set by `execute_block`. #[pallet::storage] + #[pallet::whitelist_storage] #[pallet::getter(fn block_number)] pub(super) type Number = StorageValue<_, T::BlockNumber, ValueQuery>; @@ -606,12 +608,14 @@ pub mod pallet { /// Events have a large in-memory size. Box the events to not go out-of-memory /// just in case someone still reads them from within the runtime. #[pallet::storage] + #[pallet::whitelist_storage] #[pallet::unbounded] pub(super) type Events = StorageValue<_, Vec>>, ValueQuery>; /// The number of events in the `Events` list. #[pallet::storage] + #[pallet::whitelist_storage] #[pallet::getter(fn event_count)] pub(super) type EventCount = StorageValue<_, EventIndex, ValueQuery>; @@ -647,6 +651,7 @@ pub mod pallet { /// The execution phase of the block. #[pallet::storage] + #[pallet::whitelist_storage] pub(super) type ExecutionPhase = StorageValue<_, Phase>; #[cfg_attr(feature = "std", derive(Default))] diff --git a/primitives/storage/src/lib.rs b/primitives/storage/src/lib.rs index 0948cf431158d..9c046bf9d2dd1 100644 --- a/primitives/storage/src/lib.rs +++ b/primitives/storage/src/lib.rs @@ -47,8 +47,9 @@ impl AsRef<[u8]> for StorageKey { } /// Storage key with read/write tracking information. -#[derive(PartialEq, Eq, RuntimeDebug, Clone, Encode, Decode)] -#[cfg_attr(feature = "std", derive(Hash, PartialOrd, Ord))] +#[derive( + PartialEq, Eq, Ord, PartialOrd, sp_std::hash::Hash, RuntimeDebug, Clone, Encode, Decode, +)] pub struct TrackedStorageKey { pub key: Vec, pub reads: u32,