From 2af5dc8ec24118016679202e490956c7ec98f9be Mon Sep 17 00:00:00 2001 From: 0xmovses Date: Thu, 12 Oct 2023 19:48:32 +0100 Subject: [PATCH 1/5] refactor benchmark v1 --- substrate/frame/alliance/src/benchmarking.rs | 535 +++++++++---------- 1 file changed, 259 insertions(+), 276 deletions(-) diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index eb32c6c466c9..53f6f7b84105 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,9 @@ use sp_std::{ prelude::*, }; -use frame_benchmarking::v1::{account, benchmarks_instance_pallet, BenchmarkError}; +use frame_benchmarking::{ + account, impl_benchmark_test_suite, v2::*, whitelisted_caller, BenchmarkError, +}; use frame_support::traits::{EnsureOrigin, Get, UnfilteredDispatchable}; use frame_system::{pallet_prelude::BlockNumberFor, Pallet as System, RawOrigin as SystemOrigin}; @@ -94,32 +98,31 @@ fn set_members, I: 'static>() { T::InitializeMembers::initialize_members(&[fellows.as_slice()].concat()); } -benchmarks_instance_pallet! { +#[instance_benchmarks] +mod benchmarks { + use super::*; + // 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(); + #[benchmark] + fn propose_proposed() -> Result<(), BenchmarkError> { + let b = 1..MAX_BYTES; + let m = 2..T::MaxFellows::get(); + let p = 1..T::MaxProposals::get(); 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 +131,47 @@ 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), + 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)); + ensure!(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() -> Result<(), BenchmarkError> { + let m = 5..T::MaxFellows::get(); let p = T::MaxProposals::get(); let b = MAX_BYTES; 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 +183,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 +208,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 { + + #[extrinsic_call] + _(SystemOrigin::Signed(voter), last_hash.clone(), index, approve); + + //nothing to verify + Ok(()) } - 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(); + #[benchmark] + fn close_early_disapproved() -> Result<(), BenchmarkError> { + let m = 4..T::MaxFellows::get(); + let p = 1..T::MaxProposals::get(); 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 +239,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 +255,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,30 +284,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()); - }: close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage) - verify { - // The last proposal is removed. - assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + + #[extrinsic_call] + close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + + ensure!(T::ProposalProvider::proposal_of(last_hash), None); } - close_early_approved { - let b in 1 .. MAX_BYTES; + #[benchmark] + fn close_early_approved() -> Result<(), BenchmarkError> { + let b = 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(); + let m = 4..T::MaxFellows::get(); + let p = 1..T::MaxProposals::get(); 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(); @@ -313,11 +315,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; 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, @@ -338,7 +339,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,100 +365,29 @@ benchmarks_instance_pallet! { index, true, )?; - }: close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage) - verify { - // The last proposal is removed. - assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); - } - - 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(); - - let bytes = 100; - let bytes_in_storage = bytes + size_of::() as u32 + 32; - - // Construct `members`. - let fellows = (0 .. m).map(fellow::).collect::>(); - - let members = fellows.clone(); - - Alliance::::init_members( - SystemOrigin::Root.into(), - fellows, - vec![], - )?; - - let proposer = members[0].clone(); - let voter = members[1].clone(); - - // Threshold is one less than total members so that two nays will disapprove the vote - let threshold = m - 1; - - // Add proposals - let mut last_hash = T::Hash::default(); - 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(); - Alliance::::propose( - SystemOrigin::Signed(proposer.clone()).into(), - threshold, - Box::new(proposal.clone()), - bytes_in_storage, - )?; - last_hash = T::Hashing::hash_of(&proposal); - assert_eq!(T::ProposalProvider::proposal_of(last_hash), Some(proposal)); - } - - 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 { - let voter = &members[j as usize]; - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), - index, - true, - )?; - } - - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), - index, - false, - )?; - System::::set_block_number(BlockNumberFor::::max_value()); + #[extrinsic_call] + close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); - }: close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage) - verify { - // The last proposal is removed. - assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + ensure!(T::ProposalProvider::proposal_of(last_hash), None); + Ok(()) } - close_approved { - let b in 1 .. MAX_BYTES; + #[benchmark] + fn close_disapproved() -> Result<(), BenchmarkError> { + let b = 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(); + let m = 5..T::MaxFellows::get(); + let p = 1..T::MaxProposals::get(); 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(); @@ -467,11 +397,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; 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 +416,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. - assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + #[extrinsic_call] + close( + SystemOrigin::Signed(proposer), + last_hash.clone(), + index, + Weight::MAX, + bytes_in_storage, + ); + + ensure!(T::ProposalProvider::proposal_of(last_hash), None); } - init_members { + #[benchmark] + fn init_members() -> Result<(), BenchmarkError> { // at least 1 fellow - let m in 1 .. T::MaxFellows::get(); - let z in 0 .. T::MaxAllies::get(); + let m = 1..T::MaxFellows::get(); + let z = 0..T::MaxAllies::get(); - 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_eq!(Alliance::::members(MemberRole::Fellow), fellows); - assert_eq!(Alliance::::members(MemberRole::Ally), allies); + assert_last_event::( + Event::MembersInitialized { fellows: fellows.clone(), allies: allies.clone() }.into(), + ); + ensure!(Alliance::::members(MemberRole::Fellow), fellows); + ensure!(Alliance::::members(MemberRole::Ally), allies); } - disband { + #[benchmark] + fn disband() -> Result<(), BenchmarkError> { // 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 x = 1..T::MaxFellows::get(); + let y = 0..T::MaxAllies::get(); + let z = 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, - }; + 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(); @@ -559,20 +489,26 @@ benchmarks_instance_pallet! { >::insert(&member, deposit); } - 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()); + ensure!(Alliance::::voting_members_count(), x); + ensure!(Alliance::::ally_members_count(), y); + + #[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()); } - set_rule { + #[benchmark] + fn set_rule() -> Result<(), BenchmarkError> { set_members::(); let rule = rule(b"hello world"); @@ -580,61 +516,84 @@ 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 { - assert_eq!(Alliance::::rule(), Some(rule.clone())); + + #[block] + { + call.dispatch_bypass_filter(origin)? + } + ensure!(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()); } - 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 { + ensure!(DepositOf::::get(&outsider), None); + + #[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(), + ); } - nominate_ally { + #[benchmark] + fn nominate_ally() -> Result<(), BenchmarkError> { set_members::(); let fellow1 = fellow::(1); @@ -645,19 +604,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 +628,67 @@ 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()); } - 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(), + ); } - kick_member { + #[benchmark] + fn kick_member() -> Result<(), BenchmarkError> { set_members::(); let fellow2 = fellow::(2); @@ -726,42 +697,54 @@ 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(), + ); } - add_unscrupulous_items { - let n in 0 .. T::MaxUnscrupulousItems::get(); - let l in 0 .. T::MaxWebsiteUrlLength::get(); + #[benchmark] + fn add_scrupulous_items() -> Result<(), BenchmarkError> { + let n = 0..T::MaxUnscrupulousItems::get(); + let l = 0..T::MaxWebsiteUrlLength::get(); 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(()) } +} + +benchmarks_instance_pallet! { remove_unscrupulous_items { let n in 0 .. T::MaxUnscrupulousItems::get(); From 9aa362dfece80583e958e3ee79f5a65d578ec744 Mon Sep 17 00:00:00 2001 From: 0xmovses Date: Fri, 13 Oct 2023 11:59:45 +0100 Subject: [PATCH 2/5] refactor params --- substrate/frame/alliance/src/benchmarking.rs | 181 ++++++++++--------- 1 file changed, 94 insertions(+), 87 deletions(-) diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index 53f6f7b84105..fc95c0152b68 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -27,9 +27,7 @@ use sp_std::{ prelude::*, }; -use frame_benchmarking::{ - account, impl_benchmark_test_suite, v2::*, whitelisted_caller, 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}; @@ -104,11 +102,11 @@ mod benchmarks { // This tests when proposal is created and queued as "proposed" #[benchmark] - fn propose_proposed() -> Result<(), BenchmarkError> { - let b = 1..MAX_BYTES; - let m = 2..T::MaxFellows::get(); - let p = 1..T::MaxProposals::get(); - + 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`. @@ -138,19 +136,17 @@ mod benchmarks { propose( SystemOrigin::Signed(proposer.clone()), threshold, - Box::new(proposal), + Box::new(proposal.clone()), bytes_in_storage, ); let proposal_hash = T::Hashing::hash_of(&proposal); - ensure!(T::ProposalProvider::proposal_of(proposal_hash), Some(proposal)); + assert_eq!(T::ProposalProvider::proposal_of(proposal_hash), Some(proposal)); Ok(()) } #[benchmark] - fn vote() -> Result<(), BenchmarkError> { - let m = 5..T::MaxFellows::get(); - + 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; @@ -217,10 +213,10 @@ mod benchmarks { } #[benchmark] - fn close_early_disapproved() -> Result<(), BenchmarkError> { - let m = 4..T::MaxFellows::get(); - let p = 1..T::MaxProposals::get(); - + 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; @@ -288,16 +284,17 @@ mod benchmarks { #[extrinsic_call] close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); - ensure!(T::ProposalProvider::proposal_of(last_hash), None); + assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + Ok(()) } + // 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() -> Result<(), BenchmarkError> { - let b = 1..MAX_BYTES; - // We choose 4 as a minimum so we always trigger a vote in the voting loop (`for j in ...`) - let m = 4..T::MaxFellows::get(); - let p = 1..T::MaxProposals::get(); - + 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`. @@ -308,8 +305,6 @@ mod benchmarks { 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; @@ -369,17 +364,17 @@ mod benchmarks { #[extrinsic_call] close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); - ensure!(T::ProposalProvider::proposal_of(last_hash), None); + assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); Ok(()) } + // We choose 5 fellows as a minimum so we always trigger a vote in the voting loop (`for j in ...`) #[benchmark] - fn close_disapproved() -> Result<(), BenchmarkError> { - let b = 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 = 5..T::MaxFellows::get(); - let p = 1..T::MaxProposals::get(); - + fn close_disapproved( + 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`. @@ -390,8 +385,6 @@ mod benchmarks { 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; @@ -444,15 +437,15 @@ mod benchmarks { bytes_in_storage, ); - ensure!(T::ProposalProvider::proposal_of(last_hash), None); + assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + Ok(()) } #[benchmark] - fn init_members() -> Result<(), BenchmarkError> { - // at least 1 fellow - let m = 1..T::MaxFellows::get(); - let z = 0..T::MaxAllies::get(); - + 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::>(); @@ -464,17 +457,17 @@ mod benchmarks { assert_last_event::( Event::MembersInitialized { fellows: fellows.clone(), allies: allies.clone() }.into(), ); - ensure!(Alliance::::members(MemberRole::Fellow), fellows); - ensure!(Alliance::::members(MemberRole::Ally), allies); + assert_eq!(Alliance::::members(MemberRole::Fellow), fellows); + assert_eq!(Alliance::::members(MemberRole::Ally), allies); + Ok(()) } #[benchmark] - fn disband() -> Result<(), BenchmarkError> { - // at least 1 founders - let x = 1..T::MaxFellows::get(); - let y = 0..T::MaxAllies::get(); - let z = 0..T::MaxMembersCount::get() / 2; - + 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 }; @@ -489,8 +482,8 @@ mod benchmarks { >::insert(&member, deposit); } - ensure!(Alliance::::voting_members_count(), x); - ensure!(Alliance::::ally_members_count(), y); + assert_eq!(Alliance::::voting_members_count(), x); + assert_eq!(Alliance::::ally_members_count(), y); #[extrinsic_call] _(SystemOrigin::Root, witness); @@ -505,6 +498,7 @@ mod benchmarks { ); assert!(!Alliance::::is_initialized()); + Ok(()) } #[benchmark] @@ -519,9 +513,9 @@ mod benchmarks { #[block] { - call.dispatch_bypass_filter(origin)? + call.dispatch_bypass_filter(origin)?; } - ensure!(Alliance::::rule(), Some(rule.clone())); + assert_eq!(Alliance::::rule(), Some(rule.clone())); assert_last_event::(Event::NewRuleSet { rule }.into()); Ok(()) } @@ -538,11 +532,12 @@ mod benchmarks { #[block] { - call.dispatch_bypass_filter(origin)? + call.dispatch_bypass_filter(origin)?; } assert!(Alliance::::announcements().contains(&announcement)); assert_last_event::(Event::Announced { announcement }.into()); + Ok(()) } #[benchmark] @@ -560,7 +555,7 @@ mod benchmarks { #[block] { - call.dispatch_bypass_filter(origin)? + call.dispatch_bypass_filter(origin)?; } assert!(!Alliance::::announcements().contains(&announcement)); @@ -574,7 +569,7 @@ mod benchmarks { let outsider = outsider::(1); assert!(!Alliance::::is_member(&outsider)); - ensure!(DepositOf::::get(&outsider), None); + assert_eq!(DepositOf::::get(&outsider), None); #[extrinsic_call] _(SystemOrigin::Signed(outsider.clone())); @@ -590,6 +585,7 @@ mod benchmarks { } .into(), ); + Ok(()) } #[benchmark] @@ -633,7 +629,7 @@ mod benchmarks { #[block] { - call.dispatch_bypass_filter(origin)? + call.dispatch_bypass_filter(origin)?; } assert!(!Alliance::::is_ally(&ally1)); @@ -659,6 +655,7 @@ mod benchmarks { Some(System::::block_number() + T::RetirementPeriod::get()) ); assert_last_event::(Event::MemberRetirementPeriodStarted { member: fellow2 }.into()); + Ok(()) } #[benchmark] @@ -685,6 +682,7 @@ mod benchmarks { Event::MemberRetired { member: fellow2, unreserved: Some(T::AllyDeposit::get()) } .into(), ); + Ok(()) } #[benchmark] @@ -702,7 +700,7 @@ mod benchmarks { #[block] { - call.dispatch_bypass_filter(origin)? + call.dispatch_bypass_filter(origin)?; } assert!(!Alliance::::is_member(&fellow2)); @@ -710,13 +708,14 @@ mod benchmarks { assert_last_event::( Event::MemberKicked { member: fellow2, slashed: Some(T::AllyDeposit::get()) }.into(), ); + Ok(()) } #[benchmark] - fn add_scrupulous_items() -> Result<(), BenchmarkError> { - let n = 0..T::MaxUnscrupulousItems::get(); - let l = 0..T::MaxWebsiteUrlLength::get(); - + 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)).collect::>(); @@ -736,31 +735,31 @@ mod benchmarks { #[block] { - call.dispatch_bypass_filter(origin)? + call.dispatch_bypass_filter(origin)?; } assert_last_event::(Event::UnscrupulousItemAdded { items: unscrupulous_list }.into()); Ok(()) } -} - -benchmarks_instance_pallet! { - - 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()); @@ -770,24 +769,32 @@ 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!(Alliance::::is_member_of(&fellow2, MemberRole::Retiring)); + assert_last_event::(Event::FellowAbdicated { fellow: fellow2 }.into()); + Ok(()) } impl_benchmark_test_suite!(Alliance, crate::mock::new_bench_ext(), crate::mock::Test); From 47e2bd763c5c2d30caa8e5c8d4baefba08101d87 Mon Sep 17 00:00:00 2001 From: 0xmovses Date: Fri, 13 Oct 2023 13:47:24 +0100 Subject: [PATCH 3/5] add missing benchmark --- substrate/frame/alliance/src/benchmarking.rs | 130 ++++++++++++++++++- 1 file changed, 129 insertions(+), 1 deletion(-) diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index fc95c0152b68..b9cbe5815292 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -368,9 +368,137 @@ mod benchmarks { Ok(()) } - // We choose 5 fellows as a minimum so we always trigger a vote in the voting loop (`for j in ...`) #[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 members = fellows.clone(); + + Alliance::::init_members( + SystemOrigin::Root.into(), + fellows, + vec![], + )?; + + let proposer = members[0].clone(); + let voter = members[1].clone(); + + // Threshold is one less than total members so that two nays will disapprove the vote + let threshold = m - 1; + + // Add proposals + let mut last_hash = T::Hash::default(); + 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(); + Alliance::::propose( + SystemOrigin::Signed(proposer.clone()).into(), + threshold, + Box::new(proposal.clone()), + bytes_in_storage, + )?; + last_hash = T::Hashing::hash_of(&proposal); + assert_eq!(T::ProposalProvider::proposal_of(last_hash), Some(proposal)); + } + + 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 { + let voter = &members[j as usize]; + Alliance::::vote( + SystemOrigin::Signed(voter.clone()).into(), + last_hash.clone(), + index, + true, + )?; + } + + Alliance::::vote( + SystemOrigin::Signed(voter.clone()).into(), + last_hash.clone(), + index, + false, + )?; + + System::::set_block_number(BlockNumberFor::::max_value());et bytes = 100; + let bytes_in_storage = bytes + size_of::() as u32 + 32; + + // Construct `members`. + let fellows = (0 .. m).map(fellow::).collect::>(); + + let members = fellows.clone(); + + Alliance::::init_members( + SystemOrigin::Root.into(), + fellows, + vec![], + )?; + + let proposer = members[0].clone(); + let voter = members[1].clone(); + + // Threshold is one less than total members so that two nays will disapprove the vote + let threshold = m - 1; + + // Add proposals + let mut last_hash = T::Hash::default(); + 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(); + Alliance::::propose( + SystemOrigin::Signed(proposer.clone()).into(), + threshold, + Box::new(proposal.clone()), + bytes_in_storage, + )?; + last_hash = T::Hashing::hash_of(&proposal); + assert_eq!(T::ProposalProvider::proposal_of(last_hash), Some(proposal)); + } + + 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 { + let voter = &members[j as usize]; + Alliance::::vote( + SystemOrigin::Signed(voter.clone()).into(), + last_hash.clone(), + index, + true, + )?; + } + + Alliance::::vote( + SystemOrigin::Signed(voter.clone()).into(), + last_hash.clone(), + index, + false, + )?; + + System::::set_block_number(BlockNumberFor::::max_value()); + + #[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); + } + + // 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() }>, From afd634d3f56989445772bf678d4f285eae1b1e1d Mon Sep 17 00:00:00 2001 From: 0xmovses Date: Fri, 13 Oct 2023 13:56:54 +0100 Subject: [PATCH 4/5] fmt --- substrate/frame/alliance/src/benchmarking.rs | 54 +++++++++---------- .../procedural/src/pallet/expand/warnings.rs | 8 +-- 2 files changed, 26 insertions(+), 36 deletions(-) diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index b9cbe5815292..84b5f39e4ac5 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -288,7 +288,8 @@ mod benchmarks { Ok(()) } - // We choose 4 as a minimum for param m, so we always trigger a vote in the voting loop (`for j in ...`) + // 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>, @@ -325,7 +326,8 @@ mod benchmarks { } 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(), @@ -373,19 +375,15 @@ mod benchmarks { m: Linear<2, { T::MaxFellows::get() }>, p: Linear<1, { T::MaxProposals::get() }>, ) -> Result<(), BenchmarkError> { - let bytes = 100; + 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(); @@ -395,11 +393,10 @@ mod benchmarks { // 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, @@ -413,7 +410,7 @@ mod benchmarks { 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(), @@ -430,19 +427,16 @@ mod benchmarks { false, )?; - System::::set_block_number(BlockNumberFor::::max_value());et bytes = 100; + System::::set_block_number(BlockNumberFor::::max_value()); + 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(); @@ -452,11 +446,10 @@ mod benchmarks { // 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, @@ -470,7 +463,7 @@ mod benchmarks { 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(), @@ -489,14 +482,15 @@ mod benchmarks { System::::set_block_number(BlockNumberFor::::max_value()); - #[extrinsic_call] - close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); + #[extrinsic_call] + close(SystemOrigin::Signed(voter), last_hash.clone(), index, Weight::MAX, bytes_in_storage); - // The last proposal is removed. + // The last proposal is removed. assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); } - // We choose 5 fellows as a minimum so we always trigger a vote in the voting loop (`for j in ...`) + // 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>, 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()) From 69e09197827ee85e71f0c7c6966855db7759899d Mon Sep 17 00:00:00 2001 From: 0xmovses Date: Fri, 13 Oct 2023 14:28:03 +0100 Subject: [PATCH 5/5] remove dup block --- substrate/frame/alliance/src/benchmarking.rs | 57 +------------------- 1 file changed, 2 insertions(+), 55 deletions(-) diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index 84b5f39e4ac5..77294c6b6646 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -149,7 +149,7 @@ mod benchmarks { 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::>(); @@ -427,59 +427,6 @@ mod benchmarks { false, )?; - System::::set_block_number(BlockNumberFor::::max_value()); - let bytes = 100; - let bytes_in_storage = bytes + size_of::() as u32 + 32; - - // Construct `members`. - let fellows = (0..m).map(fellow::).collect::>(); - - let members = fellows.clone(); - - Alliance::::init_members(SystemOrigin::Root.into(), fellows, vec![])?; - - let proposer = members[0].clone(); - let voter = members[1].clone(); - - // Threshold is one less than total members so that two nays will disapprove the vote - let threshold = m - 1; - - // Add proposals - let mut last_hash = T::Hash::default(); - 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(); - Alliance::::propose( - SystemOrigin::Signed(proposer.clone()).into(), - threshold, - Box::new(proposal.clone()), - bytes_in_storage, - )?; - last_hash = T::Hashing::hash_of(&proposal); - assert_eq!(T::ProposalProvider::proposal_of(last_hash), Some(proposal)); - } - - 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 { - let voter = &members[j as usize]; - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), - index, - true, - )?; - } - - Alliance::::vote( - SystemOrigin::Signed(voter.clone()).into(), - last_hash.clone(), - index, - false, - )?; - System::::set_block_number(BlockNumberFor::::max_value()); #[extrinsic_call] @@ -487,6 +434,7 @@ mod benchmarks { // The last proposal is removed. assert_eq!(T::ProposalProvider::proposal_of(last_hash), None); + Ok(()) } // We choose 5 fellows as a minimum so we always trigger a vote in the voting loop (`for j in @@ -914,7 +862,6 @@ mod benchmarks { #[extrinsic_call] _(SystemOrigin::Signed(fellow2.clone())); - assert!(Alliance::::is_member_of(&fellow2, MemberRole::Retiring)); assert_last_event::(Event::FellowAbdicated { fellow: fellow2 }.into()); Ok(()) }