-
Notifications
You must be signed in to change notification settings - Fork 256
Benchmarks for balance transfer check extension #3557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
9238983
move domains enabled check to domains extension
vedhavyas 3b0ea9d
move balance transfer check to primitives crate for reusability
vedhavyas a56a06e
define benchmarks for balance transfer extension
vedhavyas 4ae0708
add benchmarks and generate weights
vedhavyas 734a273
integrate weights to extension
vedhavyas 7a789ef
enable domins for test consensus runtime
vedhavyas fe4075a
Merge branch 'main' into benchmarks_disable_extension
vedhavyas 4f4900a
clarify and update benchmark weight usage
vedhavyas 0d37ad2
Benchmark a list of non-transfer calls instead of mixed nested calls
teor2345 7c80471
Try 5,000 benchmark calls
teor2345 2c43c56
Merge branch 'main' into benchmarks_disable_extension
vedhavyas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,200 @@ | ||
| #[cfg(feature = "runtime-benchmarks")] | ||
| pub mod benchmarking; | ||
| pub mod weights; | ||
|
|
||
| use crate::extension::weights::WeightInfo as SubstrateWeightInfo; | ||
| use crate::utility::{MaybeNestedCall, nested_call_iter}; | ||
| use core::marker::PhantomData; | ||
| use frame_support::RuntimeDebugNoBound; | ||
| use frame_support::pallet_prelude::Weight; | ||
| use frame_system::Config; | ||
| use frame_system::pallet_prelude::{OriginFor, RuntimeCallFor}; | ||
| use pallet_balances::Call as BalancesCall; | ||
| use parity_scale_codec::{Decode, Encode}; | ||
| use scale_info::TypeInfo; | ||
| use scale_info::prelude::fmt; | ||
| use sp_runtime::DispatchResult; | ||
| use sp_runtime::traits::{ | ||
| AsSystemOriginSigner, DispatchInfoOf, DispatchOriginOf, Dispatchable, PostDispatchInfoOf, | ||
| TransactionExtension, ValidateResult, | ||
| }; | ||
| use sp_runtime::transaction_validity::{ | ||
| InvalidTransaction, TransactionSource, TransactionValidityError, ValidTransaction, | ||
| }; | ||
|
|
||
| /// Maximum number of calls we benchmarked for. | ||
| const MAXIMUM_NUMBER_OF_CALLS: u32 = 1000; | ||
|
|
||
| /// Weights for the balance transfer check extension. | ||
| pub trait WeightInfo { | ||
| fn balance_transfer_check_mixed(c: u32) -> Weight; | ||
| fn balance_transfer_check_utility(c: u32) -> Weight; | ||
| fn balance_transfer_check_multisig(c: u32) -> Weight; | ||
| } | ||
|
|
||
| /// Trait to convert Runtime call to possible Balance call. | ||
| pub trait MaybeBalancesCall<Runtime> | ||
| where | ||
| Runtime: pallet_balances::Config, | ||
| { | ||
| fn maybe_balance_call(&self) -> Option<&BalancesCall<Runtime>>; | ||
| } | ||
|
|
||
| /// Trait to check if the Balance transfers are enabled. | ||
| pub trait BalanceTransferChecks { | ||
| fn is_balance_transferable() -> bool; | ||
| } | ||
|
|
||
| /// Disable balance transfers, if configured in the runtime. | ||
| #[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] | ||
| pub struct BalanceTransferCheckExtension<Runtime>(PhantomData<Runtime>); | ||
|
|
||
| impl<Runtime> Default for BalanceTransferCheckExtension<Runtime> | ||
| where | ||
| Runtime: BalanceTransferChecks + pallet_balances::Config, | ||
| RuntimeCallFor<Runtime>: MaybeBalancesCall<Runtime> + MaybeNestedCall<Runtime>, | ||
| { | ||
| fn default() -> Self { | ||
| Self(PhantomData) | ||
| } | ||
| } | ||
|
|
||
| impl<Runtime> BalanceTransferCheckExtension<Runtime> | ||
| where | ||
| Runtime: BalanceTransferChecks + pallet_balances::Config, | ||
| RuntimeCallFor<Runtime>: MaybeBalancesCall<Runtime> + MaybeNestedCall<Runtime>, | ||
| { | ||
| fn do_validate_signed( | ||
| call: &RuntimeCallFor<Runtime>, | ||
| ) -> Result<(ValidTransaction, u32), TransactionValidityError> { | ||
| if Runtime::is_balance_transferable() { | ||
| return Ok((ValidTransaction::default(), 0)); | ||
| } | ||
|
|
||
| // Disable normal balance transfers. | ||
| let (contains_balance_call, calls) = Self::contains_balance_transfer(call); | ||
| if contains_balance_call { | ||
| Err(InvalidTransaction::Call.into()) | ||
| } else { | ||
| Ok((ValidTransaction::default(), calls)) | ||
| } | ||
| } | ||
|
|
||
| fn contains_balance_transfer(call: &RuntimeCallFor<Runtime>) -> (bool, u32) { | ||
| let mut calls = 0; | ||
| for call in nested_call_iter::<Runtime>(call) { | ||
| calls += 1; | ||
| // Any other calls might contain nested calls, so we can only return early if we find a | ||
| // balance transfer call. | ||
| if let Some(balance_call) = call.maybe_balance_call() | ||
| && matches!( | ||
| balance_call, | ||
| BalancesCall::transfer_allow_death { .. } | ||
| | BalancesCall::transfer_keep_alive { .. } | ||
| | BalancesCall::transfer_all { .. } | ||
| ) | ||
| { | ||
| return (true, calls); | ||
| } | ||
| } | ||
|
|
||
| (false, calls) | ||
| } | ||
|
|
||
| fn get_weights(n: u32) -> Weight { | ||
| SubstrateWeightInfo::<Runtime>::balance_transfer_check_multisig(n) | ||
| .max(SubstrateWeightInfo::<Runtime>::balance_transfer_check_mixed(n)) | ||
| .max(SubstrateWeightInfo::<Runtime>::balance_transfer_check_utility(n)) | ||
| } | ||
| } | ||
|
|
||
| /// Data passed from prepare to post_dispatch. | ||
| #[derive(RuntimeDebugNoBound)] | ||
| pub enum Pre { | ||
| Refund(Weight), | ||
| } | ||
|
|
||
| /// Data passed from validate to prepare. | ||
| #[derive(RuntimeDebugNoBound)] | ||
| pub enum Val { | ||
| FullRefund, | ||
| PartialRefund(Option<u32>), | ||
| } | ||
|
|
||
| impl<Runtime> TransactionExtension<RuntimeCallFor<Runtime>> | ||
| for BalanceTransferCheckExtension<Runtime> | ||
| where | ||
| Runtime: Config | ||
| + pallet_balances::Config | ||
| + scale_info::TypeInfo | ||
| + fmt::Debug | ||
| + Send | ||
| + Sync | ||
| + BalanceTransferChecks, | ||
| <RuntimeCallFor<Runtime> as Dispatchable>::RuntimeOrigin: | ||
| AsSystemOriginSigner<<Runtime as Config>::AccountId> + Clone, | ||
| RuntimeCallFor<Runtime>: MaybeBalancesCall<Runtime> + MaybeNestedCall<Runtime>, | ||
| { | ||
| const IDENTIFIER: &'static str = "BalanceTransferCheckExtension"; | ||
| type Implicit = (); | ||
| type Val = Val; | ||
| type Pre = Pre; | ||
|
|
||
| fn weight(&self, _call: &RuntimeCallFor<Runtime>) -> Weight { | ||
| Self::get_weights(MAXIMUM_NUMBER_OF_CALLS) | ||
| } | ||
|
|
||
| fn validate( | ||
| &self, | ||
| origin: OriginFor<Runtime>, | ||
| call: &RuntimeCallFor<Runtime>, | ||
| _info: &DispatchInfoOf<RuntimeCallFor<Runtime>>, | ||
| _len: usize, | ||
| _self_implicit: Self::Implicit, | ||
| _inherited_implication: &impl Encode, | ||
| _source: TransactionSource, | ||
| ) -> ValidateResult<Self::Val, RuntimeCallFor<Runtime>> { | ||
| let (validity, val) = if origin.as_system_origin_signer().is_some() { | ||
| let (valid, maybe_calls) = | ||
| Self::do_validate_signed(call).map(|(valid, calls)| (valid, Some(calls)))?; | ||
| (valid, Val::PartialRefund(maybe_calls)) | ||
| } else { | ||
| (ValidTransaction::default(), Val::FullRefund) | ||
| }; | ||
|
|
||
| Ok((validity, val, origin)) | ||
| } | ||
|
|
||
| fn prepare( | ||
| self, | ||
| val: Self::Val, | ||
| _origin: &DispatchOriginOf<RuntimeCallFor<Runtime>>, | ||
| _call: &RuntimeCallFor<Runtime>, | ||
| _info: &DispatchInfoOf<RuntimeCallFor<Runtime>>, | ||
| _len: usize, | ||
| ) -> Result<Self::Pre, TransactionValidityError> { | ||
| let total_weight = Self::get_weights(MAXIMUM_NUMBER_OF_CALLS); | ||
| match val { | ||
| // not a signed transaction, so return full refund. | ||
| Val::FullRefund => Ok(Pre::Refund(total_weight)), | ||
|
|
||
| // signed transaction with a minimum of one read weight, | ||
| // so refund any extra call weight | ||
| Val::PartialRefund(maybe_calls) => { | ||
| let actual_weights = Self::get_weights(maybe_calls.unwrap_or(0)); | ||
| Ok(Pre::Refund(total_weight.saturating_sub(actual_weights))) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn post_dispatch_details( | ||
| pre: Self::Pre, | ||
| _info: &DispatchInfoOf<RuntimeCallFor<Runtime>>, | ||
| _post_info: &PostDispatchInfoOf<RuntimeCallFor<Runtime>>, | ||
| _len: usize, | ||
| _result: &DispatchResult, | ||
| ) -> Result<Weight, TransactionValidityError> { | ||
| let Pre::Refund(weight) = pre; | ||
| Ok(weight) | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the actual number of nested calls is greater than this? Does the submitter get the extra calls for free?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes if it exceeds 1000, the remaining are free but max decoding depth for extrinsics is 256. So we will never reach that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are at least 3 ways to get more than 1000 loop iterations without exceeding the decoding depth:
utility([non_transfer_call; 2000]): 2000 calls in one utility callutility([utility([non_transfer_call; 100]); 100]): 100 utility calls each containing 100 callsIt is likely that decoding will have different performance for case 3, due to branch misprediction (there are no long runs of the same item).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
MAXIMUM_NUMBER_OF_CALLSis inevitable because the weight system works as:So we have to first define the worst situation weight with
MAXIMUM_NUMBER_OF_CALLS, the value ofMAXIMUM_NUMBER_OF_CALLSis hard to determine since we don't really have an explicit limit on the maximum nested calls, options I can think of:MAXIMUM_NUMBER_OF_CALLSbyblock_size / min_extrinsic_sizeMAXIMUM_NUMBER_OF_CALLSas is or pick another value, since the check is lightweight, the resulting weight won't be much different I guess.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tried again with max of 1000 nested utility calls with 500 system.remark calls until the last nested call. Last one included on balance transfer.
Allocator failed to allocate the memory. If such a case arise in practical, that extrinsic will never be inluded
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if that's the data structure I was talking about above.
Either way, there's going to be some number of calls that fits within the allocation limits. I'll try a few things and add a benchmark if it's heavier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A list of 1000 non-transfer calls fits in memory and is heavier in some cases:
https://github.com/autonomys/subspace/tree/benchmarks_disable_extension_exploration
I haven't had time to explore the other data structures like squares and trees. Since both "long" and "tall" calls have similar results, I'm not sure it's going to be much different. Trees might be different if there's a lot of pointer dereferences, but I don't think it's a blocker for this PR.