diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index a221eccb82c85..29fa0b6a6af70 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -881,6 +881,13 @@ macro_rules! impl_bench_name_tests { "WARNING: benchmark error skipped - {}", stringify!($name), ); + }, + $crate::BenchmarkError::Weightless => { + // This is considered a success condition. + $crate::log::error!( + "WARNING: benchmark weightless skipped - {}", + stringify!($name), + ); } } }, @@ -1640,6 +1647,14 @@ macro_rules! impl_test_function { .expect("benchmark name is always a valid string!"), ); } + $crate::BenchmarkError::Weightless => { + // This is considered a success condition. + $crate::log::error!( + "WARNING: benchmark weightless skipped - {}", + $crate::str::from_utf8(benchmark_name) + .expect("benchmark name is always a valid string!"), + ); + } } }, Ok(Ok(())) => (), @@ -1792,6 +1807,17 @@ macro_rules! add_benchmark { .expect("benchmark name is always a valid string!") ); None + }, + Err($crate::BenchmarkError::Weightless) => { + $crate::log::error!( + "WARNING: benchmark weightless skipped - {}", + $crate::str::from_utf8(benchmark) + .expect("benchmark name is always a valid string!") + ); + Some(vec![$crate::BenchmarkResult { + components: selected_components.clone(), + .. Default::default() + }]) } }; diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index 753e8c1c684ee..654b1c34c0658 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -162,6 +162,11 @@ pub enum BenchmarkError { /// The benchmarking pipeline is allowed to fail here, and we should simply /// skip processing these results. Skip, + /// No weight can be determined; set the weight of this call to zero. + /// + /// You can also use `Override` instead, but this is easier to use since `Override` expects the + /// correct components to be present. + Weightless, } impl From for &'static str { @@ -170,6 +175,7 @@ impl From for &'static str { BenchmarkError::Stop(s) => s, BenchmarkError::Override(_) => "benchmark override", BenchmarkError::Skip => "benchmark skip", + BenchmarkError::Weightless => "benchmark weightless", } } } diff --git a/frame/bounties/src/benchmarking.rs b/frame/bounties/src/benchmarking.rs index 84f5665d402a1..adadb3411ac65 100644 --- a/frame/bounties/src/benchmarking.rs +++ b/frame/bounties/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; -use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller}; +use frame_benchmarking::{account, benchmarks_instance_pallet, whitelisted_caller, BenchmarkError}; use frame_system::RawOrigin; use sp_runtime::traits::Bounded; @@ -31,13 +31,14 @@ use pallet_treasury::Pallet as Treasury; const SEED: u32 = 0; // Create bounties that are approved for use in `on_initialize`. -fn create_approved_bounties, I: 'static>(n: u32) -> Result<(), &'static str> { +fn create_approved_bounties, I: 'static>(n: u32) -> Result<(), BenchmarkError> { for i in 0..n { let (caller, _curator, _fee, value, reason) = setup_bounty::(i, T::MaximumReasonLength::get()); 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::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Bounties::::approve_bounty(approve_origin, bounty_id)?; } ensure!(BountyApprovals::::get().len() == n as usize, "Not all bounty approved"); @@ -62,13 +63,14 @@ fn setup_bounty, I: 'static>( } fn create_bounty, I: 'static>( -) -> Result<(AccountIdLookupOf, BountyIndex), &'static str> { +) -> Result<(AccountIdLookupOf, BountyIndex), BenchmarkError> { let (caller, curator, fee, value, reason) = setup_bounty::(0, T::MaximumReasonLength::get()); let curator_lookup = T::Lookup::unlookup(curator.clone()); 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::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Bounties::::approve_bounty(approve_origin.clone(), bounty_id)?; Treasury::::on_initialize(T::BlockNumber::zero()); Bounties::::propose_curator(approve_origin, bounty_id, curator_lookup.clone(), fee)?; @@ -97,7 +99,7 @@ benchmarks_instance_pallet! { let (caller, curator, fee, value, reason) = setup_bounty::(0, T::MaximumReasonLength::get()); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; let bounty_id = BountyCount::::get() - 1; - let approve_origin = T::SpendOrigin::successful_origin(); + let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(approve_origin, bounty_id) propose_curator { @@ -106,10 +108,9 @@ benchmarks_instance_pallet! { let curator_lookup = T::Lookup::unlookup(curator); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; let bounty_id = BountyCount::::get() - 1; - let approve_origin = T::SpendOrigin::successful_origin(); - Bounties::::approve_bounty(approve_origin, bounty_id)?; + let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + Bounties::::approve_bounty(approve_origin.clone(), bounty_id)?; Treasury::::on_initialize(T::BlockNumber::zero()); - let approve_origin = T::SpendOrigin::successful_origin(); }: _(approve_origin, bounty_id, curator_lookup, fee) // Worst case when curator is inactive and any sender unassigns the curator. @@ -128,7 +129,7 @@ benchmarks_instance_pallet! { let curator_lookup = T::Lookup::unlookup(curator.clone()); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; let bounty_id = BountyCount::::get() - 1; - let approve_origin = T::SpendOrigin::successful_origin(); + let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Bounties::::approve_bounty(approve_origin.clone(), bounty_id)?; Treasury::::on_initialize(T::BlockNumber::zero()); Bounties::::propose_curator(approve_origin, bounty_id, curator_lookup, fee)?; diff --git a/frame/child-bounties/src/benchmarking.rs b/frame/child-bounties/src/benchmarking.rs index 697ed40e0071f..04a8583f286f1 100644 --- a/frame/child-bounties/src/benchmarking.rs +++ b/frame/child-bounties/src/benchmarking.rs @@ -21,7 +21,7 @@ use super::*; -use frame_benchmarking::{account, benchmarks, whitelisted_caller}; +use frame_benchmarking::{account, benchmarks, whitelisted_caller, BenchmarkError}; use frame_system::RawOrigin; use crate::Pallet as ChildBounties; @@ -94,7 +94,7 @@ fn setup_child_bounty(user: u32, description: u32) -> BenchmarkChildB fn activate_bounty( user: u32, description: u32, -) -> Result, &'static str> { +) -> Result, BenchmarkError> { let mut child_bounty_setup = setup_child_bounty::(user, description); let curator_lookup = T::Lookup::unlookup(child_bounty_setup.curator.clone()); Bounties::::propose_bounty( @@ -105,7 +105,9 @@ fn activate_bounty( child_bounty_setup.bounty_id = Bounties::::bounty_count() - 1; - Bounties::::approve_bounty(RawOrigin::Root.into(), child_bounty_setup.bounty_id)?; + let approve_origin = + T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + Bounties::::approve_bounty(approve_origin, child_bounty_setup.bounty_id)?; Treasury::::on_initialize(T::BlockNumber::zero()); Bounties::::propose_curator( RawOrigin::Root.into(), @@ -124,7 +126,7 @@ fn activate_bounty( fn activate_child_bounty( user: u32, description: u32, -) -> Result, &'static str> { +) -> Result, BenchmarkError> { let mut bounty_setup = activate_bounty::(user, description)?; let child_curator_lookup = T::Lookup::unlookup(bounty_setup.child_curator.clone());