Skip to content

Commit 0fc5d6a

Browse files
manuelmauroRomarQ
andcommitted
Add support for feature pallet_balances/insecure_zero_ed in benchmarks and testing (paritytech#7379)
# Description Currently benchmarks and tests on pallet_balances would fail when the feature insecure_zero_ed is enabled. This PR allows to run such benchmark and tests keeping into account the fact that accounts would not be deleted when their balance goes below a threshold. ## Integration *In depth notes about how this PR should be integrated by downstream projects. This part is mandatory, and should be reviewed by reviewers, if the PR does NOT have the `R0-Silent` label. In case of a `R0-Silent`, it can be ignored.* ## Review Notes *In depth notes about the **implementation** details of your PR. This should be the main guide for reviewers to understand your approach and effectively review it. If too long, use [`<details>`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details)*. *Imagine that someone who is depending on the old code wants to integrate your new code and the only information that they get is this section. It helps to include example usage and default value here, with a `diff` code-block to show possibly integration.* *Include your leftover TODOs, if any, here.* # Checklist * [x] My PR includes a detailed description as outlined in the "Description" and its two subsections above. * [x] My PR follows the [labeling requirements]( https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process ) of this project (at minimum one label for `T` required) * External contributors: ask maintainers to put the right label on your PR. * [x] I have made corresponding changes to the documentation (if applicable) * [x] I have added tests that prove my fix is effective or that my feature works (if applicable) You can remove the "Checklist" section once all have been checked. Thank you for your contribution! ✄ ----------------------------------------------------------------------------- --------- Co-authored-by: Rodrigo Quelhas <rodrigo_quelhas@outlook.pt>
1 parent 14624b9 commit 0fc5d6a

3 files changed

Lines changed: 41 additions & 7 deletions

File tree

prdoc/pr_7379.prdoc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
2+
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
3+
4+
title: "Add support for feature pallet_balances/insecure_zero_ed in benchmarks and testing"
5+
6+
doc:
7+
- audience: Runtime Dev
8+
description: |
9+
Currently benchmarks and tests on pallet_balances would fail when the feature insecure_zero_ed is enabled. This PR allows to run such benchmark and tests keeping into account the fact that accounts would not be deleted when their balance goes below a threshold.
10+
11+
crates:
12+
- name: pallet-balances
13+
bump: patch

substrate/frame/balances/src/benchmarking.rs

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,12 @@ mod benchmarks {
6565
#[extrinsic_call]
6666
_(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount);
6767

68-
assert_eq!(Balances::<T, I>::free_balance(&caller), Zero::zero());
68+
if cfg!(feature = "insecure_zero_ed") {
69+
assert_eq!(Balances::<T, I>::free_balance(&caller), balance - transfer_amount);
70+
} else {
71+
assert_eq!(Balances::<T, I>::free_balance(&caller), Zero::zero());
72+
}
73+
6974
assert_eq!(Balances::<T, I>::free_balance(&recipient), transfer_amount);
7075
}
7176

@@ -173,7 +178,12 @@ mod benchmarks {
173178
#[extrinsic_call]
174179
_(RawOrigin::Root, source_lookup, recipient_lookup, transfer_amount);
175180

176-
assert_eq!(Balances::<T, I>::free_balance(&source), Zero::zero());
181+
if cfg!(feature = "insecure_zero_ed") {
182+
assert_eq!(Balances::<T, I>::free_balance(&source), balance - transfer_amount);
183+
} else {
184+
assert_eq!(Balances::<T, I>::free_balance(&source), Zero::zero());
185+
}
186+
177187
assert_eq!(Balances::<T, I>::free_balance(&recipient), transfer_amount);
178188
}
179189

@@ -208,7 +218,12 @@ mod benchmarks {
208218
#[extrinsic_call]
209219
transfer_allow_death(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount);
210220

211-
assert_eq!(Balances::<T, I>::free_balance(&caller), Zero::zero());
221+
if cfg!(feature = "insecure_zero_ed") {
222+
assert_eq!(Balances::<T, I>::free_balance(&caller), balance - transfer_amount);
223+
} else {
224+
assert_eq!(Balances::<T, I>::free_balance(&caller), Zero::zero());
225+
}
226+
212227
assert_eq!(Balances::<T, I>::free_balance(&recipient), transfer_amount);
213228
}
214229

@@ -308,7 +323,7 @@ mod benchmarks {
308323
/// Benchmark `burn` extrinsic with the worst possible condition - burn kills the account.
309324
#[benchmark]
310325
fn burn_allow_death() {
311-
let existential_deposit = T::ExistentialDeposit::get();
326+
let existential_deposit: T::Balance = minimum_balance::<T, I>();
312327
let caller = whitelisted_caller();
313328

314329
// Give some multiple of the existential deposit
@@ -321,13 +336,17 @@ mod benchmarks {
321336
#[extrinsic_call]
322337
burn(RawOrigin::Signed(caller.clone()), burn_amount, false);
323338

324-
assert_eq!(Balances::<T, I>::free_balance(&caller), Zero::zero());
339+
if cfg!(feature = "insecure_zero_ed") {
340+
assert_eq!(Balances::<T, I>::free_balance(&caller), balance - burn_amount);
341+
} else {
342+
assert_eq!(Balances::<T, I>::free_balance(&caller), Zero::zero());
343+
}
325344
}
326345

327346
// Benchmark `burn` extrinsic with the case where account is kept alive.
328347
#[benchmark]
329348
fn burn_keep_alive() {
330-
let existential_deposit = T::ExistentialDeposit::get();
349+
let existential_deposit: T::Balance = minimum_balance::<T, I>();
331350
let caller = whitelisted_caller();
332351

333352
// Give some multiple of the existential deposit

substrate/frame/balances/src/tests/currency_tests.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use frame_support::{
2424
BalanceStatus::{Free, Reserved},
2525
Currency,
2626
ExistenceRequirement::{self, AllowDeath, KeepAlive},
27-
Hooks, InspectLockableCurrency, LockIdentifier, LockableCurrency, NamedReservableCurrency,
27+
InspectLockableCurrency, LockIdentifier, LockableCurrency, NamedReservableCurrency,
2828
ReservableCurrency, WithdrawReasons,
2929
},
3030
StorageNoopGuard,
@@ -1133,7 +1133,9 @@ fn operations_on_dead_account_should_not_change_state() {
11331133

11341134
#[test]
11351135
#[should_panic = "The existential deposit must be greater than zero!"]
1136+
#[cfg(not(feature = "insecure_zero_ed"))]
11361137
fn zero_ed_is_prohibited() {
1138+
use frame_support::traits::Hooks;
11371139
// These functions all use `mutate_account` which may introduce a storage change when
11381140
// the account never existed to begin with, and shouldn't exist in the end.
11391141
ExtBuilder::default().existential_deposit(0).build_and_execute_with(|| {

0 commit comments

Comments
 (0)