Refactor/tokens events (#729)

* Added new events and ensure events are deposited whenever balance and total issuances changes happen.
Added unit tests to test these functions will deposit events correctly

* Moved the event deposit of do_transfer to the outside of the mutate, so transfer event is emitted last (instead of endowed).
Removed some redundant deposit_event for transfer, to avoid double transfer event emission

* Removed all events from orml-currencies
All events are deposited at implementation level instead

* Minor tidying up. Addressed some PR review comments.

* Fixed some line spacing with event deposits

* Minor linebreak improvements

Co-authored-by: Roy Yang <[email protected]>

Author: syan095

currencies/src/lib.rs

@@ -81,8 +81,6 @@ pub mod module {
 
 	#[pallet::config]
 	pub trait Config: frame_system::Config {
-		type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
-
 		type MultiCurrency: TransferAll<Self::AccountId>
 			+ MultiCurrencyExtended<Self::AccountId>
 			+ MultiLockableCurrency<Self::AccountId>
@@ -109,36 +107,6 @@ pub mod module {
 		DepositFailed,
 	}
 
-	#[pallet::event]
-	#[pallet::generate_deposit(pub(crate) fn deposit_event)]
-	pub enum Event<T: Config> {
-		/// Currency transfer success.
-		Transferred {
-			currency_id: CurrencyIdOf<T>,
-			from: T::AccountId,
-			to: T::AccountId,
-			amount: BalanceOf<T>,
-		},
-		/// Update balance success.
-		BalanceUpdated {
-			currency_id: CurrencyIdOf<T>,
-			who: T::AccountId,
-			amount: AmountOf<T>,
-		},
-		/// Deposit success.
-		Deposited {
-			currency_id: CurrencyIdOf<T>,
-			who: T::AccountId,
-			amount: BalanceOf<T>,
-		},
-		/// Withdraw success.
-		Withdrawn {
-			currency_id: CurrencyIdOf<T>,
-			who: T::AccountId,
-			amount: BalanceOf<T>,
-		},
-	}
-
 	#[pallet::pallet]
 	pub struct Pallet<T>(_);
 
@@ -160,8 +128,7 @@ pub mod module {
 		) -> DispatchResult {
 			let from = ensure_signed(origin)?;
 			let to = T::Lookup::lookup(dest)?;
-			<Self as MultiCurrency<T::AccountId>>::transfer(currency_id, &from, &to, amount)?;
-			Ok(())
+			<Self as MultiCurrency<T::AccountId>>::transfer(currency_id, &from, &to, amount)
 		}
 
 		/// Transfer some native currency to another account.
@@ -176,15 +143,7 @@ pub mod module {
 		) -> DispatchResult {
 			let from = ensure_signed(origin)?;
 			let to = T::Lookup::lookup(dest)?;
-			T::NativeCurrency::transfer(&from, &to, amount)?;
-
-			Self::deposit_event(Event::Transferred {
-				currency_id: T::GetNativeCurrencyId::get(),
-				from,
-				to,
-				amount,
-			});
-			Ok(())
+			T::NativeCurrency::transfer(&from, &to, amount)
 		}
 
 		/// update amount of account `who` under `currency_id`.
@@ -199,8 +158,7 @@ pub mod module {
 		) -> DispatchResult {
 			ensure_root(origin)?;
 			let dest = T::Lookup::lookup(who)?;
-			<Self as MultiCurrencyExtended<T::AccountId>>::update_balance(currency_id, &dest, amount)?;
-			Ok(())
+			<Self as MultiCurrencyExtended<T::AccountId>>::update_balance(currency_id, &dest, amount)
 		}
 	}
 }
@@ -259,51 +217,32 @@ impl<T: Config> MultiCurrency<T::AccountId> for Pallet<T> {
 			return Ok(());
 		}
 		if currency_id == T::GetNativeCurrencyId::get() {
-			T::NativeCurrency::transfer(from, to, amount)?;
+			T::NativeCurrency::transfer(from, to, amount)
 		} else {
-			T::MultiCurrency::transfer(currency_id, from, to, amount)?;
+			T::MultiCurrency::transfer(currency_id, from, to, amount)
 		}
-		Self::deposit_event(Event::Transferred {
-			currency_id,
-			from: from.clone(),
-			to: to.clone(),
-			amount,
-		});
-		Ok(())
 	}
 
 	fn deposit(currency_id: Self::CurrencyId, who: &T::AccountId, amount: Self::Balance) -> DispatchResult {
 		if amount.is_zero() {
 			return Ok(());
 		}
 		if currency_id == T::GetNativeCurrencyId::get() {
-			T::NativeCurrency::deposit(who, amount)?;
+			T::NativeCurrency::deposit(who, amount)
 		} else {
-			T::MultiCurrency::deposit(currency_id, who, amount)?;
+			T::MultiCurrency::deposit(currency_id, who, amount)
 		}
-		Self::deposit_event(Event::Deposited {
-			currency_id,
-			who: who.clone(),
-			amount,
-		});
-		Ok(())
 	}
 
 	fn withdraw(currency_id: Self::CurrencyId, who: &T::AccountId, amount: Self::Balance) -> DispatchResult {
 		if amount.is_zero() {
 			return Ok(());
 		}
 		if currency_id == T::GetNativeCurrencyId::get() {
-			T::NativeCurrency::withdraw(who, amount)?;
+			T::NativeCurrency::withdraw(who, amount)
 		} else {
-			T::MultiCurrency::withdraw(currency_id, who, amount)?;
+			T::MultiCurrency::withdraw(currency_id, who, amount)
 		}
-		Self::deposit_event(Event::Withdrawn {
-			currency_id,
-			who: who.clone(),
-			amount,
-		});
-		Ok(())
 	}
 
 	fn can_slash(currency_id: Self::CurrencyId, who: &T::AccountId, amount: Self::Balance) -> bool {
@@ -328,16 +267,10 @@ impl<T: Config> MultiCurrencyExtended<T::AccountId> for Pallet<T> {
 
 	fn update_balance(currency_id: Self::CurrencyId, who: &T::AccountId, by_amount: Self::Amount) -> DispatchResult {
 		if currency_id == T::GetNativeCurrencyId::get() {
-			T::NativeCurrency::update_balance(who, by_amount)?;
+			T::NativeCurrency::update_balance(who, by_amount)
 		} else {
-			T::MultiCurrency::update_balance(currency_id, who, by_amount)?;
+			T::MultiCurrency::update_balance(currency_id, who, by_amount)
 		}
-		Self::deposit_event(Event::BalanceUpdated {
-			currency_id,
-			who: who.clone(),
-			amount: by_amount,
-		});
-		Ok(())
 	}
 }
 
@@ -608,7 +541,6 @@ where
 			let actual_deposit = deposit_result.peek();
 			ensure!(actual_deposit == amount, Error::<T>::DepositFailed);
 		}
-
 		Ok(())
 	}
 

currencies/src/mock.rs

@@ -91,7 +91,6 @@ parameter_types! {
 }
 
 impl Config for Runtime {
-	type Event = Event;
 	type MultiCurrency = Tokens;
 	type NativeCurrency = AdaptedBasicCurrency;
 	type GetNativeCurrencyId = GetNativeCurrencyId;
@@ -110,7 +109,7 @@ construct_runtime!(
 		UncheckedExtrinsic = UncheckedExtrinsic,
 	{
 		System: frame_system::{Pallet, Call, Storage, Config, Event<T>},
-		Currencies: currencies::{Pallet, Call, Event<T>},
+		Currencies: currencies::{Pallet, Call},
 		Tokens: orml_tokens::{Pallet, Storage, Event<T>, Config<T>},
 		PalletBalances: pallet_balances::{Pallet, Call, Storage, Config<T>, Event<T>},
 	}

currencies/src/tests.rs

@@ -271,7 +271,7 @@ fn call_event_should_work() {
 			assert_ok!(Currencies::transfer(Some(ALICE).into(), BOB, X_TOKEN_ID, 50));
 			assert_eq!(Currencies::free_balance(X_TOKEN_ID, &ALICE), 50);
 			assert_eq!(Currencies::free_balance(X_TOKEN_ID, &BOB), 150);
-			System::assert_last_event(Event::Currencies(crate::Event::Transferred {
+			System::assert_last_event(Event::Tokens(orml_tokens::Event::Transfer {
 				currency_id: X_TOKEN_ID,
 				from: ALICE,
 				to: BOB,
@@ -283,7 +283,7 @@ fn call_event_should_work() {
 			));
 			assert_eq!(Currencies::free_balance(X_TOKEN_ID, &ALICE), 40);
 			assert_eq!(Currencies::free_balance(X_TOKEN_ID, &BOB), 160);
-			System::assert_last_event(Event::Currencies(crate::Event::Transferred {
+			System::assert_last_event(Event::Tokens(orml_tokens::Event::Transfer {
 				currency_id: X_TOKEN_ID,
 				from: ALICE,
 				to: BOB,
@@ -294,7 +294,7 @@ fn call_event_should_work() {
 				X_TOKEN_ID, &ALICE, 100
 			));
 			assert_eq!(Currencies::free_balance(X_TOKEN_ID, &ALICE), 140);
-			System::assert_last_event(Event::Currencies(crate::Event::Deposited {
+			System::assert_last_event(Event::Tokens(orml_tokens::Event::Deposited {
 				currency_id: X_TOKEN_ID,
 				who: ALICE,
 				amount: 100,
@@ -304,7 +304,7 @@ fn call_event_should_work() {
 				X_TOKEN_ID, &ALICE, 20
 			));
 			assert_eq!(Currencies::free_balance(X_TOKEN_ID, &ALICE), 120);
-			System::assert_last_event(Event::Currencies(crate::Event::Withdrawn {
+			System::assert_last_event(Event::Tokens(orml_tokens::Event::Withdrawn {
 				currency_id: X_TOKEN_ID,
 				who: ALICE,
 				amount: 20,