From bea8d897f01d645cd49c608df7c194b2cfbae74a Mon Sep 17 00:00:00 2001 From: Xavier Lau Date: Thu, 31 Oct 2024 22:17:27 +0800 Subject: [PATCH 1/7] Format --- substrate/frame/utility/src/benchmarking.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/frame/utility/src/benchmarking.rs b/substrate/frame/utility/src/benchmarking.rs index 88556c05195a..487d82d8461c 100644 --- a/substrate/frame/utility/src/benchmarking.rs +++ b/substrate/frame/utility/src/benchmarking.rs @@ -19,7 +19,6 @@ #![cfg(feature = "runtime-benchmarks")] -use alloc::vec; use frame_benchmarking::{benchmarking::add_to_whitelist, v2::*}; use frame_system::RawOrigin; From b50a696f2a9064fc690950eb58078baf6ee6339d Mon Sep 17 00:00:00 2001 From: Xavier Lau Date: Thu, 31 Oct 2024 22:17:41 +0800 Subject: [PATCH 2/7] Migrate pallet-child-bounties benchmark to v2 --- .../frame/child-bounties/src/benchmarking.rs | 238 ++++++++++++------ 1 file changed, 156 insertions(+), 82 deletions(-) diff --git a/substrate/frame/child-bounties/src/benchmarking.rs b/substrate/frame/child-bounties/src/benchmarking.rs index b1f6370f3340..301d2b44356a 100644 --- a/substrate/frame/child-bounties/src/benchmarking.rs +++ b/substrate/frame/child-bounties/src/benchmarking.rs @@ -19,16 +19,10 @@ #![cfg(feature = "runtime-benchmarks")] -use super::*; - -use alloc::{vec, vec::Vec}; - -use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller, BenchmarkError}; +use frame_benchmarking::{v2::*, BenchmarkError}; use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; -use crate::Pallet as ChildBounties; -use pallet_bounties::Pallet as Bounties; -use pallet_treasury::Pallet as Treasury; +use crate::*; const SEED: u32 = 0; @@ -105,7 +99,7 @@ fn activate_bounty( ) -> Result, BenchmarkError> { let mut child_bounty_setup = setup_child_bounty::(user, description); let curator_lookup = T::Lookup::unlookup(child_bounty_setup.curator.clone()); - Bounties::::propose_bounty( + pallet_bounties::Pallet::::propose_bounty( RawOrigin::Signed(child_bounty_setup.caller.clone()).into(), child_bounty_setup.value, child_bounty_setup.reason.clone(), @@ -115,15 +109,15 @@ fn activate_bounty( let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - Bounties::::approve_bounty(approve_origin, child_bounty_setup.bounty_id)?; - Treasury::::on_initialize(BlockNumberFor::::zero()); - Bounties::::propose_curator( + pallet_bounties::Pallet::::approve_bounty(approve_origin, child_bounty_setup.bounty_id)?; + pallet_treasury::Pallet::::on_initialize(BlockNumberFor::::zero()); + pallet_bounties::Pallet::::propose_curator( RawOrigin::Root.into(), child_bounty_setup.bounty_id, curator_lookup, child_bounty_setup.fee, )?; - Bounties::::accept_curator( + pallet_bounties::Pallet::::accept_curator( RawOrigin::Signed(child_bounty_setup.curator.clone()).into(), child_bounty_setup.bounty_id, )?; @@ -138,7 +132,7 @@ fn activate_child_bounty( let mut bounty_setup = activate_bounty::(user, description)?; let child_curator_lookup = T::Lookup::unlookup(bounty_setup.child_curator.clone()); - ChildBounties::::add_child_bounty( + Pallet::::add_child_bounty( RawOrigin::Signed(bounty_setup.curator.clone()).into(), bounty_setup.bounty_id, bounty_setup.child_bounty_value, @@ -147,7 +141,7 @@ fn activate_child_bounty( bounty_setup.child_bounty_id = ChildBountyCount::::get() - 1; - ChildBounties::::propose_curator( + Pallet::::propose_curator( RawOrigin::Signed(bounty_setup.curator.clone()).into(), bounty_setup.bounty_id, bounty_setup.child_bounty_id, @@ -155,7 +149,7 @@ fn activate_child_bounty( bounty_setup.child_bounty_fee, )?; - ChildBounties::::accept_curator( + Pallet::::accept_curator( RawOrigin::Signed(bounty_setup.child_curator.clone()).into(), bounty_setup.bounty_id, bounty_setup.child_bounty_id, @@ -165,7 +159,7 @@ fn activate_child_bounty( } fn setup_pot_account() { - let pot_account = Bounties::::account_id(); + let pot_account = pallet_bounties::Pallet::::account_id(); let value = T::Currency::minimum_balance().saturating_mul(1_000_000_000u32.into()); let _ = T::Currency::make_free_balance_be(&pot_account, value); } @@ -174,26 +168,43 @@ fn assert_last_event(generic_event: ::RuntimeEvent) { frame_system::Pallet::::assert_last_event(generic_event.into()); } -benchmarks! { - add_child_bounty { - let d in 0 .. T::MaximumReasonLength::get(); +#[benchmarks] +mod benchmarks { + use super::*; + + #[benchmark] + fn add_child_bounty( + d: Linear<0, { T::MaximumReasonLength::get() }>, + ) -> Result<(), BenchmarkError> { setup_pot_account::(); let bounty_setup = activate_bounty::(0, d)?; - }: _(RawOrigin::Signed(bounty_setup.curator), bounty_setup.bounty_id, - bounty_setup.child_bounty_value, bounty_setup.reason.clone()) - verify { - assert_last_event::(Event::Added { - index: bounty_setup.bounty_id, - child_index: bounty_setup.child_bounty_id, - }.into()) + + #[extrinsic_call] + _( + RawOrigin::Signed(bounty_setup.curator), + bounty_setup.bounty_id, + bounty_setup.child_bounty_value, + bounty_setup.reason.clone(), + ); + + assert_last_event::( + Event::Added { + index: bounty_setup.bounty_id, + child_index: bounty_setup.child_bounty_id, + } + .into(), + ); + + Ok(()) } - propose_curator { + #[benchmark] + fn propose_curator() -> Result<(), BenchmarkError> { setup_pot_account::(); let bounty_setup = activate_bounty::(0, T::MaximumReasonLength::get())?; let child_curator_lookup = T::Lookup::unlookup(bounty_setup.child_curator.clone()); - ChildBounties::::add_child_bounty( + Pallet::::add_child_bounty( RawOrigin::Signed(bounty_setup.curator.clone()).into(), bounty_setup.bounty_id, bounty_setup.child_bounty_value, @@ -201,15 +212,25 @@ benchmarks! { )?; let child_bounty_id = ChildBountyCount::::get() - 1; - }: _(RawOrigin::Signed(bounty_setup.curator), bounty_setup.bounty_id, - child_bounty_id, child_curator_lookup, bounty_setup.child_bounty_fee) + #[extrinsic_call] + _( + RawOrigin::Signed(bounty_setup.curator), + bounty_setup.bounty_id, + child_bounty_id, + child_curator_lookup, + bounty_setup.child_bounty_fee, + ); + + Ok(()) + } - accept_curator { + #[benchmark] + fn accept_curator() -> Result<(), BenchmarkError> { setup_pot_account::(); let mut bounty_setup = activate_bounty::(0, T::MaximumReasonLength::get())?; let child_curator_lookup = T::Lookup::unlookup(bounty_setup.child_curator.clone()); - ChildBounties::::add_child_bounty( + Pallet::::add_child_bounty( RawOrigin::Signed(bounty_setup.curator.clone()).into(), bounty_setup.bounty_id, bounty_setup.child_bounty_value, @@ -217,74 +238,110 @@ benchmarks! { )?; bounty_setup.child_bounty_id = ChildBountyCount::::get() - 1; - ChildBounties::::propose_curator( + Pallet::::propose_curator( RawOrigin::Signed(bounty_setup.curator.clone()).into(), bounty_setup.bounty_id, bounty_setup.child_bounty_id, child_curator_lookup, bounty_setup.child_bounty_fee, )?; - }: _(RawOrigin::Signed(bounty_setup.child_curator), bounty_setup.bounty_id, - bounty_setup.child_bounty_id) + + #[extrinsic_call] + _( + RawOrigin::Signed(bounty_setup.child_curator), + bounty_setup.bounty_id, + bounty_setup.child_bounty_id, + ); + + Ok(()) + } // Worst case when curator is inactive and any sender un-assigns the curator. - unassign_curator { + #[benchmark] + fn unassign_curator() -> Result<(), BenchmarkError> { setup_pot_account::(); let bounty_setup = activate_child_bounty::(0, T::MaximumReasonLength::get())?; - Treasury::::on_initialize(BlockNumberFor::::zero()); + pallet_treasury::Pallet::::on_initialize(BlockNumberFor::::zero()); frame_system::Pallet::::set_block_number(T::BountyUpdatePeriod::get() + 1u32.into()); let caller = whitelisted_caller(); - }: _(RawOrigin::Signed(caller), bounty_setup.bounty_id, - bounty_setup.child_bounty_id) - award_child_bounty { + #[extrinsic_call] + _(RawOrigin::Signed(caller), bounty_setup.bounty_id, bounty_setup.child_bounty_id); + + Ok(()) + } + + #[benchmark] + fn award_child_bounty() -> Result<(), BenchmarkError> { setup_pot_account::(); let bounty_setup = activate_child_bounty::(0, T::MaximumReasonLength::get())?; - let beneficiary_account: T::AccountId = account("beneficiary", 0, SEED); + let beneficiary_account = account::("beneficiary", 0, SEED); let beneficiary = T::Lookup::unlookup(beneficiary_account.clone()); - }: _(RawOrigin::Signed(bounty_setup.child_curator), bounty_setup.bounty_id, - bounty_setup.child_bounty_id, beneficiary) - verify { - assert_last_event::(Event::Awarded { - index: bounty_setup.bounty_id, - child_index: bounty_setup.child_bounty_id, - beneficiary: beneficiary_account - }.into()) + + #[extrinsic_call] + _( + RawOrigin::Signed(bounty_setup.child_curator), + bounty_setup.bounty_id, + bounty_setup.child_bounty_id, + beneficiary, + ); + + assert_last_event::( + Event::Awarded { + index: bounty_setup.bounty_id, + child_index: bounty_setup.child_bounty_id, + beneficiary: beneficiary_account, + } + .into(), + ); + + Ok(()) } - claim_child_bounty { + #[benchmark] + fn claim_child_bounty() -> Result<(), BenchmarkError> { setup_pot_account::(); let bounty_setup = activate_child_bounty::(0, T::MaximumReasonLength::get())?; - let beneficiary_account: T::AccountId = account("beneficiary", 0, SEED); + let beneficiary_account = account("beneficiary", 0, SEED); let beneficiary = T::Lookup::unlookup(beneficiary_account); - ChildBounties::::award_child_bounty( + Pallet::::award_child_bounty( RawOrigin::Signed(bounty_setup.child_curator.clone()).into(), bounty_setup.bounty_id, bounty_setup.child_bounty_id, - beneficiary + beneficiary, )?; - let beneficiary_account: T::AccountId = account("beneficiary", 0, SEED); - let beneficiary = T::Lookup::unlookup(beneficiary_account.clone()); + let beneficiary_account = account("beneficiary", 0, SEED); frame_system::Pallet::::set_block_number(T::BountyDepositPayoutDelay::get()); - ensure!(T::Currency::free_balance(&beneficiary_account).is_zero(), - "Beneficiary already has balance."); - - }: _(RawOrigin::Signed(bounty_setup.curator), bounty_setup.bounty_id, - bounty_setup.child_bounty_id) - verify { - ensure!(!T::Currency::free_balance(&beneficiary_account).is_zero(), - "Beneficiary didn't get paid."); + assert!( + T::Currency::free_balance(&beneficiary_account).is_zero(), + "Beneficiary already has balance." + ); + + #[extrinsic_call] + _( + RawOrigin::Signed(bounty_setup.curator), + bounty_setup.bounty_id, + bounty_setup.child_bounty_id, + ); + + assert!( + !T::Currency::free_balance(&beneficiary_account).is_zero(), + "Beneficiary didn't get paid." + ); + + Ok(()) } // Best case scenario. - close_child_bounty_added { + #[benchmark] + fn close_child_bounty_added() -> Result<(), BenchmarkError> { setup_pot_account::(); let mut bounty_setup = activate_bounty::(0, T::MaximumReasonLength::get())?; - ChildBounties::::add_child_bounty( + Pallet::::add_child_bounty( RawOrigin::Signed(bounty_setup.curator.clone()).into(), bounty_setup.bounty_id, bounty_setup.child_bounty_value, @@ -292,27 +349,44 @@ benchmarks! { )?; bounty_setup.child_bounty_id = ChildBountyCount::::get() - 1; - }: close_child_bounty(RawOrigin::Root, bounty_setup.bounty_id, - bounty_setup.child_bounty_id) - verify { - assert_last_event::(Event::Canceled { - index: bounty_setup.bounty_id, - child_index: bounty_setup.child_bounty_id - }.into()) + #[extrinsic_call] + close_child_bounty(RawOrigin::Root, bounty_setup.bounty_id, bounty_setup.child_bounty_id); + + assert_last_event::( + Event::Canceled { + index: bounty_setup.bounty_id, + child_index: bounty_setup.child_bounty_id, + } + .into(), + ); + + Ok(()) } // Worst case scenario. - close_child_bounty_active { + #[benchmark] + fn close_child_bounty_active() -> Result<(), BenchmarkError> { setup_pot_account::(); let bounty_setup = activate_child_bounty::(0, T::MaximumReasonLength::get())?; - Treasury::::on_initialize(BlockNumberFor::::zero()); - }: close_child_bounty(RawOrigin::Root, bounty_setup.bounty_id, bounty_setup.child_bounty_id) - verify { - assert_last_event::(Event::Canceled { - index: bounty_setup.bounty_id, - child_index: bounty_setup.child_bounty_id, - }.into()) + pallet_treasury::Pallet::::on_initialize(BlockNumberFor::::zero()); + + #[extrinsic_call] + close_child_bounty(RawOrigin::Root, bounty_setup.bounty_id, bounty_setup.child_bounty_id); + + assert_last_event::( + Event::Canceled { + index: bounty_setup.bounty_id, + child_index: bounty_setup.child_bounty_id, + } + .into(), + ); + + Ok(()) } - impl_benchmark_test_suite!(ChildBounties, crate::tests::new_test_ext(), crate::tests::Test) + impl_benchmark_test_suite! { + Pallet, + tests::new_test_ext(), + tests::Test + } } From d5cea353c44652348a17942193d1686d4d7eaeed Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Thu, 31 Oct 2024 19:26:48 +0000 Subject: [PATCH 3/7] Update from ggwpez running command 'prdoc --bump patch --audience runtime_dev' --- prdoc/pr_6310.prdoc | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 prdoc/pr_6310.prdoc diff --git a/prdoc/pr_6310.prdoc b/prdoc/pr_6310.prdoc new file mode 100644 index 000000000000..ab421791dc72 --- /dev/null +++ b/prdoc/pr_6310.prdoc @@ -0,0 +1,12 @@ +title: Migrate pallet-child-bounties benchmark to v2 +doc: +- audience: Runtime Dev + description: |- + Part of: + + - #6202. +crates: +- name: pallet-utility + bump: patch +- name: pallet-child-bounties + bump: patch From a8a3ea936c6633f0d0b496029bd783bdec8a4b1a Mon Sep 17 00:00:00 2001 From: Xavier Lau Date: Fri, 1 Nov 2024 14:28:05 +0800 Subject: [PATCH 4/7] Change as suggested --- .../frame/child-bounties/src/benchmarking.rs | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/substrate/frame/child-bounties/src/benchmarking.rs b/substrate/frame/child-bounties/src/benchmarking.rs index 301d2b44356a..fd0eaeef4b9d 100644 --- a/substrate/frame/child-bounties/src/benchmarking.rs +++ b/substrate/frame/child-bounties/src/benchmarking.rs @@ -20,7 +20,10 @@ #![cfg(feature = "runtime-benchmarks")] use frame_benchmarking::{v2::*, BenchmarkError}; +use frame_support::ensure; use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; +use pallet_bounties::Pallet as Bounties; +use pallet_treasury as Treasury; use crate::*; @@ -99,7 +102,7 @@ fn activate_bounty( ) -> Result, BenchmarkError> { let mut child_bounty_setup = setup_child_bounty::(user, description); let curator_lookup = T::Lookup::unlookup(child_bounty_setup.curator.clone()); - pallet_bounties::Pallet::::propose_bounty( + Bounties::::propose_bounty( RawOrigin::Signed(child_bounty_setup.caller.clone()).into(), child_bounty_setup.value, child_bounty_setup.reason.clone(), @@ -109,15 +112,15 @@ fn activate_bounty( let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - pallet_bounties::Pallet::::approve_bounty(approve_origin, child_bounty_setup.bounty_id)?; - pallet_treasury::Pallet::::on_initialize(BlockNumberFor::::zero()); - pallet_bounties::Pallet::::propose_curator( + Bounties::::approve_bounty(approve_origin, child_bounty_setup.bounty_id)?; + Treasury::::on_initialize(BlockNumberFor::::zero()); + Bounties::::propose_curator( RawOrigin::Root.into(), child_bounty_setup.bounty_id, curator_lookup, child_bounty_setup.fee, )?; - pallet_bounties::Pallet::::accept_curator( + Bounties::::accept_curator( RawOrigin::Signed(child_bounty_setup.curator.clone()).into(), child_bounty_setup.bounty_id, )?; @@ -159,7 +162,7 @@ fn activate_child_bounty( } fn setup_pot_account() { - let pot_account = pallet_bounties::Pallet::::account_id(); + let pot_account = Bounties::::account_id(); let value = T::Currency::minimum_balance().saturating_mul(1_000_000_000u32.into()); let _ = T::Currency::make_free_balance_be(&pot_account, value); } @@ -261,7 +264,7 @@ mod benchmarks { fn unassign_curator() -> Result<(), BenchmarkError> { setup_pot_account::(); let bounty_setup = activate_child_bounty::(0, T::MaximumReasonLength::get())?; - pallet_treasury::Pallet::::on_initialize(BlockNumberFor::::zero()); + Treasury::::on_initialize(BlockNumberFor::::zero()); frame_system::Pallet::::set_block_number(T::BountyUpdatePeriod::get() + 1u32.into()); let caller = whitelisted_caller(); @@ -315,7 +318,7 @@ mod benchmarks { let beneficiary_account = account("beneficiary", 0, SEED); frame_system::Pallet::::set_block_number(T::BountyDepositPayoutDelay::get()); - assert!( + ensure!( T::Currency::free_balance(&beneficiary_account).is_zero(), "Beneficiary already has balance." ); @@ -327,7 +330,7 @@ mod benchmarks { bounty_setup.child_bounty_id, ); - assert!( + ensure!( !T::Currency::free_balance(&beneficiary_account).is_zero(), "Beneficiary didn't get paid." ); @@ -368,7 +371,7 @@ mod benchmarks { fn close_child_bounty_active() -> Result<(), BenchmarkError> { setup_pot_account::(); let bounty_setup = activate_child_bounty::(0, T::MaximumReasonLength::get())?; - pallet_treasury::Pallet::::on_initialize(BlockNumberFor::::zero()); + Treasury::::on_initialize(BlockNumberFor::::zero()); #[extrinsic_call] close_child_bounty(RawOrigin::Root, bounty_setup.bounty_id, bounty_setup.child_bounty_id); From 146041de10189d20a9ab0fff477e9cc439b22a5a Mon Sep 17 00:00:00 2001 From: Giuseppe Re Date: Fri, 8 Nov 2024 11:25:22 +0100 Subject: [PATCH 5/7] Update substrate/frame/utility/src/benchmarking.rs --- substrate/frame/utility/src/benchmarking.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/utility/src/benchmarking.rs b/substrate/frame/utility/src/benchmarking.rs index 487d82d8461c..88556c05195a 100644 --- a/substrate/frame/utility/src/benchmarking.rs +++ b/substrate/frame/utility/src/benchmarking.rs @@ -19,6 +19,7 @@ #![cfg(feature = "runtime-benchmarks")] +use alloc::vec; use frame_benchmarking::{benchmarking::add_to_whitelist, v2::*}; use frame_system::RawOrigin; From e7ff4ed31816dec42c3d54fb3de1d0c59208ff43 Mon Sep 17 00:00:00 2001 From: Xavier Lau Date: Sun, 10 Nov 2024 23:43:02 +0800 Subject: [PATCH 6/7] Format --- substrate/frame/child-bounties/src/benchmarking.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/substrate/frame/child-bounties/src/benchmarking.rs b/substrate/frame/child-bounties/src/benchmarking.rs index 299276cb154a..636b78d12dae 100644 --- a/substrate/frame/child-bounties/src/benchmarking.rs +++ b/substrate/frame/child-bounties/src/benchmarking.rs @@ -245,7 +245,8 @@ mod benchmarks { bounty_setup.child_bounty_value, bounty_setup.reason.clone(), )?; - bounty_setup.child_bounty_id = ParentTotalChildBounties::::get(bounty_setup.bounty_id) - 1; + bounty_setup.child_bounty_id = + ParentTotalChildBounties::::get(bounty_setup.bounty_id) - 1; Pallet::::propose_curator( RawOrigin::Signed(bounty_setup.curator.clone()).into(), @@ -356,7 +357,8 @@ mod benchmarks { bounty_setup.child_bounty_value, bounty_setup.reason.clone(), )?; - bounty_setup.child_bounty_id = ParentTotalChildBounties::::get(bounty_setup.bounty_id) - 1; + bounty_setup.child_bounty_id = + ParentTotalChildBounties::::get(bounty_setup.bounty_id) - 1; #[extrinsic_call] close_child_bounty(RawOrigin::Root, bounty_setup.bounty_id, bounty_setup.child_bounty_id); From 9bc5a88641abd77794a7963b0c8b7e728fc777e1 Mon Sep 17 00:00:00 2001 From: Xavier Lau Date: Tue, 12 Nov 2024 12:15:01 +0800 Subject: [PATCH 7/7] Fix compile --- substrate/frame/child-bounties/src/benchmarking.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/child-bounties/src/benchmarking.rs b/substrate/frame/child-bounties/src/benchmarking.rs index 636b78d12dae..4b2d62cd920e 100644 --- a/substrate/frame/child-bounties/src/benchmarking.rs +++ b/substrate/frame/child-bounties/src/benchmarking.rs @@ -19,6 +19,7 @@ #![cfg(feature = "runtime-benchmarks")] +use alloc::vec; use frame_benchmarking::{v2::*, BenchmarkError}; use frame_support::ensure; use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};