diff --git a/prdoc/pr_8108.prdoc b/prdoc/pr_8108.prdoc new file mode 100644 index 0000000000000..4b037c6f70ca7 --- /dev/null +++ b/prdoc/pr_8108.prdoc @@ -0,0 +1,11 @@ +title: "[pallet_balances] Account for reserved balance in `ensure_can_withdraw` function" + +doc: +- audience: [Runtime Dev, Runtime User] + description: |- + The function `ensure_can_withdraw` now ensures the `new free balance` is higher + or equal to the `usable balance`. + +crates: +- name: pallet-balances + bump: patch \ No newline at end of file diff --git a/substrate/frame/balances/src/impl_currency.rs b/substrate/frame/balances/src/impl_currency.rs index f453b23420c40..dfc7febd87146 100644 --- a/substrate/frame/balances/src/impl_currency.rs +++ b/substrate/frame/balances/src/impl_currency.rs @@ -315,13 +315,29 @@ where fn ensure_can_withdraw( who: &T::AccountId, amount: T::Balance, - _reasons: WithdrawReasons, + reasons: WithdrawReasons, new_balance: T::Balance, ) -> DispatchResult { if amount.is_zero() { return Ok(()) } - ensure!(new_balance >= Self::account(who).frozen, Error::::LiquidityRestrictions); + let account = Self::account(who); + + // Account for the new reserved amount only if withdrawing for a reserve. + let updated_reserved = if matches!(reasons, WithdrawReasons::RESERVE) { + account.reserved.saturating_add(amount) + } else { + account.reserved + }; + + // Frozen balance applies to total. Anything on hold therefore gets discounted from the + // limit given by the freezes + let untouchable = account + .frozen + .saturating_sub(updated_reserved) + .max(T::ExistentialDeposit::get()); + + ensure!(new_balance >= untouchable, Error::::LiquidityRestrictions); Ok(()) } diff --git a/substrate/frame/balances/src/tests/currency_tests.rs b/substrate/frame/balances/src/tests/currency_tests.rs index 0e5d7ccb46dee..726b78582f422 100644 --- a/substrate/frame/balances/src/tests/currency_tests.rs +++ b/substrate/frame/balances/src/tests/currency_tests.rs @@ -280,6 +280,48 @@ fn lock_should_work_reserve() { }); } +#[test] +fn reserve_should_work_for_usable_balance() { + frame_support::__private::sp_tracing::init_for_tests(); + ExtBuilder::default() + .existential_deposit(1) + .monied(true) + .build_and_execute_with(|| { + // Check balances + let account = Balances::account(&1); + assert_eq!(account.free, 10); + assert_eq!(account.frozen, 0); + assert_eq!(account.reserved, 0); + + Balances::set_lock(ID_1, &1, 9, WithdrawReasons::RESERVE); + + let account = Balances::account(&1); + assert_eq!(account.free, 10); + assert_eq!(account.frozen, 9); + assert_eq!(account.reserved, 0); + + assert_ok!(Balances::reserve(&1, 5)); + + let account = Balances::account(&1); + assert_eq!(account.free, 5); + assert_eq!(account.frozen, 9); + assert_eq!(account.reserved, 5); + + assert_ok!(Balances::reserve(&1, 4)); + + let account = Balances::account(&1); + assert_eq!(account.free, 1); + assert_eq!(account.frozen, 9); + assert_eq!(account.reserved, 9); + + // Check usable balance + // usable_balance = free - max(frozen - reserved, ExistentialDeposit) + // 0 = 1 - max(9 - 9, 1) + let usable_balance = Balances::usable_balance(&1); + assert_eq!(usable_balance, 0); + }); +} + #[test] fn lock_should_work_tx_fee() { ExtBuilder::default() @@ -754,7 +796,10 @@ fn existential_deposit_respected_when_reserving() { assert_eq!(Balances::reserved_balance(1), 1); // Cannot reserve any more of the free balance. - assert_noop!(Balances::reserve(&1, 1), DispatchError::ConsumerRemaining); + // free = 100 + // reserved = 1 + // existential deposit = 100 + assert_noop!(Balances::reserve(&1, 1), Error::::LiquidityRestrictions); }); } diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 1b3411b65360d..bdf157edfacaa 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -502,7 +502,7 @@ fn staking_should_work() { } ); // e.g. it cannot reserve more than 500 that it has free from the total 2000 - assert_noop!(Balances::reserve(&3, 501), DispatchError::ConsumerRemaining); + assert_noop!(Balances::reserve(&3, 501), BalancesError::::LiquidityRestrictions); assert_ok!(Balances::reserve(&3, 409)); }); }