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

Fix Origins Used in Runtime Benchmarks #12037

Merged
merged 8 commits into from
Aug 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions frame/benchmarking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ macro_rules! benchmarks_iter {
( $( $names:tt )* )
( $( $names_extra:tt )* )
( $( $names_skip_meta:tt )* )
$name:ident { $( $code:tt )* }: _ ( $origin:expr $( , $arg:expr )* )
$name:ident { $( $code:tt )* }: _ $(<$origin_type:ty>)? ( $origin:expr $( , $arg:expr )* )
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
Expand All @@ -557,7 +557,7 @@ macro_rules! benchmarks_iter {
( $( $names )* )
( $( $names_extra )* )
( $( $names_skip_meta )* )
$name { $( $code )* }: _ ( $origin $( , $arg )* )
$name { $( $code )* }: _ $(<$origin_type>)? ( $origin $( , $arg )* )
verify { }
$( $rest )*
}
Expand All @@ -570,7 +570,7 @@ macro_rules! benchmarks_iter {
( $( $names:tt )* )
( $( $names_extra:tt )* )
( $( $names_skip_meta:tt )* )
$name:ident { $( $code:tt )* }: $dispatch:ident ( $origin:expr $( , $arg:expr )* )
$name:ident { $( $code:tt )* }: $dispatch:ident $(<$origin_type:ty>)? ( $origin:expr $( , $arg:expr )* )
$( $rest:tt )*
) => {
$crate::benchmarks_iter! {
Expand All @@ -580,7 +580,7 @@ macro_rules! benchmarks_iter {
( $( $names )* )
( $( $names_extra )* )
( $( $names_skip_meta )* )
$name { $( $code )* }: $dispatch ( $origin $( , $arg )* )
$name { $( $code )* }: $dispatch $(<$origin_type>)? ( $origin $( , $arg )* )
verify { }
$( $rest )*
}
Expand All @@ -593,7 +593,7 @@ macro_rules! benchmarks_iter {
( $( $names:tt )* )
( $( $names_extra:tt )* )
( $( $names_skip_meta:tt )* )
$name:ident { $( $code:tt )* }: $eval:block
$name:ident { $( $code:tt )* }: $(<$origin_type:ty>)? $eval:block
$( $rest:tt )*
) => {
$crate::benchmarks_iter!(
Expand All @@ -603,7 +603,7 @@ macro_rules! benchmarks_iter {
( $( $names )* )
( $( $names_extra )* )
( $( $names_skip_meta )* )
$name { $( $code )* }: $eval
$name { $( $code )* }: $(<$origin_type>)? $eval
verify { }
$( $rest )*
);
Expand All @@ -617,7 +617,7 @@ macro_rules! to_origin {
$origin.into()
};
($origin:expr, $origin_type:ty) => {
<T::Origin as From<$origin_type>>::from($origin)
<<T as frame_system::Config>::Origin as From<$origin_type>>::from($origin)
};
}

Expand Down
33 changes: 18 additions & 15 deletions frame/bounties/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ fn create_approved_bounties<T: Config<I>, I: 'static>(n: u32) -> Result<(), &'st
setup_bounty::<T, I>(i, T::MaximumReasonLength::get());
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
Bounties::<T, I>::approve_bounty(RawOrigin::Root.into(), bounty_id)?;
let approve_origin = T::ApproveOrigin::successful_origin();
Bounties::<T, I>::approve_bounty(approve_origin, bounty_id)?;
}
ensure!(BountyApprovals::<T, I>::get().len() == n as usize, "Not all bounty approved");
Ok(())
Expand Down Expand Up @@ -67,14 +68,10 @@ fn create_bounty<T: Config<I>, I: 'static>(
let curator_lookup = T::Lookup::unlookup(curator.clone());
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
Bounties::<T, I>::approve_bounty(RawOrigin::Root.into(), bounty_id)?;
let approve_origin = T::ApproveOrigin::successful_origin();
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
Bounties::<T, I>::propose_curator(
RawOrigin::Root.into(),
bounty_id,
curator_lookup.clone(),
fee,
)?;
Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup.clone(), fee)?;
Bounties::<T, I>::accept_curator(RawOrigin::Signed(curator).into(), bounty_id)?;
Ok((curator_lookup, bounty_id))
}
Expand All @@ -100,17 +97,20 @@ benchmarks_instance_pallet! {
let (caller, curator, fee, value, reason) = setup_bounty::<T, I>(0, T::MaximumReasonLength::get());
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
}: _(RawOrigin::Root, bounty_id)
let approve_origin = T::ApproveOrigin::successful_origin();
}: _<T::Origin>(approve_origin, bounty_id)

propose_curator {
setup_pot_account::<T, I>();
let (caller, curator, fee, value, reason) = setup_bounty::<T, I>(0, T::MaximumReasonLength::get());
let curator_lookup = T::Lookup::unlookup(curator);
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
Bounties::<T, I>::approve_bounty(RawOrigin::Root.into(), bounty_id)?;
let approve_origin = T::ApproveOrigin::successful_origin();
Bounties::<T, I>::approve_bounty(approve_origin, bounty_id)?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
}: _(RawOrigin::Root, bounty_id, curator_lookup, fee)
let approve_origin = T::ApproveOrigin::successful_origin();
}: _<T::Origin>(approve_origin, bounty_id, curator_lookup, fee)

// Worst case when curator is inactive and any sender unassigns the curator.
unassign_curator {
Expand All @@ -128,9 +128,10 @@ benchmarks_instance_pallet! {
let curator_lookup = T::Lookup::unlookup(curator.clone());
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
Bounties::<T, I>::approve_bounty(RawOrigin::Root.into(), bounty_id)?;
let approve_origin = T::ApproveOrigin::successful_origin();
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
Bounties::<T, I>::propose_curator(RawOrigin::Root.into(), bounty_id, curator_lookup, fee)?;
Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup, fee)?;
}: _(RawOrigin::Signed(curator), bounty_id)

award_bounty {
Expand Down Expand Up @@ -169,14 +170,16 @@ 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;
}: close_bounty(RawOrigin::Root, bounty_id)
let approve_origin = T::ApproveOrigin::successful_origin();
}: close_bounty<T::Origin>(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;
}: close_bounty(RawOrigin::Root, bounty_id)
let approve_origin = T::ApproveOrigin::successful_origin();
}: close_bounty<T::Origin>(approve_origin, bounty_id)
verify {
assert_last_event::<T, I>(Event::BountyCanceled { index: bounty_id }.into())
}
Expand Down
4 changes: 3 additions & 1 deletion frame/democracy/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,9 @@ benchmarks! {
for i in 0 .. p {
add_proposal::<T>(i)?;
}
}: _(RawOrigin::Root, 0)

let cancel_origin = T::CancelProposalOrigin::successful_origin();
}: _<T::Origin>(cancel_origin, 0)

cancel_referendum {
let referendum_index = add_referendum::<T>(0)?;
Expand Down
26 changes: 18 additions & 8 deletions frame/identity/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ use super::*;

use crate::Pallet as Identity;
use frame_benchmarking::{account, benchmarks, whitelisted_caller};
use frame_support::{ensure, traits::Get};
use frame_support::{
ensure,
traits::{EnsureOrigin, Get},
};
use frame_system::RawOrigin;
use sp_runtime::traits::Bounded;

Expand All @@ -38,7 +41,8 @@ fn add_registrars<T: Config>(r: u32) -> Result<(), &'static str> {
for i in 0..r {
let registrar: T::AccountId = account("registrar", i, SEED);
let _ = T::Currency::make_free_balance_be(&registrar, BalanceOf::<T>::max_value());
Identity::<T>::add_registrar(RawOrigin::Root.into(), registrar.clone())?;
let registrar_origin = T::RegistrarOrigin::successful_origin();
Identity::<T>::add_registrar(registrar_origin, registrar.clone())?;
Identity::<T>::set_fee(RawOrigin::Signed(registrar.clone()).into(), i, 10u32.into())?;
let fields =
IdentityFields(
Expand Down Expand Up @@ -114,7 +118,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.");
}: _(RawOrigin::Root, account("registrar", r + 1, SEED))
let origin = T::RegistrarOrigin::successful_origin();
}: _<T::Origin>(origin, account("registrar", r + 1, SEED))
verify {
ensure!(Registrars::<T>::get().len() as u32 == r + 1, "Registrars not added.");
}
Expand Down Expand Up @@ -259,7 +264,8 @@ benchmarks! {

let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;

Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
let registrar_origin = T::RegistrarOrigin::successful_origin();
Identity::<T>::add_registrar(registrar_origin, caller.clone())?;
let registrars = Registrars::<T>::get();
ensure!(registrars[r as usize].as_ref().unwrap().fee == 0u32.into(), "Fee already set.");
}: _(RawOrigin::Signed(caller), r, 100u32.into())
Expand All @@ -274,7 +280,8 @@ benchmarks! {

let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;

Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
let registrar_origin = T::RegistrarOrigin::successful_origin();
Identity::<T>::add_registrar(registrar_origin, caller.clone())?;
let registrars = Registrars::<T>::get();
ensure!(registrars[r as usize].as_ref().unwrap().account == caller, "id not set.");
}: _(RawOrigin::Signed(caller), r, account("new", 0, SEED))
Expand All @@ -289,7 +296,8 @@ benchmarks! {

let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;

Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
let registrar_origin = T::RegistrarOrigin::successful_origin();
Identity::<T>::add_registrar(registrar_origin, caller.clone())?;
let fields = IdentityFields(
IdentityField::Display | IdentityField::Legal | IdentityField::Web | IdentityField::Riot
| IdentityField::Email | IdentityField::PgpFingerprint | IdentityField::Image | IdentityField::Twitter
Expand Down Expand Up @@ -318,7 +326,8 @@ benchmarks! {
Identity::<T>::set_identity(user_origin.clone(), Box::new(info))?;
};

Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
let registrar_origin = T::RegistrarOrigin::successful_origin();
Identity::<T>::add_registrar(registrar_origin, caller.clone())?;
Identity::<T>::request_judgement(user_origin, r, 10u32.into())?;
}: _(RawOrigin::Signed(caller), r, user_lookup, Judgement::Reasonable)
verify {
Expand Down Expand Up @@ -350,7 +359,8 @@ benchmarks! {
)?;
}
ensure!(IdentityOf::<T>::contains_key(&target), "Identity not set");
}: _(RawOrigin::Root, target_lookup)
let origin = T::ForceOrigin::successful_origin();
}: _<T::Origin>(origin, target_lookup)
verify {
ensure!(!IdentityOf::<T>::contains_key(&target), "Identity not removed");
}
Expand Down
15 changes: 10 additions & 5 deletions frame/scheduler/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use frame_support::{
ensure,
traits::{OnInitialize, PreimageProvider, PreimageRecipient},
};
use frame_system::RawOrigin;
use sp_runtime::traits::Hash;
use sp_std::{prelude::*, vec};

Expand All @@ -32,6 +31,8 @@ use frame_system::Pallet as System;

const BLOCK_NUMBER: u32 = 2;

type SystemOrigin<T> = <T as frame_system::Config>::Origin;

/// Add `n` named items to the schedule.
///
/// For `resolved`:
Expand Down Expand Up @@ -210,7 +211,8 @@ benchmarks! {
let call = Box::new(CallOrHashOf::<T>::Value(inner_call));

fill_schedule::<T>(when, s, true, true, Some(false))?;
}: _(RawOrigin::Root, when, periodic, priority, call)
let schedule_origin = T::ScheduleOrigin::successful_origin();
}: _<SystemOrigin<T>>(schedule_origin, when, periodic, priority, call)
verify {
ensure!(
Agenda::<T>::get(when).len() == (s + 1) as usize,
Expand All @@ -224,7 +226,8 @@ benchmarks! {

fill_schedule::<T>(when, s, true, true, Some(false))?;
assert_eq!(Agenda::<T>::get(when).len(), s as usize);
}: _(RawOrigin::Root, when, 0)
let schedule_origin = T::ScheduleOrigin::successful_origin();
}: _<SystemOrigin<T>>(schedule_origin, when, 0)
verify {
ensure!(
Lookup::<T>::get(0.encode()).is_none(),
Expand All @@ -248,7 +251,8 @@ benchmarks! {
let call = Box::new(CallOrHashOf::<T>::Value(inner_call));

fill_schedule::<T>(when, s, true, true, Some(false))?;
}: _(RawOrigin::Root, id, when, periodic, priority, call)
let schedule_origin = T::ScheduleOrigin::successful_origin();
}: _<SystemOrigin<T>>(schedule_origin, id, when, periodic, priority, call)
verify {
ensure!(
Agenda::<T>::get(when).len() == (s + 1) as usize,
Expand All @@ -261,7 +265,8 @@ benchmarks! {
let when = BLOCK_NUMBER.into();

fill_schedule::<T>(when, s, true, true, Some(false))?;
}: _(RawOrigin::Root, 0.encode())
let schedule_origin = T::ScheduleOrigin::successful_origin();
}: _<SystemOrigin<T>>(schedule_origin, 0.encode())
verify {
ensure!(
Lookup::<T>::get(0.encode()).is_none(),
Expand Down
2 changes: 1 addition & 1 deletion frame/staking/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,6 @@ runtime-benchmarks = [
"frame-benchmarking/runtime-benchmarks",
"frame-election-provider-support/runtime-benchmarks",
"rand_chacha",
"sp-staking/runtime-benchmarks"
"sp-staking/runtime-benchmarks",
]
try-runtime = ["frame-support/try-runtime"]
3 changes: 2 additions & 1 deletion frame/tips/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ benchmarks_instance_pallet! {
let reason_hash = T::Hashing::hash(&reason[..]);
let hash = T::Hashing::hash_of(&(&reason_hash, &beneficiary));
ensure!(Tips::<T, I>::contains_key(hash), "tip does not exist");
}: _(RawOrigin::Root, hash)
let reject_origin = T::RejectOrigin::successful_origin();
}: _<T::Origin>(reject_origin, hash)

impl_benchmark_test_suite!(TipsMod, crate::tests::new_test_ext(), crate::tests::Test);
}
9 changes: 6 additions & 3 deletions frame/treasury/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ benchmarks_instance_pallet! {
beneficiary_lookup
)?;
let proposal_id = Treasury::<T, _>::proposal_count() - 1;
}: _(RawOrigin::Root, proposal_id)
let reject_origin = T::RejectOrigin::successful_origin();
}: _<T::Origin>(reject_origin, proposal_id)

approve_proposal {
let p in 0 .. T::MaxApprovals::get() - 1;
Expand All @@ -111,7 +112,8 @@ benchmarks_instance_pallet! {
beneficiary_lookup
)?;
let proposal_id = Treasury::<T, _>::proposal_count() - 1;
}: _(RawOrigin::Root, proposal_id)
let approve_origin = T::ApproveOrigin::successful_origin();
}: _<T::Origin>(approve_origin, proposal_id)

remove_approval {
let (caller, value, beneficiary_lookup) = setup_proposal::<T, _>(SEED);
Expand All @@ -122,7 +124,8 @@ benchmarks_instance_pallet! {
)?;
let proposal_id = Treasury::<T, _>::proposal_count() - 1;
Treasury::<T, I>::approve_proposal(RawOrigin::Root.into(), proposal_id)?;
}: _(RawOrigin::Root, proposal_id)
let reject_origin = T::RejectOrigin::successful_origin();
}: _<T::Origin>(reject_origin, proposal_id)

on_initialize_proposals {
let p in 0 .. T::MaxApprovals::get();
Expand Down