From 24840290af20ef67956c48b7548ab184d45b8889 Mon Sep 17 00:00:00 2001 From: 0xmovses <35300528+0xmovses@users.noreply.github.com> Date: Fri, 13 Oct 2023 21:54:45 +0100 Subject: [PATCH] Refactor alliance benchmarks to v2 (#1868) - This PR refactors `alliance/src/benchmarkings.rs` to use benchmarking v2. These changes are needed to improve the readability and maintainability of the benchmarking code. - No known issue to backlink. ## Local Testing 1. `cargo build --features runtime-benchmarks` 2. `cargo run --locked --release -p node-cli --bin substrate-node --features runtime-benchmarks -- benchmark pallet --execution wasm --wasm-execution compiled --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1` --- substrate/frame/alliance/src/benchmarking.rs | 567 ++++++++++-------- .../procedural/src/pallet/expand/warnings.rs | 8 +- 2 files changed, 315 insertions(+), 260 deletions(-) diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index eb32c6c466c9..77294c6b6646 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -17,6 +17,8 @@ //! Alliance pallet benchmarking. +#![cfg(feature = "runtime-benchmarks")] + use sp_runtime::traits::{Bounded, Hash, StaticLookup}; use sp_std::{ cmp, @@ -25,7 +27,7 @@ use sp_std::{ prelude::*, }; -use frame_benchmarking::v1::{account, benchmarks_instance_pallet, BenchmarkError}; +use frame_benchmarking::{account, impl_benchmark_test_suite, v2::*, BenchmarkError}; use frame_support::traits::{EnsureOrigin, Get, UnfilteredDispatchable}; use frame_system::{pallet_prelude::BlockNumberFor, Pallet as System, RawOrigin as SystemOrigin}; @@ -94,32 +96,31 @@ fn set_members, I: 'static>() { T::InitializeMembers::initialize_members(&[fellows.as_slice()].concat()); } -benchmarks_instance_pallet! { - // This tests when proposal is created and queued as "proposed" - propose_proposed { - let b in 1 .. MAX_BYTES; - let m in 2 .. T::MaxFellows::get(); - let p in 1 .. T::MaxProposals::get(); +#[instance_benchmarks] +mod benchmarks { + use super::*; + // This tests when proposal is created and queued as "proposed" + #[benchmark] + fn propose_proposed( + b: Linear<1, MAX_BYTES>, + m: Linear<2, { T::MaxFellows::get() }>, + p: Linear<1, { T::MaxProposals::get() }>, + ) -> Result<(), BenchmarkError> { let bytes_in_storage = b + size_of::() as u32 + 32; // Construct `members`. - let fellows = (0 .. m).map(fellow::).collect::>(); + let fellows = (0..m).map(fellow::).collect::>(); let proposer = fellows[0].clone(); - Alliance::::init_members( - SystemOrigin::Root.into(), - fellows, - vec![], - )?; + Alliance::::init_members(SystemOrigin::Root.into(), fellows, vec![])?; let threshold = m; // Add previous proposals. - for i in 0 .. p - 1 { + for i in 0..p - 1 { // Proposals should be different so that different proposal hashes are generated - let proposal: T::Proposal = AllianceCall::::set_rule { - rule: rule(vec![i as u8; b as usize]) - }.into(); + let proposal: T::Proposal = + AllianceCall::::set_rule { rule: rule(vec![i as u8; b as usize]) }.into(); Alliance::::propose( SystemOrigin::Signed(proposer.clone()).into(), threshold, @@ -128,45 +129,45 @@ benchmarks_instance_pallet! { )?; } - let proposal: T::Proposal = AllianceCall::::set_rule { rule: rule(vec![p as u8; b as usize]) }.into(); + let proposal: T::Proposal = + AllianceCall::::set_rule { rule: rule(vec![p as u8; b as usize]) }.into(); + + #[extrinsic_call] + propose( + SystemOrigin::Signed(proposer.clone()), + threshold, + Box::new(proposal.clone()), + bytes_in_storage, + ); - }: propose(SystemOrigin::Signed(proposer.clone()), threshold, Box::new(proposal.clone()), bytes_in_storage) - verify { - // New proposal is recorded let proposal_hash = T::Hashing::hash_of(&proposal); assert_eq!(T::ProposalProvider::proposal_of(proposal_hash), Some(proposal)); + Ok(()) } - vote { - // We choose 5 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 5 .. T::MaxFellows::get(); - + #[benchmark] + fn vote(m: Linear<5, { T::MaxFellows::get() }>) -> Result<(), BenchmarkError> { let p = T::MaxProposals::get(); let b = MAX_BYTES; - let bytes_in_storage = b + size_of::() as u32 + 32; + let _bytes_in_storage = b + size_of::() as u32 + 32; // Construct `members`. - let fellows = (0 .. m).map(fellow::).collect::>(); + let fellows = (0..m).map(fellow::).collect::>(); let proposer = fellows[0].clone(); let members = fellows.clone(); - Alliance::::init_members( - SystemOrigin::Root.into(), - fellows, - vec![], - )?; + Alliance::::init_members(SystemOrigin::Root.into(), fellows, vec![])?; // Threshold is 1 less than the number of members so that one person can vote nay let threshold = m - 1; // Add previous proposals let mut last_hash = T::Hash::default(); - for i in 0 .. p { + for i in 0..p { // Proposals should be different so that different proposal hashes are generated - let proposal: T::Proposal = AllianceCall::::set_rule { - rule: rule(vec![i as u8; b as usize]) - }.into(); + let proposal: T::Proposal = + AllianceCall::::set_rule { rule: rule(vec![i as u8; b as usize]) }.into(); Alliance::::propose( SystemOrigin::Signed(proposer.clone()).into(), threshold, @@ -178,7 +179,7 @@ benchmarks_instance_pallet! { let index = p - 1; // Have almost everyone vote aye on last proposal, while keeping it from passing. - for j in 0 .. m - 3 { + for j in 0..m - 3 { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), @@ -203,28 +204,28 @@ benchmarks_instance_pallet! { // Whitelist voter account from further DB operations. let voter_key = frame_system::Account::::hashed_key_for(&voter); frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); - }: _(SystemOrigin::Signed(voter), last_hash.clone(), index, approve) - verify { - } - close_early_disapproved { - // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 4 .. T::MaxFellows::get(); - let p in 1 .. T::MaxProposals::get(); + #[extrinsic_call] + _(SystemOrigin::Signed(voter), last_hash.clone(), index, approve); + //nothing to verify + Ok(()) + } + + #[benchmark] + fn close_early_disapproved( + m: Linear<4, { T::MaxFellows::get() }>, + p: Linear<1, { T::MaxProposals::get() }>, + ) -> Result<(), BenchmarkError> { let bytes = 100; let bytes_in_storage = bytes + size_of::() as u32 + 32; // Construct `members`. - let fellows = (0 .. m).map(fellow::).collect::>(); + let fellows = (0..m).map(fellow::).collect::>(); let members = fellows.clone(); - Alliance::::init_members( - SystemOrigin::Root.into(), - fellows, - vec![], - )?; + Alliance::::init_members(SystemOrigin::Root.into(), fellows, vec![])?; let proposer = members[0].clone(); let voter = members[1].clone(); @@ -234,11 +235,10 @@ benchmarks_instance_pallet! { // Add previous proposals let mut last_hash = T::Hash::default(); - for i in 0 .. p { + for i in 0..p { // Proposals should be different so that different proposal hashes are generated - let proposal: T::Proposal = AllianceCall::::set_rule { - rule: rule(vec![i as u8; bytes as usize]) - }.into(); + let proposal: T::Proposal = + AllianceCall::::set_rule { rule: rule(vec![i as u8; bytes as usize]) }.into(); Alliance::::propose( SystemOrigin::Signed(proposer.clone()).into(), threshold, @@ -251,7 +251,7 @@ benchmarks_instance_pallet! { let index = p - 1; // Have most everyone vote aye on last proposal, while keeping it from passing. - for j in 2 .. m - 1 { + for j in 2..m - 1 { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), @@ -280,44 +280,41 @@ benchmarks_instance_pallet! { // Whitelist voter account from further DB operations. let voter_key = frame_system::Account::::hashed_key_for(&voter); frame_benchmarking::benchmarking::add_to_whitelist(voter_key.into()); - }: close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage) - verify { - // The last proposal is removed. + + #[extrinsic_call] + close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + Ok(()) } - close_early_approved { - let b in 1 .. MAX_BYTES; - // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 4 .. T::MaxFellows::get(); - let p in 1 .. T::MaxProposals::get(); - + // We choose 4 as a minimum for param m, so we always trigger a vote in the voting loop (`for j + // in ...`) + #[benchmark] + fn close_early_approved( + b: Linear<1, MAX_BYTES>, + m: Linear<4, { T::MaxFellows::get() }>, + p: Linear<1, { T::MaxProposals::get() }>, + ) -> Result<(), BenchmarkError> { let bytes_in_storage = b + size_of::() as u32 + 32; // Construct `members`. - let fellows = (0 .. m).map(fellow::).collect::>(); + let fellows = (0..m).map(fellow::).collect::>(); let members = fellows.clone(); - Alliance::::init_members( - SystemOrigin::Root.into(), - fellows, - vec![], - )?; + Alliance::::init_members(SystemOrigin::Root.into(), fellows, vec![])?; let proposer = members[0].clone(); - let voter = members[1].clone(); - // Threshold is 2 so any two ayes will approve the vote let threshold = 2; // Add previous proposals let mut last_hash = T::Hash::default(); - for i in 0 .. p { + for i in 0..p { // Proposals should be different so that different proposal hashes are generated - let proposal: T::Proposal = AllianceCall::::set_rule { - rule: rule(vec![i as u8; b as usize]) - }.into(); + let proposal: T::Proposal = + AllianceCall::::set_rule { rule: rule(vec![i as u8; b as usize]) }.into(); Alliance::::propose( SystemOrigin::Signed(proposer.clone()).into(), threshold, @@ -329,7 +326,8 @@ benchmarks_instance_pallet! { } let index = p - 1; - // Caller switches vote to nay on their own proposal, allowing them to be the deciding approval vote + // Caller switches vote to nay on their own proposal, allowing them to be the deciding + // approval vote Alliance::::vote( SystemOrigin::Signed(proposer.clone()).into(), last_hash.clone(), @@ -338,7 +336,7 @@ benchmarks_instance_pallet! { )?; // Have almost everyone vote nay on last proposal, while keeping it from failing. - for j in 2 .. m - 1 { + for j in 2..m - 1 { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), @@ -364,30 +362,28 @@ benchmarks_instance_pallet! { index, true, )?; - }: close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage) - verify { - // The last proposal is removed. + + #[extrinsic_call] + close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + Ok(()) } - close_disapproved { - // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 2 .. T::MaxFellows::get(); - let p in 1 .. T::MaxProposals::get(); - + #[benchmark] + fn close_disapproved( + m: Linear<2, { T::MaxFellows::get() }>, + p: Linear<1, { T::MaxProposals::get() }>, + ) -> Result<(), BenchmarkError> { let bytes = 100; let bytes_in_storage = bytes + size_of::() as u32 + 32; // Construct `members`. - let fellows = (0 .. m).map(fellow::).collect::>(); + let fellows = (0..m).map(fellow::).collect::>(); let members = fellows.clone(); - Alliance::::init_members( - SystemOrigin::Root.into(), - fellows, - vec![], - )?; + Alliance::::init_members(SystemOrigin::Root.into(), fellows, vec![])?; let proposer = members[0].clone(); let voter = members[1].clone(); @@ -397,11 +393,10 @@ benchmarks_instance_pallet! { // Add proposals let mut last_hash = T::Hash::default(); - for i in 0 .. p { + for i in 0..p { // Proposals should be different so that different proposal hashes are generated - let proposal: T::Proposal = AllianceCall::::set_rule { - rule: rule(vec![i as u8; bytes as usize]) - }.into(); + let proposal: T::Proposal = + AllianceCall::::set_rule { rule: rule(vec![i as u8; bytes as usize]) }.into(); Alliance::::propose( SystemOrigin::Signed(proposer.clone()).into(), threshold, @@ -415,7 +410,7 @@ benchmarks_instance_pallet! { let index = p - 1; // Have almost everyone vote aye on last proposal, while keeping it from passing. // A few abstainers will be the nay votes needed to fail the vote. - for j in 2 .. m - 1 { + for j in 2..m - 1 { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), @@ -434,44 +429,41 @@ benchmarks_instance_pallet! { System::::set_block_number(BlockNumberFor::::max_value()); - }: close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage) - verify { + #[extrinsic_call] + close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + // The last proposal is removed. assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + Ok(()) } - close_approved { - let b in 1 .. MAX_BYTES; - // We choose 4 fellows as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m in 5 .. T::MaxFellows::get(); - let p in 1 .. T::MaxProposals::get(); - + // We choose 5 fellows as a minimum so we always trigger a vote in the voting loop (`for j in + // ...`) + #[benchmark] + fn close_approved( + b: Linear<1, MAX_BYTES>, + m: Linear<5, { T::MaxFellows::get() }>, + p: Linear<1, { T::MaxProposals::get() }>, + ) -> Result<(), BenchmarkError> { let bytes_in_storage = b + size_of::() as u32 + 32; // Construct `members`. - let fellows = (0 .. m).map(fellow::).collect::>(); + let fellows = (0..m).map(fellow::).collect::>(); let members = fellows.clone(); - Alliance::::init_members( - SystemOrigin::Root.into(), - fellows, - vec![], - )?; + Alliance::::init_members(SystemOrigin::Root.into(), fellows, vec![])?; let proposer = members[0].clone(); - let voter = members[1].clone(); - // Threshold is two, so any two ayes will pass the vote let threshold = 2; // Add proposals let mut last_hash = T::Hash::default(); - for i in 0 .. p { + for i in 0..p { // Proposals should be different so that different proposal hashes are generated - let proposal: T::Proposal = AllianceCall::::set_rule { - rule: rule(vec![i as u8; b as usize]) - }.into(); + let proposal: T::Proposal = + AllianceCall::::set_rule { rule: rule(vec![i as u8; b as usize]) }.into(); Alliance::::propose( SystemOrigin::Signed(proposer.clone()).into(), threshold, @@ -487,70 +479,71 @@ benchmarks_instance_pallet! { SystemOrigin::Signed(proposer.clone()).into(), last_hash.clone(), p - 1, - true // Vote aye. + true, // Vote aye. )?; let index = p - 1; // Have almost everyone vote nay on last proposal, while keeping it from failing. // A few abstainers will be the aye votes needed to pass the vote. - for j in 2 .. m - 1 { + for j in 2..m - 1 { let voter = &members[j as usize]; Alliance::::vote( SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), index, - false + false, )?; } // caller is prime, prime already votes aye by creating the proposal System::::set_block_number(BlockNumberFor::::max_value()); - }: close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage) - verify { - // The last proposal is removed. + #[extrinsic_call] + close( + SystemOrigin::Signed(proposer), + last_hash.clone(), + index, + Weight::MAX, + bytes_in_storage, + ); + assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + Ok(()) } - init_members { - // at least 1 fellow - let m in 1 .. T::MaxFellows::get(); - let z in 0 .. T::MaxAllies::get(); + #[benchmark] + fn init_members( + m: Linear<1, { T::MaxFellows::get() }>, + z: Linear<0, { T::MaxAllies::get() }>, + ) -> Result<(), BenchmarkError> { + let mut fellows = (0..m).map(fellow::).collect::>(); + let mut allies = (0..z).map(ally::).collect::>(); - let mut fellows = (0 .. m).map(fellow::).collect::>(); - let mut allies = (0 .. z).map(ally::).collect::>(); + #[extrinsic_call] + _(SystemOrigin::Root, fellows.clone(), allies.clone()); - }: _(SystemOrigin::Root, fellows.clone(), allies.clone()) - verify { fellows.sort(); allies.sort(); - assert_last_event::(Event::MembersInitialized { - fellows: fellows.clone(), - allies: allies.clone(), - }.into()); + assert_last_event::( + Event::MembersInitialized { fellows: fellows.clone(), allies: allies.clone() }.into(), + ); assert_eq!(Alliance::::members(MemberRole::Fellow), fellows); assert_eq!(Alliance::::members(MemberRole::Ally), allies); + Ok(()) } - disband { - // at least 1 founders - let x in 1 .. T::MaxFellows::get(); - let y in 0 .. T::MaxAllies::get(); - let z in 0 .. T::MaxMembersCount::get() / 2; - - let fellows = (0 .. x).map(fellow::).collect::>(); - let allies = (0 .. y).map(ally::).collect::>(); - let witness = DisbandWitness{ - fellow_members: x, - ally_members: y, - }; + #[benchmark] + fn disband( + x: Linear<1, { T::MaxFellows::get() }>, + y: Linear<0, { T::MaxAllies::get() }>, + z: Linear<0, { T::MaxMembersCount::get() / 2 }>, + ) -> Result<(), BenchmarkError> { + let fellows = (0..x).map(fellow::).collect::>(); + let allies = (0..y).map(ally::).collect::>(); + let witness = DisbandWitness { fellow_members: x, ally_members: y }; // setting the Alliance to disband on the benchmark call - Alliance::::init_members( - SystemOrigin::Root.into(), - fellows.clone(), - allies.clone(), - )?; + Alliance::::init_members(SystemOrigin::Root.into(), fellows.clone(), allies.clone())?; // reserve deposits let deposit = T::AllyDeposit::get(); @@ -561,18 +554,25 @@ benchmarks_instance_pallet! { assert_eq!(Alliance::::voting_members_count(), x); assert_eq!(Alliance::::ally_members_count(), y); - }: _(SystemOrigin::Root, witness) - verify { - assert_last_event::(Event::AllianceDisbanded { - fellow_members: x, - ally_members: y, - unreserved: cmp::min(z, x + y), - }.into()); + + #[extrinsic_call] + _(SystemOrigin::Root, witness); + + assert_last_event::( + Event::AllianceDisbanded { + fellow_members: x, + ally_members: y, + unreserved: cmp::min(z, x + y), + } + .into(), + ); assert!(!Alliance::::is_initialized()); + Ok(()) } - set_rule { + #[benchmark] + fn set_rule() -> Result<(), BenchmarkError> { set_members::(); let rule = rule(b"hello world"); @@ -580,61 +580,86 @@ benchmarks_instance_pallet! { let call = Call::::set_rule { rule: rule.clone() }; let origin = T::AdminOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: { call.dispatch_bypass_filter(origin)? } - verify { + + #[block] + { + call.dispatch_bypass_filter(origin)?; + } assert_eq!(Alliance::::rule(), Some(rule.clone())); assert_last_event::(Event::NewRuleSet { rule }.into()); + Ok(()) } - announce { + #[benchmark] + fn announce() -> Result<(), BenchmarkError> { set_members::(); let announcement = announcement(b"hello world"); let call = Call::::announce { announcement: announcement.clone() }; - let origin = - T::AnnouncementOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: { call.dispatch_bypass_filter(origin)? } - verify { + let origin = T::AnnouncementOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; + + #[block] + { + call.dispatch_bypass_filter(origin)?; + } + assert!(Alliance::::announcements().contains(&announcement)); assert_last_event::(Event::Announced { announcement }.into()); + Ok(()) } - remove_announcement { + #[benchmark] + fn remove_announcement() -> Result<(), BenchmarkError> { set_members::(); let announcement = announcement(b"hello world"); - let announcements: BoundedVec<_, T::MaxAnnouncementsCount> = BoundedVec::try_from(vec![announcement.clone()]).unwrap(); + let announcements: BoundedVec<_, T::MaxAnnouncementsCount> = + BoundedVec::try_from(vec![announcement.clone()]).unwrap(); Announcements::::put(announcements); let call = Call::::remove_announcement { announcement: announcement.clone() }; - let origin = - T::AnnouncementOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: { call.dispatch_bypass_filter(origin)? } - verify { - assert!(Alliance::::announcements().is_empty()); + let origin = T::AnnouncementOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; + + #[block] + { + call.dispatch_bypass_filter(origin)?; + } + + assert!(!Alliance::::announcements().contains(&announcement)); assert_last_event::(Event::AnnouncementRemoved { announcement }.into()); + Ok(()) } - join_alliance { + #[benchmark] + fn join_alliance() -> Result<(), BenchmarkError> { set_members::(); let outsider = outsider::(1); assert!(!Alliance::::is_member(&outsider)); assert_eq!(DepositOf::::get(&outsider), None); - }: _(SystemOrigin::Signed(outsider.clone())) - verify { + + #[extrinsic_call] + _(SystemOrigin::Signed(outsider.clone())); + assert!(Alliance::::is_member_of(&outsider, MemberRole::Ally)); // outsider is now an ally assert_eq!(DepositOf::::get(&outsider), Some(T::AllyDeposit::get())); // with a deposit assert!(!Alliance::::has_voting_rights(&outsider)); // allies don't have voting rights - assert_last_event::(Event::NewAllyJoined { - ally: outsider, - nominator: None, - reserved: Some(T::AllyDeposit::get()) - }.into()); + assert_last_event::( + Event::NewAllyJoined { + ally: outsider, + nominator: None, + reserved: Some(T::AllyDeposit::get()), + } + .into(), + ); + Ok(()) } - nominate_ally { + #[benchmark] + fn nominate_ally() -> Result<(), BenchmarkError> { set_members::(); let fellow1 = fellow::(1); @@ -645,19 +670,23 @@ benchmarks_instance_pallet! { assert_eq!(DepositOf::::get(&outsider), None); let outsider_lookup = T::Lookup::unlookup(outsider.clone()); - }: _(SystemOrigin::Signed(fellow1.clone()), outsider_lookup) - verify { + + #[extrinsic_call] + _(SystemOrigin::Signed(fellow1.clone()), outsider_lookup); + assert!(Alliance::::is_member_of(&outsider, MemberRole::Ally)); // outsider is now an ally assert_eq!(DepositOf::::get(&outsider), None); // without a deposit assert!(!Alliance::::has_voting_rights(&outsider)); // allies don't have voting rights - assert_last_event::(Event::NewAllyJoined { - ally: outsider, - nominator: Some(fellow1), - reserved: None - }.into()); + assert_last_event::( + Event::NewAllyJoined { ally: outsider, nominator: Some(fellow1), reserved: None } + .into(), + ); + + Ok(()) } - elevate_ally { + #[benchmark] + fn elevate_ally() -> Result<(), BenchmarkError> { set_members::(); let ally1 = ally::(1); @@ -665,59 +694,69 @@ benchmarks_instance_pallet! { let ally1_lookup = T::Lookup::unlookup(ally1.clone()); let call = Call::::elevate_ally { ally: ally1_lookup }; - let origin = - T::MembershipManager::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: { call.dispatch_bypass_filter(origin)? } - verify { + let origin = T::MembershipManager::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; + + #[block] + { + call.dispatch_bypass_filter(origin)?; + } + assert!(!Alliance::::is_ally(&ally1)); assert!(Alliance::::has_voting_rights(&ally1)); assert_last_event::(Event::AllyElevated { ally: ally1 }.into()); + Ok(()) } - give_retirement_notice { + #[benchmark] + fn give_retirement_notice() -> Result<(), BenchmarkError> { set_members::(); let fellow2 = fellow::(2); assert!(Alliance::::has_voting_rights(&fellow2)); - }: _(SystemOrigin::Signed(fellow2.clone())) - verify { + + #[extrinsic_call] + _(SystemOrigin::Signed(fellow2.clone())); + assert!(Alliance::::is_member_of(&fellow2, MemberRole::Retiring)); assert_eq!( RetiringMembers::::get(&fellow2), Some(System::::block_number() + T::RetirementPeriod::get()) ); - assert_last_event::( - Event::MemberRetirementPeriodStarted {member: fellow2}.into() - ); + assert_last_event::(Event::MemberRetirementPeriodStarted { member: fellow2 }.into()); + Ok(()) } - retire { + #[benchmark] + fn retire() -> Result<(), BenchmarkError> { set_members::(); let fellow2 = fellow::(2); assert!(Alliance::::has_voting_rights(&fellow2)); assert_eq!( - Alliance::::give_retirement_notice( - SystemOrigin::Signed(fellow2.clone()).into() - ), + Alliance::::give_retirement_notice(SystemOrigin::Signed(fellow2.clone()).into()), Ok(()) ); System::::set_block_number(System::::block_number() + T::RetirementPeriod::get()); assert_eq!(DepositOf::::get(&fellow2), Some(T::AllyDeposit::get())); - }: _(SystemOrigin::Signed(fellow2.clone())) - verify { + + #[extrinsic_call] + _(SystemOrigin::Signed(fellow2.clone())); + assert!(!Alliance::::is_member(&fellow2)); assert_eq!(DepositOf::::get(&fellow2), None); - assert_last_event::(Event::MemberRetired { - member: fellow2, - unreserved: Some(T::AllyDeposit::get()) - }.into()); + assert_last_event::( + Event::MemberRetired { member: fellow2, unreserved: Some(T::AllyDeposit::get()) } + .into(), + ); + Ok(()) } - kick_member { + #[benchmark] + fn kick_member() -> Result<(), BenchmarkError> { set_members::(); let fellow2 = fellow::(2); @@ -726,58 +765,71 @@ benchmarks_instance_pallet! { let fellow2_lookup = T::Lookup::unlookup(fellow2.clone()); let call = Call::::kick_member { who: fellow2_lookup }; - let origin = - T::MembershipManager::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: { call.dispatch_bypass_filter(origin)? } - verify { + let origin = T::MembershipManager::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; + + #[block] + { + call.dispatch_bypass_filter(origin)?; + } + assert!(!Alliance::::is_member(&fellow2)); assert_eq!(DepositOf::::get(&fellow2), None); - assert_last_event::(Event::MemberKicked { - member: fellow2, - slashed: Some(T::AllyDeposit::get()) - }.into()); + assert_last_event::( + Event::MemberKicked { member: fellow2, slashed: Some(T::AllyDeposit::get()) }.into(), + ); + Ok(()) } - add_unscrupulous_items { - let n in 0 .. T::MaxUnscrupulousItems::get(); - let l in 0 .. T::MaxWebsiteUrlLength::get(); - + #[benchmark] + fn add_scrupulous_items( + n: Linear<0, { T::MaxUnscrupulousItems::get() }>, + l: Linear<0, { T::MaxWebsiteUrlLength::get() }>, + ) -> Result<(), BenchmarkError> { set_members::(); - let accounts = (0 .. n) - .map(|i| generate_unscrupulous_account::(i)) + let accounts = (0..n).map(|i| generate_unscrupulous_account::(i)).collect::>(); + let websites = (0..n) + .map(|i| -> BoundedVec { + BoundedVec::try_from(vec![i as u8; l as usize]).unwrap() + }) .collect::>(); - let websites = (0 .. n).map(|i| -> BoundedVec { - BoundedVec::try_from(vec![i as u8; l as usize]).unwrap() - }).collect::>(); let mut unscrupulous_list = Vec::with_capacity(accounts.len() + websites.len()); unscrupulous_list.extend(accounts.into_iter().map(UnscrupulousItem::AccountId)); unscrupulous_list.extend(websites.into_iter().map(UnscrupulousItem::Website)); let call = Call::::add_unscrupulous_items { items: unscrupulous_list.clone() }; - let origin = - T::AnnouncementOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: { call.dispatch_bypass_filter(origin)? } - verify { + let origin = T::AnnouncementOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; + + #[block] + { + call.dispatch_bypass_filter(origin)?; + } + assert_last_event::(Event::UnscrupulousItemAdded { items: unscrupulous_list }.into()); + Ok(()) } - remove_unscrupulous_items { - let n in 0 .. T::MaxUnscrupulousItems::get(); - let l in 0 .. T::MaxWebsiteUrlLength::get(); - + #[benchmark] + fn remove_unscrupulous_items( + n: Linear<0, { T::MaxUnscrupulousItems::get() }>, + l: Linear<0, { T::MaxWebsiteUrlLength::get() }>, + ) -> Result<(), BenchmarkError> { set_members::(); - let mut accounts = (0 .. n) - .map(|i| generate_unscrupulous_account::(i)) - .collect::>(); + let mut accounts = + (0..n).map(|i| generate_unscrupulous_account::(i)).collect::>(); accounts.sort(); let accounts: BoundedVec<_, T::MaxUnscrupulousItems> = accounts.try_into().unwrap(); UnscrupulousAccounts::::put(accounts.clone()); - let mut websites = (0 .. n).map(|i| -> BoundedVec<_, T::MaxWebsiteUrlLength> - { BoundedVec::try_from(vec![i as u8; l as usize]).unwrap() }).collect::>(); + let mut websites = (0..n) + .map(|i| -> BoundedVec<_, T::MaxWebsiteUrlLength> { + BoundedVec::try_from(vec![i as u8; l as usize]).unwrap() + }) + .collect::>(); websites.sort(); let websites: BoundedVec<_, T::MaxUnscrupulousItems> = websites.try_into().unwrap(); UnscrupulousWebsites::::put(websites.clone()); @@ -787,24 +839,31 @@ benchmarks_instance_pallet! { unscrupulous_list.extend(websites.into_iter().map(UnscrupulousItem::Website)); let call = Call::::remove_unscrupulous_items { items: unscrupulous_list.clone() }; - let origin = - T::AnnouncementOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: { call.dispatch_bypass_filter(origin)? } - verify { - assert_last_event::(Event::UnscrupulousItemRemoved { items: unscrupulous_list }.into()); + let origin = T::AnnouncementOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; + + #[block] + { + call.dispatch_bypass_filter(origin)?; + } + + assert_last_event::( + Event::UnscrupulousItemRemoved { items: unscrupulous_list }.into(), + ); + Ok(()) } - abdicate_fellow_status { + #[benchmark] + fn abdicate_fellow_status() -> Result<(), BenchmarkError> { set_members::(); let fellow2 = fellow::(2); assert!(Alliance::::has_voting_rights(&fellow2)); - }: _(SystemOrigin::Signed(fellow2.clone())) - verify { - assert!(Alliance::::is_member_of(&fellow2, MemberRole::Ally)); - assert_last_event::( - Event::FellowAbdicated {fellow: fellow2}.into() - ); + #[extrinsic_call] + _(SystemOrigin::Signed(fellow2.clone())); + + assert_last_event::(Event::FellowAbdicated { fellow: fellow2 }.into()); + Ok(()) } impl_benchmark_test_suite!(Alliance, crate::mock::new_bench_ext(), crate::mock::Test); diff --git a/substrate/frame/support/procedural/src/pallet/expand/warnings.rs b/substrate/frame/support/procedural/src/pallet/expand/warnings.rs index ae5890878a2f..030e3ddaf323 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/warnings.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/warnings.rs @@ -33,9 +33,7 @@ pub(crate) fn weight_witness_warning( if dev_mode { return } - let CallWeightDef::Immediate(w) = &method.weight else { - return; - }; + let CallWeightDef::Immediate(w) = &method.weight else { return }; let partial_warning = Warning::new_deprecated("UncheckedWeightWitness") .old("not check weight witness data") @@ -66,9 +64,7 @@ pub(crate) fn weight_constant_warning( if dev_mode { return } - let syn::Expr::Lit(lit) = weight else { - return; - }; + let syn::Expr::Lit(lit) = weight else { return }; let warning = Warning::new_deprecated("ConstantWeight") .index(warnings.len())