From 408ede3f406077b0dc758c1fa832c2aeadadd450 Mon Sep 17 00:00:00 2001
From: Keith Yeung <kungfukeith11@gmail.com>
Date: Wed, 10 Aug 2022 23:51:27 +0800
Subject: [PATCH 01/14] Use can_hold instead of can_reserve in fn hold

---
 frame/balances/src/lib.rs | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
index 683ebce2b1693..a0f9f6543b363 100644
--- a/frame/balances/src/lib.rs
+++ b/frame/balances/src/lib.rs
@@ -1182,12 +1182,15 @@ impl<T: Config<I>, I: 'static> fungible::MutateHold<T::AccountId> for Pallet<T,
 		if amount.is_zero() {
 			return Ok(())
 		}
-		ensure!(Self::can_reserve(who, amount), Error::<T, I>::InsufficientBalance);
-		Self::mutate_account(who, |a| {
-			a.free -= amount;
-			a.reserved += amount;
-		})?;
-		Ok(())
+		ensure!(
+			<Self as fungible::InspectHold<T::AccountId>>::can_hold(who, amount),
+			Error::<T, I>::InsufficientBalance,
+		);
+		Self::try_mutate_account(who, |a, _| -> DispatchResult {
+			a.free = a.free.checked_sub(&amount).ok_or(Error::<T, I>::InsufficientBalance)?;
+			a.reserved = a.reserved.checked_add(&amount).ok_or(ArithmeticError::Overflow)?;
+			Ok(())
+		})
 	}
 	fn release(
 		who: &T::AccountId,

From 462a9dec16b164e84b1dd6d54203788d33502930 Mon Sep 17 00:00:00 2001
From: Keith Yeung <kungfukeith11@gmail.com>
Date: Wed, 28 Sep 2022 03:34:27 +0800
Subject: [PATCH 02/14] Add tests for fungible::hold

---
 frame/balances/src/tests.rs | 126 ++++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/frame/balances/src/tests.rs b/frame/balances/src/tests.rs
index 8f5470ae3cac2..3440f67bc325c 100644
--- a/frame/balances/src/tests.rs
+++ b/frame/balances/src/tests.rs
@@ -28,6 +28,7 @@ macro_rules! decl_tests {
 		use frame_support::{
 			assert_noop, assert_storage_noop, assert_ok, assert_err,
 			traits::{
+				fungible::{self, BalancedHold, Inspect, InspectHold, MutateHold},
 				LockableCurrency, LockIdentifier, WithdrawReasons,
 				Currency, ReservableCurrency, ExistenceRequirement::AllowDeath
 			}
@@ -393,6 +394,23 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn holding_fungible_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
+
+				assert_eq!(Balances::balance(&1), 111);
+				assert_eq!(Balances::reducible_balance(&1, false), 111);
+				assert_eq!(Balances::balance_on_hold(&1), 0);
+
+				assert_ok!(Balances::hold(&1, 69));
+
+				assert_eq!(Balances::balance(&1), 111);
+				assert_eq!(Balances::reducible_balance(&1, false), 42);
+				assert_eq!(Balances::balance_on_hold(&1), 69);
+			});
+		}
+
 		#[test]
 		fn balance_transfer_when_reserved_should_not_work() {
 			<$ext_builder>::default().build().execute_with(|| {
@@ -404,6 +422,18 @@ macro_rules! decl_tests {
 				);
 			});
 		}
+		
+		#[test]
+		fn balance_transfer_when_held_should_not_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
+				assert_ok!(Balances::hold(&1, 69));
+				assert_noop!(
+					<Balances as fungible::Transfer<_>>::transfer(&1, &2, 69, false),
+					Error::<$test, _>::InsufficientBalance,
+				);
+			});
+		}
 
 		#[test]
 		fn deducting_balance_should_work() {
@@ -437,6 +467,18 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn slashing_fungible_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
+				assert_ok!(Balances::hold(&1, 69));
+				assert_eq!(<Balances as fungible::Mutate<_>>::slash(&1, 69), Ok(42));
+				assert_eq!(Balances::reducible_balance(&1, false), 0);
+				assert_eq!(Balances::balance_on_hold(&1), 69);
+				assert_eq!(Balances::total_issuance(), 69);
+			});
+		}
+
 		#[test]
 		fn withdrawing_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
@@ -462,6 +504,18 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn slashing_incomplete_fungible_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 42);
+				assert_ok!(Balances::hold(&1, 21));
+				assert_eq!(<Balances as fungible::Mutate<_>>::slash(&1, 69), Ok(21));
+				assert_eq!(Balances::reducible_balance(&1, false), 0);
+				assert_eq!(Balances::balance_on_hold(&1), 21);
+				assert_eq!(Balances::total_issuance(), 21);
+			});
+		}
+
 		#[test]
 		fn unreserving_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
@@ -473,6 +527,17 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn releasing_fungible_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
+				assert_ok!(Balances::hold(&1, 111));
+				assert_eq!(Balances::release(&1, 42, false), Ok(42));
+				assert_eq!(Balances::balance_on_hold(&1), 69);
+				assert_eq!(Balances::reducible_balance(&1, false), 42);
+			});
+		}
+
 		#[test]
 		fn slashing_reserved_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
@@ -485,6 +550,18 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn slashing_held_fungible_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
+				assert_ok!(Balances::hold(&1, 111));
+				assert_eq!(Balances::slash_held(&1, 42).1, 0);
+				assert_eq!(Balances::balance_on_hold(&1), 69);
+				assert_eq!(Balances::reducible_balance(&1, false), 0);
+				assert_eq!(Balances::total_issuance(), 69);
+			});
+		}
+
 		#[test]
 		fn slashing_incomplete_reserved_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
@@ -497,6 +574,18 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn slashing_incomplete_held_fungible_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
+				assert_ok!(Balances::hold(&1, 42));
+				assert_eq!(Balances::slash_held(&1, 69).1, 0);
+				assert_eq!(Balances::reducible_balance(&1, false), 69);
+				assert_eq!(Balances::balance_on_hold(&1), 0);
+				assert_eq!(Balances::total_issuance(), 69);
+			});
+		}
+
 		#[test]
 		fn repatriating_reserved_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
@@ -528,6 +617,20 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn transferring_held_fungible_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 110);
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&2, 1);
+				assert_ok!(Balances::hold(&1, 110));
+				assert_ok!(Balances::transfer_held(&1, &2, 41, false, true), 41);
+				assert_eq!(Balances::balance_on_hold(&1), 69);
+				assert_eq!(Balances::reducible_balance(&1, false), 0);
+				assert_eq!(Balances::balance_on_hold(&2), 41);
+				assert_eq!(Balances::reducible_balance(&2, false), 1);
+			});
+		}
+
 		#[test]
 		fn transferring_reserved_balance_to_nonexistent_should_fail() {
 			<$ext_builder>::default().build().execute_with(|| {
@@ -537,6 +640,15 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn transferring_held_fungible_to_nonexistent_should_fail() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
+				assert_ok!(Balances::hold(&1, 111));
+				assert_noop!(Balances::transfer_held(&1, &2, 42, false, false), Error::<$test, _>::DeadAccount);
+			});
+		}
+
 		#[test]
 		fn transferring_incomplete_reserved_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
@@ -551,6 +663,20 @@ macro_rules! decl_tests {
 			});
 		}
 
+		#[test]
+		fn transferring_incomplete_held_fungible_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 110);
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&2, 1);
+				assert_ok!(Balances::hold(&1, 41));
+				assert_ok!(Balances::transfer_held(&1, &2, 69, true, false), 41);
+				assert_eq!(Balances::balance_on_hold(&1), 0);
+				assert_eq!(Balances::reducible_balance(&1, false), 69);
+				assert_eq!(Balances::balance_on_hold(&2), 0);
+				assert_eq!(Balances::reducible_balance(&2, false), 42);
+			});
+		}
+
 		#[test]
 		fn transferring_too_high_value_should_not_panic() {
 			<$ext_builder>::default().build().execute_with(|| {

From 13d2261f927607206fba7009bc2336a2267433dd Mon Sep 17 00:00:00 2001
From: Keith Yeung <kungfukeith11@gmail.com>
Date: Wed, 28 Sep 2022 03:44:44 +0800
Subject: [PATCH 03/14] cargo fmt

---
 frame/balances/src/tests.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frame/balances/src/tests.rs b/frame/balances/src/tests.rs
index 5bdfd1031f544..4ea298b951d34 100644
--- a/frame/balances/src/tests.rs
+++ b/frame/balances/src/tests.rs
@@ -422,7 +422,7 @@ macro_rules! decl_tests {
 				);
 			});
 		}
-		
+
 		#[test]
 		fn balance_transfer_when_held_should_not_work() {
 			<$ext_builder>::default().build().execute_with(|| {

From 7a6db84850d68985e51e5fbd662f3bccaa208648 Mon Sep 17 00:00:00 2001
From: Keith Yeung <kungfukeith11@gmail.com>
Date: Fri, 7 Oct 2022 21:27:28 +0800
Subject: [PATCH 04/14] Add keep_alive parameter to can_hold

---
 frame/balances/src/lib.rs                    | 25 ++++++++++++--------
 frame/support/src/traits/tokens/fungible.rs  |  6 ++---
 frame/support/src/traits/tokens/fungibles.rs | 10 +++++++-
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
index 163f3ca4f3e51..0342cf8e33be5 100644
--- a/frame/balances/src/lib.rs
+++ b/frame/balances/src/lib.rs
@@ -1169,19 +1169,24 @@ impl<T: Config<I>, I: 'static> fungible::InspectHold<T::AccountId> for Pallet<T,
 	fn balance_on_hold(who: &T::AccountId) -> T::Balance {
 		Self::account(who).reserved
 	}
-	fn can_hold(who: &T::AccountId, amount: T::Balance) -> bool {
+	fn can_hold(who: &T::AccountId, amount: T::Balance, keep_alive: bool) -> bool {
 		let a = Self::account(who);
-		let min_balance = T::ExistentialDeposit::get().max(a.frozen(Reasons::All));
 		if a.reserved.checked_add(&amount).is_none() {
 			return false
 		}
-		// We require it to be min_balance + amount to ensure that the full reserved funds may be
-		// slashed without compromising locked funds or destroying the account.
-		let required_free = match min_balance.checked_add(&amount) {
-			Some(x) => x,
-			None => return false,
-		};
-		a.free >= required_free
+
+		if keep_alive {
+			let min_balance = T::ExistentialDeposit::get().max(a.frozen(Reasons::All));
+			// We require it to be min_balance + amount to ensure that the full reserved funds may
+			// be slashed without compromising locked funds or destroying the account.
+			let required_free = match min_balance.checked_add(&amount) {
+				Some(x) => x,
+				None => return false,
+			};
+			a.free >= required_free
+		} else {
+			a.free >= amount
+		}
 	}
 }
 impl<T: Config<I>, I: 'static> fungible::MutateHold<T::AccountId> for Pallet<T, I> {
@@ -1190,7 +1195,7 @@ impl<T: Config<I>, I: 'static> fungible::MutateHold<T::AccountId> for Pallet<T,
 			return Ok(())
 		}
 		ensure!(
-			<Self as fungible::InspectHold<T::AccountId>>::can_hold(who, amount),
+			<Self as fungible::InspectHold<T::AccountId>>::can_hold(who, amount, false),
 			Error::<T, I>::InsufficientBalance,
 		);
 		Self::try_mutate_account(who, |a, _| -> DispatchResult {
diff --git a/frame/support/src/traits/tokens/fungible.rs b/frame/support/src/traits/tokens/fungible.rs
index 90aadb6d8daa6..8b19f4f4d2280 100644
--- a/frame/support/src/traits/tokens/fungible.rs
+++ b/frame/support/src/traits/tokens/fungible.rs
@@ -128,7 +128,7 @@ pub trait InspectHold<AccountId>: Inspect<AccountId> {
 	fn balance_on_hold(who: &AccountId) -> Self::Balance;
 
 	/// Check to see if some `amount` of funds of `who` may be placed on hold.
-	fn can_hold(who: &AccountId, amount: Self::Balance) -> bool;
+	fn can_hold(who: &AccountId, amount: Self::Balance, keep_alive: bool) -> bool;
 }
 
 /// Trait for mutating a fungible asset which can be reserved.
@@ -269,8 +269,8 @@ impl<
 	fn balance_on_hold(who: &AccountId) -> Self::Balance {
 		<F as fungibles::InspectHold<AccountId>>::balance_on_hold(A::get(), who)
 	}
-	fn can_hold(who: &AccountId, amount: Self::Balance) -> bool {
-		<F as fungibles::InspectHold<AccountId>>::can_hold(A::get(), who, amount)
+	fn can_hold(who: &AccountId, amount: Self::Balance, keep_alive: bool) -> bool {
+		<F as fungibles::InspectHold<AccountId>>::can_hold(A::get(), who, amount, keep_alive)
 	}
 }
 
diff --git a/frame/support/src/traits/tokens/fungibles.rs b/frame/support/src/traits/tokens/fungibles.rs
index e4108b7f80a98..e8c0c88495ad3 100644
--- a/frame/support/src/traits/tokens/fungibles.rs
+++ b/frame/support/src/traits/tokens/fungibles.rs
@@ -183,7 +183,15 @@ pub trait InspectHold<AccountId>: Inspect<AccountId> {
 	fn balance_on_hold(asset: Self::AssetId, who: &AccountId) -> Self::Balance;
 
 	/// Check to see if some `amount` of `asset` may be held on the account of `who`.
-	fn can_hold(asset: Self::AssetId, who: &AccountId, amount: Self::Balance) -> bool;
+	///
+	/// If `keep_alive` is set to `true`, then it also checks so that the free balance of `asset`
+	/// does not go below its existential deposit after holding `amount`.
+	fn can_hold(
+		asset: Self::AssetId,
+		who: &AccountId,
+		amount: Self::Balance,
+		keep_alive: bool,
+	) -> bool;
 }
 
 /// Trait for mutating a set of named fungible assets which can be placed on hold.

From 9420645ed0d722c067bd50a54ecc99707bdba3c0 Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Tue, 29 Nov 2022 15:59:22 +0000
Subject: [PATCH 05/14] Revert "Add keep_alive parameter to can_hold"

As per this comment: https://github.com/paritytech/substrate/pull/12004#pullrequestreview-1183868550

This reverts commit 7a6db84850d68985e51e5fbd662f3bccaa208648.
---
 frame/balances/src/lib.rs                    | 25 ++++++++------------
 frame/support/src/traits/tokens/fungible.rs  |  6 ++---
 frame/support/src/traits/tokens/fungibles.rs | 10 +-------
 3 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
index 5dbff3f1a9aec..ad1601b23c9f3 100644
--- a/frame/balances/src/lib.rs
+++ b/frame/balances/src/lib.rs
@@ -1173,24 +1173,19 @@ impl<T: Config<I>, I: 'static> fungible::InspectHold<T::AccountId> for Pallet<T,
 	fn balance_on_hold(who: &T::AccountId) -> T::Balance {
 		Self::account(who).reserved
 	}
-	fn can_hold(who: &T::AccountId, amount: T::Balance, keep_alive: bool) -> bool {
+	fn can_hold(who: &T::AccountId, amount: T::Balance) -> bool {
 		let a = Self::account(who);
+		let min_balance = T::ExistentialDeposit::get().max(a.frozen(Reasons::All));
 		if a.reserved.checked_add(&amount).is_none() {
 			return false
 		}
-
-		if keep_alive {
-			let min_balance = T::ExistentialDeposit::get().max(a.frozen(Reasons::All));
-			// We require it to be min_balance + amount to ensure that the full reserved funds may
-			// be slashed without compromising locked funds or destroying the account.
-			let required_free = match min_balance.checked_add(&amount) {
-				Some(x) => x,
-				None => return false,
-			};
-			a.free >= required_free
-		} else {
-			a.free >= amount
-		}
+		// We require it to be min_balance + amount to ensure that the full reserved funds may be
+		// slashed without compromising locked funds or destroying the account.
+		let required_free = match min_balance.checked_add(&amount) {
+			Some(x) => x,
+			None => return false,
+		};
+		a.free >= required_free
 	}
 }
 impl<T: Config<I>, I: 'static> fungible::MutateHold<T::AccountId> for Pallet<T, I> {
@@ -1199,7 +1194,7 @@ impl<T: Config<I>, I: 'static> fungible::MutateHold<T::AccountId> for Pallet<T,
 			return Ok(())
 		}
 		ensure!(
-			<Self as fungible::InspectHold<T::AccountId>>::can_hold(who, amount, false),
+			<Self as fungible::InspectHold<T::AccountId>>::can_hold(who, amount),
 			Error::<T, I>::InsufficientBalance,
 		);
 		Self::try_mutate_account(who, |a, _| -> DispatchResult {
diff --git a/frame/support/src/traits/tokens/fungible.rs b/frame/support/src/traits/tokens/fungible.rs
index 8b19f4f4d2280..90aadb6d8daa6 100644
--- a/frame/support/src/traits/tokens/fungible.rs
+++ b/frame/support/src/traits/tokens/fungible.rs
@@ -128,7 +128,7 @@ pub trait InspectHold<AccountId>: Inspect<AccountId> {
 	fn balance_on_hold(who: &AccountId) -> Self::Balance;
 
 	/// Check to see if some `amount` of funds of `who` may be placed on hold.
-	fn can_hold(who: &AccountId, amount: Self::Balance, keep_alive: bool) -> bool;
+	fn can_hold(who: &AccountId, amount: Self::Balance) -> bool;
 }
 
 /// Trait for mutating a fungible asset which can be reserved.
@@ -269,8 +269,8 @@ impl<
 	fn balance_on_hold(who: &AccountId) -> Self::Balance {
 		<F as fungibles::InspectHold<AccountId>>::balance_on_hold(A::get(), who)
 	}
-	fn can_hold(who: &AccountId, amount: Self::Balance, keep_alive: bool) -> bool {
-		<F as fungibles::InspectHold<AccountId>>::can_hold(A::get(), who, amount, keep_alive)
+	fn can_hold(who: &AccountId, amount: Self::Balance) -> bool {
+		<F as fungibles::InspectHold<AccountId>>::can_hold(A::get(), who, amount)
 	}
 }
 
diff --git a/frame/support/src/traits/tokens/fungibles.rs b/frame/support/src/traits/tokens/fungibles.rs
index 20e609f95d8bb..8c370e9a0d8b5 100644
--- a/frame/support/src/traits/tokens/fungibles.rs
+++ b/frame/support/src/traits/tokens/fungibles.rs
@@ -185,15 +185,7 @@ pub trait InspectHold<AccountId>: Inspect<AccountId> {
 	fn balance_on_hold(asset: Self::AssetId, who: &AccountId) -> Self::Balance;
 
 	/// Check to see if some `amount` of `asset` may be held on the account of `who`.
-	///
-	/// If `keep_alive` is set to `true`, then it also checks so that the free balance of `asset`
-	/// does not go below its existential deposit after holding `amount`.
-	fn can_hold(
-		asset: Self::AssetId,
-		who: &AccountId,
-		amount: Self::Balance,
-		keep_alive: bool,
-	) -> bool;
+	fn can_hold(asset: Self::AssetId, who: &AccountId, amount: Self::Balance) -> bool;
 }
 
 /// Trait for mutating a set of named fungible assets which can be placed on hold.

From 17e226b898b24cfcee8e23d572b8599411db6044 Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Tue, 29 Nov 2022 17:17:44 +0000
Subject: [PATCH 06/14] Make sure `ReservableCurrency::can_reserve` takes
 `ExistentialDeposit` into account.

---
 frame/balances/src/lib.rs | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
index ad1601b23c9f3..553efafafec78 100644
--- a/frame/balances/src/lib.rs
+++ b/frame/balances/src/lib.rs
@@ -1758,16 +1758,23 @@ impl<T: Config<I>, I: 'static> ReservableCurrency<T::AccountId> for Pallet<T, I>
 where
 	T::Balance: MaybeSerializeDeserialize + Debug,
 {
-	/// Check if `who` can reserve `value` from their free balance.
+	/// Check if `who` can reserve `value` from their free balance taking into account the
+	/// `ExistentialDeposit` making sure the account is not going to be destroyed in case of
+	/// overdraft.
 	///
 	/// Always `true` if value to be reserved is zero.
 	fn can_reserve(who: &T::AccountId, value: Self::Balance) -> bool {
 		if value.is_zero() {
 			return true
 		}
-		Self::account(who).free.checked_sub(&value).map_or(false, |new_balance| {
-			Self::ensure_can_withdraw(who, value, WithdrawReasons::RESERVE, new_balance).is_ok()
-		})
+
+		let account = Self::account(who);
+		let min_balance = T::ExistentialDeposit::get().max(account.frozen(Reasons::All));
+
+		Self::account(who)
+			.free
+			.checked_sub(&value)
+			.map_or(false, |new_balance| new_balance >= min_balance)
 	}
 
 	fn reserved_balance(who: &T::AccountId) -> Self::Balance {

From 5153396e50f251cb95a823458e1b71694b05f96d Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Tue, 29 Nov 2022 17:29:26 +0000
Subject: [PATCH 07/14] Enforce  within .

---
 frame/balances/src/lib.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
index 553efafafec78..c7c24d8161057 100644
--- a/frame/balances/src/lib.rs
+++ b/frame/balances/src/lib.rs
@@ -1794,7 +1794,7 @@ 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)
+			ensure!(Self::can_reserve(who, value), Error::<T, I>::LiquidityRestrictions);
 		})?;
 
 		Self::deposit_event(Event::Reserved { who: who.clone(), amount: value });

From b41b36b8bf2abfffa0b34add696398898fcf1bbf Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Tue, 29 Nov 2022 19:16:58 +0000
Subject: [PATCH 08/14] fix reserve

---
 frame/balances/src/lib.rs | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
index c7c24d8161057..2b891ea95a512 100644
--- a/frame/balances/src/lib.rs
+++ b/frame/balances/src/lib.rs
@@ -1794,7 +1794,9 @@ where
 				account.free.checked_sub(&value).ok_or(Error::<T, I>::InsufficientBalance)?;
 			account.reserved =
 				account.reserved.checked_add(&value).ok_or(ArithmeticError::Overflow)?;
-			ensure!(Self::can_reserve(who, value), Error::<T, I>::LiquidityRestrictions);
+			Self::can_reserve(who, value)
+				.then(|| Ok::<(), sp_runtime::DispatchError>(()))
+				.ok_or(Error::<T, I>::LiquidityRestrictions)?
 		})?;
 
 		Self::deposit_event(Event::Reserved { who: who.clone(), amount: value });

From 7916a0ab992f8e63dc911de7f13ae122c93bd5bd Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Thu, 1 Dec 2022 17:48:23 +0100
Subject: [PATCH 09/14] fix tests

---
 frame/balances/src/benchmarking.rs     |  4 +-
 frame/balances/src/lib.rs              |  2 +-
 frame/balances/src/tests.rs            | 90 ++++++++++++--------------
 frame/balances/src/tests_reentrancy.rs | 55 +---------------
 4 files changed, 47 insertions(+), 104 deletions(-)

diff --git a/frame/balances/src/benchmarking.rs b/frame/balances/src/benchmarking.rs
index 206adba0f044b..c4bed51ea930c 100644
--- a/frame/balances/src/benchmarking.rs
+++ b/frame/balances/src/benchmarking.rs
@@ -201,12 +201,12 @@ benchmarks_instance_pallet! {
 		// Give some multiple of the existential deposit
 		let existential_deposit = T::ExistentialDeposit::get();
 		let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
-		let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&user, balance);
+		let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&user, balance + existential_deposit);
 
 		// Reserve the balance
 		<Balances<T, I> as ReservableCurrency<_>>::reserve(&user, balance)?;
 		assert_eq!(Balances::<T, I>::reserved_balance(&user), balance);
-		assert!(Balances::<T, I>::free_balance(&user).is_zero());
+		assert_eq!(Balances::<T, I>::free_balance(&user), existential_deposit);
 
 	}: _(RawOrigin::Root, user_lookup, balance)
 	verify {
diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
index 2b891ea95a512..46e60777c5b24 100644
--- a/frame/balances/src/lib.rs
+++ b/frame/balances/src/lib.rs
@@ -1796,7 +1796,7 @@ where
 				account.reserved.checked_add(&value).ok_or(ArithmeticError::Overflow)?;
 			Self::can_reserve(who, value)
 				.then(|| Ok::<(), sp_runtime::DispatchError>(()))
-				.ok_or(Error::<T, I>::LiquidityRestrictions)?
+				.ok_or(Error::<T, I>::InsufficientBalance)?
 		})?;
 
 		Self::deposit_event(Event::Reserved { who: who.clone(), amount: value });
diff --git a/frame/balances/src/tests.rs b/frame/balances/src/tests.rs
index 6a27ff473a506..f2fae41c6ffe5 100644
--- a/frame/balances/src/tests.rs
+++ b/frame/balances/src/tests.rs
@@ -183,7 +183,7 @@ macro_rules! decl_tests {
 					);
 					assert_noop!(
 						<Balances as ReservableCurrency<_>>::reserve(&1, 1),
-						Error::<$test, _>::LiquidityRestrictions,
+						Error::<$test, _>::InsufficientBalance,
 					);
 					assert!(<ChargeTransactionPayment<$test> as SignedExtension>::pre_dispatch(
 						ChargeTransactionPayment::from(1),
@@ -202,7 +202,9 @@ macro_rules! decl_tests {
 
 					Balances::set_lock(ID_1, &1, 10, WithdrawReasons::TRANSACTION_PAYMENT);
 					assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
-					assert_ok!(<Balances as ReservableCurrency<_>>::reserve(&1, 1));
+					assert_noop!(<Balances as ReservableCurrency<_>>::reserve(&1, 1),
+					             Error::<$test, _>::InsufficientBalance,
+					);
 					assert!(<ChargeTransactionPayment<$test> as SignedExtension>::pre_dispatch(
 						ChargeTransactionPayment::from(1),
 						&1,
@@ -290,16 +292,17 @@ macro_rules! decl_tests {
 					System::inc_account_nonce(&2);
 					assert_eq!(Balances::total_balance(&2), 256 * 20);
 
-					assert_ok!(Balances::reserve(&2, 256 * 19 + 1)); // account 2 becomes mostly reserved
+					assert_ok!(Balances::reserve(&2, 256 * 19)); // account 2 becomes mostly reserved
+					assert_ok!(Balances::transfer(Some(2).into(), 12, 1)); // account 2 free balance below ED
 					assert_eq!(Balances::free_balance(2), 255); // "free" account deleted."
-					assert_eq!(Balances::total_balance(&2), 256 * 20); // reserve still exists.
+					assert_eq!(Balances::total_balance(&2), 256 * 20-1); // reserve still exists.
 					assert_eq!(System::account_nonce(&2), 1);
 
 					// account 4 tries to take index 1 for account 5.
 					assert_ok!(Balances::transfer(Some(4).into(), 5, 256 * 1 + 0x69));
 					assert_eq!(Balances::total_balance(&5), 256 * 1 + 0x69);
 
-					assert!(Balances::slash(&2, 256 * 19 + 2).1.is_zero()); // account 2 gets slashed
+					assert!(Balances::slash(&2, 256 * 19 + 1).1.is_zero()); // account 2 gets slashed
 					// "reserve" account reduced to 255 (below ED) so account deleted
 					assert_eq!(Balances::total_balance(&2), 0);
 					assert_eq!(System::account_nonce(&2), 0);    // nonce zero
@@ -519,22 +522,30 @@ macro_rules! decl_tests {
 		#[test]
 		fn unreserving_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
-				let _ = Balances::deposit_creating(&1, 111);
+				let _ = Balances::deposit_creating(&1, 112);
 				assert_ok!(Balances::reserve(&1, 111));
 				Balances::unreserve(&1, 42);
 				assert_eq!(Balances::reserved_balance(1), 69);
-				assert_eq!(Balances::free_balance(1), 42);
+				assert_eq!(Balances::free_balance(1), 43);
 			});
 		}
 
 		#[test]
-		fn releasing_fungible_should_work() {
+		fn hold_should_respect_ed() {
 			<$ext_builder>::default().build().execute_with(|| {
 				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
+				assert_err!(Balances::hold(&1, 111), Error::<$test, _>::InsufficientBalance);
+			});
+		}
+
+		#[test]
+		fn releasing_fungible_should_work() {
+			<$ext_builder>::default().build().execute_with(|| {
+				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 112);
 				assert_ok!(Balances::hold(&1, 111));
 				assert_eq!(Balances::release(&1, 42, false), Ok(42));
 				assert_eq!(Balances::balance_on_hold(&1), 69);
-				assert_eq!(Balances::reducible_balance(&1, false), 42);
+				assert_eq!(Balances::reducible_balance(&1, false), 43);
 			});
 		}
 
@@ -542,10 +553,10 @@ macro_rules! decl_tests {
 		fn slashing_reserved_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
 				let _ = Balances::deposit_creating(&1, 111);
-				assert_ok!(Balances::reserve(&1, 111));
+				assert_ok!(Balances::reserve(&1, 110));
 				assert_eq!(Balances::slash_reserved(&1, 42).1, 0);
-				assert_eq!(Balances::reserved_balance(1), 69);
-				assert_eq!(Balances::free_balance(1), 0);
+				assert_eq!(Balances::reserved_balance(1), 68);
+				assert_eq!(Balances::free_balance(1), 1);
 				assert_eq!(<TotalIssuance<$test>>::get(), 69);
 			});
 		}
@@ -554,10 +565,10 @@ macro_rules! decl_tests {
 		fn slashing_held_fungible_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
 				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
-				assert_ok!(Balances::hold(&1, 111));
+				assert_ok!(Balances::hold(&1, 110));
 				assert_eq!(Balances::slash_held(&1, 42).1, 0);
-				assert_eq!(Balances::balance_on_hold(&1), 69);
-				assert_eq!(Balances::reducible_balance(&1, false), 0);
+				assert_eq!(Balances::balance_on_hold(&1), 68);
+				assert_eq!(Balances::reducible_balance(&1, false), 1);
 				assert_eq!(Balances::total_issuance(), 69);
 			});
 		}
@@ -589,7 +600,7 @@ macro_rules! decl_tests {
 		#[test]
 		fn repatriating_reserved_balance_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {
-				let _ = Balances::deposit_creating(&1, 110);
+				let _ = Balances::deposit_creating(&1, 111);
 				let _ = Balances::deposit_creating(&2, 1);
 				assert_ok!(Balances::reserve(&1, 110));
 				assert_ok!(Balances::repatriate_reserved(&1, &2, 41, Status::Free), 0);
@@ -597,7 +608,7 @@ macro_rules! decl_tests {
 					RuntimeEvent::Balances(crate::Event::ReserveRepatriated { from: 1, to: 2, amount: 41, destination_status: Status::Free })
 				);
 				assert_eq!(Balances::reserved_balance(1), 69);
-				assert_eq!(Balances::free_balance(1), 0);
+				assert_eq!(Balances::free_balance(1), 1);
 				assert_eq!(Balances::reserved_balance(2), 0);
 				assert_eq!(Balances::free_balance(2), 42);
 			});
@@ -608,10 +619,10 @@ macro_rules! decl_tests {
 			<$ext_builder>::default().build().execute_with(|| {
 				let _ = Balances::deposit_creating(&1, 110);
 				let _ = Balances::deposit_creating(&2, 1);
-				assert_ok!(Balances::reserve(&1, 110));
+				assert_ok!(Balances::reserve(&1, 109));
 				assert_ok!(Balances::repatriate_reserved(&1, &2, 41, Status::Reserved), 0);
-				assert_eq!(Balances::reserved_balance(1), 69);
-				assert_eq!(Balances::free_balance(1), 0);
+				assert_eq!(Balances::reserved_balance(1), 68);
+				assert_eq!(Balances::free_balance(1), 1);
 				assert_eq!(Balances::reserved_balance(2), 41);
 				assert_eq!(Balances::free_balance(2), 1);
 			});
@@ -622,10 +633,10 @@ macro_rules! decl_tests {
 			<$ext_builder>::default().build().execute_with(|| {
 				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 110);
 				let _ = <Balances as fungible::Mutate<_>>::mint_into(&2, 1);
-				assert_ok!(Balances::hold(&1, 110));
+				assert_ok!(Balances::hold(&1, 109));
 				assert_ok!(Balances::transfer_held(&1, &2, 41, false, true), 41);
-				assert_eq!(Balances::balance_on_hold(&1), 69);
-				assert_eq!(Balances::reducible_balance(&1, false), 0);
+				assert_eq!(Balances::balance_on_hold(&1), 68);
+				assert_eq!(Balances::reducible_balance(&1, false), 1);
 				assert_eq!(Balances::balance_on_hold(&2), 41);
 				assert_eq!(Balances::reducible_balance(&2, false), 1);
 			});
@@ -667,7 +678,7 @@ macro_rules! decl_tests {
 		fn transferring_reserved_balance_to_nonexistent_should_fail() {
 			<$ext_builder>::default().build().execute_with(|| {
 				let _ = Balances::deposit_creating(&1, 111);
-				assert_ok!(Balances::reserve(&1, 111));
+				assert_ok!(Balances::reserve(&1, 110));
 				assert_noop!(Balances::repatriate_reserved(&1, &2, 42, Status::Free), Error::<$test, _>::DeadAccount);
 			});
 		}
@@ -676,7 +687,7 @@ macro_rules! decl_tests {
 		fn transferring_held_fungible_to_nonexistent_should_fail() {
 			<$ext_builder>::default().build().execute_with(|| {
 				let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
-				assert_ok!(Balances::hold(&1, 111));
+				assert_ok!(Balances::hold(&1, 110));
 				assert_noop!(Balances::transfer_held(&1, &2, 42, false, false), Error::<$test, _>::DeadAccount);
 			});
 		}
@@ -819,34 +830,19 @@ macro_rules! decl_tests {
 		}
 
 		#[test]
-		fn dust_moves_between_free_and_reserved() {
+		fn reserve_should_respect_ed() {
 			<$ext_builder>::default()
-				.existential_deposit(100)
 				.build()
 				.execute_with(|| {
-					// Set balance to free and reserved at the existential deposit
+					// Set balance to ED
 					assert_ok!(Balances::set_balance(RawOrigin::Root.into(), 1, 100, 0));
 					// Check balance
 					assert_eq!(Balances::free_balance(1), 100);
-					assert_eq!(Balances::reserved_balance(1), 0);
-
-					// Reserve some free balance
-					assert_ok!(Balances::reserve(&1, 50));
-					// Check balance, the account should be ok.
-					assert_eq!(Balances::free_balance(1), 50);
-					assert_eq!(Balances::reserved_balance(1), 50);
-
-					// Reserve the rest of the free balance
-					assert_ok!(Balances::reserve(&1, 50));
-					// Check balance, the account should be ok.
-					assert_eq!(Balances::free_balance(1), 0);
-					assert_eq!(Balances::reserved_balance(1), 100);
-
-					// Unreserve everything
-					Balances::unreserve(&1, 100);
-					// Check balance, all 100 should move to free_balance
-					assert_eq!(Balances::free_balance(1), 100);
-					assert_eq!(Balances::reserved_balance(1), 0);
+					// Reserve over ED
+					assert_noop!(
+						Balances::reserve(&1, 100),
+						Error::<$test, _>::InsufficientBalance
+					);
 				});
 		}
 
diff --git a/frame/balances/src/tests_reentrancy.rs b/frame/balances/src/tests_reentrancy.rs
index 90363140000e8..a18d64d6c27a1 100644
--- a/frame/balances/src/tests_reentrancy.rs
+++ b/frame/balances/src/tests_reentrancy.rs
@@ -29,10 +29,7 @@ use sp_io;
 use sp_runtime::{testing::Header, traits::IdentityLookup};
 
 use crate::*;
-use frame_support::{
-	assert_ok,
-	traits::{Currency, ReservableCurrency},
-};
+use frame_support::{assert_ok, traits::Currency};
 use frame_system::RawOrigin;
 
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
@@ -212,53 +209,3 @@ fn transfer_dust_removal_tst2_should_work() {
 		}));
 	});
 }
-
-#[test]
-fn repatriating_reserved_balance_dust_removal_should_work() {
-	ExtBuilder::default().existential_deposit(100).build().execute_with(|| {
-		// Verification of reentrancy in dust removal
-		assert_ok!(Balances::set_balance(RawOrigin::Root.into(), 1, 1000, 0));
-		assert_ok!(Balances::set_balance(RawOrigin::Root.into(), 2, 500, 0));
-
-		// Reserve a value on account 2,
-		// Such that free balance is lower than
-		// Exestintial deposit.
-		assert_ok!(Balances::reserve(&2, 450));
-
-		// Transfer of reserved fund from slashed account 2 to
-		// beneficiary account 1
-		assert_ok!(Balances::repatriate_reserved(&2, &1, 450, Status::Free), 0);
-
-		// Since free balance of account 2 is lower than
-		// existential deposit, dust amount is
-		// removed from the account 2
-		assert_eq!(Balances::reserved_balance(2), 0);
-		assert_eq!(Balances::free_balance(2), 0);
-
-		// account 1 is credited with reserved amount
-		// together with dust balance during dust
-		// removal.
-		assert_eq!(Balances::reserved_balance(1), 0);
-		assert_eq!(Balances::free_balance(1), 1500);
-
-		// Verify the events
-		assert_eq!(System::events().len(), 11);
-
-		System::assert_has_event(RuntimeEvent::Balances(crate::Event::ReserveRepatriated {
-			from: 2,
-			to: 1,
-			amount: 450,
-			destination_status: Status::Free,
-		}));
-
-		System::assert_has_event(RuntimeEvent::Balances(crate::Event::DustLost {
-			account: 2,
-			amount: 50,
-		}));
-
-		System::assert_last_event(RuntimeEvent::Balances(crate::Event::Deposit {
-			who: 1,
-			amount: 50,
-		}));
-	});
-}

From b58674d0dc13d37d8158ee8cd3c8a7685d602a45 Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Thu, 1 Dec 2022 17:57:21 +0100
Subject: [PATCH 10/14] fix bench

---
 frame/balances/src/benchmarking.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frame/balances/src/benchmarking.rs b/frame/balances/src/benchmarking.rs
index c4bed51ea930c..5f01e97c16952 100644
--- a/frame/balances/src/benchmarking.rs
+++ b/frame/balances/src/benchmarking.rs
@@ -211,7 +211,7 @@ benchmarks_instance_pallet! {
 	}: _(RawOrigin::Root, user_lookup, balance)
 	verify {
 		assert!(Balances::<T, I>::reserved_balance(&user).is_zero());
-		assert_eq!(Balances::<T, I>::free_balance(&user), balance);
+		assert_eq!(Balances::<T, I>::free_balance(&user), balance + existential_deposit);
 	}
 
 	impl_benchmark_test_suite!(

From 84e648c59577dafb7c511c6144f47d5ca51175b1 Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Thu, 1 Dec 2022 20:25:57 +0100
Subject: [PATCH 11/14] fix assets tests

---
 frame/assets/src/tests.rs | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs
index d5fcece0e91d8..c0ab225c733bd 100644
--- a/frame/assets/src/tests.rs
+++ b/frame/assets/src/tests.rs
@@ -163,7 +163,7 @@ fn approval_lifecycle_works() {
 		// so we create it :)
 		assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1));
 		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100));
-		Balances::make_free_balance_be(&1, 1);
+		Balances::make_free_balance_be(&1, 2);
 		assert_ok!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 50));
 		assert_eq!(Asset::<Test>::get(0).unwrap().approvals, 1);
 		assert_eq!(Balances::reserved_balance(&1), 1);
@@ -189,7 +189,7 @@ fn transfer_approved_all_funds() {
 		// so we create it :)
 		assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1));
 		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100));
-		Balances::make_free_balance_be(&1, 1);
+		Balances::make_free_balance_be(&1, 2);
 		assert_ok!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 50));
 		assert_eq!(Asset::<Test>::get(0).unwrap().approvals, 1);
 		assert_eq!(Balances::reserved_balance(&1), 1);
@@ -211,7 +211,7 @@ fn approval_deposits_work() {
 		let e = BalancesError::<Test>::InsufficientBalance;
 		assert_noop!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 50), e);
 
-		Balances::make_free_balance_be(&1, 1);
+		Balances::make_free_balance_be(&1, 2);
 		assert_ok!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 50));
 		assert_eq!(Balances::reserved_balance(&1), 1);
 
@@ -229,7 +229,7 @@ fn cannot_transfer_more_than_approved() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1));
 		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100));
-		Balances::make_free_balance_be(&1, 1);
+		Balances::make_free_balance_be(&1, 2);
 		assert_ok!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 50));
 		let e = Error::<Test>::Unapproved;
 		assert_noop!(Assets::transfer_approved(RuntimeOrigin::signed(2), 0, 1, 3, 51), e);
@@ -241,7 +241,7 @@ fn cannot_transfer_more_than_exists() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1));
 		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100));
-		Balances::make_free_balance_be(&1, 1);
+		Balances::make_free_balance_be(&1, 2);
 		assert_ok!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 101));
 		let e = Error::<Test>::BalanceLow;
 		assert_noop!(Assets::transfer_approved(RuntimeOrigin::signed(2), 0, 1, 3, 101), e);
@@ -253,7 +253,7 @@ fn cancel_approval_works() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1));
 		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100));
-		Balances::make_free_balance_be(&1, 1);
+		Balances::make_free_balance_be(&1, 2);
 		assert_ok!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 50));
 		assert_eq!(Asset::<Test>::get(0).unwrap().approvals, 1);
 		assert_noop!(
@@ -283,7 +283,7 @@ fn force_cancel_approval_works() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1));
 		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100));
-		Balances::make_free_balance_be(&1, 1);
+		Balances::make_free_balance_be(&1, 2);
 		assert_ok!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 50));
 		assert_eq!(Asset::<Test>::get(0).unwrap().approvals, 1);
 		let e = Error::<Test>::NoPermission;
@@ -509,13 +509,6 @@ fn min_balance_should_work() {
 		assert!(Assets::maybe_balance(0, 1).is_none());
 		assert_eq!(Asset::<Test>::get(0).unwrap().accounts, 0);
 		assert_eq!(take_hooks(), vec![Hook::Died(0, 1)]);
-
-		// Death by `transfer_approved`.
-		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100));
-		Balances::make_free_balance_be(&1, 1);
-		assert_ok!(Assets::approve_transfer(RuntimeOrigin::signed(1), 0, 2, 100));
-		assert_ok!(Assets::transfer_approved(RuntimeOrigin::signed(2), 0, 1, 3, 91));
-		assert_eq!(take_hooks(), vec![Hook::Died(0, 1)]);
 	});
 }
 
@@ -1154,7 +1147,7 @@ fn querying_allowance_should_work() {
 		use frame_support::traits::tokens::fungibles::approvals::{Inspect, Mutate};
 		assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1));
 		assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100));
-		Balances::make_free_balance_be(&1, 1);
+		Balances::make_free_balance_be(&1, 2);
 		assert_ok!(Assets::approve(0, &1, &2, 50));
 		assert_eq!(Assets::allowance(0, &1, &2), 50);
 		// Transfer asset 0, from owner 1 and delegate 2 to destination 3

From 8b503974017e753b32c0f090f0f6db15dbc88641 Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Thu, 1 Dec 2022 20:44:13 +0100
Subject: [PATCH 12/14] fix asset bench

---
 frame/assets/src/benchmarking.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frame/assets/src/benchmarking.rs b/frame/assets/src/benchmarking.rs
index ede5b4e77fac6..bff24b4653534 100644
--- a/frame/assets/src/benchmarking.rs
+++ b/frame/assets/src/benchmarking.rs
@@ -105,7 +105,7 @@ fn add_sufficients<T: Config<I>, I: 'static>(minter: T::AccountId, n: u32) {
 
 fn add_approvals<T: Config<I>, I: 'static>(minter: T::AccountId, n: u32) {
 	let asset_id = default_asset_id::<T, I>();
-	T::Currency::deposit_creating(&minter, T::ApprovalDeposit::get() * n.into());
+	T::Currency::deposit_creating(&minter, T::ApprovalDeposit::get() * (n + 1).into());
 	let minter_lookup = T::Lookup::unlookup(minter.clone());
 	let origin = SystemOrigin::Signed(minter);
 	Assets::<T, I>::mint(origin.clone().into(), asset_id, minter_lookup, (100 * (n + 1)).into())

From 37ab667a747256c2d95d97c47f6eae92cf6d270f Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Thu, 1 Dec 2022 20:45:10 +0100
Subject: [PATCH 13/14] add a clarification

---
 frame/assets/src/benchmarking.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/frame/assets/src/benchmarking.rs b/frame/assets/src/benchmarking.rs
index bff24b4653534..66f486c7bfde5 100644
--- a/frame/assets/src/benchmarking.rs
+++ b/frame/assets/src/benchmarking.rs
@@ -105,6 +105,7 @@ fn add_sufficients<T: Config<I>, I: 'static>(minter: T::AccountId, n: u32) {
 
 fn add_approvals<T: Config<I>, I: 'static>(minter: T::AccountId, n: u32) {
 	let asset_id = default_asset_id::<T, I>();
+	// taking into account the ED requirement enforced by `Balances::reserve` we do (n + 1).
 	T::Currency::deposit_creating(&minter, T::ApprovalDeposit::get() * (n + 1).into());
 	let minter_lookup = T::Lookup::unlookup(minter.clone());
 	let origin = SystemOrigin::Signed(minter);

From 6b7da3104d9d78f6fe2d23699644c10b6cf84e0a Mon Sep 17 00:00:00 2001
From: Roman Useinov <roman.useinov@gmail.com>
Date: Fri, 2 Dec 2022 13:59:53 +0100
Subject: [PATCH 14/14] fix benches

---
 frame/bounties/src/benchmarking.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/frame/bounties/src/benchmarking.rs b/frame/bounties/src/benchmarking.rs
index 07dd781c29af3..7d31c9c3a474a 100644
--- a/frame/bounties/src/benchmarking.rs
+++ b/frame/bounties/src/benchmarking.rs
@@ -54,9 +54,9 @@ fn setup_bounty<T: Config<I>, I: 'static>(
 	let fee = value / 2u32.into();
 	let deposit = T::BountyDepositBase::get() +
 		T::DataDepositPerByte::get() * T::MaximumReasonLength::get().into();
-	let _ = T::Currency::make_free_balance_be(&caller, deposit);
+	let _ = T::Currency::make_free_balance_be(&caller, deposit + 1u32.into());
 	let curator = account("curator", u, SEED);
-	let _ = T::Currency::make_free_balance_be(&curator, fee / 2u32.into());
+	let _ = T::Currency::make_free_balance_be(&curator, fee / 2u32.into() + 1u32.into());
 	let reason = vec![0; d as usize];
 	(caller, curator, fee, value, reason)
 }