From ad912ede497711e600352b597ad9e6e544402651 Mon Sep 17 00:00:00 2001 From: Joseph Zhao Date: Wed, 30 Oct 2024 22:09:08 +0800 Subject: [PATCH 1/5] refactor to v2 --- substrate/frame/indices/src/benchmarking.rs | 56 +++++++++++++------ .../transaction-storage/src/benchmarking.rs | 41 +++++++++----- 2 files changed, 67 insertions(+), 30 deletions(-) diff --git a/substrate/frame/indices/src/benchmarking.rs b/substrate/frame/indices/src/benchmarking.rs index bd173815cb34..9b76b34f2195 100644 --- a/substrate/frame/indices/src/benchmarking.rs +++ b/substrate/frame/indices/src/benchmarking.rs @@ -20,7 +20,7 @@ #![cfg(feature = "runtime-benchmarks")] use super::*; -use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller}; +use frame_benchmarking::{account, v2::*, whitelisted_caller, BenchmarkError}; use frame_system::RawOrigin; use sp_runtime::traits::Bounded; @@ -28,17 +28,24 @@ use crate::Pallet as Indices; const SEED: u32 = 0; -benchmarks! { - claim { +#[benchmarks] +mod benchmarks { + use super::*; + + #[benchmark] + fn claim() { let account_index = T::AccountIndex::from(SEED); let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - }: _(RawOrigin::Signed(caller.clone()), account_index) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), account_index); + assert_eq!(Accounts::::get(account_index).unwrap().0, caller); } - transfer { + #[benchmark] + fn transfer() -> Result<(), BenchmarkError> { let account_index = T::AccountIndex::from(SEED); // Setup accounts let caller: T::AccountId = whitelisted_caller(); @@ -48,24 +55,32 @@ benchmarks! { T::Currency::make_free_balance_be(&recipient, BalanceOf::::max_value()); // Claim the index Indices::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; - }: _(RawOrigin::Signed(caller.clone()), recipient_lookup, account_index) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), recipient_lookup, account_index); + assert_eq!(Accounts::::get(account_index).unwrap().0, recipient); + Ok(()) } - free { + #[benchmark] + fn free() -> Result<(), BenchmarkError> { let account_index = T::AccountIndex::from(SEED); // Setup accounts let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); // Claim the index Indices::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; - }: _(RawOrigin::Signed(caller.clone()), account_index) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), account_index); + assert_eq!(Accounts::::get(account_index), None); + Ok(()) } - force_transfer { + #[benchmark] + fn force_transfer() -> Result<(), BenchmarkError> { let account_index = T::AccountIndex::from(SEED); // Setup accounts let original: T::AccountId = account("original", 0, SEED); @@ -75,21 +90,28 @@ benchmarks! { T::Currency::make_free_balance_be(&recipient, BalanceOf::::max_value()); // Claim the index Indices::::claim(RawOrigin::Signed(original).into(), account_index)?; - }: _(RawOrigin::Root, recipient_lookup, account_index, false) - verify { + + #[extrinsic_call] + _(RawOrigin::Root, recipient_lookup, account_index, false); + assert_eq!(Accounts::::get(account_index).unwrap().0, recipient); + Ok(()) } - freeze { + #[benchmark] + fn freeze() -> Result<(), BenchmarkError> { let account_index = T::AccountIndex::from(SEED); // Setup accounts let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); // Claim the index Indices::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; - }: _(RawOrigin::Signed(caller.clone()), account_index) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), account_index); + assert_eq!(Accounts::::get(account_index).unwrap().2, true); + Ok(()) } // TODO in another PR: lookup and unlookup trait weights (not critical) diff --git a/substrate/frame/transaction-storage/src/benchmarking.rs b/substrate/frame/transaction-storage/src/benchmarking.rs index f360e9847a1e..12a9d4eed822 100644 --- a/substrate/frame/transaction-storage/src/benchmarking.rs +++ b/substrate/frame/transaction-storage/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; use alloc::{vec, vec::Vec}; -use frame_benchmarking::v1::{benchmarks, whitelisted_caller}; +use frame_benchmarking::{v2::*, whitelisted_caller, BenchmarkError}; use frame_support::traits::{Get, OnFinalize, OnInitialize}; use frame_system::{pallet_prelude::BlockNumberFor, EventRecord, Pallet as System, RawOrigin}; use sp_runtime::traits::{Bounded, CheckedDiv, One, Zero}; @@ -122,19 +122,25 @@ pub fn run_to_block(n: frame_system::pallet_prelude::BlockNumberFor) { let caller: T::AccountId = whitelisted_caller(); let initial_balance = BalanceOf::::max_value().checked_div(&2u32.into()).unwrap(); T::Currency::set_balance(&caller, initial_balance); - }: _(RawOrigin::Signed(caller.clone()), vec![0u8; l as usize]) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), vec![0u8; l as usize]); + assert!(!BlockTransactions::::get().is_empty()); assert_last_event::(Event::Stored { index: 0 }.into()); } - renew { + #[benchmark] + fn renew() -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); let initial_balance = BalanceOf::::max_value().checked_div(&2u32.into()).unwrap(); T::Currency::set_balance(&caller, initial_balance); @@ -143,17 +149,22 @@ benchmarks! { vec![0u8; T::MaxTransactionSize::get() as usize], )?; run_to_block::(1u32.into()); - }: _(RawOrigin::Signed(caller.clone()), BlockNumberFor::::zero(), 0) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), BlockNumberFor::::zero(), 0); + assert_last_event::(Event::Renewed { index: 0 }.into()); + + Ok(()) } - check_proof_max { + #[benchmark] + fn check_proof_max() -> Result<(), BenchmarkError> { run_to_block::(1u32.into()); let caller: T::AccountId = whitelisted_caller(); let initial_balance = BalanceOf::::max_value().checked_div(&2u32.into()).unwrap(); T::Currency::set_balance(&caller, initial_balance); - for _ in 0 .. T::MaxBlockTransactions::get() { + for _ in 0..T::MaxBlockTransactions::get() { TransactionStorage::::store( RawOrigin::Signed(caller.clone()).into(), vec![0u8; T::MaxTransactionSize::get() as usize], @@ -162,9 +173,13 @@ benchmarks! { run_to_block::(StoragePeriod::::get() + BlockNumberFor::::one()); let encoded_proof = proof(); let proof = TransactionStorageProof::decode(&mut &*encoded_proof).unwrap(); - }: check_proof(RawOrigin::None, proof) - verify { + + #[extrinsic_call] + check_proof(RawOrigin::None, proof); + assert_last_event::(Event::ProofChecked.into()); + + Ok(()) } impl_benchmark_test_suite!(TransactionStorage, crate::mock::new_test_ext(), crate::mock::Test); From e8020142d57b11077f1e83e99254ec8c227640ef Mon Sep 17 00:00:00 2001 From: Joseph Zhao Date: Thu, 31 Oct 2024 15:20:54 +0800 Subject: [PATCH 2/5] remove some import --- substrate/frame/indices/src/benchmarking.rs | 14 ++++++-------- .../frame/transaction-storage/src/benchmarking.rs | 10 ++++------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/substrate/frame/indices/src/benchmarking.rs b/substrate/frame/indices/src/benchmarking.rs index 9b76b34f2195..61adc3a5e233 100644 --- a/substrate/frame/indices/src/benchmarking.rs +++ b/substrate/frame/indices/src/benchmarking.rs @@ -19,13 +19,11 @@ #![cfg(feature = "runtime-benchmarks")] -use super::*; +use crate::*; use frame_benchmarking::{account, v2::*, whitelisted_caller, BenchmarkError}; use frame_system::RawOrigin; use sp_runtime::traits::Bounded; -use crate::Pallet as Indices; - const SEED: u32 = 0; #[benchmarks] @@ -54,7 +52,7 @@ mod benchmarks { let recipient_lookup = T::Lookup::unlookup(recipient.clone()); T::Currency::make_free_balance_be(&recipient, BalanceOf::::max_value()); // Claim the index - Indices::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; + Pallet::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; #[extrinsic_call] _(RawOrigin::Signed(caller.clone()), recipient_lookup, account_index); @@ -70,7 +68,7 @@ mod benchmarks { let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); // Claim the index - Indices::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; + Pallet::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; #[extrinsic_call] _(RawOrigin::Signed(caller.clone()), account_index); @@ -89,7 +87,7 @@ mod benchmarks { let recipient_lookup = T::Lookup::unlookup(recipient.clone()); T::Currency::make_free_balance_be(&recipient, BalanceOf::::max_value()); // Claim the index - Indices::::claim(RawOrigin::Signed(original).into(), account_index)?; + Pallet::::claim(RawOrigin::Signed(original).into(), account_index)?; #[extrinsic_call] _(RawOrigin::Root, recipient_lookup, account_index, false); @@ -105,7 +103,7 @@ mod benchmarks { let caller: T::AccountId = whitelisted_caller(); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); // Claim the index - Indices::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; + Pallet::::claim(RawOrigin::Signed(caller.clone()).into(), account_index)?; #[extrinsic_call] _(RawOrigin::Signed(caller.clone()), account_index); @@ -116,5 +114,5 @@ mod benchmarks { // TODO in another PR: lookup and unlookup trait weights (not critical) - impl_benchmark_test_suite!(Indices, crate::mock::new_test_ext(), crate::mock::Test); + impl_benchmark_test_suite!(Pallet, mock::new_test_ext(), mock::Test); } diff --git a/substrate/frame/transaction-storage/src/benchmarking.rs b/substrate/frame/transaction-storage/src/benchmarking.rs index 12a9d4eed822..f71f6ec53a35 100644 --- a/substrate/frame/transaction-storage/src/benchmarking.rs +++ b/substrate/frame/transaction-storage/src/benchmarking.rs @@ -19,7 +19,7 @@ #![cfg(feature = "runtime-benchmarks")] -use super::*; +use crate::*; use alloc::{vec, vec::Vec}; use frame_benchmarking::{v2::*, whitelisted_caller, BenchmarkError}; use frame_support::traits::{Get, OnFinalize, OnInitialize}; @@ -27,8 +27,6 @@ use frame_system::{pallet_prelude::BlockNumberFor, EventRecord, Pallet as System use sp_runtime::traits::{Bounded, CheckedDiv, One, Zero}; use sp_transaction_storage_proof::TransactionStorageProof; -use crate::Pallet as TransactionStorage; - // Proof generated from max size storage: // ``` // let mut transactions = Vec::new(); @@ -144,7 +142,7 @@ mod benchmarks { let caller: T::AccountId = whitelisted_caller(); let initial_balance = BalanceOf::::max_value().checked_div(&2u32.into()).unwrap(); T::Currency::set_balance(&caller, initial_balance); - TransactionStorage::::store( + Pallet::::store( RawOrigin::Signed(caller.clone()).into(), vec![0u8; T::MaxTransactionSize::get() as usize], )?; @@ -165,7 +163,7 @@ mod benchmarks { let initial_balance = BalanceOf::::max_value().checked_div(&2u32.into()).unwrap(); T::Currency::set_balance(&caller, initial_balance); for _ in 0..T::MaxBlockTransactions::get() { - TransactionStorage::::store( + Pallet::::store( RawOrigin::Signed(caller.clone()).into(), vec![0u8; T::MaxTransactionSize::get() as usize], )?; @@ -182,5 +180,5 @@ mod benchmarks { Ok(()) } - impl_benchmark_test_suite!(TransactionStorage, crate::mock::new_test_ext(), crate::mock::Test); + impl_benchmark_test_suite!(Pallet, mock::new_test_ext(), mock::Test); } From 746c089a1c20d551521fd64e55b00f70ffdcc713 Mon Sep 17 00:00:00 2001 From: Giuseppe Re Date: Fri, 8 Nov 2024 11:39:04 +0100 Subject: [PATCH 3/5] Update substrate/frame/indices/src/benchmarking.rs --- substrate/frame/indices/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/indices/src/benchmarking.rs b/substrate/frame/indices/src/benchmarking.rs index 61adc3a5e233..28f5e3bf5cf0 100644 --- a/substrate/frame/indices/src/benchmarking.rs +++ b/substrate/frame/indices/src/benchmarking.rs @@ -20,7 +20,7 @@ #![cfg(feature = "runtime-benchmarks")] use crate::*; -use frame_benchmarking::{account, v2::*, whitelisted_caller, BenchmarkError}; +use frame_benchmarking::v2::*; use frame_system::RawOrigin; use sp_runtime::traits::Bounded; From 71b8b13673a79128c4583ad5bbe8d6ec39f8a10b Mon Sep 17 00:00:00 2001 From: Giuseppe Re Date: Fri, 8 Nov 2024 11:39:10 +0100 Subject: [PATCH 4/5] Update substrate/frame/transaction-storage/src/benchmarking.rs --- substrate/frame/transaction-storage/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/transaction-storage/src/benchmarking.rs b/substrate/frame/transaction-storage/src/benchmarking.rs index f71f6ec53a35..0b5b0dc99405 100644 --- a/substrate/frame/transaction-storage/src/benchmarking.rs +++ b/substrate/frame/transaction-storage/src/benchmarking.rs @@ -21,7 +21,7 @@ use crate::*; use alloc::{vec, vec::Vec}; -use frame_benchmarking::{v2::*, whitelisted_caller, BenchmarkError}; +use frame_benchmarking::v2::*; use frame_support::traits::{Get, OnFinalize, OnInitialize}; use frame_system::{pallet_prelude::BlockNumberFor, EventRecord, Pallet as System, RawOrigin}; use sp_runtime::traits::{Bounded, CheckedDiv, One, Zero}; From 543ce23026fdf75da7b354daaee623cdc91248e1 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Fri, 8 Nov 2024 13:40:21 +0000 Subject: [PATCH 5/5] Update from re-gius running command 'prdoc --bump patch --audience runtime_dev' --- prdoc/pr_6290.prdoc | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 prdoc/pr_6290.prdoc diff --git a/prdoc/pr_6290.prdoc b/prdoc/pr_6290.prdoc new file mode 100644 index 000000000000..a05d0cd15acf --- /dev/null +++ b/prdoc/pr_6290.prdoc @@ -0,0 +1,11 @@ +title: Migrate pallet-transaction-storage and pallet-indices to benchmark v2 +doc: +- audience: Runtime Dev + description: |- + Part of: + #6202 +crates: +- name: pallet-indices + bump: patch +- name: pallet-transaction-storage + bump: patch