diff --git a/prdoc/pr_7379.prdoc b/prdoc/pr_7379.prdoc new file mode 100644 index 000000000000..0bd904346d68 --- /dev/null +++ b/prdoc/pr_7379.prdoc @@ -0,0 +1,13 @@ +# 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: "Add support for feature pallet_balances/insecure_zero_ed in benchmarks and testing" + +doc: + - audience: Runtime Dev + 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. + +crates: + - name: pallet-balances + bump: patch diff --git a/substrate/frame/balances/src/benchmarking.rs b/substrate/frame/balances/src/benchmarking.rs index c825300218d4..a761f8e2af82 100644 --- a/substrate/frame/balances/src/benchmarking.rs +++ b/substrate/frame/balances/src/benchmarking.rs @@ -65,7 +65,12 @@ mod benchmarks { #[extrinsic_call] _(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount); - assert_eq!(Balances::::free_balance(&caller), Zero::zero()); + if cfg!(feature = "insecure_zero_ed") { + assert_eq!(Balances::::free_balance(&caller), balance - transfer_amount); + } else { + assert_eq!(Balances::::free_balance(&caller), Zero::zero()); + } + assert_eq!(Balances::::free_balance(&recipient), transfer_amount); } @@ -173,7 +178,12 @@ mod benchmarks { #[extrinsic_call] _(RawOrigin::Root, source_lookup, recipient_lookup, transfer_amount); - assert_eq!(Balances::::free_balance(&source), Zero::zero()); + if cfg!(feature = "insecure_zero_ed") { + assert_eq!(Balances::::free_balance(&source), balance - transfer_amount); + } else { + assert_eq!(Balances::::free_balance(&source), Zero::zero()); + } + assert_eq!(Balances::::free_balance(&recipient), transfer_amount); } @@ -208,7 +218,12 @@ mod benchmarks { #[extrinsic_call] transfer_allow_death(RawOrigin::Signed(caller.clone()), recipient_lookup, transfer_amount); - assert_eq!(Balances::::free_balance(&caller), Zero::zero()); + if cfg!(feature = "insecure_zero_ed") { + assert_eq!(Balances::::free_balance(&caller), balance - transfer_amount); + } else { + assert_eq!(Balances::::free_balance(&caller), Zero::zero()); + } + assert_eq!(Balances::::free_balance(&recipient), transfer_amount); } @@ -308,7 +323,7 @@ mod benchmarks { /// Benchmark `burn` extrinsic with the worst possible condition - burn kills the account. #[benchmark] fn burn_allow_death() { - let existential_deposit = T::ExistentialDeposit::get(); + let existential_deposit: T::Balance = minimum_balance::(); let caller = whitelisted_caller(); // Give some multiple of the existential deposit @@ -321,13 +336,17 @@ mod benchmarks { #[extrinsic_call] burn(RawOrigin::Signed(caller.clone()), burn_amount, false); - assert_eq!(Balances::::free_balance(&caller), Zero::zero()); + if cfg!(feature = "insecure_zero_ed") { + assert_eq!(Balances::::free_balance(&caller), balance - burn_amount); + } else { + assert_eq!(Balances::::free_balance(&caller), Zero::zero()); + } } // Benchmark `burn` extrinsic with the case where account is kept alive. #[benchmark] fn burn_keep_alive() { - let existential_deposit = T::ExistentialDeposit::get(); + let existential_deposit: T::Balance = minimum_balance::(); let caller = whitelisted_caller(); // Give some multiple of the existential deposit diff --git a/substrate/frame/balances/src/tests/currency_tests.rs b/substrate/frame/balances/src/tests/currency_tests.rs index a6377c3ad72e..0e5d7ccb46de 100644 --- a/substrate/frame/balances/src/tests/currency_tests.rs +++ b/substrate/frame/balances/src/tests/currency_tests.rs @@ -24,7 +24,7 @@ use frame_support::{ BalanceStatus::{Free, Reserved}, Currency, ExistenceRequirement::{self, AllowDeath, KeepAlive}, - Hooks, InspectLockableCurrency, LockIdentifier, LockableCurrency, NamedReservableCurrency, + InspectLockableCurrency, LockIdentifier, LockableCurrency, NamedReservableCurrency, ReservableCurrency, WithdrawReasons, }, StorageNoopGuard, @@ -1136,7 +1136,9 @@ fn operations_on_dead_account_should_not_change_state() { #[test] #[should_panic = "The existential deposit must be greater than zero!"] +#[cfg(not(feature = "insecure_zero_ed"))] fn zero_ed_is_prohibited() { + use frame_support::traits::Hooks; // These functions all use `mutate_account` which may introduce a storage change when // the account never existed to begin with, and shouldn't exist in the end. ExtBuilder::default().existential_deposit(0).build_and_execute_with(|| {