From 5861a10d255590b105bca2126757e5bd95fbd391 Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Fri, 2 Aug 2024 09:41:48 +0100 Subject: [PATCH 1/6] account for all parameters on_initialize --- substrate/frame/parameters/src/lib.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/substrate/frame/parameters/src/lib.rs b/substrate/frame/parameters/src/lib.rs index 55a6f1ff91de..b073773d0712 100644 --- a/substrate/frame/parameters/src/lib.rs +++ b/substrate/frame/parameters/src/lib.rs @@ -167,6 +167,15 @@ pub mod pallet { type WeightInfo: WeightInfo; } + #[pallet::hooks] + impl Hooks> for Pallet { + fn on_initialize(_: BlockNumberFor) -> Weight { + let items = Parameters::::iter().count() as u64; + + Weight::zero().saturating_add(T::DbWeight::get().reads(items)) + } + } + #[pallet::event] #[pallet::generate_deposit(pub(crate) fn deposit_event)] pub enum Event { From 60d3b3a55e8f2385ca5223da4ca50419ca9ffceb Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Fri, 2 Aug 2024 09:44:05 +0100 Subject: [PATCH 2/6] whitelist storage --- substrate/frame/parameters/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/substrate/frame/parameters/src/lib.rs b/substrate/frame/parameters/src/lib.rs index b073773d0712..eb35c3e447ce 100644 --- a/substrate/frame/parameters/src/lib.rs +++ b/substrate/frame/parameters/src/lib.rs @@ -193,7 +193,12 @@ pub mod pallet { } /// Stored parameters. + /// + /// ## Storage Whitelist + /// Since we account for all parameters read weight for each block, + /// don't double count it in other pallet benchmarks #[pallet::storage] + #[pallet::whitelist_storage] pub type Parameters = StorageMap<_, Blake2_128Concat, KeyOf, ValueOf, OptionQuery>; From 5276bf9f7bbaa8db20d7a49366cf6431367dca08 Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Mon, 5 Aug 2024 10:09:27 +0100 Subject: [PATCH 3/6] remove whitelist --- substrate/frame/parameters/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/parameters/src/lib.rs b/substrate/frame/parameters/src/lib.rs index eb35c3e447ce..3f85522a3d6b 100644 --- a/substrate/frame/parameters/src/lib.rs +++ b/substrate/frame/parameters/src/lib.rs @@ -198,7 +198,7 @@ pub mod pallet { /// Since we account for all parameters read weight for each block, /// don't double count it in other pallet benchmarks #[pallet::storage] - #[pallet::whitelist_storage] + // #[pallet::whitelist_storage] pub type Parameters = StorageMap<_, Blake2_128Concat, KeyOf, ValueOf, OptionQuery>; From 34af46a718cb7ded0757d1ff57df187e6ec411ce Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Mon, 5 Aug 2024 14:04:14 +0100 Subject: [PATCH 4/6] add proof size --- Cargo.lock | 1 + substrate/frame/parameters/Cargo.toml | 2 ++ substrate/frame/parameters/src/lib.rs | 13 +++++++++++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e877cd9d2486..b80dbc771382 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11132,6 +11132,7 @@ dependencies = [ name = "pallet-parameters" version = "0.1.0" dependencies = [ + "cumulus-primitives-storage-weight-reclaim", "docify", "frame-benchmarking", "frame-support", diff --git a/substrate/frame/parameters/Cargo.toml b/substrate/frame/parameters/Cargo.toml index b718b391019a..7504e67cedb0 100644 --- a/substrate/frame/parameters/Cargo.toml +++ b/substrate/frame/parameters/Cargo.toml @@ -19,6 +19,7 @@ frame-system = { path = "../system", default-features = false } sp-core = { path = "../../primitives/core", default-features = false } sp-runtime = { path = "../../primitives/runtime", default-features = false } sp-std = { path = "../../primitives/std", default-features = false } +cumulus-primitives-storage-weight-reclaim = { path = "../../../cumulus/primitives/storage-weight-reclaim", default-features = false} frame-benchmarking = { path = "../benchmarking", default-features = false, optional = true } [dev-dependencies] @@ -39,6 +40,7 @@ std = [ "sp-core/std", "sp-runtime/std", "sp-std/std", + "cumulus-primitives-storage-weight-reclaim/std", ] runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", diff --git a/substrate/frame/parameters/src/lib.rs b/substrate/frame/parameters/src/lib.rs index 3f85522a3d6b..773a880f37a3 100644 --- a/substrate/frame/parameters/src/lib.rs +++ b/substrate/frame/parameters/src/lib.rs @@ -125,6 +125,7 @@ use frame_support::traits::{ dynamic_params::{AggregatedKeyValue, IntoKey, Key, RuntimeParameterStore, TryIntoKey}, EnsureOriginWithArg, }; +use cumulus_primitives_storage_weight_reclaim::get_proof_size; mod benchmarking; #[cfg(test)] @@ -170,9 +171,17 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { fn on_initialize(_: BlockNumberFor) -> Weight { + let proof_size_before: u64 = get_proof_size().unwrap_or(0); + let items = Parameters::::iter().count() as u64; - Weight::zero().saturating_add(T::DbWeight::get().reads(items)) + let proof_size_after: u64 = get_proof_size().unwrap_or(0); + + let proof_size_diff = proof_size_after.saturating_sub(proof_size_before); + + Weight::zero() + .saturating_add(T::DbWeight::get().reads(items)) + .saturating_add(Weight::from_parts(0, proof_size_diff)) } } @@ -193,7 +202,7 @@ pub mod pallet { } /// Stored parameters. - /// + /// /// ## Storage Whitelist /// Since we account for all parameters read weight for each block, /// don't double count it in other pallet benchmarks From f9559c8a6095480175517b8392da93caf7ea1743 Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Mon, 5 Aug 2024 17:58:17 +0300 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Rodrigo Quelhas <22591718+RomarQ@users.noreply.github.com> --- substrate/frame/parameters/src/lib.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/substrate/frame/parameters/src/lib.rs b/substrate/frame/parameters/src/lib.rs index 773a880f37a3..50a3d7219138 100644 --- a/substrate/frame/parameters/src/lib.rs +++ b/substrate/frame/parameters/src/lib.rs @@ -202,12 +202,7 @@ pub mod pallet { } /// Stored parameters. - /// - /// ## Storage Whitelist - /// Since we account for all parameters read weight for each block, - /// don't double count it in other pallet benchmarks #[pallet::storage] - // #[pallet::whitelist_storage] pub type Parameters = StorageMap<_, Blake2_128Concat, KeyOf, ValueOf, OptionQuery>; From 3c15d2d02e0c26cc9ec794699faa0ee59936764b Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Mon, 5 Aug 2024 16:27:57 +0100 Subject: [PATCH 6/6] rewrite weight --- substrate/frame/parameters/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/substrate/frame/parameters/src/lib.rs b/substrate/frame/parameters/src/lib.rs index 50a3d7219138..c170d4e270b3 100644 --- a/substrate/frame/parameters/src/lib.rs +++ b/substrate/frame/parameters/src/lib.rs @@ -179,9 +179,8 @@ pub mod pallet { let proof_size_diff = proof_size_after.saturating_sub(proof_size_before); - Weight::zero() + Weight::from_parts(0, proof_size_diff) .saturating_add(T::DbWeight::get().reads(items)) - .saturating_add(Weight::from_parts(0, proof_size_diff)) } }