From f08d3ae24ae4263afd42ec4751e61efa120583e0 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 30 Aug 2022 04:05:45 -0400 Subject: [PATCH 01/72] initial setup --- frame/support/procedural/src/lib.rs | 11 +++++++++++ frame/support/procedural/src/storage/mod.rs | 1 + frame/support/src/storage/mod.rs | 2 ++ 3 files changed, 14 insertions(+) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 00204b7a4d906..ee74c3c1cae78 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -36,6 +36,7 @@ mod transactional; mod tt_macro; use proc_macro::TokenStream; +use syn::{ItemType, parse_macro_input}; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; @@ -583,3 +584,13 @@ pub fn storage_alias(_: TokenStream, input: TokenStream) -> TokenStream { .unwrap_or_else(|r| r.into_compile_error()) .into() } + +#[proc_macro_attribute] +pub fn cached(_attr: TokenStream, item: TokenStream) -> TokenStream { + let typ = item.clone(); + let typ: ItemType = parse_macro_input!(typ as ItemType); + let type_name = typ.ident.to_string(); + // need name of pallet + // use macro_state here + item +} 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/storage/mod.rs b/frame/support/src/storage/mod.rs index b70436785af12..9d2fbbf999900 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -1470,6 +1470,8 @@ pub fn storage_prefix(pallet_name: &[u8], storage_name: &[u8]) -> [u8; 32] { final_key } +pub use frame_support_procedural::cached; + #[cfg(test)] mod test { use super::*; From c381aeff0c714d1a11aa551a0ce2f4cb7e5b4d9c Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 30 Aug 2022 13:28:50 -0400 Subject: [PATCH 02/72] add WhitelistedStorageKeys trait --- frame/support/src/traits/storage.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index d40d82c28e87e..87f39a64411b3 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -18,6 +18,7 @@ //! Traits for encoding data related to pallet's storage items. use impl_trait_for_tuples::impl_for_tuples; +use sp_core::storage::TrackedStorageKey; use sp_std::prelude::*; /// An instance of a pallet in the storage. @@ -90,3 +91,13 @@ 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; +} From 1275c426ed9a113509a4609e240dbf90c5e57edb Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 31 Aug 2022 14:50:23 -0400 Subject: [PATCH 03/72] add (A, B) tuple implementation for whitelisted_storage_keys() --- frame/support/src/traits/storage.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 87f39a64411b3..142744a2c7846 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -20,6 +20,7 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_core::storage::TrackedStorageKey; use sp_std::prelude::*; +use crate::sp_std::collections::btree_set::BTreeSet; /// An instance of a pallet in the storage. /// @@ -101,3 +102,26 @@ pub trait WhitelistedStorageKeys { /// will be excluded from the benchmarking performance calculation. fn whitelisted_storage_keys() -> Vec; } + +impl WhitelistedStorageKeys for (A, B) +where + A: WhitelistedStorageKeys, + B: WhitelistedStorageKeys, +{ + fn whitelisted_storage_keys() -> Vec { + // use BTreeSet so the resulting list of keys is unique + let mut combined_keys = BTreeSet::new(); + for key in A::whitelisted_storage_keys() { + combined_keys.insert(key); + } + for key in B::whitelisted_storage_keys() { + combined_keys.insert(key); + } + // flatten BTreeSet down to a vec + let mut combined_keys_vec = Vec::new(); + for key in combined_keys { + combined_keys_vec.push(key); + } + combined_keys_vec + } +} From 84beb0fc66f94ef9540e8cf4358c525eeef3b28c Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 31 Aug 2022 15:18:40 -0400 Subject: [PATCH 04/72] fix formatting --- frame/support/procedural/src/lib.rs | 2 +- frame/support/src/traits/storage.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index ee74c3c1cae78..ddf8cfbcfd20d 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -36,9 +36,9 @@ mod transactional; mod tt_macro; use proc_macro::TokenStream; -use syn::{ItemType, parse_macro_input}; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; +use syn::{parse_macro_input, ItemType}; thread_local! { /// A global counter, can be used to generate a relatively unique identifier. diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 142744a2c7846..011ef5d60c582 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -17,10 +17,10 @@ //! 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; use sp_core::storage::TrackedStorageKey; use sp_std::prelude::*; -use crate::sp_std::collections::btree_set::BTreeSet; /// An instance of a pallet in the storage. /// @@ -100,15 +100,15 @@ 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; + fn whitelisted_storage_keys() -> Vec; } impl WhitelistedStorageKeys for (A, B) where - A: WhitelistedStorageKeys, - B: WhitelistedStorageKeys, + A: WhitelistedStorageKeys, + B: WhitelistedStorageKeys, { - fn whitelisted_storage_keys() -> Vec { + fn whitelisted_storage_keys() -> Vec { // use BTreeSet so the resulting list of keys is unique let mut combined_keys = BTreeSet::new(); for key in A::whitelisted_storage_keys() { @@ -123,5 +123,5 @@ where combined_keys_vec.push(key); } combined_keys_vec - } + } } From 3699c19af232fd06007142a5c9d9b75859b4f4be Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 31 Aug 2022 15:38:26 -0400 Subject: [PATCH 05/72] implement WhitelistedStorageKeys for all tuple combinations --- frame/support/src/traits/storage.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 011ef5d60c582..65ab9199a839e 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -103,20 +103,16 @@ pub trait WhitelistedStorageKeys { fn whitelisted_storage_keys() -> Vec; } -impl WhitelistedStorageKeys for (A, B) -where - A: WhitelistedStorageKeys, - B: WhitelistedStorageKeys, -{ +#[impl_for_tuples(5)] +impl WhitelistedStorageKeys for Tuple { fn whitelisted_storage_keys() -> Vec { // use BTreeSet so the resulting list of keys is unique let mut combined_keys = BTreeSet::new(); - for key in A::whitelisted_storage_keys() { - combined_keys.insert(key); - } - for key in B::whitelisted_storage_keys() { - combined_keys.insert(key); - } + for_tuples!( #( + for key in Tuple::whitelisted_storage_keys() { + combined_keys.insert(key); + } + )* ); // flatten BTreeSet down to a vec let mut combined_keys_vec = Vec::new(); for key in combined_keys { From 3056275f2d9c2afb8ee2425512dd25e8712cd0cf Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 31 Aug 2022 15:44:30 -0400 Subject: [PATCH 06/72] impl_for_tuples up to 128 for WhitelistedStorageKeys --- frame/support/src/traits/storage.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 65ab9199a839e..e5abf970f6454 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -103,7 +103,9 @@ pub trait WhitelistedStorageKeys { fn whitelisted_storage_keys() -> Vec; } -#[impl_for_tuples(5)] +#[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 { // use BTreeSet so the resulting list of keys is unique From adc75859bd0237a930adb4547cd76cd13cd20d65 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 31 Aug 2022 16:51:08 -0400 Subject: [PATCH 07/72] refactor to #[benchmarking(cached)] --- frame/support/procedural/src/lib.rs | 16 +++++++++------- frame/support/src/lib.rs | 3 +++ frame/support/src/storage/mod.rs | 2 -- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index ddf8cfbcfd20d..906a68bf10740 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -38,7 +38,7 @@ mod tt_macro; use proc_macro::TokenStream; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; -use syn::{parse_macro_input, ItemType}; +use syn::{parse_macro_input, Ident}; thread_local! { /// A global counter, can be used to generate a relatively unique identifier. @@ -586,11 +586,13 @@ pub fn storage_alias(_: TokenStream, input: TokenStream) -> TokenStream { } #[proc_macro_attribute] -pub fn cached(_attr: TokenStream, item: TokenStream) -> TokenStream { - let typ = item.clone(); - let typ: ItemType = parse_macro_input!(typ as ItemType); - let type_name = typ.ident.to_string(); - // need name of pallet - // use macro_state here +pub fn benchmarking(attr: TokenStream, item: TokenStream) -> TokenStream { + match parse_macro_input!(attr as Ident).to_string().as_str() { + "cached" => benchmarking_cached(item), + _ => panic!("unsupported argument provided to benchmarking macro"), + } +} + +fn benchmarking_cached(item: TokenStream) -> TokenStream { item } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 55ded6da9c5a1..0e800f38149ea 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -210,6 +210,9 @@ impl TypeId for PalletId { /// ``` pub use frame_support_procedural::storage_alias; +/// TODO: docs for benchmarking +pub use frame_support_procedural::benchmarking; + /// Create new implementations of the [`Get`](crate::traits::Get) trait. /// /// The so-called parameter type can be created in four different ways: diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 9d2fbbf999900..b70436785af12 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -1470,8 +1470,6 @@ pub fn storage_prefix(pallet_name: &[u8], storage_name: &[u8]) -> [u8; 32] { final_key } -pub use frame_support_procedural::cached; - #[cfg(test)] mod test { use super::*; From f645511cb797008635ea539f4b1218c78389e580 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 31 Aug 2022 16:59:38 -0400 Subject: [PATCH 08/72] tweak error message and mark BlockNumber as cached --- frame/support/procedural/src/lib.rs | 2 +- frame/system/src/lib.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 906a68bf10740..61719e792086d 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -589,7 +589,7 @@ pub fn storage_alias(_: TokenStream, input: TokenStream) -> TokenStream { pub fn benchmarking(attr: TokenStream, item: TokenStream) -> TokenStream { match parse_macro_input!(attr as Ident).to_string().as_str() { "cached" => benchmarking_cached(item), - _ => panic!("unsupported argument provided to benchmarking macro"), + _ => panic!("only benchmarking(cached) is supported at this time"), } } diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index ba076d963bda7..c11025a615811 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -84,6 +84,7 @@ use sp_version::RuntimeVersion; use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen}; use frame_support::{ + benchmarking, dispatch::{DispatchResult, DispatchResultWithPostInfo}, storage, traits::{ @@ -584,6 +585,7 @@ pub mod pallet { /// The current block number being processed. Set by `execute_block`. #[pallet::storage] + #[benchmarking(cached)] #[pallet::getter(fn block_number)] pub(super) type Number = StorageValue<_, T::BlockNumber, ValueQuery>; From 1b8af15d5f3f9d6fdd245b719a3df38302b564d7 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 1 Sep 2022 01:53:57 -0400 Subject: [PATCH 09/72] add benchmarking(cached) to the other default types --- frame/balances/src/lib.rs | 3 ++- frame/system/src/lib.rs | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 0a45350366449..d9dd86d31093c 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -167,7 +167,7 @@ use codec::{Codec, Decode, Encode, MaxEncodedLen}; #[cfg(feature = "std")] use frame_support::traits::GenesisBuild; use frame_support::{ - ensure, + benchmarking, ensure, pallet_prelude::DispatchResult, traits::{ tokens::{fungible, BalanceStatus as Status, DepositConsequence, WithdrawConsequence}, @@ -492,6 +492,7 @@ pub mod pallet { /// The total units issued in the system. #[pallet::storage] #[pallet::getter(fn total_issuance)] + #[benchmarking(cached)] pub type TotalIssuance, I: 'static = ()> = StorageValue<_, T::Balance, ValueQuery>; /// The Balances pallet example of storing the balance of an account. diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index c11025a615811..39f22209dbfd0 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -563,6 +563,7 @@ pub mod pallet { /// The current weight for the block. #[pallet::storage] + #[benchmarking(cached)] #[pallet::getter(fn block_weight)] pub(super) type BlockWeight = StorageValue<_, ConsumedWeight, ValueQuery>; @@ -608,12 +609,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] + #[benchmarking(cached)] #[pallet::unbounded] pub(super) type Events = StorageValue<_, Vec>>, ValueQuery>; /// The number of events in the `Events` list. #[pallet::storage] + #[benchmarking(cached)] #[pallet::getter(fn event_count)] pub(super) type EventCount = StorageValue<_, EventIndex, ValueQuery>; @@ -649,6 +652,7 @@ pub mod pallet { /// The execution phase of the block. #[pallet::storage] + #[benchmarking(cached)] pub(super) type ExecutionPhase = StorageValue<_, Phase>; #[cfg_attr(feature = "std", derive(Default))] From 2a1ae01fc8059bb286b0b4d69d7e4bea83f734ce Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 2 Sep 2022 11:49:14 -0400 Subject: [PATCH 10/72] add docs for benchmarking(cached) --- frame/support/src/lib.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 0e800f38149ea..05ab2e3549315 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -210,7 +210,27 @@ impl TypeId for PalletId { /// ``` pub use frame_support_procedural::storage_alias; -/// TODO: docs for benchmarking +/// A series of attribute macros related to benchmarking. Right now just +/// [`benchmarking(cached)`](frame_support_procedural::benchmarking(cached)) is provided. +/// +/// # benchmarking(cached) +/// +/// This is an attribute macro that can be attached to a [`StorageValue`]. Doing +/// so will exclude reads of that value's storage key from counting towards +/// weight calculations during benchmarking and will also ensure that the value +/// is automatically cached/read at the beginning of every block. +/// +/// This attribute should be attached to [`StorageValue`]s that are known to be +/// read/used in every block. This will result in a more efficient read pattern +/// for the value and a more accurate benchmarking weight +/// +/// ## Example +/// ```rust +/// #[pallet::storage] +/// #[benchmarking(cached)] +/// #[pallet::getter(fn block_number)] +/// pub(super) type Number = StorageValue<_, T::BlockNumber, ValueQuery>; +/// ``` pub use frame_support_procedural::benchmarking; /// Create new implementations of the [`Get`](crate::traits::Get) trait. From 586bafb45eb91b338a60a1831c8246e429468b92 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 6 Sep 2022 04:16:37 +0000 Subject: [PATCH 11/72] properly parse storage type declaration --- frame/support/procedural/src/lib.rs | 3 +++ frame/support/procedural/src/storage_alias.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 61719e792086d..c92fc6dd0a5a1 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -35,6 +35,7 @@ mod storage_alias; mod transactional; mod tt_macro; +use crate::storage_alias::Input; use proc_macro::TokenStream; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; @@ -594,5 +595,7 @@ pub fn benchmarking(attr: TokenStream, item: TokenStream) -> TokenStream { } fn benchmarking_cached(item: TokenStream) -> TokenStream { + let _input = syn::parse2::(item.clone().into()) + .expect("benchmarking(cached) can only be attached to a valid storage type declaration"); item } diff --git a/frame/support/procedural/src/storage_alias.rs b/frame/support/procedural/src/storage_alias.rs index e0df0123595b9..510c0f89445af 100644 --- a/frame/support/procedural/src/storage_alias.rs +++ b/frame/support/procedural/src/storage_alias.rs @@ -406,7 +406,7 @@ impl Parse for StorageType { } /// The input expected by this macro. -struct Input { +pub struct Input { attributes: Vec, visibility: Visibility, _type: Token![type], From cfcd07390f2689214862094908a8c8034215d609 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 6 Sep 2022 04:36:25 +0000 Subject: [PATCH 12/72] make storage_alias structs public so we can use them in this macro --- frame/support/procedural/src/lib.rs | 3 ++- frame/support/procedural/src/storage_alias.rs | 22 +++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index c92fc6dd0a5a1..29a6d1688326c 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -595,7 +595,8 @@ pub fn benchmarking(attr: TokenStream, item: TokenStream) -> TokenStream { } fn benchmarking_cached(item: TokenStream) -> TokenStream { - let _input = syn::parse2::(item.clone().into()) + let input = syn::parse2::(item.clone().into()) .expect("benchmarking(cached) can only be attached to a valid storage type declaration"); + println!("{}", input.storage_name); item } diff --git a/frame/support/procedural/src/storage_alias.rs b/frame/support/procedural/src/storage_alias.rs index 510c0f89445af..6e558532fabe6 100644 --- a/frame/support/procedural/src/storage_alias.rs +++ b/frame/support/procedural/src/storage_alias.rs @@ -29,7 +29,7 @@ use syn::{ }; /// Represents a path that only consists of [`Ident`] separated by `::`. -struct SimplePath { +pub struct SimplePath { leading_colon: Option, segments: Punctuated, } @@ -65,7 +65,7 @@ impl ToTokens for SimplePath { /// Represents generics which only support [`Ident`] separated by commas as you would pass it to a /// type. -struct TypeGenerics { +pub struct TypeGenerics { lt_token: Token![<], params: Punctuated, gt_token: Token![>], @@ -97,9 +97,9 @@ impl ToTokens for TypeGenerics { } /// Represents generics which only support [`TypeParam`] separated by commas. -struct SimpleGenerics { +pub struct SimpleGenerics { lt_token: Token![<], - params: Punctuated, + pub params: Punctuated, gt_token: Token![>], } @@ -141,7 +141,7 @@ mod storage_types { } /// The supported storage types -enum StorageType { +pub enum StorageType { Value { _kw: storage_types::StorageValue, _lt_token: Token![<], @@ -407,14 +407,14 @@ impl Parse for StorageType { /// The input expected by this macro. pub struct Input { - attributes: Vec, - visibility: Visibility, + pub attributes: Vec, + pub visibility: Visibility, _type: Token![type], - storage_name: Ident, - storage_generics: Option, - where_clause: Option, + pub storage_name: Ident, + pub storage_generics: Option, + pub where_clause: Option, _equal: Token![=], - storage_type: StorageType, + pub storage_type: StorageType, _semicolon: Token![;], } From 85348911edf0b0f58e9199df73646df02a1b5d56 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 6 Sep 2022 05:19:19 +0000 Subject: [PATCH 13/72] use BTreeMap since TrackedStorageKey missing Ord outside of std --- frame/support/src/traits/storage.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index e5abf970f6454..72aa4dcca54eb 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -17,7 +17,7 @@ //! Traits for encoding data related to pallet's storage items. -use crate::sp_std::collections::btree_set::BTreeSet; +use crate::sp_std::collections::btree_map::BTreeMap; use impl_trait_for_tuples::impl_for_tuples; use sp_core::storage::TrackedStorageKey; use sp_std::prelude::*; @@ -108,18 +108,19 @@ pub trait WhitelistedStorageKeys { #[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] impl WhitelistedStorageKeys for Tuple { fn whitelisted_storage_keys() -> Vec { - // use BTreeSet so the resulting list of keys is unique - let mut combined_keys = BTreeSet::new(); + // de-duplicate the storage keys + // use BTreeMap so the resulting list of keys is unique + let mut combined_keys: BTreeMap, TrackedStorageKey> = BTreeMap::new(); for_tuples!( #( - for key in Tuple::whitelisted_storage_keys() { - combined_keys.insert(key); + for storage_key in Tuple::whitelisted_storage_keys() { + combined_keys.insert(storage_key.key.clone(), storage_key); } )* ); - // flatten BTreeSet down to a vec - let mut combined_keys_vec = Vec::new(); - for key in combined_keys { - combined_keys_vec.push(key); + // flatten BTreeMap down to a vec + let mut final_combined_keys: Vec = Vec::new(); + for storage_key in combined_keys.values() { + final_combined_keys.push(storage_key.clone()); } - combined_keys_vec + final_combined_keys } } From e2550b4fc1bfd0ecef18694f6dda6f24ac4eb2b2 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 6 Sep 2022 06:06:46 +0000 Subject: [PATCH 14/72] make WhitelistedStorageKeys accessible --- frame/support/src/traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index d4f0e73557c77..359edce3ae6b4 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -88,7 +88,7 @@ pub use hooks::{ pub mod schedule; mod storage; pub use storage::{ - Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, + Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, WhitelistedStorageKeys }; mod dispatch; From 157fb297e6f8c7df7d8a0753e4dcb8bf1dfbea86 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 6 Sep 2022 10:03:42 +0000 Subject: [PATCH 15/72] basic detection of benchmarking(cached) :boom: --- frame/support/procedural/src/lib.rs | 6 +++--- frame/support/procedural/src/pallet/parse/mod.rs | 13 +++++++++++-- frame/support/src/traits.rs | 3 ++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 29a6d1688326c..6548d55baef16 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -35,7 +35,6 @@ mod storage_alias; mod transactional; mod tt_macro; -use crate::storage_alias::Input; use proc_macro::TokenStream; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; @@ -595,8 +594,9 @@ pub fn benchmarking(attr: TokenStream, item: TokenStream) -> TokenStream { } fn benchmarking_cached(item: TokenStream) -> TokenStream { - let input = syn::parse2::(item.clone().into()) + // re-use storage_alias's Input parser since it is accessible + // and valid for all storage item declarations + syn::parse2::(item.clone().into()) .expect("benchmarking(cached) can only be attached to a valid storage type declaration"); - println!("{}", input.storage_name); item } diff --git a/frame/support/procedural/src/pallet/parse/mod.rs b/frame/support/procedural/src/pallet/parse/mod.rs index a436f7e09c1d7..566ea7a2c278b 100644 --- a/frame/support/procedural/src/pallet/parse/mod.rs +++ b/frame/support/procedural/src/pallet/parse/mod.rs @@ -36,6 +36,7 @@ pub mod type_value; pub mod validate_unsigned; use frame_support_procedural_tools::generate_crate_access_2018; +use quote::ToTokens; use syn::spanned::Spanned; /// Parsed definition of a pallet. @@ -123,8 +124,16 @@ impl Def { origin = Some(origin::OriginDef::try_from(index, item)?), Some(PalletAttr::Inherent(_)) if inherent.is_none() => inherent = Some(inherent::InherentDef::try_from(index, item)?), - Some(PalletAttr::Storage(span)) => - storages.push(storage::StorageDef::try_from(span, index, item)?), + Some(PalletAttr::Storage(span)) => { + let st = item.to_token_stream().to_string(); + let storage_def = storage::StorageDef::try_from(span, index, item)?; + if st.contains("#[benchmarking(cached)]") { + println!("{} cached!", storage_def.ident.to_string()); + } else { + println!("{} NOT cached!", storage_def.ident.to_string()); + } + storages.push(storage_def) + }, Some(PalletAttr::ValidateUnsigned(_)) if validate_unsigned.is_none() => { let v = validate_unsigned::ValidateUnsignedDef::try_from(index, item)?; validate_unsigned = Some(v); diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 359edce3ae6b4..191b7e756d7c2 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -88,7 +88,8 @@ pub use hooks::{ pub mod schedule; mod storage; pub use storage::{ - Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, WhitelistedStorageKeys + Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, + WhitelistedStorageKeys, }; mod dispatch; From 531816757c827df1c70580ed3a2144043a735a3d Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 7 Sep 2022 03:15:03 +0000 Subject: [PATCH 16/72] proper parsing of #[benchmarking(cached)] from pallet parse macro --- .../support/procedural/src/pallet/parse/mod.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/mod.rs b/frame/support/procedural/src/pallet/parse/mod.rs index 566ea7a2c278b..f390efeeceac7 100644 --- a/frame/support/procedural/src/pallet/parse/mod.rs +++ b/frame/support/procedural/src/pallet/parse/mod.rs @@ -125,13 +125,19 @@ impl Def { Some(PalletAttr::Inherent(_)) if inherent.is_none() => inherent = Some(inherent::InherentDef::try_from(index, item)?), Some(PalletAttr::Storage(span)) => { - let st = item.to_token_stream().to_string(); - let storage_def = storage::StorageDef::try_from(span, index, item)?; - if st.contains("#[benchmarking(cached)]") { - println!("{} cached!", storage_def.ident.to_string()); - } else { - println!("{} NOT cached!", storage_def.ident.to_string()); + // check for #[benchmarking(cached)] calls + if let syn::Item::Type(typ) = item { + for attr in typ.attrs.as_slice() { + if let Some(seg) = attr.path.segments.last() { + if seg.to_token_stream().to_string() == "benchmarking" && + attr.tokens.to_string() == "(cached)" + { + println!("cached!"); + } + } + } } + let storage_def = storage::StorageDef::try_from(span, index, item)?; storages.push(storage_def) }, Some(PalletAttr::ValidateUnsigned(_)) if validate_unsigned.is_none() => { From 8cde8fe1a3032b2332aff34eacdc2375653d32a7 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 7 Sep 2022 03:51:16 +0000 Subject: [PATCH 17/72] store presence of #[benchmarking(cached)] macro on StorageDef * will be used for later expansion --- frame/support/procedural/src/pallet/parse/mod.rs | 6 +++--- frame/support/procedural/src/pallet/parse/storage.rs | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/mod.rs b/frame/support/procedural/src/pallet/parse/mod.rs index f390efeeceac7..fc55a27458e9b 100644 --- a/frame/support/procedural/src/pallet/parse/mod.rs +++ b/frame/support/procedural/src/pallet/parse/mod.rs @@ -125,6 +125,7 @@ impl Def { Some(PalletAttr::Inherent(_)) if inherent.is_none() => inherent = Some(inherent::InherentDef::try_from(index, item)?), Some(PalletAttr::Storage(span)) => { + let mut storage_def = storage::StorageDef::try_from(span, index, item)?; // check for #[benchmarking(cached)] calls if let syn::Item::Type(typ) = item { for attr in typ.attrs.as_slice() { @@ -132,13 +133,12 @@ impl Def { if seg.to_token_stream().to_string() == "benchmarking" && attr.tokens.to_string() == "(cached)" { - println!("cached!"); + storage_def.benchmarking_cached = true; } } } } - let storage_def = storage::StorageDef::try_from(span, index, item)?; - storages.push(storage_def) + storages.push(storage_def); }, Some(PalletAttr::ValidateUnsigned(_)) if validate_unsigned.is_none() => { let v = validate_unsigned::ValidateUnsignedDef::try_from(index, item)?; diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index f0e1353774910..2610540b105dd 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -171,6 +171,7 @@ pub struct StorageDef { pub named_generics: Option, /// If the value stored in this storage is unbounded. pub unbounded: bool, + pub benchmarking_cached: bool, } /// The parsed generic from the @@ -814,6 +815,7 @@ impl StorageDef { cfg_attrs, named_generics, unbounded, + benchmarking_cached: false, }) } } From 4cccd8ab0078ae7b318d73aeca4ceec4f1908fac Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 8 Sep 2022 04:37:08 +0000 Subject: [PATCH 18/72] compiling blank impl for WhitelistedStorageKeys --- frame/support/procedural/src/pallet/expand/storage.rs | 10 ++++++++++ frame/support/src/traits.rs | 2 +- frame/support/src/traits/storage.rs | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index 181f35b545496..3aaf99ec560be 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -637,6 +637,15 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { let type_impl_gen = &def.type_impl_generics(proc_macro2::Span::call_site()); let type_use_gen = &def.type_use_generics(proc_macro2::Span::call_site()); + let whitelisted_storage_keys_impl = quote::quote![ + impl frame_support::traits::WhitelistedStorageKeys for Pallet { + fn whitelisted_storage_keys( + ) -> frame_support::inherent::Vec { + frame_support::inherent::Vec::new() + } + } + ]; + quote::quote!( impl<#type_impl_gen> #pallet_ident<#type_use_gen> #completed_where_clause @@ -662,5 +671,6 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { #( #getters )* #( #prefix_structs )* #( #on_empty_structs )* + #whitelisted_storage_keys_impl ) } diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 191b7e756d7c2..16ac7929831b4 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -89,7 +89,7 @@ pub mod schedule; mod storage; pub use storage::{ Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, - WhitelistedStorageKeys, + TrackedStorageKey, WhitelistedStorageKeys, }; mod dispatch; diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 72aa4dcca54eb..0aeebb2cb21ca 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -19,7 +19,7 @@ use crate::sp_std::collections::btree_map::BTreeMap; use impl_trait_for_tuples::impl_for_tuples; -use sp_core::storage::TrackedStorageKey; +pub use sp_core::storage::TrackedStorageKey; use sp_std::prelude::*; /// An instance of a pallet in the storage. From f21315c2bb033b921aef16b3048956eb2ff06fd9 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 8 Sep 2022 17:08:02 +0000 Subject: [PATCH 19/72] move impl to expand_pallet_struct --- .../procedural/src/pallet/expand/pallet_struct.rs | 10 ++++++++++ frame/support/procedural/src/pallet/expand/storage.rs | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index f0fb6bacedffb..90ca2b01233fa 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -166,6 +166,15 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { quote::quote! { #frame_support::traits::StorageVersion::default() } }; + let whitelisted_storage_keys_impl = quote::quote![ + impl frame_support::traits::WhitelistedStorageKeys for Pallet { + fn whitelisted_storage_keys( + ) -> frame_support::inherent::Vec { + frame_support::inherent::Vec::new() + } + } + ]; + quote::quote_spanned!(def.pallet_struct.attr_span => #pallet_error_metadata @@ -253,5 +262,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/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs index 3aaf99ec560be..181f35b545496 100644 --- a/frame/support/procedural/src/pallet/expand/storage.rs +++ b/frame/support/procedural/src/pallet/expand/storage.rs @@ -637,15 +637,6 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { let type_impl_gen = &def.type_impl_generics(proc_macro2::Span::call_site()); let type_use_gen = &def.type_use_generics(proc_macro2::Span::call_site()); - let whitelisted_storage_keys_impl = quote::quote![ - impl frame_support::traits::WhitelistedStorageKeys for Pallet { - fn whitelisted_storage_keys( - ) -> frame_support::inherent::Vec { - frame_support::inherent::Vec::new() - } - } - ]; - quote::quote!( impl<#type_impl_gen> #pallet_ident<#type_use_gen> #completed_where_clause @@ -671,6 +662,5 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { #( #getters )* #( #prefix_structs )* #( #on_empty_structs )* - #whitelisted_storage_keys_impl ) } From faa63a7ead21b047fedf8ceb7aa0f7fe7df5a340 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 8 Sep 2022 17:25:50 +0000 Subject: [PATCH 20/72] use frame_support::sp_std::vec::Vec properly --- frame/support/procedural/src/pallet/expand/pallet_struct.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 90ca2b01233fa..1f721befe3183 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -169,8 +169,8 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { let whitelisted_storage_keys_impl = quote::quote![ impl frame_support::traits::WhitelistedStorageKeys for Pallet { fn whitelisted_storage_keys( - ) -> frame_support::inherent::Vec { - frame_support::inherent::Vec::new() + ) -> frame_support::sp_std::vec::Vec { + frame_support::sp_std::vec::Vec::new() } } ]; From 73b01f1da30bb7d02385ed3585c8f4e7d3575e8f Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 9 Sep 2022 01:23:29 +0000 Subject: [PATCH 21/72] successfully compiling with storage info loaded into a variable :boom: --- .../procedural/src/pallet/expand/pallet_struct.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 1f721befe3183..174b40f13a2e7 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -167,13 +167,15 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { }; let whitelisted_storage_keys_impl = quote::quote![ - impl frame_support::traits::WhitelistedStorageKeys for Pallet { - fn whitelisted_storage_keys( - ) -> frame_support::sp_std::vec::Vec { - frame_support::sp_std::vec::Vec::new() + use #frame_support::traits::StorageInfoTrait; + impl<#type_impl_gen> #frame_support::traits::WhitelistedStorageKeys for #pallet_ident<#type_use_gen> { + fn whitelisted_storage_keys() -> #frame_support::sp_std::vec::Vec<#frame_support::traits::TrackedStorageKey> { + let info = #pallet_ident::<#type_use_gen>::storage_info(); + #frame_support::sp_std::vec::Vec::new() } } ]; + println!("{}", whitelisted_storage_keys_impl); quote::quote_spanned!(def.pallet_struct.attr_span => #pallet_error_metadata From fc6d239bf7a8188ecbca3ca29a28e6e0f6fa5490 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 9 Sep 2022 04:31:05 +0000 Subject: [PATCH 22/72] plausible implementation for whitelisted_storage_keys() * depends on the assumption that storage_info.encode() can be loaded into TrackedStorageKey::new(..) --- frame/support/procedural/src/pallet/expand/pallet_struct.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 174b40f13a2e7..66670fea79cb1 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -170,8 +170,9 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { use #frame_support::traits::StorageInfoTrait; impl<#type_impl_gen> #frame_support::traits::WhitelistedStorageKeys for #pallet_ident<#type_use_gen> { fn whitelisted_storage_keys() -> #frame_support::sp_std::vec::Vec<#frame_support::traits::TrackedStorageKey> { - let info = #pallet_ident::<#type_use_gen>::storage_info(); - #frame_support::sp_std::vec::Vec::new() + #pallet_ident::<#type_use_gen>::storage_info().iter().map(|info| { + #frame_support::traits::TrackedStorageKey::new(info.encode()) + }).collect() } } ]; From f920bb9834f64db1ed5df246bb32d04f73624959 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 9 Sep 2022 15:27:18 +0000 Subject: [PATCH 23/72] use Pallet::whitelisted_storage_keys() instead of hard-coded list --- bin/node-template/runtime/src/lib.rs | 25 +++++++++++++------------ bin/node/runtime/src/lib.rs | 5 +++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index e28a3bb2adb9d..6ad3719b0dfcc 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -514,18 +514,19 @@ 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(), - ]; + let whitelist: Vec = Pallet::whitelisted_storage_keys(); + // 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(), + // ]; let mut batches = Vec::::new(); let params = (&config, &whitelist); diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 5aa488f328a5e..07fe6f8cc8f29 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2142,7 +2142,8 @@ impl_runtime_apis! { impl baseline::Config for Runtime {} impl pallet_nomination_pools_benchmarking::Config for Runtime {} - let whitelist: Vec = vec![ + let whitelist: Vec = Pallet::whitelisted_storage_keys(); + /*let whitelist: Vec = vec![ // Block Number hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(), // Total Issuance @@ -2157,7 +2158,7 @@ impl_runtime_apis! { hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef734abf5cb34d6244378cddbf18e849d96").to_vec().into(), // Treasury Account hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into(), - ]; + ];*/ let mut batches = Vec::::new(); let params = (&config, &whitelist); From 0040a4526bef4f142a6813649bd69d09f309f708 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 9 Sep 2022 16:07:17 +0000 Subject: [PATCH 24/72] AllPallets::whitelisted_storage_keys() properly working :boom: --- bin/node-template/runtime/src/lib.rs | 3 ++- bin/node/runtime/src/lib.rs | 3 ++- frame/support/procedural/src/pallet/expand/pallet_struct.rs | 1 - 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 6ad3719b0dfcc..e4f469e74cf87 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -514,7 +514,8 @@ impl_runtime_apis! { impl frame_system_benchmarking::Config for Runtime {} impl baseline::Config for Runtime {} - let whitelist: Vec = Pallet::whitelisted_storage_keys(); + use crate::sp_api_hidden_includes_construct_runtime::hidden_include::traits::WhitelistedStorageKeys; + let whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); // let whitelist: Vec = vec![ // // Block Number // hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(), diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 07fe6f8cc8f29..7ca4e90165ab8 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2142,7 +2142,8 @@ impl_runtime_apis! { impl baseline::Config for Runtime {} impl pallet_nomination_pools_benchmarking::Config for Runtime {} - let whitelist: Vec = Pallet::whitelisted_storage_keys(); + use crate::sp_api_hidden_includes_construct_runtime::hidden_include::traits::WhitelistedStorageKeys; + let whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); /*let whitelist: Vec = vec![ // Block Number hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef702a5c1b19ab7a04f536c519aca4983ac").to_vec().into(), diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 66670fea79cb1..82a2af6104db1 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -176,7 +176,6 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { } } ]; - println!("{}", whitelisted_storage_keys_impl); quote::quote_spanned!(def.pallet_struct.attr_span => #pallet_error_metadata From 4a0fe7f5030af23c9d50ee22ceef74b3056dcdc2 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 9 Sep 2022 19:28:56 +0000 Subject: [PATCH 25/72] collect storage names --- .../support/procedural/src/pallet/expand/pallet_struct.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 82a2af6104db1..26fb19f1ac348 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -166,6 +166,14 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { quote::quote! { #frame_support::traits::StorageVersion::default() } }; + let whitelisted_storage_names: Vec = def + .storages + .iter() + .filter(|s| s.benchmarking_cached) + .map(|s| s.ident.to_string()) + .collect(); + println!("{:?}", whitelisted_storage_names); + let whitelisted_storage_keys_impl = quote::quote![ use #frame_support::traits::StorageInfoTrait; impl<#type_impl_gen> #frame_support::traits::WhitelistedStorageKeys for #pallet_ident<#type_use_gen> { From 8ab6ab3e84e29a11402c319b547008d87fd59b6e Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 9 Sep 2022 20:16:31 +0000 Subject: [PATCH 26/72] whitelisted_storage_keys() impl working :boom: --- .../src/pallet/expand/pallet_struct.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 26fb19f1ac348..ef5bcb8930fe6 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -166,24 +166,24 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { quote::quote! { #frame_support::traits::StorageVersion::default() } }; - let whitelisted_storage_names: Vec = def + let whitelisted_storage_names: Vec = def .storages .iter() .filter(|s| s.benchmarking_cached) - .map(|s| s.ident.to_string()) + .map(|s| s.ident.clone()) .collect(); - println!("{:?}", whitelisted_storage_names); let whitelisted_storage_keys_impl = quote::quote![ - use #frame_support::traits::StorageInfoTrait; - impl<#type_impl_gen> #frame_support::traits::WhitelistedStorageKeys for #pallet_ident<#type_use_gen> { - fn whitelisted_storage_keys() -> #frame_support::sp_std::vec::Vec<#frame_support::traits::TrackedStorageKey> { - #pallet_ident::<#type_use_gen>::storage_info().iter().map(|info| { - #frame_support::traits::TrackedStorageKey::new(info.encode()) - }).collect() + use #frame_support::traits::{StorageInfoTrait, TrackedStorageKey, WhitelistedStorageKeys}; + impl<#type_impl_gen> WhitelistedStorageKeys for #pallet_ident<#type_use_gen> { + fn whitelisted_storage_keys() -> #frame_support::sp_std::vec::Vec { + use #frame_support::sp_std::vec; + use #frame_support::sp_std::vec::Vec; + vec![#(TrackedStorageKey::new(#whitelisted_storage_names::<#type_use_gen>::hashed_key().to_vec())), *] } } ]; + println!("{}", whitelisted_storage_keys_impl); quote::quote_spanned!(def.pallet_struct.attr_span => #pallet_error_metadata From 8ca026e69e43e06e54cff56ee2a3f4c0788fc7ad Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 9 Sep 2022 21:30:19 +0000 Subject: [PATCH 27/72] clean up --- .../support/procedural/src/pallet/expand/pallet_struct.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index ef5bcb8930fe6..0d101e4c22fd8 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -166,7 +166,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { quote::quote! { #frame_support::traits::StorageVersion::default() } }; - let whitelisted_storage_names: Vec = def + let whitelisted_storage_idents: Vec = def .storages .iter() .filter(|s| s.benchmarking_cached) @@ -177,13 +177,11 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { use #frame_support::traits::{StorageInfoTrait, TrackedStorageKey, WhitelistedStorageKeys}; impl<#type_impl_gen> WhitelistedStorageKeys for #pallet_ident<#type_use_gen> { fn whitelisted_storage_keys() -> #frame_support::sp_std::vec::Vec { - use #frame_support::sp_std::vec; - use #frame_support::sp_std::vec::Vec; - vec![#(TrackedStorageKey::new(#whitelisted_storage_names::<#type_use_gen>::hashed_key().to_vec())), *] + use #frame_support::sp_std::{vec, vec::Vec}; + vec![#(TrackedStorageKey::new(#whitelisted_storage_idents::<#type_use_gen>::hashed_key().to_vec())), *] } } ]; - println!("{}", whitelisted_storage_keys_impl); quote::quote_spanned!(def.pallet_struct.attr_span => #pallet_error_metadata From 58a622905ff0ff604e970834c7fc91e0f2067af2 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Sat, 10 Sep 2022 04:34:42 +0000 Subject: [PATCH 28/72] fix compiler error --- frame/support/procedural/src/pallet/expand/pallet_struct.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 0d101e4c22fd8..498a9a6557e54 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -175,7 +175,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { 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> { + impl<#type_impl_gen> WhitelistedStorageKeys for #pallet_ident<#type_use_gen> #storages_where_clause { fn whitelisted_storage_keys() -> #frame_support::sp_std::vec::Vec { use #frame_support::sp_std::{vec, vec::Vec}; vec![#(TrackedStorageKey::new(#whitelisted_storage_idents::<#type_use_gen>::hashed_key().to_vec())), *] From 854a102120e2438c613d9e0a14c7c577ed9ba903 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Sat, 10 Sep 2022 16:13:33 +0000 Subject: [PATCH 29/72] just one compiler error --- frame/support/procedural/src/pallet/expand/pallet_struct.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 498a9a6557e54..2efc9d45f87e1 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -175,7 +175,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { 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_clause { + 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::Vec}; vec![#(TrackedStorageKey::new(#whitelisted_storage_idents::<#type_use_gen>::hashed_key().to_vec())), *] From 771202c405c63ac46fda2be81fcec6c749e286b7 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Mon, 12 Sep 2022 06:23:43 +0000 Subject: [PATCH 30/72] fix doc compiler error --- frame/support/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 05ab2e3549315..fd7bacddc92d7 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -225,7 +225,7 @@ pub use frame_support_procedural::storage_alias; /// for the value and a more accurate benchmarking weight /// /// ## Example -/// ```rust +/// ```no_run /// #[pallet::storage] /// #[benchmarking(cached)] /// #[pallet::getter(fn block_number)] From fe856ac266bcacb18d270917172ba7e43f5c29aa Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Mon, 12 Sep 2022 17:48:43 +0000 Subject: [PATCH 31/72] use better import path --- bin/node-template/runtime/src/lib.rs | 2 +- bin/node/runtime/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index e4f469e74cf87..0fcbe471e4936 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -514,7 +514,7 @@ impl_runtime_apis! { impl frame_system_benchmarking::Config for Runtime {} impl baseline::Config for Runtime {} - use crate::sp_api_hidden_includes_construct_runtime::hidden_include::traits::WhitelistedStorageKeys; + use frame_support::traits::WhitelistedStorageKeys let whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); // let whitelist: Vec = vec![ // // Block Number diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 7ca4e90165ab8..c53843efa5ead 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2142,7 +2142,7 @@ impl_runtime_apis! { impl baseline::Config for Runtime {} impl pallet_nomination_pools_benchmarking::Config for Runtime {} - use crate::sp_api_hidden_includes_construct_runtime::hidden_include::traits::WhitelistedStorageKeys; + use frame_support::traits::WhitelistedStorageKeys let whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); /*let whitelist: Vec = vec![ // Block Number From 0936ca4af5a42e964be9689d6b51db72ced5bb5e Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Mon, 12 Sep 2022 17:52:11 +0000 Subject: [PATCH 32/72] fix comment --- frame/support/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index fd7bacddc92d7..d92469fb09067 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -217,8 +217,7 @@ pub use frame_support_procedural::storage_alias; /// /// This is an attribute macro that can be attached to a [`StorageValue`]. Doing /// so will exclude reads of that value's storage key from counting towards -/// weight calculations during benchmarking and will also ensure that the value -/// is automatically cached/read at the beginning of every block. +/// weight calculations during benchmarking. /// /// This attribute should be attached to [`StorageValue`]s that are known to be /// read/used in every block. This will result in a more efficient read pattern From a72a28086a7a53bc55094dd018efdd87ef3d7b48 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Mon, 12 Sep 2022 18:08:49 +0000 Subject: [PATCH 33/72] whoops --- bin/node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index c53843efa5ead..56ea449377b2f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2142,7 +2142,7 @@ impl_runtime_apis! { impl baseline::Config for Runtime {} impl pallet_nomination_pools_benchmarking::Config for Runtime {} - use frame_support::traits::WhitelistedStorageKeys + use frame_support::traits::WhitelistedStorageKeys; let whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); /*let whitelist: Vec = vec![ // Block Number From c4ecb9a7ed15f78b6aa4901eb7345bc05eba2c21 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Mon, 12 Sep 2022 18:10:45 +0000 Subject: [PATCH 34/72] whoops again --- bin/node-template/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 0fcbe471e4936..eec1e048bbfb7 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -514,7 +514,7 @@ impl_runtime_apis! { impl frame_system_benchmarking::Config for Runtime {} impl baseline::Config for Runtime {} - use frame_support::traits::WhitelistedStorageKeys + use frame_support::traits::WhitelistedStorageKeys; let whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); // let whitelist: Vec = vec![ // // Block Number From c6fd28147c40e5f14775c498f76a3c2ee985a403 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Mon, 12 Sep 2022 18:46:23 +0000 Subject: [PATCH 35/72] fix macro import issue --- frame/system/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 39f22209dbfd0..5b4e8fec979ca 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -84,7 +84,6 @@ use sp_version::RuntimeVersion; use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen}; use frame_support::{ - benchmarking, dispatch::{DispatchResult, DispatchResultWithPostInfo}, storage, traits::{ @@ -198,6 +197,7 @@ impl, MaxOverflow: Get> ConsumerLimits for (MaxNormal, pub mod pallet { use crate::{self as frame_system, pallet_prelude::*, *}; use frame_support::pallet_prelude::*; + use frame_support::benchmarking; use sp_runtime::DispatchErrorWithPostInfo; /// System configuration trait. Implemented by runtime. From f360944d84cfabc229c6329df0d9157e6fb037df Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Mon, 12 Sep 2022 19:10:19 +0000 Subject: [PATCH 36/72] cargo fmt --- frame/system/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 5b4e8fec979ca..a574d6bf0aea2 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -196,8 +196,7 @@ impl, MaxOverflow: Get> ConsumerLimits for (MaxNormal, #[frame_support::pallet] pub mod pallet { use crate::{self as frame_system, pallet_prelude::*, *}; - use frame_support::pallet_prelude::*; - use frame_support::benchmarking; + use frame_support::{benchmarking, pallet_prelude::*}; use sp_runtime::DispatchErrorWithPostInfo; /// System configuration trait. Implemented by runtime. From 92d5f5e2ad909594dcd138309cd3226dace93bc3 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Mon, 12 Sep 2022 19:38:47 +0000 Subject: [PATCH 37/72] mark example as ignore --- frame/support/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index d92469fb09067..45b89b247834f 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -224,7 +224,7 @@ pub use frame_support_procedural::storage_alias; /// for the value and a more accurate benchmarking weight /// /// ## Example -/// ```no_run +/// ```ignore /// #[pallet::storage] /// #[benchmarking(cached)] /// #[pallet::getter(fn block_number)] From da51def03061efff9b0a7163cecf4ea1a675c1cb Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 15:08:36 +0000 Subject: [PATCH 38/72] use keyword tokens instead of string parsing --- frame/support/procedural/src/pallet/parse/mod.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/mod.rs b/frame/support/procedural/src/pallet/parse/mod.rs index fc55a27458e9b..81d8e0a8eee0c 100644 --- a/frame/support/procedural/src/pallet/parse/mod.rs +++ b/frame/support/procedural/src/pallet/parse/mod.rs @@ -130,10 +130,14 @@ impl Def { if let syn::Item::Type(typ) = item { for attr in typ.attrs.as_slice() { if let Some(seg) = attr.path.segments.last() { - if seg.to_token_stream().to_string() == "benchmarking" && - attr.tokens.to_string() == "(cached)" + if let Ok(_) = + syn::parse2::(seg.to_token_stream()) { - storage_def.benchmarking_cached = true; + if let Ok(_) = + syn::parse2::(attr.path.to_token_stream()) + { + storage_def.benchmarking_cached = true; + } } } } @@ -398,6 +402,8 @@ mod keyword { syn::custom_keyword!(generate_store); syn::custom_keyword!(Store); syn::custom_keyword!(extra_constants); + syn::custom_keyword!(cached); + syn::custom_keyword!(benchmarking); } /// Parse attributes for item in pallet module From e00420fe079c25af98210c90f1f3d252b9d9e62c Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 19:16:02 +0000 Subject: [PATCH 39/72] fix keyword-based parsing of benchmarking(cached) --- .../procedural/src/pallet/parse/mod.rs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/mod.rs b/frame/support/procedural/src/pallet/parse/mod.rs index 81d8e0a8eee0c..dc3b1f8c8d6cd 100644 --- a/frame/support/procedural/src/pallet/parse/mod.rs +++ b/frame/support/procedural/src/pallet/parse/mod.rs @@ -36,8 +36,9 @@ pub mod type_value; pub mod validate_unsigned; use frame_support_procedural_tools::generate_crate_access_2018; +use proc_macro2::Group; use quote::ToTokens; -use syn::spanned::Spanned; +use syn::{spanned::Spanned, AttrStyle}; /// Parsed definition of a pallet. pub struct Def { @@ -129,14 +130,19 @@ impl Def { // check for #[benchmarking(cached)] calls if let syn::Item::Type(typ) = item { for attr in typ.attrs.as_slice() { - if let Some(seg) = attr.path.segments.last() { - if let Ok(_) = - syn::parse2::(seg.to_token_stream()) - { + if attr.style == AttrStyle::Outer { + if let Some(seg) = attr.path.segments.last() { if let Ok(_) = - syn::parse2::(attr.path.to_token_stream()) + syn::parse2::(seg.to_token_stream()) { - storage_def.benchmarking_cached = true; + if let Ok(group) = syn::parse2::(attr.tokens.clone()) + { + if let Ok(_) = + syn::parse2::(group.stream()) + { + storage_def.benchmarking_cached = true; + } + } } } } From a63acdf068699692ec475fe723f972d8128d76a6 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 19:18:58 +0000 Subject: [PATCH 40/72] preliminary spec for check_whitelist() --- bin/node-template/runtime/src/lib.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index eec1e048bbfb7..f888ea7c9f3a2 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -558,3 +558,26 @@ impl_runtime_apis! { } } } + +#[test] +fn check_whitelist() { + use frame_support::traits::WhitelistedStorageKeys; + use sp_core::hexdisplay::HexDisplay; + use std::collections::HashSet; + + 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")); +} From 1a0ba3f4313d5ae7151f1d5c92344043829693d3 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 19:56:23 +0000 Subject: [PATCH 41/72] add additional test for benchmarking whitelist --- bin/node/runtime/src/lib.rs | 63 +++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 17 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 56ea449377b2f..1acd1c71c9469 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2143,23 +2143,12 @@ impl_runtime_apis! { impl pallet_nomination_pools_benchmarking::Config for Runtime {} use frame_support::traits::WhitelistedStorageKeys; - let whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); - /*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(), - ];*/ + let mut whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); + + // Treasury Account (manually whitelist this for now since no + // accessible storage definition we can attach the + // benchmarking(cached) attr macro to) + whitelist.push(hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into()); let mut batches = Vec::::new(); let params = (&config, &whitelist); @@ -2173,8 +2162,48 @@ 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") + ); + // Treasury Account + assert!( + whitelist.contains("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000") + ); + } #[test] fn validate_transaction_submitter_bounds() { From d0ee2c60d53d4f9285e1a74d8a137c4d65545879 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 19:59:53 +0000 Subject: [PATCH 42/72] add TODO note --- bin/node/runtime/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 1acd1c71c9469..036e978b9bf0f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2148,6 +2148,8 @@ impl_runtime_apis! { // Treasury Account (manually whitelist this for now since no // accessible storage definition we can attach the // benchmarking(cached) attr macro to) + // TODO: figure out a workaround for this that doesn't involve + // manually hard-coding whitelist.push(hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into()); let mut batches = Vec::::new(); From 2d1d637eeea22db2a3d02a53ef84fbd03b8bb13e Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 20:01:06 +0000 Subject: [PATCH 43/72] remove irrelevant line from example --- frame/support/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 45b89b247834f..8615a069a67fe 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -227,7 +227,6 @@ pub use frame_support_procedural::storage_alias; /// ```ignore /// #[pallet::storage] /// #[benchmarking(cached)] -/// #[pallet::getter(fn block_number)] /// pub(super) type Number = StorageValue<_, T::BlockNumber, ValueQuery>; /// ``` pub use frame_support_procedural::benchmarking; From 9ce39d2223dbdc477eb4ea4646c94dda902b46e2 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 20:02:53 +0000 Subject: [PATCH 44/72] use filter_map instead of filter and map --- frame/support/procedural/src/pallet/expand/pallet_struct.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 2efc9d45f87e1..10efcd20ff522 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -169,8 +169,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { let whitelisted_storage_idents: Vec = def .storages .iter() - .filter(|s| s.benchmarking_cached) - .map(|s| s.ident.clone()) + .filter_map(|s| s.benchmarking_cached.then_some(s.ident.clone())) .collect(); let whitelisted_storage_keys_impl = quote::quote![ From 33828eaac6c1387253e0f0bd02a5e73ece518123 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 16:06:18 -0400 Subject: [PATCH 45/72] simplify syntax Co-authored-by: Keith Yeung --- frame/support/procedural/src/pallet/expand/pallet_struct.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 10efcd20ff522..1899577d9ebc2 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -177,7 +177,9 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { 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::Vec}; - vec![#(TrackedStorageKey::new(#whitelisted_storage_idents::<#type_use_gen>::hashed_key().to_vec())), *] + vec![#( + TrackedStorageKey::new(#whitelisted_storage_idents::<#type_use_gen>::hashed_key().to_vec()) + ),*] } } ]; From 49b13acecd582b2fea96198e59fd6575908f84ef Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 20:28:42 +0000 Subject: [PATCH 46/72] clean up --- bin/node-template/runtime/src/lib.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index f888ea7c9f3a2..5054ea57c9ac6 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -516,18 +516,6 @@ impl_runtime_apis! { use frame_support::traits::WhitelistedStorageKeys; let whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); - // 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(), - // ]; let mut batches = Vec::::new(); let params = (&config, &whitelist); From a2e8185ccd9e217ef66944f8c06821b0c0d543ce Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 20:29:45 +0000 Subject: [PATCH 47/72] fix test --- bin/node/runtime/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 036e978b9bf0f..4ac64610485e7 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2201,10 +2201,6 @@ mod tests { assert!( whitelist.contains("26aa394eea5630e07c48ae0c9558cef734abf5cb34d6244378cddbf18e849d96") ); - // Treasury Account - assert!( - whitelist.contains("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000") - ); } #[test] From 557a0c2bfa84be9268480e55b7c349d20500c45a Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 13 Sep 2022 20:32:33 +0000 Subject: [PATCH 48/72] fix tests --- bin/node-template/runtime/src/lib.rs | 48 ++++++++++++++++++---------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 5054ea57c9ac6..fd82b92cae64c 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -547,25 +547,39 @@ impl_runtime_apis! { } } -#[test] -fn check_whitelist() { +#[cfg(test)] +mod tests { + use super::*; use frame_support::traits::WhitelistedStorageKeys; use sp_core::hexdisplay::HexDisplay; use std::collections::HashSet; - 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")); + #[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") + ); + } } From d20bfe9b263643e1bb7d4c3c1aaf26453132601e Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 14 Sep 2022 06:03:17 +0000 Subject: [PATCH 49/72] use keyword parsing instead of string parsing --- frame/support/procedural/src/lib.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 6548d55baef16..fb62d67fe6936 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -38,7 +38,6 @@ mod tt_macro; use proc_macro::TokenStream; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; -use syn::{parse_macro_input, Ident}; thread_local! { /// A global counter, can be used to generate a relatively unique identifier. @@ -585,11 +584,15 @@ pub fn storage_alias(_: TokenStream, input: TokenStream) -> TokenStream { .into() } +mod kw { + syn::custom_keyword!(cached); +} + #[proc_macro_attribute] pub fn benchmarking(attr: TokenStream, item: TokenStream) -> TokenStream { - match parse_macro_input!(attr as Ident).to_string().as_str() { - "cached" => benchmarking_cached(item), - _ => panic!("only benchmarking(cached) is supported at this time"), + match syn::parse::(attr) { + Ok(_) => benchmarking_cached(item), + Err(_) => panic!("only benchmarking(cached) is supported at this time"), } } From 1d31abe54d4cfeed3137fa98a18988d008beb614 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 14 Sep 2022 15:56:34 -0400 Subject: [PATCH 50/72] use collect() instead of a for loop Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/support/src/traits/storage.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 0aeebb2cb21ca..2aabba8fa63ab 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -117,10 +117,7 @@ impl WhitelistedStorageKeys for Tuple { } )* ); // flatten BTreeMap down to a vec - let mut final_combined_keys: Vec = Vec::new(); - for storage_key in combined_keys.values() { - final_combined_keys.push(storage_key.clone()); - } + combined_keys.values().collect::>() final_combined_keys } } From dcb6312937ffac1cd033bdfd3490c4dd4ec84eba Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 15 Sep 2022 02:38:46 +0000 Subject: [PATCH 51/72] fix compiler error --- frame/support/src/traits/storage.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 2aabba8fa63ab..dba40ce2750d8 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -117,7 +117,6 @@ impl WhitelistedStorageKeys for Tuple { } )* ); // flatten BTreeMap down to a vec - combined_keys.values().collect::>() - final_combined_keys + combined_keys.values().map(|e| e.clone()).collect::>() } } From 6e66eb11c2c6273986db9924c2bbd7a9b95f94c1 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 15 Sep 2022 05:31:14 +0000 Subject: [PATCH 52/72] clean up benchmarking(cached) marking code --- .../procedural/src/pallet/parse/mod.rs | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/mod.rs b/frame/support/procedural/src/pallet/parse/mod.rs index dc3b1f8c8d6cd..7bfcabab494ac 100644 --- a/frame/support/procedural/src/pallet/parse/mod.rs +++ b/frame/support/procedural/src/pallet/parse/mod.rs @@ -40,6 +40,8 @@ use proc_macro2::Group; use quote::ToTokens; use syn::{spanned::Spanned, AttrStyle}; +use self::storage::StorageDef; + /// Parsed definition of a pallet. pub struct Def { /// The module items. @@ -64,6 +66,25 @@ pub struct Def { } impl Def { + fn mark_benchmarking_cached_calls(storage_def: &mut StorageDef, item: &mut syn::Item) { + if let syn::Item::Type(typ) = item { + for attr in typ.attrs.as_slice() { + if attr.style == AttrStyle::Outer { + if let Some(seg) = attr.path.segments.last() { + if let Ok(_) = syn::parse2::(seg.to_token_stream()) { + if let Ok(group) = syn::parse2::(attr.tokens.clone()) { + if let Ok(_) = syn::parse2::(group.stream()) { + storage_def.benchmarking_cached = true; + break + } + } + } + } + } + } + } + } + pub fn try_from(mut item: syn::ItemMod) -> syn::Result { let frame_system = generate_crate_access_2018("frame-system")?; let frame_support = generate_crate_access_2018("frame-support")?; @@ -127,27 +148,7 @@ impl Def { inherent = Some(inherent::InherentDef::try_from(index, item)?), Some(PalletAttr::Storage(span)) => { let mut storage_def = storage::StorageDef::try_from(span, index, item)?; - // check for #[benchmarking(cached)] calls - if let syn::Item::Type(typ) = item { - for attr in typ.attrs.as_slice() { - if attr.style == AttrStyle::Outer { - if let Some(seg) = attr.path.segments.last() { - if let Ok(_) = - syn::parse2::(seg.to_token_stream()) - { - if let Ok(group) = syn::parse2::(attr.tokens.clone()) - { - if let Ok(_) = - syn::parse2::(group.stream()) - { - storage_def.benchmarking_cached = true; - } - } - } - } - } - } - } + Self::mark_benchmarking_cached_calls(&mut storage_def, item); storages.push(storage_def); }, Some(PalletAttr::ValidateUnsigned(_)) if validate_unsigned.is_none() => { From dcda7352db60000a279a479d9db0cbe6c782c4fe Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 15 Sep 2022 14:16:01 +0000 Subject: [PATCH 53/72] use cloned() --- frame/support/src/traits/storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index dba40ce2750d8..08f648fadda14 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -117,6 +117,6 @@ impl WhitelistedStorageKeys for Tuple { } )* ); // flatten BTreeMap down to a vec - combined_keys.values().map(|e| e.clone()).collect::>() + combined_keys.values().cloned().collect::>() } } From 3f7bbc5b5ef5998bf09a8ff9fc49aa252fe60b75 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 15 Sep 2022 15:31:07 +0000 Subject: [PATCH 54/72] refactor to not use panic! and remove need for pub types in storage_alias --- frame/support/procedural/src/lib.rs | 21 ++++++++++--------- frame/support/procedural/src/storage_alias.rs | 9 ++++++++ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index fb62d67fe6936..20e2b29f09059 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -38,6 +38,7 @@ mod tt_macro; use proc_macro::TokenStream; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; +use storage_alias::validate_storage_item; thread_local! { /// A global counter, can be used to generate a relatively unique identifier. @@ -591,15 +592,15 @@ mod kw { #[proc_macro_attribute] pub fn benchmarking(attr: TokenStream, item: TokenStream) -> TokenStream { match syn::parse::(attr) { - Ok(_) => benchmarking_cached(item), - Err(_) => panic!("only benchmarking(cached) is supported at this time"), + Err(_) => + quote::quote!(compile_error!("only benchmarking(cached) is supported at this time")) + .into(), + Ok(_) => match validate_storage_item(item.clone()) { + Err(_) => quote::quote!(compile_error!( + "benchmarking(cached) can only be attached to valid storage type declarations" + )) + .into(), + Ok(_) => item, + }, } } - -fn benchmarking_cached(item: TokenStream) -> TokenStream { - // re-use storage_alias's Input parser since it is accessible - // and valid for all storage item declarations - syn::parse2::(item.clone().into()) - .expect("benchmarking(cached) can only be attached to a valid storage type declaration"); - item -} diff --git a/frame/support/procedural/src/storage_alias.rs b/frame/support/procedural/src/storage_alias.rs index 6e558532fabe6..33e93dfbaefb5 100644 --- a/frame/support/procedural/src/storage_alias.rs +++ b/frame/support/procedural/src/storage_alias.rs @@ -568,3 +568,12 @@ fn generate_storage_instance( Ok(StorageInstance { name, code }) } + +pub fn validate_storage_item(item: proc_macro::TokenStream) -> Result<()> { + // re-use storage_alias's Input parser since it is accessible + // and valid for all storage item declarations + match syn::parse2::(item.clone().into()) { + Ok(_) => Ok(()), + Err(e) => Err(e), + } +} From 975f3713cae66c69dd0365bc1ba77110b5bd0028 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 15 Sep 2022 11:59:21 -0400 Subject: [PATCH 55/72] remove unneeded use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/support/procedural/src/pallet/expand/pallet_struct.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 1899577d9ebc2..e97721a9140c5 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -176,7 +176,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { 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::Vec}; + use #frame_support::sp_std::vec; vec![#( TrackedStorageKey::new(#whitelisted_storage_idents::<#type_use_gen>::hashed_key().to_vec()) ),*] From 5ff3623d65a82ab2ef7c053100cf0a38d837241f Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Thu, 15 Sep 2022 15:49:50 +0000 Subject: [PATCH 56/72] remove unneeded visibility changes --- frame/support/procedural/src/storage_alias.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/frame/support/procedural/src/storage_alias.rs b/frame/support/procedural/src/storage_alias.rs index 33e93dfbaefb5..8acd153405320 100644 --- a/frame/support/procedural/src/storage_alias.rs +++ b/frame/support/procedural/src/storage_alias.rs @@ -29,7 +29,7 @@ use syn::{ }; /// Represents a path that only consists of [`Ident`] separated by `::`. -pub struct SimplePath { +struct SimplePath { leading_colon: Option, segments: Punctuated, } @@ -65,7 +65,7 @@ impl ToTokens for SimplePath { /// Represents generics which only support [`Ident`] separated by commas as you would pass it to a /// type. -pub struct TypeGenerics { +struct TypeGenerics { lt_token: Token![<], params: Punctuated, gt_token: Token![>], @@ -97,9 +97,9 @@ impl ToTokens for TypeGenerics { } /// Represents generics which only support [`TypeParam`] separated by commas. -pub struct SimpleGenerics { +struct SimpleGenerics { lt_token: Token![<], - pub params: Punctuated, + params: Punctuated, gt_token: Token![>], } @@ -141,7 +141,7 @@ mod storage_types { } /// The supported storage types -pub enum StorageType { +enum StorageType { Value { _kw: storage_types::StorageValue, _lt_token: Token![<], @@ -406,15 +406,15 @@ impl Parse for StorageType { } /// The input expected by this macro. -pub struct Input { - pub attributes: Vec, - pub visibility: Visibility, +struct Input { + attributes: Vec, + visibility: Visibility, _type: Token![type], - pub storage_name: Ident, - pub storage_generics: Option, - pub where_clause: Option, + storage_name: Ident, + storage_generics: Option, + where_clause: Option, _equal: Token![=], - pub storage_type: StorageType, + storage_type: StorageType, _semicolon: Token![;], } From 7214d0f9a2afee5b1bafc7888609e77c6798a393 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 05:26:09 +0000 Subject: [PATCH 57/72] don't manually hard code hash for treasury account as hex --- bin/node/runtime/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 4ac64610485e7..39a53854fd6bf 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2150,7 +2150,8 @@ impl_runtime_apis! { // benchmarking(cached) attr macro to) // TODO: figure out a workaround for this that doesn't involve // manually hard-coding - whitelist.push(hex_literal::hex!("26aa394eea5630e07c48ae0c9558cef7b99d880ec681799c0cf30e8886371da95ecffd7b6c0f78751baa9d281e0bfa3a6d6f646c70792f74727372790000000000000000000000000000000000000000").to_vec().into()); + 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); From 6e490696d77c74283c7dc00f8ec8d86faeeed200 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 09:37:39 +0000 Subject: [PATCH 58/72] proper Ord, PartialOrd, and Hash impls for TrackedStorageKey * now based just on key, and available in no-std --- primitives/storage/src/lib.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/primitives/storage/src/lib.rs b/primitives/storage/src/lib.rs index 0948cf431158d..fc854e16965c7 100644 --- a/primitives/storage/src/lib.rs +++ b/primitives/storage/src/lib.rs @@ -47,8 +47,7 @@ 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, RuntimeDebug, Clone, Encode, Decode, PartialOrd)] pub struct TrackedStorageKey { pub key: Vec, pub reads: u32, @@ -88,6 +87,18 @@ impl TrackedStorageKey { } } +impl Ord for TrackedStorageKey { + fn cmp(&self, other: &Self) -> core::cmp::Ordering { + self.key.cmp(&other.key) + } +} + +impl sp_std::hash::Hash for TrackedStorageKey { + fn hash(&self, state: &mut H) { + self.key.hash(state); + } +} + // Easily convert a key to a `TrackedStorageKey` that has been whitelisted. impl From> for TrackedStorageKey { fn from(key: Vec) -> Self { From 4a71bd40159b03177b69fa6b468154a50658083b Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 09:38:16 +0000 Subject: [PATCH 59/72] use BTreeSet instead of BTreeMap --- frame/support/src/traits/storage.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 08f648fadda14..b3262a4086513 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -17,7 +17,7 @@ //! Traits for encoding data related to pallet's storage items. -use crate::sp_std::collections::btree_map::BTreeMap; +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::*; @@ -110,13 +110,13 @@ impl WhitelistedStorageKeys for Tuple { fn whitelisted_storage_keys() -> Vec { // de-duplicate the storage keys // use BTreeMap so the resulting list of keys is unique - let mut combined_keys: BTreeMap, TrackedStorageKey> = BTreeMap::new(); + let mut combined_keys: BTreeSet = BTreeSet::new(); for_tuples!( #( for storage_key in Tuple::whitelisted_storage_keys() { - combined_keys.insert(storage_key.key.clone(), storage_key); + combined_keys.insert(storage_key); } )* ); // flatten BTreeMap down to a vec - combined_keys.values().cloned().collect::>() + combined_keys.iter().cloned().collect::>() } } From 7752321005ba67691214e3188d75f260389f1719 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 09:47:10 +0000 Subject: [PATCH 60/72] fix comments --- frame/support/src/traits/storage.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index b3262a4086513..ed87fa4793065 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -109,14 +109,12 @@ pub trait WhitelistedStorageKeys { impl WhitelistedStorageKeys for Tuple { fn whitelisted_storage_keys() -> Vec { // de-duplicate the storage keys - // use BTreeMap so the resulting list of keys is unique let mut combined_keys: BTreeSet = BTreeSet::new(); for_tuples!( #( for storage_key in Tuple::whitelisted_storage_keys() { combined_keys.insert(storage_key); } )* ); - // flatten BTreeMap down to a vec combined_keys.iter().cloned().collect::>() } } From 72a8ac8aed4bc344843916289ff7569d40e3ccdf Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 09:49:47 +0000 Subject: [PATCH 61/72] cargo fmt --- primitives/storage/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/primitives/storage/src/lib.rs b/primitives/storage/src/lib.rs index fc854e16965c7..b3bd112a8d71f 100644 --- a/primitives/storage/src/lib.rs +++ b/primitives/storage/src/lib.rs @@ -88,15 +88,15 @@ impl TrackedStorageKey { } impl Ord for TrackedStorageKey { - fn cmp(&self, other: &Self) -> core::cmp::Ordering { - self.key.cmp(&other.key) - } + fn cmp(&self, other: &Self) -> core::cmp::Ordering { + self.key.cmp(&other.key) + } } impl sp_std::hash::Hash for TrackedStorageKey { - fn hash(&self, state: &mut H) { - self.key.hash(state); - } + fn hash(&self, state: &mut H) { + self.key.hash(state); + } } // Easily convert a key to a `TrackedStorageKey` that has been whitelisted. From 20e13759579a7aefc4301c5cd4d700ddd30c329f Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 10:49:32 +0000 Subject: [PATCH 62/72] switch to pallet::whitelist and re-do it basti's way :D --- bin/node/runtime/src/lib.rs | 2 +- frame/balances/src/lib.rs | 4 +- frame/support/procedural/src/lib.rs | 17 --------- .../src/pallet/expand/pallet_struct.rs | 2 +- .../procedural/src/pallet/parse/mod.rs | 28 +------------- .../procedural/src/pallet/parse/storage.rs | 22 ++++++++--- frame/support/procedural/src/storage_alias.rs | 9 ----- frame/support/src/lib.rs | 37 ++++++++----------- frame/system/src/lib.rs | 12 +++--- 9 files changed, 45 insertions(+), 88 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 39a53854fd6bf..9c1d5d12e9cea 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2147,7 +2147,7 @@ impl_runtime_apis! { // Treasury Account (manually whitelist this for now since no // accessible storage definition we can attach the - // benchmarking(cached) attr macro to) + // pallet::whitelist_storage attr macro to) // TODO: figure out a workaround for this that doesn't involve // manually hard-coding let treasury_key = frame_system::Account::::hashed_key_for(Treasury::account_id()); diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index d9dd86d31093c..aceb92172582f 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -167,7 +167,7 @@ use codec::{Codec, Decode, Encode, MaxEncodedLen}; #[cfg(feature = "std")] use frame_support::traits::GenesisBuild; use frame_support::{ - benchmarking, ensure, + ensure, pallet_prelude::DispatchResult, traits::{ tokens::{fungible, BalanceStatus as Status, DepositConsequence, WithdrawConsequence}, @@ -492,7 +492,7 @@ pub mod pallet { /// The total units issued in the system. #[pallet::storage] #[pallet::getter(fn total_issuance)] - #[benchmarking(cached)] + #[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/lib.rs b/frame/support/procedural/src/lib.rs index 20e2b29f09059..989b482115cc1 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -38,7 +38,6 @@ mod tt_macro; use proc_macro::TokenStream; use std::{cell::RefCell, str::FromStr}; pub(crate) use storage::INHERENT_INSTANCE_NAME; -use storage_alias::validate_storage_item; thread_local! { /// A global counter, can be used to generate a relatively unique identifier. @@ -588,19 +587,3 @@ pub fn storage_alias(_: TokenStream, input: TokenStream) -> TokenStream { mod kw { syn::custom_keyword!(cached); } - -#[proc_macro_attribute] -pub fn benchmarking(attr: TokenStream, item: TokenStream) -> TokenStream { - match syn::parse::(attr) { - Err(_) => - quote::quote!(compile_error!("only benchmarking(cached) is supported at this time")) - .into(), - Ok(_) => match validate_storage_item(item.clone()) { - Err(_) => quote::quote!(compile_error!( - "benchmarking(cached) can only be attached to valid storage type declarations" - )) - .into(), - Ok(_) => item, - }, - } -} diff --git a/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/frame/support/procedural/src/pallet/expand/pallet_struct.rs index e97721a9140c5..e5941a33fee13 100644 --- a/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -169,7 +169,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { let whitelisted_storage_idents: Vec = def .storages .iter() - .filter_map(|s| s.benchmarking_cached.then_some(s.ident.clone())) + .filter_map(|s| s.whitelisted.then_some(s.ident.clone())) .collect(); let whitelisted_storage_keys_impl = quote::quote![ diff --git a/frame/support/procedural/src/pallet/parse/mod.rs b/frame/support/procedural/src/pallet/parse/mod.rs index 7bfcabab494ac..a5f2458ab7746 100644 --- a/frame/support/procedural/src/pallet/parse/mod.rs +++ b/frame/support/procedural/src/pallet/parse/mod.rs @@ -36,11 +36,7 @@ pub mod type_value; pub mod validate_unsigned; use frame_support_procedural_tools::generate_crate_access_2018; -use proc_macro2::Group; -use quote::ToTokens; -use syn::{spanned::Spanned, AttrStyle}; - -use self::storage::StorageDef; +use syn::spanned::Spanned; /// Parsed definition of a pallet. pub struct Def { @@ -66,25 +62,6 @@ pub struct Def { } impl Def { - fn mark_benchmarking_cached_calls(storage_def: &mut StorageDef, item: &mut syn::Item) { - if let syn::Item::Type(typ) = item { - for attr in typ.attrs.as_slice() { - if attr.style == AttrStyle::Outer { - if let Some(seg) = attr.path.segments.last() { - if let Ok(_) = syn::parse2::(seg.to_token_stream()) { - if let Ok(group) = syn::parse2::(attr.tokens.clone()) { - if let Ok(_) = syn::parse2::(group.stream()) { - storage_def.benchmarking_cached = true; - break - } - } - } - } - } - } - } - } - pub fn try_from(mut item: syn::ItemMod) -> syn::Result { let frame_system = generate_crate_access_2018("frame-system")?; let frame_support = generate_crate_access_2018("frame-support")?; @@ -147,8 +124,7 @@ impl Def { Some(PalletAttr::Inherent(_)) if inherent.is_none() => inherent = Some(inherent::InherentDef::try_from(index, item)?), Some(PalletAttr::Storage(span)) => { - let mut storage_def = storage::StorageDef::try_from(span, index, item)?; - Self::mark_benchmarking_cached_calls(&mut storage_def, item); + let storage_def = storage::StorageDef::try_from(span, index, item)?; storages.push(storage_def); }, Some(PalletAttr::ValidateUnsigned(_)) if validate_unsigned.is_none() => { diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index 2610540b105dd..ecfd560fe2453 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,7 +183,7 @@ pub struct StorageDef { pub named_generics: Option, /// If the value stored in this storage is unbounded. pub unbounded: bool, - pub benchmarking_cached: bool, + pub whitelisted: bool, } /// The parsed generic from the @@ -673,7 +685,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); @@ -815,7 +827,7 @@ impl StorageDef { cfg_attrs, named_generics, unbounded, - benchmarking_cached: false, + whitelisted, }) } } diff --git a/frame/support/procedural/src/storage_alias.rs b/frame/support/procedural/src/storage_alias.rs index 8acd153405320..e0df0123595b9 100644 --- a/frame/support/procedural/src/storage_alias.rs +++ b/frame/support/procedural/src/storage_alias.rs @@ -568,12 +568,3 @@ fn generate_storage_instance( Ok(StorageInstance { name, code }) } - -pub fn validate_storage_item(item: proc_macro::TokenStream) -> Result<()> { - // re-use storage_alias's Input parser since it is accessible - // and valid for all storage item declarations - match syn::parse2::(item.clone().into()) { - Ok(_) => Ok(()), - Err(e) => Err(e), - } -} diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 8615a069a67fe..fb965cf42ff15 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -210,27 +210,6 @@ impl TypeId for PalletId { /// ``` pub use frame_support_procedural::storage_alias; -/// A series of attribute macros related to benchmarking. Right now just -/// [`benchmarking(cached)`](frame_support_procedural::benchmarking(cached)) is provided. -/// -/// # benchmarking(cached) -/// -/// This is an attribute macro that can be attached to a [`StorageValue`]. Doing -/// so will exclude reads of that value's storage key from counting towards -/// weight calculations during benchmarking. -/// -/// This attribute should be attached to [`StorageValue`]s that are known to be -/// read/used in every block. This will result in a more efficient read pattern -/// for the value and a more accurate benchmarking weight -/// -/// ## Example -/// ```ignore -/// #[pallet::storage] -/// #[benchmarking(cached)] -/// pub(super) type Number = StorageValue<_, T::BlockNumber, ValueQuery>; -/// ``` -pub use frame_support_procedural::benchmarking; - /// Create new implementations of the [`Get`](crate::traits::Get) trait. /// /// The so-called parameter type can be created in four different ways: @@ -1876,6 +1855,22 @@ 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 efficient read pattern +/// for the value and 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/system/src/lib.rs b/frame/system/src/lib.rs index a574d6bf0aea2..fdb2a2644020c 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -196,7 +196,7 @@ impl, MaxOverflow: Get> ConsumerLimits for (MaxNormal, #[frame_support::pallet] pub mod pallet { use crate::{self as frame_system, pallet_prelude::*, *}; - use frame_support::{benchmarking, pallet_prelude::*}; + use frame_support::pallet_prelude::*; use sp_runtime::DispatchErrorWithPostInfo; /// System configuration trait. Implemented by runtime. @@ -562,7 +562,7 @@ pub mod pallet { /// The current weight for the block. #[pallet::storage] - #[benchmarking(cached)] + #[pallet::whitelist_storage] #[pallet::getter(fn block_weight)] pub(super) type BlockWeight = StorageValue<_, ConsumedWeight, ValueQuery>; @@ -585,7 +585,7 @@ pub mod pallet { /// The current block number being processed. Set by `execute_block`. #[pallet::storage] - #[benchmarking(cached)] + #[pallet::whitelist_storage] #[pallet::getter(fn block_number)] pub(super) type Number = StorageValue<_, T::BlockNumber, ValueQuery>; @@ -608,14 +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] - #[benchmarking(cached)] + #[pallet::whitelist_storage] #[pallet::unbounded] pub(super) type Events = StorageValue<_, Vec>>, ValueQuery>; /// The number of events in the `Events` list. #[pallet::storage] - #[benchmarking(cached)] + #[pallet::whitelist_storage] #[pallet::getter(fn event_count)] pub(super) type EventCount = StorageValue<_, EventIndex, ValueQuery>; @@ -651,7 +651,7 @@ pub mod pallet { /// The execution phase of the block. #[pallet::storage] - #[benchmarking(cached)] + #[pallet::whitelist_storage] pub(super) type ExecutionPhase = StorageValue<_, Phase>; #[cfg_attr(feature = "std", derive(Default))] From eb46fbb5f643b0e4a0d20fdb973ad6671651f1c6 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 11:00:36 +0000 Subject: [PATCH 63/72] make PartialOrd for TrackedStorageKey consistent with Ord --- primitives/storage/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/primitives/storage/src/lib.rs b/primitives/storage/src/lib.rs index b3bd112a8d71f..25edded86d67f 100644 --- a/primitives/storage/src/lib.rs +++ b/primitives/storage/src/lib.rs @@ -47,7 +47,7 @@ impl AsRef<[u8]> for StorageKey { } /// Storage key with read/write tracking information. -#[derive(PartialEq, Eq, RuntimeDebug, Clone, Encode, Decode, PartialOrd)] +#[derive(PartialEq, Eq, RuntimeDebug, Clone, Encode, Decode)] pub struct TrackedStorageKey { pub key: Vec, pub reads: u32, @@ -93,6 +93,12 @@ impl Ord for TrackedStorageKey { } } +impl PartialOrd for TrackedStorageKey { + fn partial_cmp(&self, other: &Self) -> Option { + self.key.partial_cmp(&other.key) + } +} + impl sp_std::hash::Hash for TrackedStorageKey { fn hash(&self, state: &mut H) { self.key.hash(state); From fd23e9e6dca37de84bf4bab7a58f0eb4a024e196 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 11:26:46 +0000 Subject: [PATCH 64/72] more correct implementation of hash-related traits for TrackedStorageKey --- primitives/storage/src/lib.rs | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/primitives/storage/src/lib.rs b/primitives/storage/src/lib.rs index 25edded86d67f..9c046bf9d2dd1 100644 --- a/primitives/storage/src/lib.rs +++ b/primitives/storage/src/lib.rs @@ -47,7 +47,9 @@ impl AsRef<[u8]> for StorageKey { } /// Storage key with read/write tracking information. -#[derive(PartialEq, Eq, RuntimeDebug, Clone, Encode, Decode)] +#[derive( + PartialEq, Eq, Ord, PartialOrd, sp_std::hash::Hash, RuntimeDebug, Clone, Encode, Decode, +)] pub struct TrackedStorageKey { pub key: Vec, pub reads: u32, @@ -87,24 +89,6 @@ impl TrackedStorageKey { } } -impl Ord for TrackedStorageKey { - fn cmp(&self, other: &Self) -> core::cmp::Ordering { - self.key.cmp(&other.key) - } -} - -impl PartialOrd for TrackedStorageKey { - fn partial_cmp(&self, other: &Self) -> Option { - self.key.partial_cmp(&other.key) - } -} - -impl sp_std::hash::Hash for TrackedStorageKey { - fn hash(&self, state: &mut H) { - self.key.hash(state); - } -} - // Easily convert a key to a `TrackedStorageKey` that has been whitelisted. impl From> for TrackedStorageKey { fn from(key: Vec) -> Self { From d483a7ab15a99b4bf7d070658392102b98b3bb9c Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 11:28:26 +0000 Subject: [PATCH 65/72] fix integration test --- .../test/tests/pallet_ui/storage_invalid_attribute.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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)] From 05db4e5961cf71fd4540ec135906965117f8e50f Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 11:33:38 +0000 Subject: [PATCH 66/72] update TODO --- bin/node/runtime/src/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 9c1d5d12e9cea..c6e733502f416 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -2145,11 +2145,9 @@ impl_runtime_apis! { use frame_support::traits::WhitelistedStorageKeys; let mut whitelist: Vec = AllPalletsWithSystem::whitelisted_storage_keys(); - // Treasury Account (manually whitelist this for now since no - // accessible storage definition we can attach the - // pallet::whitelist_storage attr macro to) - // TODO: figure out a workaround for this that doesn't involve - // manually hard-coding + // 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()); From cb4ed97c3cfc3271828408ceee4023a4214db649 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 11:35:34 +0000 Subject: [PATCH 67/72] remove unused keyword --- frame/support/procedural/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 989b482115cc1..00204b7a4d906 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -583,7 +583,3 @@ pub fn storage_alias(_: TokenStream, input: TokenStream) -> TokenStream { .unwrap_or_else(|r| r.into_compile_error()) .into() } - -mod kw { - syn::custom_keyword!(cached); -} From 4faf150928a137069a3580013984d26f426d16d5 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 11:36:49 +0000 Subject: [PATCH 68/72] remove more unused keywords --- frame/support/procedural/src/pallet/parse/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/mod.rs b/frame/support/procedural/src/pallet/parse/mod.rs index a5f2458ab7746..09fe9b7128262 100644 --- a/frame/support/procedural/src/pallet/parse/mod.rs +++ b/frame/support/procedural/src/pallet/parse/mod.rs @@ -385,8 +385,6 @@ mod keyword { syn::custom_keyword!(generate_store); syn::custom_keyword!(Store); syn::custom_keyword!(extra_constants); - syn::custom_keyword!(cached); - syn::custom_keyword!(benchmarking); } /// Parse attributes for item in pallet module From 91f33f043cada1e8574510e383bce4ae636055bc Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 07:46:40 -0400 Subject: [PATCH 69/72] use into_iter() Co-authored-by: Keith Yeung --- frame/support/src/traits/storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index ed87fa4793065..24653d1899836 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -115,6 +115,6 @@ impl WhitelistedStorageKeys for Tuple { combined_keys.insert(storage_key); } )* ); - combined_keys.iter().cloned().collect::>() + combined_keys.into_iter().collect::>() } } From 42127fc016a6cf21bcc649a251e9c2dde7678592 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 16:54:22 -0400 Subject: [PATCH 70/72] Update frame/support/procedural/src/pallet/parse/mod.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/support/procedural/src/pallet/parse/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/mod.rs b/frame/support/procedural/src/pallet/parse/mod.rs index 09fe9b7128262..a436f7e09c1d7 100644 --- a/frame/support/procedural/src/pallet/parse/mod.rs +++ b/frame/support/procedural/src/pallet/parse/mod.rs @@ -123,10 +123,8 @@ impl Def { origin = Some(origin::OriginDef::try_from(index, item)?), Some(PalletAttr::Inherent(_)) if inherent.is_none() => inherent = Some(inherent::InherentDef::try_from(index, item)?), - Some(PalletAttr::Storage(span)) => { - let storage_def = storage::StorageDef::try_from(span, index, item)?; - storages.push(storage_def); - }, + Some(PalletAttr::Storage(span)) => + storages.push(storage::StorageDef::try_from(span, index, item)?), Some(PalletAttr::ValidateUnsigned(_)) if validate_unsigned.is_none() => { let v = validate_unsigned::ValidateUnsignedDef::try_from(index, item)?; validate_unsigned = Some(v); From cbf595940e5792d5fd74afe1a308d86393533a26 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 20:56:19 +0000 Subject: [PATCH 71/72] add docs for whitelisted --- frame/support/procedural/src/pallet/parse/storage.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs index ecfd560fe2453..321c4dd5d4914 100644 --- a/frame/support/procedural/src/pallet/parse/storage.rs +++ b/frame/support/procedural/src/pallet/parse/storage.rs @@ -183,6 +183,7 @@ 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, } From ae1bf10968cdae2fdb0d1ea9c03f288c01da094a Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Fri, 16 Sep 2022 20:57:22 +0000 Subject: [PATCH 72/72] fix comment --- frame/support/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index fb965cf42ff15..b8b4635862c38 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1861,8 +1861,7 @@ pub mod pallet_prelude { /// 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 efficient read pattern -/// for the value and a more accurate benchmarking weight. +/// read/used in every block. This will result in a more accurate benchmarking weight. /// /// ### Example /// ```ignore