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

Add Weightless benchmark bailing #12829

Merged
ggwpez merged 4 commits into from
Dec 2, 2022
Merged

Add Weightless benchmark bailing #12829

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a78c6c9
Calls can be 'Weightless'
ggwpez Dec 2, 2022
3bfe859
Fix (child)-bounties benches
ggwpez Dec 2, 2022
166de20
Just use one dummy value
ggwpez Dec 2, 2022
a7ecf5b
:facepalm:
ggwpez Dec 2, 2022
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
Prev Previous commit
Next Next commit
Fix (child)-bounties benches 
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
  • Loading branch information
@ggwpez
ggwpez committed Dec 2, 2022
commit 3bfe859e7a0b113649ba99f2ed704efdbca68681
21 changes: 11 additions & 10 deletions frame/bounties/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<T: Config<I>, I: 'static>(n: u32) -> Result<(), &'static str> {
fn create_approved_bounties<T: Config<I>, I: 'static>(n: u32) -> Result<(), BenchmarkError> {
for i in 0..n {
let (caller, _curator, _fee, value, reason) =
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;
let approve_origin = T::ApproveOrigin::successful_origin();
let approve_origin =
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Copy link
Member

@bkchr bkchr Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably change all benchmarking code to use try_successful_origin and remove successful_origin?

Copy link
Member Author

@ggwpez ggwpez Dec 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes eventually… this was an oversight when originally writing them.

Bounties::<T, I>::approve_bounty(approve_origin, bounty_id)?;
}
ensure!(BountyApprovals::<T, I>::get().len() == n as usize, "Not all bounty approved");
Expand All @@ -62,13 +63,14 @@ fn setup_bounty<T: Config<I>, I: 'static>(
}

fn create_bounty<T: Config<I>, I: 'static>(
) -> Result<(AccountIdLookupOf<T>, BountyIndex), &'static str> {
) -> Result<(AccountIdLookupOf<T>, BountyIndex), BenchmarkError> {
let (caller, curator, fee, value, reason) =
setup_bounty::<T, I>(0, T::MaximumReasonLength::get());
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;
let approve_origin = T::ApproveOrigin::successful_origin();
let approve_origin =
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup.clone(), fee)?;
Expand Down Expand Up @@ -97,7 +99,7 @@ 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;
let approve_origin = T::SpendOrigin::successful_origin();
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
}: _<T::RuntimeOrigin>(approve_origin, bounty_id)

propose_curator {
Expand All @@ -106,10 +108,9 @@ benchmarks_instance_pallet! {
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;
let approve_origin = T::SpendOrigin::successful_origin();
Bounties::<T, I>::approve_bounty(approve_origin, bounty_id)?;
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
let approve_origin = T::SpendOrigin::successful_origin();
}: _<T::RuntimeOrigin>(approve_origin, bounty_id, curator_lookup, fee)

// Worst case when curator is inactive and any sender unassigns the curator.
Expand All @@ -128,7 +129,7 @@ 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;
let approve_origin = T::SpendOrigin::successful_origin();
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup, fee)?;
Expand Down
10 changes: 6 additions & 4 deletions frame/child-bounties/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -94,7 +94,7 @@ fn setup_child_bounty<T: Config>(user: u32, description: u32) -> BenchmarkChildB
fn activate_bounty<T: Config>(
user: u32,
description: u32,
) -> Result<BenchmarkChildBounty<T>, &'static str> {
) -> Result<BenchmarkChildBounty<T>, BenchmarkError> {
let mut child_bounty_setup = setup_child_bounty::<T>(user, description);
let curator_lookup = T::Lookup::unlookup(child_bounty_setup.curator.clone());
Bounties::<T>::propose_bounty(
Expand All @@ -105,7 +105,9 @@ fn activate_bounty<T: Config>(

child_bounty_setup.bounty_id = Bounties::<T>::bounty_count() - 1;

Bounties::<T>::approve_bounty(RawOrigin::Root.into(), child_bounty_setup.bounty_id)?;
let approve_origin =
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T>::approve_bounty(approve_origin, child_bounty_setup.bounty_id)?;
Treasury::<T>::on_initialize(T::BlockNumber::zero());
Bounties::<T>::propose_curator(
RawOrigin::Root.into(),
Expand All @@ -124,7 +126,7 @@ fn activate_bounty<T: Config>(
fn activate_child_bounty<T: Config>(
user: u32,
description: u32,
) -> Result<BenchmarkChildBounty<T>, &'static str> {
) -> Result<BenchmarkChildBounty<T>, BenchmarkError> {
let mut bounty_setup = activate_bounty::<T>(user, description)?;
let child_curator_lookup = T::Lookup::unlookup(bounty_setup.child_curator.clone());

Expand Down