Add events for balance reserve and unreserve functions (#6330)

* almost works

* add clone to BalanceStatus

* reserve event

* fix staking tests

* fix balances tests

* Update frame/balances/src/tests.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* restore tests and move event emission

* move repatriate reserved event outside of mutate_account

* clean up events in tests

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

substrate/frame/balances/src/lib.rs

@@ -230,6 +230,13 @@ decl_event!(
 		BalanceSet(AccountId, Balance, Balance),
 		/// Some amount was deposited (e.g. for transaction fees).
 		Deposit(AccountId, Balance),
+		/// Some balance was reserved (moved from free to reserved).
+		Reserved(AccountId, Balance),
+		/// Some balance was unreserved (moved from reserved to free).
+		Unreserved(AccountId, Balance),
+		/// Some balance was moved from the reserve of the first account to the second account.
+		/// Final argument indicates the destination balance type.
+		ReserveRepatriated(AccountId, AccountId, Balance, Status),
 	}
 );
 
@@ -1150,8 +1157,11 @@ impl<T: Trait<I>, I: Instance> ReservableCurrency<T::AccountId> for Module<T, I>
 		Self::try_mutate_account(who, |account, _| -> DispatchResult {
 			account.free = account.free.checked_sub(&value).ok_or(Error::<T, I>::InsufficientBalance)?;
 			account.reserved = account.reserved.checked_add(&value).ok_or(Error::<T, I>::Overflow)?;
-			Self::ensure_can_withdraw(who, value, WithdrawReason::Reserve.into(), account.free)
-		})
+			Self::ensure_can_withdraw(&who, value.clone(), WithdrawReason::Reserve.into(), account.free)
+		})?;
+
+		Self::deposit_event(RawEvent::Reserved(who.clone(), value));
+		Ok(())
 	}
 
 	/// Unreserve some funds, returning any amount that was unable to be unreserved.
@@ -1160,14 +1170,17 @@ impl<T: Trait<I>, I: Instance> ReservableCurrency<T::AccountId> for Module<T, I>
 	fn unreserve(who: &T::AccountId, value: Self::Balance) -> Self::Balance {
 		if value.is_zero() { return Zero::zero() }
 
-		Self::mutate_account(who, |account| {
+		let actual = Self::mutate_account(who, |account| {
 			let actual = cmp::min(account.reserved, value);
 			account.reserved -= actual;
 			// defensive only: this can never fail since total issuance which is at least free+reserved
 			// fits into the same data type.
 			account.free = account.free.saturating_add(actual);
-			value - actual
-		})
+			actual
+		});
+
+		Self::deposit_event(RawEvent::Unreserved(who.clone(), actual.clone()));
+		value - actual
 	}
 
 	/// Slash from reserved balance, returning the negative imbalance created,
@@ -1208,7 +1221,7 @@ impl<T: Trait<I>, I: Instance> ReservableCurrency<T::AccountId> for Module<T, I>
 			};
 		}
 
-		Self::try_mutate_account(beneficiary, |to_account, is_new| -> Result<Self::Balance, DispatchError> {
+		let actual = Self::try_mutate_account(beneficiary, |to_account, is_new|-> Result<Self::Balance, DispatchError> {
 			ensure!(!is_new, Error::<T, I>::DeadAccount);
 			Self::try_mutate_account(slashed, |from_account, _| -> Result<Self::Balance, DispatchError> {
 				let actual = cmp::min(from_account.reserved, value);
@@ -1217,9 +1230,12 @@ impl<T: Trait<I>, I: Instance> ReservableCurrency<T::AccountId> for Module<T, I>
 					Status::Reserved => to_account.reserved = to_account.reserved.checked_add(&actual).ok_or(Error::<T, I>::Overflow)?,
 				}
 				from_account.reserved -= actual;
-				Ok(value - actual)
+				Ok(actual)
 			})
-		})
+		})?;
+
+		Self::deposit_event(RawEvent::ReserveRepatriated(slashed.clone(), beneficiary.clone(), actual, status));
+		Ok(value - actual)
 	}
 }
 

substrate/frame/balances/src/tests.rs

@@ -69,6 +69,10 @@ macro_rules! decl_tests {
 			evt
 		}
 
+		fn last_event() -> Event {
+			system::Module::<Test>::events().pop().expect("Event expected").event
+		}
+
 		#[test]
 		fn basic_locking_should_work() {
 			<$ext_builder>::default().existential_deposit(1).monied(true).build().execute_with(|| {
@@ -170,7 +174,7 @@ macro_rules! decl_tests {
 					);
 					assert_noop!(
 						<Balances as ReservableCurrency<_>>::reserve(&1, 1),
-						Error::<$test, _>::LiquidityRestrictions
+						Error::<$test, _>::LiquidityRestrictions,
 					);
 					assert!(<ChargeTransactionPayment<$test> as SignedExtension>::pre_dispatch(
 						ChargeTransactionPayment::from(1),
@@ -485,6 +489,10 @@ macro_rules! decl_tests {
 				let _ = Balances::deposit_creating(&2, 1);
 				assert_ok!(Balances::reserve(&1, 110));
 				assert_ok!(Balances::repatriate_reserved(&1, &2, 41, Status::Free), 0);
+				assert_eq!(
+					last_event(),
+					Event::balances(RawEvent::ReserveRepatriated(1, 2, 41, Status::Free)),
+				);
 				assert_eq!(Balances::reserved_balance(1), 69);
 				assert_eq!(Balances::free_balance(1), 0);
 				assert_eq!(Balances::reserved_balance(2), 0);
@@ -683,6 +691,40 @@ macro_rules! decl_tests {
 				});
 		}
 
+		#[test]
+		fn emit_events_with_reserve_and_unreserve() {
+			<$ext_builder>::default()
+				.build()
+				.execute_with(|| {
+					let _ = Balances::deposit_creating(&1, 100);
+
+					System::set_block_number(2);
+					let _ = Balances::reserve(&1, 10);
+
+					assert_eq!(
+						last_event(),
+						Event::balances(RawEvent::Reserved(1, 10)),
+					);
+
+					System::set_block_number(3);
+					let _ = Balances::unreserve(&1, 5);
+
+					assert_eq!(
+						last_event(),
+						Event::balances(RawEvent::Unreserved(1, 5)),
+					);
+
+					System::set_block_number(4);
+					let _ = Balances::unreserve(&1, 6);
+
+					// should only unreserve 5
+					assert_eq!(
+						last_event(),
+						Event::balances(RawEvent::Unreserved(1, 5)),
+					);
+				});
+		}
+
 		#[test]
 		fn emit_events_with_existential_deposit() {
 			<$ext_builder>::default()

substrate/frame/staking/src/tests.rs

@@ -337,7 +337,7 @@ fn staking_should_work() {
 					claimed_rewards: vec![0],
 				})
 			);
-			// e.g. it cannot spend more than 500 that it has free from the total 2000
+			// e.g. it cannot reserve more than 500 that it has free from the total 2000
 			assert_noop!(
 				Balances::reserve(&3, 501),
 				BalancesError::<Test, _>::LiquidityRestrictions
@@ -783,10 +783,10 @@ fn cannot_reserve_staked_balance() {
 		assert_eq!(Balances::free_balance(11), 1000);
 		// Confirm account 11 (via controller 10) is totally staked
 		assert_eq!(Staking::eras_stakers(Staking::active_era().unwrap().index, 11).own, 1000);
-		// Confirm account 11 cannot transfer as a result
+		// Confirm account 11 cannot reserve as a result
 		assert_noop!(
 			Balances::reserve(&11, 1),
-			BalancesError::<Test, _>::LiquidityRestrictions
+			BalancesError::<Test, _>::LiquidityRestrictions,
 		);
 
 		// Give account 11 extra free balance

substrate/frame/support/src/traits.rs

@@ -1008,6 +1008,7 @@ pub trait Currency<AccountId> {
 }
 
 /// Status of funds.
+#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug)]
 pub enum BalanceStatus {
 	/// Funds are free, as corresponding to `free` item in Balances.
 	Free,