Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Benchmark's successful origin api update (#13146)
Browse files Browse the repository at this point in the history
* try successful origin unimplemented by default

* error as a default impl for try_successful_origin

* remove successful_origin func of EnsureOrigin trait

* default impl -> unimplemented!()

* update EnsureOriginWithArg

* fix EnsureOriginWithArg

* prefix unused arg with underscore

* use try_successful_origin instead successful_origin, map err to Weightless

* fix tests

* remove default impl

* unwrap for indirect origin dep

* replace unwrap by expect with a message

---------

Co-authored-by: parity-processbot <>
  • Loading branch information
muharem authored and Ank4n committed Feb 28, 2023
1 parent 162ba81 commit e35b92d
Show file tree
Hide file tree
Showing 21 changed files with 402 additions and 243 deletions.
23 changes: 15 additions & 8 deletions frame/alliance/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -581,7 +581,8 @@ benchmarks_instance_pallet! {
let rule = rule(b"hello world");

let call = Call::<T, I>::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::<T, I>::rule(), Some(rule.clone()));
Expand All @@ -594,7 +595,8 @@ benchmarks_instance_pallet! {
let announcement = announcement(b"hello world");

let call = Call::<T, I>::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::<T, I>::announcements().contains(&announcement));
Expand All @@ -609,7 +611,8 @@ benchmarks_instance_pallet! {
Announcements::<T, I>::put(announcements);

let call = Call::<T, I>::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::<T, I>::announcements().is_empty());
Expand Down Expand Up @@ -665,7 +668,8 @@ benchmarks_instance_pallet! {

let ally1_lookup = T::Lookup::unlookup(ally1.clone());
let call = Call::<T, I>::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::<T, I>::is_ally(&ally1));
Expand Down Expand Up @@ -725,7 +729,8 @@ benchmarks_instance_pallet! {

let fellow2_lookup = T::Lookup::unlookup(fellow2.clone());
let call = Call::<T, I>::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::<T, I>::is_member(&fellow2));
Expand Down Expand Up @@ -754,7 +759,8 @@ benchmarks_instance_pallet! {
unscrupulous_list.extend(websites.into_iter().map(UnscrupulousItem::Website));

let call = Call::<T, I>::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::<T, I>(Event::UnscrupulousItemAdded { items: unscrupulous_list }.into());
Expand Down Expand Up @@ -784,7 +790,8 @@ benchmarks_instance_pallet! {
unscrupulous_list.extend(websites.into_iter().map(UnscrupulousItem::Website));

let call = Call::<T, I>::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::<T, I>(Event::UnscrupulousItemRemoved { items: unscrupulous_list }.into());
Expand Down
14 changes: 9 additions & 5 deletions frame/assets/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -135,7 +135,8 @@ fn assert_event<T: Config<I>, I: 'static>(generic_event: <T as Config<I>>::Runti
benchmarks_instance_pallet! {
create {
let asset_id = default_asset_id::<T, I>();
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::<T, I>::max_value());
Expand Down Expand Up @@ -362,7 +363,8 @@ benchmarks_instance_pallet! {

let (asset_id, _, _) = create_default_asset::<T, I>(true);

let origin = T::ForceOrigin::successful_origin();
let origin =
T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
let call = Call::<T, I>::force_set_metadata {
id: asset_id,
name: name.clone(),
Expand All @@ -382,7 +384,8 @@ benchmarks_instance_pallet! {
let origin = SystemOrigin::Signed(caller).into();
Assets::<T, I>::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::<T, I>::force_clear_metadata { id: asset_id };
}: { call.dispatch_bypass_filter(origin)? }
verify {
Expand All @@ -392,7 +395,8 @@ benchmarks_instance_pallet! {
force_asset_status {
let (asset_id, caller, caller_lookup) = create_default_asset::<T, I>(true);

let origin = T::ForceOrigin::successful_origin();
let origin =
T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
let call = Call::<T, I>::force_asset_status {
id: asset_id,
owner: caller_lookup.clone(),
Expand Down
6 changes: 4 additions & 2 deletions frame/bounties/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,17 @@ benchmarks_instance_pallet! {
let (caller, curator, fee, value, reason) = setup_bounty::<T, I>(0, 0);
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
let approve_origin = T::ApproveOrigin::successful_origin();
let approve_origin =
T::ApproveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
}: close_bounty<T::RuntimeOrigin>(approve_origin, bounty_id)

close_bounty_active {
setup_pot_account::<T, I>();
let (curator_lookup, bounty_id) = create_bounty::<T, I>()?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
let bounty_id = BountyCount::<T, I>::get() - 1;
let approve_origin = T::ApproveOrigin::successful_origin();
let approve_origin =
T::ApproveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
}: close_bounty<T::RuntimeOrigin>(approve_origin, bounty_id)
verify {
assert_last_event::<T, I>(Event::BountyCanceled { index: bounty_id }.into())
Expand Down
42 changes: 27 additions & 15 deletions frame/democracy/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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::<T>(0).0;
assert_ok!(Democracy::<T>::referendum_status(ref_index));
}: _<T::RuntimeOrigin>(origin, ref_index)
Expand All @@ -200,10 +201,13 @@ benchmarks! {
let (ref_index, hash) = add_referendum::<T>(0);
assert_ok!(Democracy::<T>::referendum_status(ref_index));
// Place our proposal in the external queue, too.
assert_ok!(
Democracy::<T>::external_propose(T::ExternalOrigin::successful_origin(), make_proposal::<T>(0))
);
let origin = T::BlacklistOrigin::successful_origin();
assert_ok!(Democracy::<T>::external_propose(
T::ExternalOrigin::try_successful_origin()
.expect("ExternalOrigin has no successful origin required for the benchmark"),
make_proposal::<T>(0)
));
let origin =
T::BlacklistOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
}: _<T::RuntimeOrigin>(origin, hash, Some(ref_index))
verify {
// Referendum has been canceled
Expand All @@ -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::<T>(0);
// Add proposal to blacklist with block number 0

Expand All @@ -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::<T>(0);
}: _<T::RuntimeOrigin>(origin, proposal)
verify {
Expand All @@ -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::<T>(0);
}: _<T::RuntimeOrigin>(origin, proposal)
verify {
Expand All @@ -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::<T>(0);
let proposal_hash = proposal.hash();
Democracy::<T>::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;
}: _<T::RuntimeOrigin>(origin_fast_track, proposal_hash, voting_period, delay.into())
Expand All @@ -269,7 +278,8 @@ benchmarks! {
let proposal = make_proposal::<T>(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::<T>::external_propose_default(origin_propose, proposal)?;

let mut vetoers: BoundedVec<T::AccountId, _> = Default::default();
Expand All @@ -279,7 +289,7 @@ benchmarks! {
vetoers.sort();
Blacklist::<T>::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::<T>::get().is_some(), "no external proposal");
}: _<T::RuntimeOrigin>(origin, proposal_hash)
verify {
Expand All @@ -293,7 +303,8 @@ benchmarks! {
for i in 0 .. T::MaxProposals::get() {
add_proposal::<T>(i)?;
}
let cancel_origin = T::CancelProposalOrigin::successful_origin();
let cancel_origin = T::CancelProposalOrigin::try_successful_origin()
.map_err(|_| BenchmarkError::Weightless)?;
}: _<T::RuntimeOrigin>(cancel_origin, 0)

cancel_referendum {
Expand All @@ -313,7 +324,8 @@ benchmarks! {
// Launch external
LastTabledWasExternal::<T>::put(false);

let origin = T::ExternalMajorityOrigin::successful_origin();
let origin = T::ExternalMajorityOrigin::try_successful_origin()
.map_err(|_| BenchmarkError::Weightless)?;
let proposal = make_proposal::<T>(r);
let call = Call::<T>::external_propose_majority { proposal };
call.dispatch_bypass_filter(origin)?;
Expand Down
5 changes: 3 additions & 2 deletions frame/fast-unstake/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -192,7 +192,8 @@ benchmarks! {
}

control {
let origin = <T as Config>::ControlOrigin::successful_origin();
let origin = <T as Config>::ControlOrigin::try_successful_origin()
.map_err(|_| BenchmarkError::Weightless)?;
}
: _<T::RuntimeOrigin>(origin, T::MaxErasToCheckPerBlock::get())
verify {}
Expand Down
23 changes: 15 additions & 8 deletions frame/identity/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -42,7 +42,8 @@ fn add_registrars<T: Config>(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(&registrar, BalanceOf::<T>::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::<T>::add_registrar(registrar_origin, registrar_lookup)?;
Identity::<T>::set_fee(RawOrigin::Signed(registrar.clone()).into(), i, 10u32.into())?;
let fields =
Expand Down Expand Up @@ -121,7 +122,8 @@ benchmarks! {
add_registrar {
let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
ensure!(Registrars::<T>::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));
}: _<T::RuntimeOrigin>(origin, account)
verify {
Expand Down Expand Up @@ -280,7 +282,8 @@ benchmarks! {

let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(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::<T>::add_registrar(registrar_origin, caller_lookup)?;
let registrars = Registrars::<T>::get();
ensure!(registrars[r as usize].as_ref().unwrap().fee == 0u32.into(), "Fee already set.");
Expand All @@ -297,7 +300,8 @@ benchmarks! {

let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(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::<T>::add_registrar(registrar_origin, caller_lookup)?;
let registrars = Registrars::<T>::get();
ensure!(registrars[r as usize].as_ref().unwrap().account == caller, "id not set.");
Expand All @@ -315,7 +319,8 @@ benchmarks! {

let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(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::<T>::add_registrar(registrar_origin, caller_lookup)?;
let fields = IdentityFields(
IdentityField::Display | IdentityField::Legal | IdentityField::Web | IdentityField::Riot
Expand Down Expand Up @@ -347,7 +352,8 @@ benchmarks! {
let info_hash = T::Hashing::hash_of(&info);
Identity::<T>::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::<T>::add_registrar(registrar_origin, caller_lookup)?;
Identity::<T>::request_judgement(user_origin, r, 10u32.into())?;
}: _(RawOrigin::Signed(caller), r, user_lookup, Judgement::Reasonable, info_hash)
Expand Down Expand Up @@ -385,7 +391,8 @@ benchmarks! {
)?;
}
ensure!(IdentityOf::<T>::contains_key(&target), "Identity not set");
let origin = T::ForceOrigin::successful_origin();
let origin =
T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
}: _<T::RuntimeOrigin>(origin, target_lookup)
verify {
ensure!(!IdentityOf::<T>::contains_key(&target), "Identity not removed");
Expand Down
Loading

0 comments on commit e35b92d

Please sign in to comment.