[stable2509] Backport #8108

prdoc/pr_8108.prdoc

@@ -0,0 +1,16 @@
+title: "Fix `reserve` and `can_reserve` methods in `pallet-balances` to properly handle reserved balances"
+
+doc:
+- audience: Runtime Dev
+  description: |-
+    Fixes a bug in `pallet-balances` where the `reserve` and `can_reserve` methods were incorrectly 
+    preventing new reserves when an account's free balance was lower than its frozen balance, 
+    even if the usable balance was sufficient after accounting for the reserved balance.
+- audience: Runtime User
+  description: |-
+    Fixes a bug that was causing unexpected `LiquidityRestrictions` errors when attempting to 
+    reserve funds.
+
+crates: 
+- name: pallet-balances
+  bump: patch

substrate/frame/balances/src/impl_currency.rs

@@ -535,6 +535,29 @@ where
 	}
 }
 
+/// Validates whether an account can create a reserve without violating
+/// liquidity constraints.
+///
+/// This method performs liquidity checks without modifying the account state.
+fn ensure_can_reserve<T: Config<I>, I: 'static>(
+	who: &T::AccountId,
+	value: T::Balance,
+	check_existential_deposit: bool,
+) -> DispatchResult {
+	let AccountData { free, .. } = Pallet::<T, I>::account(who);
+
+	// Early validation: Check sufficient free balance
+	let new_free_balance = free.checked_sub(&value).ok_or(Error::<T, I>::InsufficientBalance)?;
+
+	// Conditionally validate existential deposit preservation
+	if check_existential_deposit {
+		let existential_deposit = T::ExistentialDeposit::get();
+		ensure!(new_free_balance >= existential_deposit, Error::<T, I>::Expendability);
+	}
+
+	Ok(())
+}
+
 impl<T: Config<I>, I: 'static> ReservableCurrency<T::AccountId> for Pallet<T, I>
 where
 	T::Balance: MaybeSerializeDeserialize + Debug,
@@ -546,11 +569,7 @@ where
 		if value.is_zero() {
 			return true
 		}
-		Self::account(who).free.checked_sub(&value).map_or(false, |new_balance| {
-			new_balance >= T::ExistentialDeposit::get() &&
-				Self::ensure_can_withdraw(who, value, WithdrawReasons::RESERVE, new_balance)
-					.is_ok()
-		})
+		ensure_can_reserve::<T, I>(who, value, true).is_ok()
 	}
 
 	fn reserved_balance(who: &T::AccountId) -> Self::Balance {
@@ -570,7 +589,9 @@ where
 				account.free.checked_sub(&value).ok_or(Error::<T, I>::InsufficientBalance)?;
 			account.reserved =
 				account.reserved.checked_add(&value).ok_or(ArithmeticError::Overflow)?;
-			Self::ensure_can_withdraw(&who, value, WithdrawReasons::RESERVE, account.free)
+
+			// Check if it is possible to reserve before trying to mutate the account
+			ensure_can_reserve::<T, I>(who, value, false)
 		})?;
 
 		Self::deposit_event(Event::Reserved { who: who.clone(), amount: value });

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

@@ -258,7 +258,9 @@ fn lock_should_work_reserve() {
 				<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath),
 				TokenError::Frozen
 			);
-			assert_noop!(Balances::reserve(&1, 1), Error::<Test>::LiquidityRestrictions,);
+			// We can use frozen balance for reserves
+			assert_ok!(Balances::reserve(&1, 9));
+			assert_noop!(Balances::reserve(&1, 1), DispatchError::ConsumerRemaining,);
 			assert!(ChargeTransactionPayment::<Test>::validate_and_prepare(
 				ChargeTransactionPayment::from(1),
 				Some(1).into(),
@@ -280,6 +282,47 @@ fn lock_should_work_reserve() {
 		});
 }
 
+#[test]
+fn reserve_should_work_for_frozen_balance() {
+	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()
@@ -291,7 +334,9 @@ fn lock_should_work_tx_fee() {
 				<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath),
 				TokenError::Frozen
 			);
-			assert_noop!(Balances::reserve(&1, 1), Error::<Test>::LiquidityRestrictions,);
+			// We can use frozen balance for reserves
+			assert_ok!(Balances::reserve(&1, 9));
+			assert_noop!(Balances::reserve(&1, 1), DispatchError::ConsumerRemaining,);
 			assert!(ChargeTransactionPayment::<Test>::validate_and_prepare(
 				ChargeTransactionPayment::from(1),
 				Some(1).into(),
@@ -1273,8 +1318,14 @@ fn reserve_must_succeed_if_can_reserve_does() {
 	ExtBuilder::default().build_and_execute_with(|| {
 		let _ = Balances::deposit_creating(&1, 1);
 		let _ = Balances::deposit_creating(&2, 2);
+		let _ = Balances::deposit_creating(&3, 3);
 		assert!(Balances::can_reserve(&1, 1) == Balances::reserve(&1, 1).is_ok());
 		assert!(Balances::can_reserve(&2, 1) == Balances::reserve(&2, 1).is_ok());
+
+		// Ensure that we can reserve as long (free + reserved) remains above
+		// the maximum of frozen balance.
+		Balances::set_lock(ID_1, &3, 2, WithdrawReasons::RESERVE);
+		assert_eq!(Balances::can_reserve(&3, 2), Balances::reserve(&3, 2).is_ok());
 	});
 }