Add events for balance reserve and unreserve functions (paritytech#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 <[email protected]>

* restore tests and move event emission

* move repatriate reserved event outside of mutate_account

* clean up events in tests

Co-authored-by: Kian Paimani <[email protected]>

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)
  }
 }
 

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()

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

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,