From 6ec82c7a83294e6027dc6e793510a787d9181a02 Mon Sep 17 00:00:00 2001
From: Falco Hirschenberger <falco.hirschenberger@gmail.com>
Date: Mon, 27 Sep 2021 17:17:38 +0200
Subject: [PATCH] Add weight for decoding the call to benchmarks (#9781)
* First two bechmarks converted
* Add decoding weight to benchmarks.
* Update frame/democracy/src/benchmarking.rs
* Adapt to new Call-ing convention
* Resolve conflicts and change more calls
* Remove error impl for codec and use plain `expect` for error handling instead
* Compile fix
* Spaces to tabs
* Update frame/democracy/src/benchmarking.rs
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
* Add origin-type specifier to benchmarks macro
* formatting
* Update frame/benchmarking/src/lib.rs
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
* Change manual to automatic benchmark syntax
* Formatting
* Revert "Change manual to automatic benchmark syntax"
This reverts commit ea5b5d906b318b6525c1e6d2bd05c5011595c21a.
Because tests are lost and cleanup code in the verify function is not run on failing calls.
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
---
substrate/frame/benchmarking/src/lib.rs | 33 ++++++++++++----
substrate/frame/benchmarking/src/utils.rs | 1 -
substrate/frame/democracy/src/benchmarking.rs | 38 +++++++------------
.../src/benchmarking.rs | 1 -
.../elections-phragmen/src/benchmarking.rs | 15 +++++++-
substrate/frame/gilt/src/benchmarking.rs | 15 +++-----
substrate/frame/im-online/src/benchmarking.rs | 8 ++--
substrate/frame/lottery/src/benchmarking.rs | 13 ++-----
substrate/frame/staking/src/benchmarking.rs | 11 +++++-
9 files changed, 75 insertions(+), 60 deletions(-)
diff --git a/substrate/frame/benchmarking/src/lib.rs b/substrate/frame/benchmarking/src/lib.rs
index 6c124a8a757..4a6c5e15ae2 100644
--- a/substrate/frame/benchmarking/src/lib.rs
+++ b/substrate/frame/benchmarking/src/lib.rs
@@ -131,6 +131,13 @@ macro_rules! whitelist {
/// let c = 0 .. 10 => setup_c_in_some_other_way(&caller, c);
/// }: baz(Origin::Signed(caller))
///
+/// // You may optionally specify the origin type if it can't be determined automatically like
+/// // this.
+/// baz3 {
+/// let caller = account::<T>(b"caller", 0, benchmarks_seed);
+/// let l in 1 .. MAX_LENGTH => initialize_l(l);
+/// }: baz<T::Origin>(Origin::Signed(caller), vec![0u8; l])
+///
/// // this is benchmarking some code that is not a dispatchable.
/// populate_a_set {
/// let x in 0 .. 10_000;
@@ -305,7 +312,7 @@ macro_rules! benchmarks_iter {
( $( $names:tt )* ) // This contains $( $( { $instance } )? $name:ident )*
( $( $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 )* )
verify $postcode:block
$( $rest:tt )*
) => {
@@ -315,7 +322,7 @@ macro_rules! benchmarks_iter {
( $( $names )* )
( $( $names_extra )* )
( $( $names_skip_meta )* )
- $name { $( $code )* }: $name ( $origin $( , $arg )* )
+ $name { $( $code )* }: $name $(< $origin_type >)? ( $origin $( , $arg )* )
verify $postcode
$( $rest )*
}
@@ -327,7 +334,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 )* )
verify $postcode:block
$( $rest:tt )*
) => {
@@ -350,15 +357,14 @@ macro_rules! benchmarks_iter {
&__call
);
}: {
- let call_decoded = <
+ let __call_decoded = <
Call<T $(, $instance )?>
as $crate::frame_support::codec::Decode
>::decode(&mut &__benchmarked_call_encoded[..])
.expect("call is encoded above, encoding must be correct");
-
- <
- Call<T $(, $instance)? > as $crate::frame_support::traits::UnfilteredDispatchable
- >::dispatch_bypass_filter(call_decoded, $origin.into())?;
+ let __origin = $crate::to_origin!($origin $(, $origin_type)?);
+ <Call<T $(, $instance)? > as $crate::frame_support::traits::UnfilteredDispatchable
+ >::dispatch_bypass_filter(__call_decoded, __origin)?;
}
verify $postcode
$( $rest )*
@@ -488,6 +494,17 @@ macro_rules! benchmarks_iter {
};
}
+#[macro_export]
+#[doc(hidden)]
+macro_rules! to_origin {
+ ($origin:expr) => {
+ $origin.into()
+ };
+ ($origin:expr, $origin_type:ty) => {
+ <T::Origin as From<$origin_type>>::from($origin)
+ };
+}
+
#[macro_export]
#[doc(hidden)]
macro_rules! benchmark_backend {
diff --git a/substrate/frame/benchmarking/src/utils.rs b/substrate/frame/benchmarking/src/utils.rs
index 158f5c5b575..c24ad2f64e1 100644
--- a/substrate/frame/benchmarking/src/utils.rs
+++ b/substrate/frame/benchmarking/src/utils.rs
@@ -16,7 +16,6 @@
// limitations under the License.
//! Interfaces, types and utils for benchmarking a FRAME runtime.
-
use codec::{Decode, Encode};
use frame_support::{
dispatch::{DispatchError, DispatchErrorWithPostInfo},
diff --git a/substrate/frame/democracy/src/benchmarking.rs b/substrate/frame/democracy/src/benchmarking.rs
index 7d4d7aee140..a00e6f4686f 100644
--- a/substrate/frame/democracy/src/benchmarking.rs
+++ b/substrate/frame/democracy/src/benchmarking.rs
@@ -22,6 +22,7 @@ use super::*;
use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelist_account};
use frame_support::{
assert_noop, assert_ok,
+ codec::Decode,
traits::{
schedule::DispatchTime, Currency, EnsureOrigin, Get, OnInitialize, UnfilteredDispatchable,
},
@@ -194,9 +195,8 @@ benchmarks! {
emergency_cancel {
let origin = T::CancellationOrigin::successful_origin();
let referendum_index = add_referendum::<T>(0)?;
- let call = Call::<T>::emergency_cancel { ref_index: referendum_index };
assert_ok!(Democracy::<T>::referendum_status(referendum_index));
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin, referendum_index)
verify {
// Referendum has been canceled
assert_noop!(
@@ -219,14 +219,11 @@ benchmarks! {
assert_ok!(
Democracy::<T>::external_propose(T::ExternalOrigin::successful_origin(), hash.clone())
);
-
+ let origin = T::BlacklistOrigin::successful_origin();
// Add a referendum of our proposal.
let referendum_index = add_referendum::<T>(0)?;
assert_ok!(Democracy::<T>::referendum_status(referendum_index));
-
- let call = Call::<T>::blacklist { proposal_hash: hash, maybe_ref_index: Some(referendum_index) };
- let origin = T::BlacklistOrigin::successful_origin();
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin, hash, Some(referendum_index))
verify {
// Referendum has been canceled
assert_noop!(
@@ -246,9 +243,7 @@ benchmarks! {
proposal_hash,
(T::BlockNumber::zero(), vec![T::AccountId::default(); v as usize])
);
-
- let call = Call::<T>::external_propose { proposal_hash };
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin, proposal_hash)
verify {
// External proposal created
ensure!(<NextExternal<T>>::exists(), "External proposal didn't work");
@@ -257,8 +252,7 @@ benchmarks! {
external_propose_majority {
let origin = T::ExternalMajorityOrigin::successful_origin();
let proposal_hash = T::Hashing::hash_of(&0);
- let call = Call::<T>::external_propose_majority { proposal_hash };
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin, proposal_hash)
verify {
// External proposal created
ensure!(<NextExternal<T>>::exists(), "External proposal didn't work");
@@ -267,8 +261,7 @@ benchmarks! {
external_propose_default {
let origin = T::ExternalDefaultOrigin::successful_origin();
let proposal_hash = T::Hashing::hash_of(&0);
- let call = Call::<T>::external_propose_default { proposal_hash };
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin, proposal_hash)
verify {
// External proposal created
ensure!(<NextExternal<T>>::exists(), "External proposal didn't work");
@@ -283,13 +276,7 @@ benchmarks! {
let origin_fast_track = T::FastTrackOrigin::successful_origin();
let voting_period = T::FastTrackVotingPeriod::get();
let delay = 0u32;
- let call = Call::<T>::fast_track {
- proposal_hash,
- voting_period: voting_period.into(),
- delay: delay.into()
- };
-
- }: { call.dispatch_bypass_filter(origin_fast_track)? }
+ }: _<T::Origin>(origin_fast_track, proposal_hash, voting_period.into(), delay.into())
verify {
assert_eq!(Democracy::<T>::referendum_count(), 1, "referendum not created")
}
@@ -310,10 +297,9 @@ benchmarks! {
vetoers.sort();
Blacklist::<T>::insert(proposal_hash, (T::BlockNumber::zero(), vetoers));
- let call = Call::<T>::veto_external { proposal_hash };
let origin = T::VetoOrigin::successful_origin();
ensure!(NextExternal::<T>::get().is_some(), "no external proposal");
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin, proposal_hash)
verify {
assert!(NextExternal::<T>::get().is_none());
let (_, new_vetoers) = <Blacklist<T>>::get(&proposal_hash).ok_or("no blacklist")?;
@@ -774,9 +760,13 @@ benchmarks! {
Some(PreimageStatus::Available { .. }) => (),
_ => return Err("preimage not available".into())
}
+ let origin = RawOrigin::Root.into();
+ let call = Call::<T>::enact_proposal { proposal_hash, index: 0 }.encode();
}: {
assert_eq!(
- Democracy::<T>::enact_proposal(RawOrigin::Root.into(), proposal_hash, 0),
+ <Call<T> as Decode>::decode(&mut &*call)
+ .expect("call is encoded above, encoding must be correct")
+ .dispatch_bypass_filter(origin),
Err(Error::<T>::PreimageInvalid.into())
);
}
diff --git a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs
index a5bb0d6351c..b8d7bc45c44 100644
--- a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs
+++ b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs
@@ -350,7 +350,6 @@ frame_benchmarking::benchmarks! {
assert!(<MultiPhase<T>>::queued_solution().is_none());
<CurrentPhase<T>>::put(Phase::Unsigned((true, 1u32.into())));
-
}: _(RawOrigin::None, Box::new(raw_solution), witness)
verify {
assert!(<MultiPhase<T>>::queued_solution().is_some());
diff --git a/substrate/frame/elections-phragmen/src/benchmarking.rs b/substrate/frame/elections-phragmen/src/benchmarking.rs
index 7cb83b3dd77..6e3ce0234c4 100644
--- a/substrate/frame/elections-phragmen/src/benchmarking.rs
+++ b/substrate/frame/elections-phragmen/src/benchmarking.rs
@@ -24,7 +24,10 @@ use super::*;
use frame_benchmarking::{
account, benchmarks, impl_benchmark_test_suite, whitelist, BenchmarkError, BenchmarkResult,
};
-use frame_support::{dispatch::DispatchResultWithPostInfo, traits::OnInitialize};
+use frame_support::{
+ dispatch::{DispatchResultWithPostInfo, UnfilteredDispatchable},
+ traits::OnInitialize,
+};
use frame_system::RawOrigin;
use crate::Pallet as Elections;
@@ -401,15 +404,23 @@ benchmarks! {
let _ = fill_seats_up_to::<T>(m)?;
let removing = as_lookup::<T>(<Elections<T>>::members_ids()[0].clone());
+ let who = T::Lookup::lookup(removing.clone()).expect("member was added above");
+ let call = Call::<T>::remove_member { who: removing, has_replacement: false }.encode();
}: {
assert_eq!(
- <Elections<T>>::remove_member(RawOrigin::Root.into(), removing, false).unwrap_err().error,
+ <Call<T> as Decode>::decode(&mut &*call)
+ .expect("call is encoded above, encoding must be correct")
+ .dispatch_bypass_filter(RawOrigin::Root.into())
+ .unwrap_err()
+ .error,
Error::<T>::InvalidReplacement.into(),
);
}
verify {
// must still have enough members.
assert_eq!(<Elections<T>>::members().len() as u32, T::DesiredMembers::get());
+ // on fail, `who` must still be a member
+ assert!(<Elections<T>>::members_ids().contains(&who));
#[cfg(test)]
{
// reset members in between benchmark tests.
diff --git a/substrate/frame/gilt/src/benchmarking.rs b/substrate/frame/gilt/src/benchmarking.rs
index 55d34a35a7c..cfc503cf897 100644
--- a/substrate/frame/gilt/src/benchmarking.rs
+++ b/substrate/frame/gilt/src/benchmarking.rs
@@ -50,17 +50,12 @@ benchmarks! {
place_bid_max {
let caller: T::AccountId = whitelisted_caller();
+ let origin = RawOrigin::Signed(caller.clone());
T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
for i in 0..T::MaxQueueLen::get() {
- Gilt::<T>::place_bid(RawOrigin::Signed(caller.clone()).into(), T::MinFreeze::get(), 1)?;
+ Gilt::<T>::place_bid(origin.clone().into(), T::MinFreeze::get(), 1)?;
}
- }: {
- Gilt::<T>::place_bid(
- RawOrigin::Signed(caller.clone()).into(),
- T::MinFreeze::get() * BalanceOf::<T>::from(2u32),
- 1,
- )?
- }
+ }: place_bid(origin, T::MinFreeze::get() * BalanceOf::<T>::from(2u32), 1)
verify {
assert_eq!(QueueTotals::<T>::get()[0], (
T::MaxQueueLen::get(),
@@ -81,9 +76,9 @@ benchmarks! {
}
set_target {
- let call = Call::<T>::set_target { target: Default::default() };
let origin = T::AdminOrigin::successful_origin();
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin, Default::default())
+ verify {}
thaw {
let caller: T::AccountId = whitelisted_caller();
diff --git a/substrate/frame/im-online/src/benchmarking.rs b/substrate/frame/im-online/src/benchmarking.rs
index 20812f03d28..b39b0057c48 100644
--- a/substrate/frame/im-online/src/benchmarking.rs
+++ b/substrate/frame/im-online/src/benchmarking.rs
@@ -93,10 +93,12 @@ benchmarks! {
let e in 1 .. MAX_EXTERNAL_ADDRESSES;
let (input_heartbeat, signature) = create_heartbeat::<T>(k, e)?;
let call = Call::heartbeat { heartbeat: input_heartbeat, signature };
+ let call_enc = call.encode();
}: {
- ImOnline::<T>::validate_unsigned(TransactionSource::InBlock, &call)
- .map_err(<&str>::from)?;
- call.dispatch_bypass_filter(RawOrigin::None.into())?;
+ ImOnline::<T>::validate_unsigned(TransactionSource::InBlock, &call).map_err(<&str>::from)?;
+ <Call<T> as Decode>::decode(&mut &*call_enc)
+ .expect("call is encoded above, encoding must be correct")
+ .dispatch_bypass_filter(RawOrigin::None.into())?;
}
}
diff --git a/substrate/frame/lottery/src/benchmarking.rs b/substrate/frame/lottery/src/benchmarking.rs
index 3b7035c72de..7af20bbb0e1 100644
--- a/substrate/frame/lottery/src/benchmarking.rs
+++ b/substrate/frame/lottery/src/benchmarking.rs
@@ -22,7 +22,7 @@
use super::*;
use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};
-use frame_support::traits::{EnsureOrigin, OnInitialize, UnfilteredDispatchable};
+use frame_support::traits::{EnsureOrigin, OnInitialize};
use frame_system::RawOrigin;
use sp_runtime::traits::{Bounded, Zero};
@@ -73,11 +73,9 @@ benchmarks! {
set_calls {
let n in 0 .. T::MaxCalls::get() as u32;
let calls = vec![frame_system::Call::<T>::remark { remark: vec![] }.into(); n as usize];
-
- let call = Call::<T>::set_calls { calls };
let origin = T::ManagerOrigin::successful_origin();
assert!(CallIndices::<T>::get().is_empty());
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin, calls)
verify {
if !n.is_zero() {
assert!(!CallIndices::<T>::get().is_empty());
@@ -88,10 +86,8 @@ benchmarks! {
let price = BalanceOf::<T>::max_value();
let end = 10u32.into();
let payout = 5u32.into();
-
- let call = Call::<T>::start_lottery { price, length: end, delay: payout, repeat: true };
let origin = T::ManagerOrigin::successful_origin();
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin, price, end, payout, true)
verify {
assert!(crate::Lottery::<T>::get().is_some());
}
@@ -99,9 +95,8 @@ benchmarks! {
stop_repeat {
setup_lottery::<T>(true)?;
assert_eq!(crate::Lottery::<T>::get().unwrap().repeat, true);
- let call = Call::<T>::stop_repeat {};
let origin = T::ManagerOrigin::successful_origin();
- }: { call.dispatch_bypass_filter(origin)? }
+ }: _<T::Origin>(origin)
verify {
assert_eq!(crate::Lottery::<T>::get().unwrap().repeat, false);
}
diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs
index f3def720632..fe60d516e14 100644
--- a/substrate/frame/staking/src/benchmarking.rs
+++ b/substrate/frame/staking/src/benchmarking.rs
@@ -23,6 +23,7 @@ use testing_utils::*;
use frame_election_provider_support::SortedListProvider;
use frame_support::{
+ dispatch::UnfilteredDispatchable,
pallet_prelude::*,
traits::{Currency, CurrencyToVote, Get, Imbalance},
};
@@ -764,9 +765,15 @@ benchmarks! {
<ErasValidatorReward<T>>::insert(current_era, total_payout);
let caller: T::AccountId = whitelisted_caller();
+ let origin = RawOrigin::Signed(caller);
+ let calls: Vec<_> = payout_calls_arg.iter().map(|arg|
+ Call::<T>::payout_stakers { validator_stash: arg.0.clone(), era: arg.1 }.encode()
+ ).collect();
}: {
- for arg in payout_calls_arg {
- <Staking<T>>::payout_stakers(RawOrigin::Signed(caller.clone()).into(), arg.0, arg.1)?;
+ for call in calls {
+ <Call<T> as Decode>::decode(&mut &*call)
+ .expect("call is encoded above, encoding must be correct")
+ .dispatch_bypass_filter(origin.clone().into())?;
}
}
--
GitLab