Skip to content

Commit

Permalink
Fix benchmark failures when using insecure_zero_ed flag (#5354)
Browse files Browse the repository at this point in the history
Currently, when the pallet is compiled with the `insecure_zero_ed flag`,
benchmarks fail because the minimum balance is set to zero.

The PR aims to resolve this issue by implementing a placeholder value
for the minimum balance when the `insecure_zero_ed` flag is active. it
ensures that benchmarks run successfully regardless of whether this flag
is used or not
  • Loading branch information
TarekkMA authored Aug 28, 2024
1 parent 8e0cefc commit 1c4141a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 11 deletions.
15 changes: 15 additions & 0 deletions prdoc/pr_5354.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Fix benchmark failures when using insecure_zero_ed flag

doc:
- audience: Runtime Dev
description: |
Currently, when the pallet is compiled with the insecure_zero_ed flag, benchmarks fail because the minimum balance is set to zero.

The PR aims to resolve this issue by implementing a placeholder value for the minimum balance when the insecure_zero_ed flag is active. it ensures that benchmarks run successfully regardless of whether this flag is used or not

crates:
- name: pallet-balances
bump: minor
30 changes: 19 additions & 11 deletions substrate/frame/balances/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ const SEED: u32 = 0;
// existential deposit multiplier
const ED_MULTIPLIER: u32 = 10;

fn minimum_balance<T: Config<I>, I: 'static>() -> T::Balance {
if cfg!(feature = "insecure_zero_ed") {
100u32.into()
} else {
T::ExistentialDeposit::get()
}
}

#[instance_benchmarks]
mod benchmarks {
use super::*;
Expand All @@ -40,7 +48,7 @@ mod benchmarks {
// * Transfer will create the recipient account.
#[benchmark]
fn transfer_allow_death() {
let existential_deposit = T::ExistentialDeposit::get();
let existential_deposit: T::Balance = minimum_balance::<T, I>();
let caller = whitelisted_caller();

// Give some multiple of the existential deposit
Expand Down Expand Up @@ -75,7 +83,7 @@ mod benchmarks {
<Balances<T, I> as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value());

// Give the recipient account existential deposit (thus their account already exists).
let existential_deposit = T::ExistentialDeposit::get();
let existential_deposit: T::Balance = minimum_balance::<T, I>();
let _ =
<Balances<T, I> as Currency<_>>::make_free_balance_be(&recipient, existential_deposit);
let transfer_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
Expand All @@ -98,7 +106,7 @@ mod benchmarks {
// Give the sender account max funds, thus a transfer will not kill account.
let _ =
<Balances<T, I> as Currency<_>>::make_free_balance_be(&caller, T::Balance::max_value());
let existential_deposit = T::ExistentialDeposit::get();
let existential_deposit: T::Balance = minimum_balance::<T, I>();
let transfer_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into());

#[extrinsic_call]
Expand All @@ -115,7 +123,7 @@ mod benchmarks {
let user_lookup = T::Lookup::unlookup(user.clone());

// Give the user some initial balance.
let existential_deposit = T::ExistentialDeposit::get();
let existential_deposit: T::Balance = minimum_balance::<T, I>();
let balance_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&user, balance_amount);

Expand All @@ -132,7 +140,7 @@ mod benchmarks {
let user_lookup = T::Lookup::unlookup(user.clone());

// Give the user some initial balance.
let existential_deposit = T::ExistentialDeposit::get();
let existential_deposit: T::Balance = minimum_balance::<T, I>();
let balance_amount = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&user, balance_amount);

Expand All @@ -147,7 +155,7 @@ mod benchmarks {
// * Transfer will create the recipient account.
#[benchmark]
fn force_transfer() {
let existential_deposit = T::ExistentialDeposit::get();
let existential_deposit: T::Balance = minimum_balance::<T, I>();
let source: T::AccountId = account("source", 0, SEED);
let source_lookup = T::Lookup::unlookup(source.clone());

Expand Down Expand Up @@ -175,7 +183,7 @@ mod benchmarks {
#[benchmark(extra)]
fn transfer_increasing_users(u: Linear<0, 1_000>) {
// 1_000 is not very much, but this upper bound can be controlled by the CLI.
let existential_deposit = T::ExistentialDeposit::get();
let existential_deposit: T::Balance = minimum_balance::<T, I>();
let caller = whitelisted_caller();

// Give some multiple of the existential deposit
Expand Down Expand Up @@ -214,7 +222,7 @@ mod benchmarks {
let recipient_lookup = T::Lookup::unlookup(recipient.clone());

// Give some multiple of the existential deposit
let existential_deposit = T::ExistentialDeposit::get();
let existential_deposit: T::Balance = minimum_balance::<T, I>();
let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&caller, balance);

Expand All @@ -231,7 +239,7 @@ mod benchmarks {
let user_lookup = T::Lookup::unlookup(user.clone());

// Give some multiple of the existential deposit
let ed = T::ExistentialDeposit::get();
let ed = minimum_balance::<T, I>();
let balance = ed + ed;
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&user, balance);

Expand All @@ -257,8 +265,8 @@ mod benchmarks {
.map(|i| -> T::AccountId {
let user = account("old_user", i, SEED);
let account = AccountData {
free: T::ExistentialDeposit::get(),
reserved: T::ExistentialDeposit::get(),
free: minimum_balance::<T, I>(),
reserved: minimum_balance::<T, I>(),
frozen: Zero::zero(),
flags: ExtraFlags::old_logic(),
};
Expand Down

0 comments on commit 1c4141a

Please sign in to comment.