diff --git a/frame/alliance/src/benchmarking.rs b/frame/alliance/src/benchmarking.rs index f312e032bbb0f..01b81ed66e999 100644 --- a/frame/alliance/src/benchmarking.rs +++ b/frame/alliance/src/benchmarking.rs @@ -25,7 +25,7 @@ use sp_std::{ prelude::*, }; -use frame_benchmarking::v1::{account, benchmarks_instance_pallet}; +use frame_benchmarking::v1::{account, benchmarks_instance_pallet, BenchmarkError}; use frame_support::traits::{EnsureOrigin, Get, UnfilteredDispatchable}; use frame_system::{Pallet as System, RawOrigin as SystemOrigin}; @@ -581,7 +581,8 @@ benchmarks_instance_pallet! { let rule = rule(b"hello world"); let call = Call::::set_rule { rule: rule.clone() }; - let origin = T::AdminOrigin::successful_origin(); + let origin = + T::AdminOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: { call.dispatch_bypass_filter(origin)? } verify { assert_eq!(Alliance::::rule(), Some(rule.clone())); @@ -594,7 +595,8 @@ benchmarks_instance_pallet! { let announcement = announcement(b"hello world"); let call = Call::::announce { announcement: announcement.clone() }; - let origin = T::AnnouncementOrigin::successful_origin(); + let origin = + T::AnnouncementOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: { call.dispatch_bypass_filter(origin)? } verify { assert!(Alliance::::announcements().contains(&announcement)); @@ -609,7 +611,8 @@ benchmarks_instance_pallet! { Announcements::::put(announcements); let call = Call::::remove_announcement { announcement: announcement.clone() }; - let origin = T::AnnouncementOrigin::successful_origin(); + let origin = + T::AnnouncementOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: { call.dispatch_bypass_filter(origin)? } verify { assert!(Alliance::::announcements().is_empty()); @@ -665,7 +668,8 @@ benchmarks_instance_pallet! { let ally1_lookup = T::Lookup::unlookup(ally1.clone()); let call = Call::::elevate_ally { ally: ally1_lookup }; - let origin = T::MembershipManager::successful_origin(); + let origin = + T::MembershipManager::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: { call.dispatch_bypass_filter(origin)? } verify { assert!(!Alliance::::is_ally(&ally1)); @@ -725,7 +729,8 @@ benchmarks_instance_pallet! { let fellow2_lookup = T::Lookup::unlookup(fellow2.clone()); let call = Call::::kick_member { who: fellow2_lookup }; - let origin = T::MembershipManager::successful_origin(); + let origin = + T::MembershipManager::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: { call.dispatch_bypass_filter(origin)? } verify { assert!(!Alliance::::is_member(&fellow2)); @@ -754,7 +759,8 @@ benchmarks_instance_pallet! { unscrupulous_list.extend(websites.into_iter().map(UnscrupulousItem::Website)); let call = Call::::add_unscrupulous_items { items: unscrupulous_list.clone() }; - let origin = T::AnnouncementOrigin::successful_origin(); + let origin = + T::AnnouncementOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: { call.dispatch_bypass_filter(origin)? } verify { assert_last_event::(Event::UnscrupulousItemAdded { items: unscrupulous_list }.into()); @@ -784,7 +790,8 @@ 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::successful_origin(); + 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()); diff --git a/frame/assets/src/benchmarking.rs b/frame/assets/src/benchmarking.rs index 9acf69f1e4ef4..cf0ec353f6350 100644 --- a/frame/assets/src/benchmarking.rs +++ b/frame/assets/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; use frame_benchmarking::v1::{ - account, benchmarks_instance_pallet, whitelist_account, whitelisted_caller, + account, benchmarks_instance_pallet, whitelist_account, whitelisted_caller, BenchmarkError, }; use frame_support::{ dispatch::UnfilteredDispatchable, @@ -135,7 +135,8 @@ fn assert_event, I: 'static>(generic_event: >::Runti benchmarks_instance_pallet! { create { let asset_id = default_asset_id::(); - let origin = T::CreateOrigin::successful_origin(&asset_id.into()); + let origin = T::CreateOrigin::try_successful_origin(&asset_id.into()) + .map_err(|_| BenchmarkError::Weightless)?; let caller = T::CreateOrigin::ensure_origin(origin, &asset_id.into()).unwrap(); let caller_lookup = T::Lookup::unlookup(caller.clone()); T::Currency::make_free_balance_be(&caller, DepositBalanceOf::::max_value()); @@ -362,7 +363,8 @@ benchmarks_instance_pallet! { let (asset_id, _, _) = create_default_asset::(true); - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call = Call::::force_set_metadata { id: asset_id, name: name.clone(), @@ -382,7 +384,8 @@ benchmarks_instance_pallet! { let origin = SystemOrigin::Signed(caller).into(); Assets::::set_metadata(origin, asset_id, dummy.clone(), dummy, 12)?; - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call = Call::::force_clear_metadata { id: asset_id }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -392,7 +395,8 @@ benchmarks_instance_pallet! { force_asset_status { let (asset_id, caller, caller_lookup) = create_default_asset::(true); - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call = Call::::force_asset_status { id: asset_id, owner: caller_lookup.clone(), diff --git a/frame/bounties/src/benchmarking.rs b/frame/bounties/src/benchmarking.rs index a43372978e5ca..881f1c912e38b 100644 --- a/frame/bounties/src/benchmarking.rs +++ b/frame/bounties/src/benchmarking.rs @@ -173,7 +173,8 @@ benchmarks_instance_pallet! { let (caller, curator, fee, value, reason) = setup_bounty::(0, 0); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; let bounty_id = BountyCount::::get() - 1; - let approve_origin = T::ApproveOrigin::successful_origin(); + let approve_origin = + T::ApproveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: close_bounty(approve_origin, bounty_id) close_bounty_active { @@ -181,7 +182,8 @@ benchmarks_instance_pallet! { let (curator_lookup, bounty_id) = create_bounty::()?; Treasury::::on_initialize(T::BlockNumber::zero()); let bounty_id = BountyCount::::get() - 1; - let approve_origin = T::ApproveOrigin::successful_origin(); + let approve_origin = + T::ApproveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: close_bounty(approve_origin, bounty_id) verify { assert_last_event::(Event::BountyCanceled { index: bounty_id }.into()) diff --git a/frame/democracy/src/benchmarking.rs b/frame/democracy/src/benchmarking.rs index 66d9e2a007afa..f2d45b719f678 100644 --- a/frame/democracy/src/benchmarking.rs +++ b/frame/democracy/src/benchmarking.rs @@ -19,7 +19,7 @@ use super::*; -use frame_benchmarking::v1::{account, benchmarks, whitelist_account}; +use frame_benchmarking::v1::{account, benchmarks, whitelist_account, BenchmarkError}; use frame_support::{ assert_noop, assert_ok, traits::{Currency, EnsureOrigin, Get, OnInitialize, UnfilteredDispatchable}, @@ -177,7 +177,8 @@ benchmarks! { } emergency_cancel { - let origin = T::CancellationOrigin::successful_origin(); + let origin = + T::CancellationOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let ref_index = add_referendum::(0).0; assert_ok!(Democracy::::referendum_status(ref_index)); }: _(origin, ref_index) @@ -200,10 +201,13 @@ benchmarks! { let (ref_index, hash) = add_referendum::(0); assert_ok!(Democracy::::referendum_status(ref_index)); // Place our proposal in the external queue, too. - assert_ok!( - Democracy::::external_propose(T::ExternalOrigin::successful_origin(), make_proposal::(0)) - ); - let origin = T::BlacklistOrigin::successful_origin(); + assert_ok!(Democracy::::external_propose( + T::ExternalOrigin::try_successful_origin() + .expect("ExternalOrigin has no successful origin required for the benchmark"), + make_proposal::(0) + )); + let origin = + T::BlacklistOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin, hash, Some(ref_index)) verify { // Referendum has been canceled @@ -215,7 +219,8 @@ benchmarks! { // Worst case scenario, we external propose a previously blacklisted proposal external_propose { - let origin = T::ExternalOrigin::successful_origin(); + let origin = + T::ExternalOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let proposal = make_proposal::(0); // Add proposal to blacklist with block number 0 @@ -233,7 +238,8 @@ benchmarks! { } external_propose_majority { - let origin = T::ExternalMajorityOrigin::successful_origin(); + let origin = T::ExternalMajorityOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; let proposal = make_proposal::(0); }: _(origin, proposal) verify { @@ -242,7 +248,8 @@ benchmarks! { } external_propose_default { - let origin = T::ExternalDefaultOrigin::successful_origin(); + let origin = T::ExternalDefaultOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; let proposal = make_proposal::(0); }: _(origin, proposal) verify { @@ -251,13 +258,15 @@ benchmarks! { } fast_track { - let origin_propose = T::ExternalDefaultOrigin::successful_origin(); + let origin_propose = T::ExternalDefaultOrigin::try_successful_origin() + .expect("ExternalDefaultOrigin has no successful origin required for the benchmark"); let proposal = make_proposal::(0); let proposal_hash = proposal.hash(); Democracy::::external_propose_default(origin_propose, proposal)?; // NOTE: Instant origin may invoke a little bit more logic, but may not always succeed. - let origin_fast_track = T::FastTrackOrigin::successful_origin(); + let origin_fast_track = + T::FastTrackOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let voting_period = T::FastTrackVotingPeriod::get(); let delay = 0u32; }: _(origin_fast_track, proposal_hash, voting_period, delay.into()) @@ -269,7 +278,8 @@ benchmarks! { let proposal = make_proposal::(0); let proposal_hash = proposal.hash(); - let origin_propose = T::ExternalDefaultOrigin::successful_origin(); + let origin_propose = T::ExternalDefaultOrigin::try_successful_origin() + .expect("ExternalDefaultOrigin has no successful origin required for the benchmark"); Democracy::::external_propose_default(origin_propose, proposal)?; let mut vetoers: BoundedVec = Default::default(); @@ -279,7 +289,7 @@ benchmarks! { vetoers.sort(); Blacklist::::insert(proposal_hash, (T::BlockNumber::zero(), vetoers)); - let origin = T::VetoOrigin::successful_origin(); + let origin = T::VetoOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; ensure!(NextExternal::::get().is_some(), "no external proposal"); }: _(origin, proposal_hash) verify { @@ -293,7 +303,8 @@ benchmarks! { for i in 0 .. T::MaxProposals::get() { add_proposal::(i)?; } - let cancel_origin = T::CancelProposalOrigin::successful_origin(); + let cancel_origin = T::CancelProposalOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; }: _(cancel_origin, 0) cancel_referendum { @@ -313,7 +324,8 @@ benchmarks! { // Launch external LastTabledWasExternal::::put(false); - let origin = T::ExternalMajorityOrigin::successful_origin(); + let origin = T::ExternalMajorityOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; let proposal = make_proposal::(r); let call = Call::::external_propose_majority { proposal }; call.dispatch_bypass_filter(origin)?; diff --git a/frame/fast-unstake/src/benchmarking.rs b/frame/fast-unstake/src/benchmarking.rs index 77488b6447c61..e0e4186d78014 100644 --- a/frame/fast-unstake/src/benchmarking.rs +++ b/frame/fast-unstake/src/benchmarking.rs @@ -20,7 +20,7 @@ #![cfg(feature = "runtime-benchmarks")] use crate::{types::*, Pallet as FastUnstake, *}; -use frame_benchmarking::v1::{benchmarks, whitelist_account}; +use frame_benchmarking::v1::{benchmarks, whitelist_account, BenchmarkError}; use frame_support::{ assert_ok, traits::{Currency, EnsureOrigin, Get, Hooks}, @@ -192,7 +192,8 @@ benchmarks! { } control { - let origin = ::ControlOrigin::successful_origin(); + let origin = ::ControlOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; } : _(origin, T::MaxErasToCheckPerBlock::get()) verify {} diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs index 356bcb2e80174..2d485af0f698b 100644 --- a/frame/identity/src/benchmarking.rs +++ b/frame/identity/src/benchmarking.rs @@ -22,7 +22,7 @@ use super::*; use crate::Pallet as Identity; -use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller}; +use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller, BenchmarkError}; use frame_support::{ ensure, traits::{EnsureOrigin, Get}, @@ -42,7 +42,8 @@ fn add_registrars(r: u32) -> Result<(), &'static str> { let registrar: T::AccountId = account("registrar", i, SEED); let registrar_lookup = T::Lookup::unlookup(registrar.clone()); let _ = T::Currency::make_free_balance_be(®istrar, BalanceOf::::max_value()); - let registrar_origin = T::RegistrarOrigin::successful_origin(); + let registrar_origin = T::RegistrarOrigin::try_successful_origin() + .expect("RegistrarOrigin has no successful origin required for the benchmark"); Identity::::add_registrar(registrar_origin, registrar_lookup)?; Identity::::set_fee(RawOrigin::Signed(registrar.clone()).into(), i, 10u32.into())?; let fields = @@ -121,7 +122,8 @@ benchmarks! { add_registrar { let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; ensure!(Registrars::::get().len() as u32 == r, "Registrars not set up correctly."); - let origin = T::RegistrarOrigin::successful_origin(); + let origin = + T::RegistrarOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let account = T::Lookup::unlookup(account("registrar", r + 1, SEED)); }: _(origin, account) verify { @@ -280,7 +282,8 @@ benchmarks! { let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; - let registrar_origin = T::RegistrarOrigin::successful_origin(); + let registrar_origin = T::RegistrarOrigin::try_successful_origin() + .expect("RegistrarOrigin has no successful origin required for the benchmark"); Identity::::add_registrar(registrar_origin, caller_lookup)?; let registrars = Registrars::::get(); ensure!(registrars[r as usize].as_ref().unwrap().fee == 0u32.into(), "Fee already set."); @@ -297,7 +300,8 @@ benchmarks! { let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; - let registrar_origin = T::RegistrarOrigin::successful_origin(); + let registrar_origin = T::RegistrarOrigin::try_successful_origin() + .expect("RegistrarOrigin has no successful origin required for the benchmark"); Identity::::add_registrar(registrar_origin, caller_lookup)?; let registrars = Registrars::::get(); ensure!(registrars[r as usize].as_ref().unwrap().account == caller, "id not set."); @@ -315,7 +319,8 @@ benchmarks! { let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; - let registrar_origin = T::RegistrarOrigin::successful_origin(); + let registrar_origin = T::RegistrarOrigin::try_successful_origin() + .expect("RegistrarOrigin has no successful origin required for the benchmark"); Identity::::add_registrar(registrar_origin, caller_lookup)?; let fields = IdentityFields( IdentityField::Display | IdentityField::Legal | IdentityField::Web | IdentityField::Riot @@ -347,7 +352,8 @@ benchmarks! { let info_hash = T::Hashing::hash_of(&info); Identity::::set_identity(user_origin.clone(), Box::new(info))?; - let registrar_origin = T::RegistrarOrigin::successful_origin(); + let registrar_origin = T::RegistrarOrigin::try_successful_origin() + .expect("RegistrarOrigin has no successful origin required for the benchmark"); Identity::::add_registrar(registrar_origin, caller_lookup)?; Identity::::request_judgement(user_origin, r, 10u32.into())?; }: _(RawOrigin::Signed(caller), r, user_lookup, Judgement::Reasonable, info_hash) @@ -385,7 +391,8 @@ benchmarks! { )?; } ensure!(IdentityOf::::contains_key(&target), "Identity not set"); - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin, target_lookup) verify { ensure!(!IdentityOf::::contains_key(&target), "Identity not removed"); diff --git a/frame/lottery/src/benchmarking.rs b/frame/lottery/src/benchmarking.rs index 658dd25a98a27..0db7fabb365ef 100644 --- a/frame/lottery/src/benchmarking.rs +++ b/frame/lottery/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; -use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller}; +use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller, BenchmarkError}; use frame_support::{ storage::bounded_vec::BoundedVec, traits::{EnsureOrigin, OnInitialize}, @@ -43,7 +43,8 @@ fn setup_lottery(repeat: bool) -> Result<(), &'static str> { ]; // Last call will be the match for worst case scenario. calls.push(frame_system::Call::::remark { remark: vec![] }.into()); - let origin = T::ManagerOrigin::successful_origin(); + let origin = T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"); Lottery::::set_calls(origin.clone(), calls)?; Lottery::::start_lottery(origin, price, length, delay, repeat)?; Ok(()) @@ -76,7 +77,8 @@ benchmarks! { set_calls { let n in 0 .. T::MaxCalls::get() as u32; let calls = vec![frame_system::Call::::remark { remark: vec![] }.into(); n as usize]; - let origin = T::ManagerOrigin::successful_origin(); + let origin = + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; assert!(CallIndices::::get().is_empty()); }: _(origin, calls) verify { @@ -89,7 +91,8 @@ benchmarks! { let price = BalanceOf::::max_value(); let end = 10u32.into(); let payout = 5u32.into(); - let origin = T::ManagerOrigin::successful_origin(); + let origin = + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin, price, end, payout, true) verify { assert!(crate::Lottery::::get().is_some()); @@ -98,7 +101,8 @@ benchmarks! { stop_repeat { setup_lottery::(true)?; assert_eq!(crate::Lottery::::get().unwrap().repeat, true); - let origin = T::ManagerOrigin::successful_origin(); + let origin = + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin) verify { assert_eq!(crate::Lottery::::get().unwrap().repeat, false); diff --git a/frame/membership/src/lib.rs b/frame/membership/src/lib.rs index 6d973a15fe032..8b39a08ce6699 100644 --- a/frame/membership/src/lib.rs +++ b/frame/membership/src/lib.rs @@ -362,15 +362,17 @@ impl, I: 'static> SortedMembers for Pallet { #[cfg(feature = "runtime-benchmarks")] mod benchmark { use super::{Pallet as Membership, *}; - use frame_benchmarking::v1::{account, benchmarks_instance_pallet, whitelist}; + use frame_benchmarking::v1::{account, benchmarks_instance_pallet, whitelist, BenchmarkError}; use frame_support::{assert_ok, traits::EnsureOrigin}; use frame_system::RawOrigin; const SEED: u32 = 0; fn set_members, I: 'static>(members: Vec, prime: Option) { - let reset_origin = T::ResetOrigin::successful_origin(); - let prime_origin = T::PrimeOrigin::successful_origin(); + let reset_origin = T::ResetOrigin::try_successful_origin() + .expect("ResetOrigin has no successful origin required for the benchmark"); + let prime_origin = T::PrimeOrigin::try_successful_origin() + .expect("PrimeOrigin has no successful origin required for the benchmark"); assert_ok!(>::reset_members(reset_origin, members.clone())); if let Some(prime) = prime.map(|i| members[i].clone()) { @@ -390,9 +392,11 @@ mod benchmark { let new_member = account::("add", m, SEED); let new_member_lookup = T::Lookup::unlookup(new_member.clone()); }: { - assert_ok!(>::add_member(T::AddOrigin::successful_origin(), new_member_lookup)); - } - verify { + assert_ok!(>::add_member( + T::AddOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + new_member_lookup, + )); + } verify { assert!(>::get().contains(&new_member)); #[cfg(test)] crate::tests::clean(); } @@ -408,7 +412,10 @@ mod benchmark { let to_remove = members.first().cloned().unwrap(); let to_remove_lookup = T::Lookup::unlookup(to_remove.clone()); }: { - assert_ok!(>::remove_member(T::RemoveOrigin::successful_origin(), to_remove_lookup)); + assert_ok!(>::remove_member( + T::RemoveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + to_remove_lookup, + )); } verify { assert!(!>::get().contains(&to_remove)); // prime is rejigged @@ -428,7 +435,7 @@ mod benchmark { let remove_lookup = T::Lookup::unlookup(remove.clone()); }: { assert_ok!(>::swap_member( - T::SwapOrigin::successful_origin(), + T::SwapOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, remove_lookup, add_lookup, )); @@ -448,7 +455,10 @@ mod benchmark { set_members::(members.clone(), Some(members.len() - 1)); let mut new_members = (m..2*m).map(|i| account("member", i, SEED)).collect::>(); }: { - assert_ok!(>::reset_members(T::ResetOrigin::successful_origin(), new_members.clone())); + assert_ok!(>::reset_members( + T::ResetOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + new_members.clone(), + )); } verify { new_members.sort(); assert_eq!(>::get(), new_members); @@ -485,7 +495,10 @@ mod benchmark { let prime_lookup = T::Lookup::unlookup(prime.clone()); set_members::(members, None); }: { - assert_ok!(>::set_prime(T::PrimeOrigin::successful_origin(), prime_lookup)); + assert_ok!(>::set_prime( + T::PrimeOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + prime_lookup, + )); } verify { assert!(>::get().is_some()); assert!(::get_prime().is_some()); @@ -498,7 +511,9 @@ mod benchmark { let prime = members.last().cloned().unwrap(); set_members::(members, None); }: { - assert_ok!(>::clear_prime(T::PrimeOrigin::successful_origin())); + assert_ok!(>::clear_prime( + T::PrimeOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + )); } verify { assert!(>::get().is_none()); assert!(::get_prime().is_none()); diff --git a/frame/nfts/src/benchmarking.rs b/frame/nfts/src/benchmarking.rs index 17ff28b822814..a4007f6b2c133 100644 --- a/frame/nfts/src/benchmarking.rs +++ b/frame/nfts/src/benchmarking.rs @@ -22,7 +22,7 @@ use super::*; use enumflags2::{BitFlag, BitFlags}; use frame_benchmarking::v1::{ - account, benchmarks_instance_pallet, whitelist_account, whitelisted_caller, + account, benchmarks_instance_pallet, whitelist_account, whitelisted_caller, BenchmarkError, }; use frame_support::{ assert_ok, @@ -151,7 +151,8 @@ fn default_item_config() -> ItemConfig { benchmarks_instance_pallet! { create { let collection = T::Helper::collection(0); - let origin = T::CreateOrigin::successful_origin(&collection); + let origin = T::CreateOrigin::try_successful_origin(&collection) + .map_err(|_| BenchmarkError::Weightless)?; let caller = T::CreateOrigin::ensure_origin(origin.clone(), &collection).unwrap(); whitelist_account!(caller); let admin = T::Lookup::unlookup(caller.clone()); @@ -311,7 +312,8 @@ benchmarks_instance_pallet! { force_collection_owner { let (collection, _, _) = create_collection::(); - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let target: T::AccountId = account("target", 0, SEED); let target_lookup = T::Lookup::unlookup(target.clone()); T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance()); @@ -326,7 +328,8 @@ benchmarks_instance_pallet! { force_collection_config { let (collection, caller, _) = create_collection::(); - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call = Call::::force_collection_config { collection, config: make_collection_config::(CollectionSetting::DepositRequired.into()), diff --git a/frame/nis/src/benchmarking.rs b/frame/nis/src/benchmarking.rs index b01d414c4b251..10d3a238a3f5e 100644 --- a/frame/nis/src/benchmarking.rs +++ b/frame/nis/src/benchmarking.rs @@ -20,7 +20,7 @@ #![cfg(feature = "runtime-benchmarks")] use super::*; -use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller}; +use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller, BenchmarkError}; use frame_support::traits::{nonfungible::Inspect, Currency, EnsureOrigin, Get}; use frame_system::RawOrigin; use sp_arithmetic::Perquintill; @@ -100,7 +100,8 @@ benchmarks! { } fund_deficit { - let origin = T::FundOrigin::successful_origin(); + let origin = + T::FundOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let caller: T::AccountId = whitelisted_caller(); let bid = T::MinBid::get().max(One::one()); T::Currency::make_free_balance_be(&caller, bid); diff --git a/frame/preimage/src/benchmarking.rs b/frame/preimage/src/benchmarking.rs index f9526b67d4bc8..56361b88c2583 100644 --- a/frame/preimage/src/benchmarking.rs +++ b/frame/preimage/src/benchmarking.rs @@ -18,7 +18,7 @@ //! Preimage pallet benchmarking. use super::*; -use frame_benchmarking::v1::{account, benchmarks, whitelist_account}; +use frame_benchmarking::v1::{account, benchmarks, whitelist_account, BenchmarkError}; use frame_support::assert_ok; use frame_system::RawOrigin; use sp_runtime::traits::Bounded; @@ -62,7 +62,11 @@ benchmarks! { let caller = funded_account::("caller", 0); whitelist_account!(caller); let (preimage, hash) = sized_preimage_and_hash::(s); - assert_ok!(Preimage::::request_preimage(T::ManagerOrigin::successful_origin(), hash)); + assert_ok!(Preimage::::request_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + hash, + )); }: note_preimage(RawOrigin::Signed(caller), preimage) verify { assert!(Preimage::::have_preimage(&hash)); @@ -71,9 +75,15 @@ benchmarks! { note_no_deposit_preimage { let s in 0 .. MAX_SIZE; let (preimage, hash) = sized_preimage_and_hash::(s); - assert_ok!(Preimage::::request_preimage(T::ManagerOrigin::successful_origin(), hash)); - }: note_preimage(T::ManagerOrigin::successful_origin(), preimage) - verify { + assert_ok!(Preimage::::request_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + hash, + )); + }: note_preimage( + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + preimage + ) verify { assert!(Preimage::::have_preimage(&hash)); } @@ -90,9 +100,15 @@ benchmarks! { // Cheap unnote - will not unreserve since there's no deposit held. unnote_no_deposit_preimage { let (preimage, hash) = preimage_and_hash::(); - assert_ok!(Preimage::::note_preimage(T::ManagerOrigin::successful_origin(), preimage)); - }: unnote_preimage(T::ManagerOrigin::successful_origin(), hash) - verify { + assert_ok!(Preimage::::note_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + preimage, + )); + }: unnote_preimage( + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + hash + ) verify { assert!(!Preimage::::have_preimage(&hash)); } @@ -102,8 +118,10 @@ benchmarks! { let noter = funded_account::("noter", 0); whitelist_account!(noter); assert_ok!(Preimage::::note_preimage(RawOrigin::Signed(noter.clone()).into(), preimage)); - }: _(T::ManagerOrigin::successful_origin(), hash) - verify { + }: _( + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + hash + ) verify { let deposit = T::BaseDeposit::get() + T::ByteDeposit::get() * MAX_SIZE.into(); let s = RequestStatus::Requested { deposit: Some((noter, deposit)), count: 1, len: Some(MAX_SIZE) }; assert_eq!(StatusFor::::get(&hash), Some(s)); @@ -111,26 +129,40 @@ benchmarks! { // Cheap request - would unreserve the deposit but none was held. request_no_deposit_preimage { let (preimage, hash) = preimage_and_hash::(); - assert_ok!(Preimage::::note_preimage(T::ManagerOrigin::successful_origin(), preimage)); - }: request_preimage(T::ManagerOrigin::successful_origin(), hash) - verify { + assert_ok!(Preimage::::note_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + preimage, + )); + }: request_preimage( + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + hash + ) verify { let s = RequestStatus::Requested { deposit: None, count: 2, len: Some(MAX_SIZE) }; assert_eq!(StatusFor::::get(&hash), Some(s)); } // Cheap request - the preimage is not yet noted, so deposit to unreserve. request_unnoted_preimage { let (_, hash) = preimage_and_hash::(); - }: request_preimage(T::ManagerOrigin::successful_origin(), hash) - verify { + }: request_preimage( + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + hash + ) verify { let s = RequestStatus::Requested { deposit: None, count: 1, len: None }; assert_eq!(StatusFor::::get(&hash), Some(s)); } // Cheap request - the preimage is already requested, so just a counter bump. request_requested_preimage { let (_, hash) = preimage_and_hash::(); - assert_ok!(Preimage::::request_preimage(T::ManagerOrigin::successful_origin(), hash)); - }: request_preimage(T::ManagerOrigin::successful_origin(), hash) - verify { + assert_ok!(Preimage::::request_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + hash, + )); + }: request_preimage( + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + hash + ) verify { let s = RequestStatus::Requested { deposit: None, count: 2, len: None }; assert_eq!(StatusFor::::get(&hash), Some(s)); } @@ -138,27 +170,53 @@ benchmarks! { // Expensive unrequest - last reference and it's noted, so will destroy the preimage. unrequest_preimage { let (preimage, hash) = preimage_and_hash::(); - assert_ok!(Preimage::::request_preimage(T::ManagerOrigin::successful_origin(), hash)); - assert_ok!(Preimage::::note_preimage(T::ManagerOrigin::successful_origin(), preimage)); - }: _(T::ManagerOrigin::successful_origin(), hash) - verify { + assert_ok!(Preimage::::request_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + hash, + )); + assert_ok!(Preimage::::note_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + preimage, + )); + }: _( + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + hash + ) verify { assert_eq!(StatusFor::::get(&hash), None); } // Cheap unrequest - last reference, but it's not noted. unrequest_unnoted_preimage { let (_, hash) = preimage_and_hash::(); - assert_ok!(Preimage::::request_preimage(T::ManagerOrigin::successful_origin(), hash)); - }: unrequest_preimage(T::ManagerOrigin::successful_origin(), hash) - verify { + assert_ok!(Preimage::::request_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + hash, + )); + }: unrequest_preimage( + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + hash + ) verify { assert_eq!(StatusFor::::get(&hash), None); } // Cheap unrequest - not the last reference. unrequest_multi_referenced_preimage { let (_, hash) = preimage_and_hash::(); - assert_ok!(Preimage::::request_preimage(T::ManagerOrigin::successful_origin(), hash)); - assert_ok!(Preimage::::request_preimage(T::ManagerOrigin::successful_origin(), hash)); - }: unrequest_preimage(T::ManagerOrigin::successful_origin(), hash) - verify { + assert_ok!(Preimage::::request_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + hash, + )); + assert_ok!(Preimage::::request_preimage( + T::ManagerOrigin::try_successful_origin() + .expect("ManagerOrigin has no successful origin required for the benchmark"), + hash, + )); + }: unrequest_preimage( + T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + hash + ) verify { let s = RequestStatus::Requested { deposit: None, count: 1, len: None }; assert_eq!(StatusFor::::get(&hash), Some(s)); } diff --git a/frame/ranked-collective/src/benchmarking.rs b/frame/ranked-collective/src/benchmarking.rs index 48df7f1f1e6ab..ad81325becb5e 100644 --- a/frame/ranked-collective/src/benchmarking.rs +++ b/frame/ranked-collective/src/benchmarking.rs @@ -21,7 +21,9 @@ use super::*; #[allow(unused_imports)] use crate::Pallet as RankedCollective; -use frame_benchmarking::v1::{account, benchmarks_instance_pallet, whitelisted_caller}; +use frame_benchmarking::v1::{ + account, benchmarks_instance_pallet, whitelisted_caller, BenchmarkError, +}; use frame_support::{assert_ok, dispatch::UnfilteredDispatchable}; use frame_system::RawOrigin as SystemOrigin; @@ -35,13 +37,15 @@ fn make_member, I: 'static>(rank: Rank) -> T::AccountId { let who = account::("member", MemberCount::::get(0), SEED); let who_lookup = T::Lookup::unlookup(who.clone()); assert_ok!(Pallet::::add_member( - T::PromoteOrigin::successful_origin(), - who_lookup.clone() + T::PromoteOrigin::try_successful_origin() + .expect("PromoteOrigin has no successful origin required for the benchmark"), + who_lookup.clone(), )); for _ in 0..rank { assert_ok!(Pallet::::promote_member( - T::PromoteOrigin::successful_origin(), - who_lookup.clone() + T::PromoteOrigin::try_successful_origin() + .expect("PromoteOrigin has no successful origin required for the benchmark"), + who_lookup.clone(), )); } who @@ -51,7 +55,8 @@ benchmarks_instance_pallet! { add_member { let who = account::("member", 0, SEED); let who_lookup = T::Lookup::unlookup(who.clone()); - let origin = T::PromoteOrigin::successful_origin(); + let origin = + T::PromoteOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call = Call::::add_member { who: who_lookup }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -67,7 +72,8 @@ benchmarks_instance_pallet! { let who_lookup = T::Lookup::unlookup(who.clone()); let last = make_member::(rank); let last_index = (0..=rank).map(|r| IdToIndex::::get(r, &last).unwrap()).collect::>(); - let origin = T::DemoteOrigin::successful_origin(); + let origin = + T::DemoteOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call = Call::::remove_member { who: who_lookup, min_rank: rank }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -83,7 +89,8 @@ benchmarks_instance_pallet! { let rank = r as u16; let who = make_member::(rank); let who_lookup = T::Lookup::unlookup(who.clone()); - let origin = T::PromoteOrigin::successful_origin(); + let origin = + T::PromoteOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call = Call::::promote_member { who: who_lookup }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -99,7 +106,8 @@ benchmarks_instance_pallet! { let who_lookup = T::Lookup::unlookup(who.clone()); let last = make_member::(rank); let last_index = IdToIndex::::get(rank, &last).unwrap(); - let origin = T::DemoteOrigin::successful_origin(); + let origin = + T::DemoteOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call = Call::::demote_member { who: who_lookup }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -115,14 +123,19 @@ benchmarks_instance_pallet! { vote { let caller: T::AccountId = whitelisted_caller(); let caller_lookup = T::Lookup::unlookup(caller.clone()); - assert_ok!(Pallet::::add_member(T::PromoteOrigin::successful_origin(), caller_lookup.clone())); + assert_ok!(Pallet::::add_member( + T::PromoteOrigin::try_successful_origin() + .expect("PromoteOrigin has no successful origin required for the benchmark"), + caller_lookup.clone(), + )); // Create a poll let class = T::Polls::classes().into_iter().next().unwrap(); let rank = T::MinRankOfClass::convert(class.clone()); for _ in 0..rank { assert_ok!(Pallet::::promote_member( - T::PromoteOrigin::successful_origin(), - caller_lookup.clone() + T::PromoteOrigin::try_successful_origin() + .expect("PromoteOrigin has no successful origin required for the benchmark"), + caller_lookup.clone(), )); } diff --git a/frame/ranked-collective/src/lib.rs b/frame/ranked-collective/src/lib.rs index b057a57508023..84be1243c8afc 100644 --- a/frame/ranked-collective/src/lib.rs +++ b/frame/ranked-collective/src/lib.rs @@ -259,19 +259,6 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin::get(MIN_RANK, 0).ok_or(())?; Ok(frame_system::RawOrigin::Signed(who).into()) } - - #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> T::RuntimeOrigin { - match Self::try_successful_origin() { - Ok(o) => o, - Err(()) => { - let who: T::AccountId = frame_benchmarking::whitelisted_caller(); - crate::Pallet::::do_add_member_to_rank(who.clone(), MIN_RANK) - .expect("failed to add ranked member"); - frame_system::RawOrigin::Signed(who).into() - }, - } - } } /// Guard to ensure that the given origin is a member of the collective. The account ID of the @@ -295,19 +282,6 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin::get(MIN_RANK, 0).ok_or(())?; Ok(frame_system::RawOrigin::Signed(who).into()) } - - #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> T::RuntimeOrigin { - match Self::try_successful_origin() { - Ok(o) => o, - Err(()) => { - let who: T::AccountId = frame_benchmarking::whitelisted_caller(); - crate::Pallet::::do_add_member_to_rank(who.clone(), MIN_RANK) - .expect("failed to add ranked member"); - frame_system::RawOrigin::Signed(who).into() - }, - } - } } /// Guard to ensure that the given origin is a member of the collective. The pair of both the @@ -331,19 +305,6 @@ impl, I: 'static, const MIN_RANK: u16> EnsureOrigin::get(MIN_RANK, 0).ok_or(())?; Ok(frame_system::RawOrigin::Signed(who).into()) } - - #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> T::RuntimeOrigin { - match Self::try_successful_origin() { - Ok(o) => o, - Err(()) => { - let who: T::AccountId = frame_benchmarking::whitelisted_caller(); - crate::Pallet::::do_add_member_to_rank(who.clone(), MIN_RANK) - .expect("failed to add ranked member"); - frame_system::RawOrigin::Signed(who).into() - }, - } - } } #[frame_support::pallet] diff --git a/frame/referenda/src/benchmarking.rs b/frame/referenda/src/benchmarking.rs index db5299e0d5bb2..62b37857f2567 100644 --- a/frame/referenda/src/benchmarking.rs +++ b/frame/referenda/src/benchmarking.rs @@ -20,7 +20,9 @@ use super::*; use crate::Pallet as Referenda; use assert_matches::assert_matches; -use frame_benchmarking::v1::{account, benchmarks_instance_pallet, whitelist_account}; +use frame_benchmarking::v1::{ + account, benchmarks_instance_pallet, whitelist_account, BenchmarkError, +}; use frame_support::{ assert_ok, dispatch::UnfilteredDispatchable, @@ -48,8 +50,7 @@ fn dummy_call, I: 'static>() -> Bounded<>::RuntimeCa T::Preimages::bound(call).unwrap() } -fn create_referendum, I: 'static>() -> (T::RuntimeOrigin, ReferendumIndex) { - let origin: T::RuntimeOrigin = T::SubmitOrigin::successful_origin(); +fn create_referendum, I: 'static>(origin: T::RuntimeOrigin) -> ReferendumIndex { if let Ok(caller) = frame_system::ensure_signed(origin.clone()) { T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); whitelist_account!(caller); @@ -61,7 +62,7 @@ fn create_referendum, I: 'static>() -> (T::RuntimeOrigin, Referendu let call = crate::Call::::submit { proposal_origin, proposal, enactment_moment }; assert_ok!(call.dispatch_bypass_filter(origin.clone())); let index = ReferendumCount::::get() - 1; - (origin, index) + index } fn place_deposit, I: 'static>(index: ReferendumIndex) { @@ -75,6 +76,7 @@ fn nudge, I: 'static>(index: ReferendumIndex) { } fn fill_queue, I: 'static>( + origin: T::RuntimeOrigin, index: ReferendumIndex, spaces: u32, pass_after: u32, @@ -82,7 +84,7 @@ fn fill_queue, I: 'static>( // First, create enough other referendums to fill the track. let mut others = vec![]; for _ in 0..info::(index).max_deciding { - let (_origin, index) = create_referendum::(); + let index = create_referendum::(origin.clone()); place_deposit::(index); others.push(index); } @@ -90,7 +92,7 @@ fn fill_queue, I: 'static>( // We will also need enough referenda which are queued and passing, we want `MaxQueued - 1` // in order to force the maximum amount of work to insert ours into the queue. for _ in spaces..T::MaxQueued::get() { - let (_origin, index) = create_referendum::(); + let index = create_referendum::(origin.clone()); place_deposit::(index); make_passing_after::(index, Perbill::from_percent(pass_after)); others.push(index); @@ -194,7 +196,8 @@ fn is_not_confirming, I: 'static>(index: ReferendumIndex) -> bool { benchmarks_instance_pallet! { submit { - let origin: T::RuntimeOrigin = T::SubmitOrigin::successful_origin(); + let origin = + T::SubmitOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; if let Ok(caller) = frame_system::ensure_signed(origin.clone()) { T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); whitelist_account!(caller); @@ -210,15 +213,19 @@ benchmarks_instance_pallet! { } place_decision_deposit_preparing { - let (origin, index) = create_referendum::(); + let origin = + T::SubmitOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let index = create_referendum::(origin.clone()); }: place_decision_deposit(origin, index) verify { assert!(Referenda::::ensure_ongoing(index).unwrap().decision_deposit.is_some()); } place_decision_deposit_queued { - let (origin, index) = create_referendum::(); - fill_queue::(index, 1, 90); + let origin = + T::SubmitOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let index = create_referendum::(origin.clone()); + fill_queue::(origin.clone(), index, 1, 90); }: place_decision_deposit(origin, index) verify { let track = Referenda::::ensure_ongoing(index).unwrap().track; @@ -227,8 +234,10 @@ benchmarks_instance_pallet! { } place_decision_deposit_not_queued { - let (origin, index) = create_referendum::(); - fill_queue::(index, 0, 90); + let origin = + T::SubmitOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let index = create_referendum::(origin.clone()); + fill_queue::(origin.clone(), index, 0, 90); let track = Referenda::::ensure_ongoing(index).unwrap().track; assert_eq!(TrackQueue::::get(&track).len() as u32, T::MaxQueued::get()); assert!(TrackQueue::::get(&track).into_iter().all(|(i, _)| i != index)); @@ -239,7 +248,9 @@ benchmarks_instance_pallet! { } place_decision_deposit_passing { - let (origin, index) = create_referendum::(); + let origin = + T::SubmitOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let index = create_referendum::(origin.clone()); skip_prepare_period::(index); make_passing::(index); }: place_decision_deposit(origin, index) @@ -248,7 +259,9 @@ benchmarks_instance_pallet! { } place_decision_deposit_failing { - let (origin, index) = create_referendum::(); + let origin = + T::SubmitOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let index = create_referendum::(origin.clone()); skip_prepare_period::(index); }: place_decision_deposit(origin, index) verify { @@ -256,19 +269,31 @@ benchmarks_instance_pallet! { } refund_decision_deposit { - let (origin, index) = create_referendum::(); + let origin = + T::SubmitOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let index = create_referendum::(origin.clone()); place_deposit::(index); - assert_ok!(Referenda::::cancel(T::CancelOrigin::successful_origin(), index)); + assert_ok!(Referenda::::cancel( + T::CancelOrigin::try_successful_origin() + .expect("CancelOrigin has no successful origin required for the benchmark"), + index, + )); }: _(origin, index) verify { assert_matches!(ReferendumInfoFor::::get(index), Some(ReferendumInfo::Cancelled(_, _, None))); } refund_submission_deposit { - let (origin, index) = create_referendum::(); + let origin = + T::SubmitOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let index = create_referendum::(origin.clone()); let caller = frame_system::ensure_signed(origin.clone()).unwrap(); let balance = T::Currency::free_balance(&caller); - assert_ok!(Referenda::::cancel(T::CancelOrigin::successful_origin(), index)); + assert_ok!(Referenda::::cancel( + T::CancelOrigin::try_successful_origin() + .expect("CancelOrigin has no successful origin required for the benchmark"), + index, + )); assert_matches!(ReferendumInfoFor::::get(index), Some(ReferendumInfo::Cancelled(_, Some(_), _))); }: _(origin, index) verify { @@ -279,28 +304,42 @@ benchmarks_instance_pallet! { } cancel { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); - }: _(T::CancelOrigin::successful_origin(), index) - verify { + }: _( + T::CancelOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + index + ) verify { assert_matches!(ReferendumInfoFor::::get(index), Some(ReferendumInfo::Cancelled(..))); } kill { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); - }: _(T::KillOrigin::successful_origin(), index) - verify { + }: _( + T::KillOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?, + index + ) verify { assert_matches!(ReferendumInfoFor::::get(index), Some(ReferendumInfo::Killed(..))); } one_fewer_deciding_queue_empty { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); skip_prepare_period::(index); nudge::(index); let track = Referenda::::ensure_ongoing(index).unwrap().track; - assert_ok!(Referenda::::cancel(T::CancelOrigin::successful_origin(), index)); + assert_ok!(Referenda::::cancel( + T::CancelOrigin::try_successful_origin() + .expect("CancelOrigin has no successful origin required for the benchmark"), + index, + )); assert_eq!(DecidingCount::::get(&track), 1); }: one_fewer_deciding(RawOrigin::Root, track) verify { @@ -308,11 +347,17 @@ benchmarks_instance_pallet! { } one_fewer_deciding_failing { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin.clone()); // No spaces free in the queue. - let queued = fill_queue::(index, 0, 90); + let queued = fill_queue::(origin, index, 0, 90); let track = Referenda::::ensure_ongoing(index).unwrap().track; - assert_ok!(Referenda::::cancel(T::CancelOrigin::successful_origin(), queued[0])); + assert_ok!(Referenda::::cancel( + T::CancelOrigin::try_successful_origin() + .expect("CancelOrigin has no successful origin required for the benchmark"), + queued[0], + )); assert_eq!(TrackQueue::::get(&track).len() as u32, T::MaxQueued::get()); let deciding_count = DecidingCount::::get(&track); }: one_fewer_deciding(RawOrigin::Root, track) @@ -327,11 +372,17 @@ benchmarks_instance_pallet! { } one_fewer_deciding_passing { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin.clone()); // No spaces free in the queue. - let queued = fill_queue::(index, 0, 0); + let queued = fill_queue::(origin, index, 0, 0); let track = Referenda::::ensure_ongoing(index).unwrap().track; - assert_ok!(Referenda::::cancel(T::CancelOrigin::successful_origin(), queued[0])); + assert_ok!(Referenda::::cancel( + T::CancelOrigin::try_successful_origin() + .expect("CancelOrigin has no successful origin required for the benchmark"), + queued[0], + )); assert_eq!(TrackQueue::::get(&track).len() as u32, T::MaxQueued::get()); let deciding_count = DecidingCount::::get(&track); }: one_fewer_deciding(RawOrigin::Root, track) @@ -346,10 +397,12 @@ benchmarks_instance_pallet! { } nudge_referendum_requeued_insertion { + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); // First create our referendum and place the deposit. It will be failing. - let (_origin, index) = create_referendum::(); + let index = create_referendum::(origin.clone()); place_deposit::(index); - fill_queue::(index, 0, 90); + fill_queue::(origin, index, 0, 90); // Now nudge ours, with the track now full and the queue full of referenda with votes, // ours will not be in the queue. @@ -367,10 +420,12 @@ benchmarks_instance_pallet! { } nudge_referendum_requeued_slide { + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); // First create our referendum and place the deposit. It will be failing. - let (_origin, index) = create_referendum::(); + let index = create_referendum::(origin.clone()); place_deposit::(index); - fill_queue::(index, 1, 90); + fill_queue::(origin, index, 1, 90); // Now nudge ours, with the track now full, ours will be queued, but with no votes, it // will have the worst position. @@ -393,10 +448,12 @@ benchmarks_instance_pallet! { // free and this failing. It would result in `QUEUE_SIZE - 1` items being shifted for the // insertion at the beginning. + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); // First create our referendum and place the deposit. It will be failing. - let (_origin, index) = create_referendum::(); + let index = create_referendum::(origin.clone()); place_deposit::(index); - fill_queue::(index, 1, 0); + fill_queue::(origin, index, 1, 0); let track = Referenda::::ensure_ongoing(index).unwrap().track; assert_eq!(TrackQueue::::get(&track).len() as u32, T::MaxQueued::get() - 1); @@ -410,10 +467,12 @@ benchmarks_instance_pallet! { } nudge_referendum_not_queued { + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); // First create our referendum and place the deposit. It will be failing. - let (_origin, index) = create_referendum::(); + let index = create_referendum::(origin.clone()); place_deposit::(index); - fill_queue::(index, 0, 0); + fill_queue::(origin, index, 0, 0); let track = Referenda::::ensure_ongoing(index).unwrap().track; assert_eq!(TrackQueue::::get(&track).len() as u32, T::MaxQueued::get()); @@ -427,7 +486,9 @@ benchmarks_instance_pallet! { } nudge_referendum_no_deposit { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); skip_prepare_period::(index); }: nudge_referendum(RawOrigin::Root, index) verify { @@ -436,7 +497,9 @@ benchmarks_instance_pallet! { } nudge_referendum_preparing { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); }: nudge_referendum(RawOrigin::Root, index) verify { @@ -445,7 +508,9 @@ benchmarks_instance_pallet! { } nudge_referendum_timed_out { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); skip_timeout_period::(index); }: nudge_referendum(RawOrigin::Root, index) verify { @@ -454,7 +519,9 @@ benchmarks_instance_pallet! { } nudge_referendum_begin_deciding_failing { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); skip_prepare_period::(index); }: nudge_referendum(RawOrigin::Root, index) @@ -463,7 +530,9 @@ benchmarks_instance_pallet! { } nudge_referendum_begin_deciding_passing { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); make_passing::(index); skip_prepare_period::(index); @@ -473,7 +542,9 @@ benchmarks_instance_pallet! { } nudge_referendum_begin_confirming { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); skip_prepare_period::(index); nudge::(index); @@ -485,7 +556,9 @@ benchmarks_instance_pallet! { } nudge_referendum_end_confirming { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); skip_prepare_period::(index); make_passing::(index); @@ -498,7 +571,9 @@ benchmarks_instance_pallet! { } nudge_referendum_continue_not_confirming { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); skip_prepare_period::(index); nudge::(index); @@ -512,7 +587,9 @@ benchmarks_instance_pallet! { } nudge_referendum_continue_confirming { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); make_passing::(index); skip_prepare_period::(index); @@ -525,7 +602,9 @@ benchmarks_instance_pallet! { } nudge_referendum_approved { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); skip_prepare_period::(index); make_passing::(index); @@ -538,7 +617,9 @@ benchmarks_instance_pallet! { } nudge_referendum_rejected { - let (_origin, index) = create_referendum::(); + let origin = T::SubmitOrigin::try_successful_origin() + .expect("SubmitOrigin has no successful origin required for the benchmark"); + let index = create_referendum::(origin); place_deposit::(index); skip_prepare_period::(index); make_failing::(index); diff --git a/frame/scheduler/src/benchmarking.rs b/frame/scheduler/src/benchmarking.rs index 9ec82f5c8447e..94a833bd98e0a 100644 --- a/frame/scheduler/src/benchmarking.rs +++ b/frame/scheduler/src/benchmarking.rs @@ -18,7 +18,7 @@ //! Scheduler pallet benchmarking. use super::*; -use frame_benchmarking::v1::{account, benchmarks}; +use frame_benchmarking::v1::{account, benchmarks, BenchmarkError}; use frame_support::{ ensure, traits::{schedule::Priority, BoundedInline}, @@ -244,7 +244,8 @@ benchmarks! { fill_schedule::(when, s)?; assert_eq!(Agenda::::get(when).len(), s as usize); - let schedule_origin = T::ScheduleOrigin::successful_origin(); + let schedule_origin = + T::ScheduleOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _>(schedule_origin, when, 0) verify { ensure!( diff --git a/frame/support/src/traits/dispatch.rs b/frame/support/src/traits/dispatch.rs index 36ddf5b507c0c..50c6c22791944 100644 --- a/frame/support/src/traits/dispatch.rs +++ b/frame/support/src/traits/dispatch.rs @@ -40,25 +40,12 @@ pub trait EnsureOrigin { /// Perform the origin check. fn try_origin(o: OuterOrigin) -> Result; - /// Returns an outer origin capable of passing `try_origin` check. - /// - /// NOTE: This should generally *NOT* be reimplemented. Instead implement - /// `try_successful_origin`. + /// Attempt to get an outer origin capable of passing `try_origin` check. May return `Err` if it + /// is impossible. /// /// ** Should be used for benchmarking only!!! ** #[cfg(feature = "runtime-benchmarks")] - fn successful_origin() -> OuterOrigin { - Self::try_successful_origin().expect("No origin exists which can satisfy the guard") - } - - /// Attept to get an outer origin capable of passing `try_origin` check. May return `Err` if it - /// is impossible. Default implementation just uses `successful_origin()`. - /// - /// ** Should be used for benchmarking only!!! ** - #[cfg(feature = "runtime-benchmarks")] - fn try_successful_origin() -> Result { - Ok(Self::successful_origin()) - } + fn try_successful_origin() -> Result; } /// [`EnsureOrigin`] implementation that always fails. @@ -171,25 +158,12 @@ pub trait EnsureOriginWithArg { /// Perform the origin check, returning the origin value if unsuccessful. This allows chaining. fn try_origin(o: OuterOrigin, a: &Argument) -> Result; - /// Returns an outer origin capable of passing `try_origin` check. - /// - /// NOTE: This should generally *NOT* be reimplemented. Instead implement - /// `try_successful_origin`. - /// - /// ** Should be used for benchmarking only!!! ** - #[cfg(feature = "runtime-benchmarks")] - fn successful_origin(a: &Argument) -> OuterOrigin { - Self::try_successful_origin(a).expect("No origin exists which can satisfy the guard") - } - - /// Attept to get an outer origin capable of passing `try_origin` check. May return `Err` if it - /// is impossible. Default implementation just uses `successful_origin()`. + /// Attempt to get an outer origin capable of passing `try_origin` check. May return `Err` if it + /// is impossible. /// /// ** Should be used for benchmarking only!!! ** #[cfg(feature = "runtime-benchmarks")] - fn try_successful_origin(a: &Argument) -> Result { - Ok(Self::successful_origin(a)) - } + fn try_successful_origin(a: &Argument) -> Result; } pub struct AsEnsureOriginWithArg(sp_std::marker::PhantomData); diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index 8223f66744151..f441e785599eb 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -679,10 +679,13 @@ fn ensure_signed_stuff_works() { #[cfg(feature = "runtime-benchmarks")] { - let successful_origin: RuntimeOrigin = EnsureSigned::successful_origin(); + let successful_origin: RuntimeOrigin = EnsureSigned::try_successful_origin() + .expect("EnsureSigned has no successful origin required for the test"); assert_ok!(EnsureSigned::try_origin(successful_origin)); - let successful_origin: RuntimeOrigin = EnsureSignedBy::::successful_origin(); + let successful_origin: RuntimeOrigin = + EnsureSignedBy::::try_successful_origin() + .expect("EnsureSignedBy has no successful origin required for the test"); assert_ok!(EnsureSignedBy::::try_origin(successful_origin)); } } diff --git a/frame/tips/src/benchmarking.rs b/frame/tips/src/benchmarking.rs index 8ffc53b45e4ca..5b1a102b8eb30 100644 --- a/frame/tips/src/benchmarking.rs +++ b/frame/tips/src/benchmarking.rs @@ -19,7 +19,9 @@ #![cfg(feature = "runtime-benchmarks")] -use frame_benchmarking::v1::{account, benchmarks_instance_pallet, whitelisted_caller}; +use frame_benchmarking::v1::{ + account, benchmarks_instance_pallet, whitelisted_caller, BenchmarkError, +}; use frame_support::ensure; use frame_system::RawOrigin; use sp_runtime::traits::Saturating; @@ -196,7 +198,8 @@ benchmarks_instance_pallet! { let reason_hash = T::Hashing::hash(&reason[..]); let hash = T::Hashing::hash_of(&(&reason_hash, &beneficiary)); ensure!(Tips::::contains_key(hash), "tip does not exist"); - let reject_origin = T::RejectOrigin::successful_origin(); + let reject_origin = + T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(reject_origin, hash) impl_benchmark_test_suite!(TipsMod, crate::tests::new_test_ext(), crate::tests::Test); diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index eb44744506bf4..0947618a2b02f 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::{Pallet as Treasury, *}; -use frame_benchmarking::v1::{account, benchmarks_instance_pallet}; +use frame_benchmarking::v1::{account, benchmarks_instance_pallet, BenchmarkError}; use frame_support::{ dispatch::UnfilteredDispatchable, ensure, @@ -99,7 +99,8 @@ benchmarks_instance_pallet! { beneficiary_lookup )?; let proposal_id = Treasury::::proposal_count() - 1; - let reject_origin = T::RejectOrigin::successful_origin(); + let reject_origin = + T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(reject_origin, proposal_id) approve_proposal { @@ -112,7 +113,8 @@ benchmarks_instance_pallet! { beneficiary_lookup )?; let proposal_id = Treasury::::proposal_count() - 1; - let approve_origin = T::ApproveOrigin::successful_origin(); + let approve_origin = + T::ApproveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(approve_origin, proposal_id) remove_approval { @@ -124,7 +126,8 @@ benchmarks_instance_pallet! { )?; let proposal_id = Treasury::::proposal_count() - 1; Treasury::::approve_proposal(RawOrigin::Root.into(), proposal_id)?; - let reject_origin = T::RejectOrigin::successful_origin(); + let reject_origin = + T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(reject_origin, proposal_id) on_initialize_proposals { diff --git a/frame/uniques/src/benchmarking.rs b/frame/uniques/src/benchmarking.rs index 5ecb093a5d010..2656582f0e073 100644 --- a/frame/uniques/src/benchmarking.rs +++ b/frame/uniques/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; use frame_benchmarking::v1::{ - account, benchmarks_instance_pallet, whitelist_account, whitelisted_caller, + account, benchmarks_instance_pallet, whitelist_account, whitelisted_caller, BenchmarkError, }; use frame_support::{ dispatch::UnfilteredDispatchable, @@ -137,7 +137,8 @@ fn assert_last_event, I: 'static>(generic_event: >:: benchmarks_instance_pallet! { create { let collection = T::Helper::collection(0); - let origin = T::CreateOrigin::successful_origin(&collection); + let origin = T::CreateOrigin::try_successful_origin(&collection) + .map_err(|_| BenchmarkError::Weightless)?; let caller = T::CreateOrigin::ensure_origin(origin.clone(), &collection).unwrap(); whitelist_account!(caller); let admin = T::Lookup::unlookup(caller.clone()); @@ -290,7 +291,8 @@ benchmarks_instance_pallet! { force_item_status { let (collection, caller, caller_lookup) = create_collection::(); - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call = Call::::force_item_status { collection, owner: caller_lookup.clone(), diff --git a/frame/whitelist/src/benchmarking.rs b/frame/whitelist/src/benchmarking.rs index e64842b34df59..582f4a7997c67 100644 --- a/frame/whitelist/src/benchmarking.rs +++ b/frame/whitelist/src/benchmarking.rs @@ -20,7 +20,7 @@ #![cfg(feature = "runtime-benchmarks")] use super::*; -use frame_benchmarking::v1::benchmarks; +use frame_benchmarking::v1::{benchmarks, BenchmarkError}; use frame_support::{ensure, traits::EnsureOrigin}; #[cfg(test)] @@ -28,7 +28,8 @@ use crate::Pallet as Whitelist; benchmarks! { whitelist_call { - let origin = T::WhitelistOrigin::successful_origin(); + let origin = + T::WhitelistOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call_hash = Default::default(); }: _(origin, call_hash) verify { @@ -43,7 +44,8 @@ benchmarks! { } remove_whitelisted_call { - let origin = T::WhitelistOrigin::successful_origin(); + let origin = + T::WhitelistOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let call_hash = Default::default(); Pallet::::whitelist_call(origin.clone(), call_hash) .expect("whitelisting call must be successful"); @@ -70,7 +72,8 @@ benchmarks! { // NOTE: we remove `10` because we need some bytes to encode the variants and vec length let n in 1 .. T::Preimages::MAX_LENGTH as u32 - 10; - let origin = T::DispatchWhitelistedOrigin::successful_origin(); + let origin = T::DispatchWhitelistedOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; let remark = sp_std::vec![1u8; n as usize]; let call: ::RuntimeCall = frame_system::Call::remark { remark }.into(); let call_weight = call.get_dispatch_info().weight; @@ -98,7 +101,8 @@ benchmarks! { dispatch_whitelisted_call_with_preimage { let n in 1 .. 10_000; - let origin = T::DispatchWhitelistedOrigin::successful_origin(); + let origin = T::DispatchWhitelistedOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; let remark = sp_std::vec![1u8; n as usize]; let call: ::RuntimeCall = frame_system::Call::remark { remark }.into();