Benchmark's successful origin api update (#13146)

* 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 <>

Author: muharem

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::<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()));
@@ -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));
@@ -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());
@@ -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));
@@ -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));
@@ -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());
@@ -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());

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<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());
@@ -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(),
@@ -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 {
@@ -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(),

frame/bounties/src/benchmarking.rs

@@ -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())

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::<T>(0).0;
 		assert_ok!(Democracy::<T>::referendum_status(ref_index));
 	}: _<T::RuntimeOrigin>(origin, ref_index)
@@ -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
@@ -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
 
@@ -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 {
@@ -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 {
@@ -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())
@@ -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();
@@ -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 {
@@ -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 {
@@ -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)?;

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 = <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 {}

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<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 =
@@ -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 {
@@ -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.");
@@ -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.");
@@ -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
@@ -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)
@@ -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");