From 0930f3a576722405c8f89884ffee61374c613c21 Mon Sep 17 00:00:00 2001 From: Daniel Savu Date: Sat, 5 Nov 2022 13:38:35 +0100 Subject: [PATCH 1/3] feat(tokens): Refactor hooks into single trait --- asset-registry/src/mock/para.rs | 7 +- currencies/src/mock.rs | 22 +++++-- payments/src/mock.rs | 7 +- tokens/src/lib.rs | 57 ++++++++-------- tokens/src/mock.rs | 91 ++++++++++++++++++-------- tokens/src/tests.rs | 19 +++--- traits/src/currency.rs | 31 ++++++++- traits/src/lib.rs | 2 +- xtokens/src/mock/para.rs | 7 +- xtokens/src/mock/para_relative_view.rs | 7 +- xtokens/src/mock/para_teleport.rs | 7 +- 11 files changed, 154 insertions(+), 103 deletions(-) diff --git a/asset-registry/src/mock/para.rs b/asset-registry/src/mock/para.rs index 585f29280..e8900a634 100644 --- a/asset-registry/src/mock/para.rs +++ b/asset-registry/src/mock/para.rs @@ -92,16 +92,11 @@ impl orml_tokens::Config for Runtime { type CurrencyId = CurrencyId; type WeightInfo = (); type ExistentialDeposits = ExistentialDeposits; - type OnDust = (); - type OnSlash = (); - type OnDeposit = (); - type OnTransfer = (); + type CurrencyHooks = (); type ReserveIdentifier = [u8; 8]; type MaxReserves = (); type MaxLocks = ConstU32<50>; type DustRemovalWhitelist = Nothing; - type OnNewTokenAccount = (); - type OnKilledTokenAccount = (); } #[derive(scale_info::TypeInfo, Encode, Decode, Clone, Eq, PartialEq, Debug)] diff --git a/currencies/src/mock.rs b/currencies/src/mock.rs index 64a398a7a..e42804cc2 100644 --- a/currencies/src/mock.rs +++ b/currencies/src/mock.rs @@ -8,7 +8,7 @@ use frame_support::{ traits::{ConstU32, ConstU64, Everything, Nothing}, PalletId, }; -use orml_traits::parameter_type_with_key; +use orml_traits::{currency::CurrencyMutationHooks, parameter_type_with_key}; use sp_core::H256; use sp_runtime::{ testing::Header, @@ -73,6 +73,19 @@ parameter_types! { pub DustAccount: AccountId = PalletId(*b"orml/dst").into_account_truncating(); } +pub struct CurrencyHooks(marker::PhantomData); +impl CurrencyMutationHooks for CurrencyHooks +where + T::AccountId: From, +{ + type OnDust = orml_tokens::TransferDust; + type OnSlash = (); + type OnDeposit = (); + type OnTransfer = (); + type OnNewTokenAccount = (); + type OnKilledTokenAccount = (); +} + impl orml_tokens::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = Balance; @@ -80,16 +93,11 @@ impl orml_tokens::Config for Runtime { type CurrencyId = CurrencyId; type WeightInfo = (); type ExistentialDeposits = ExistentialDeposits; - type OnDust = orml_tokens::TransferDust; - type OnSlash = (); - type OnDeposit = (); - type OnTransfer = (); + type CurrencyHooks = CurrencyHooks; type MaxLocks = ConstU32<100_000>; type MaxReserves = ConstU32<100_000>; type ReserveIdentifier = ReserveIdentifier; type DustRemovalWhitelist = Nothing; - type OnNewTokenAccount = (); - type OnKilledTokenAccount = (); } pub const NATIVE_CURRENCY_ID: CurrencyId = 1; diff --git a/payments/src/mock.rs b/payments/src/mock.rs index ac32214ba..53bc87e3c 100644 --- a/payments/src/mock.rs +++ b/payments/src/mock.rs @@ -98,17 +98,12 @@ impl orml_tokens::Config for Test { type CurrencyId = u32; type RuntimeEvent = RuntimeEvent; type ExistentialDeposits = ExistentialDeposits; - type OnDust = (); - type OnSlash = (); - type OnDeposit = (); - type OnTransfer = (); + type CurrencyHooks = (); type WeightInfo = (); type MaxLocks = MaxLocks; type DustRemovalWhitelist = MockDustRemovalWhitelist; type MaxReserves = ConstU32<2>; type ReserveIdentifier = ReserveIdentifier; - type OnNewTokenAccount = (); - type OnKilledTokenAccount = (); } pub struct MockDisputeResolver; diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index 59a9db044..6d0803f63 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -66,9 +66,9 @@ use sp_std::{cmp, convert::Infallible, marker, prelude::*, vec::Vec}; use orml_traits::{ arithmetic::{self, Signed}, - currency::{OnDeposit, OnSlash, OnTransfer, TransferAll}, + currency::{CurrencyMutationHooks, OnDeposit, OnDust, OnSlash, OnTransfer, TransferAll}, BalanceStatus, GetByKey, Happened, LockIdentifier, MultiCurrency, MultiCurrencyExtended, MultiLockableCurrency, - MultiReservableCurrency, NamedMultiReservableCurrency, OnDust, + MultiReservableCurrency, NamedMultiReservableCurrency, }; mod imbalances; @@ -173,7 +173,7 @@ pub use module::*; #[frame_support::pallet] pub mod module { - use orml_traits::currency::{OnDeposit, OnSlash, OnTransfer}; + use orml_traits::currency::CurrencyMutationHooks; use super::*; @@ -216,23 +216,9 @@ pub mod module { /// System::AccountInfo, zero ED may cause some problems. type ExistentialDeposits: GetByKey; - /// Handler to burn or transfer account's dust - type OnDust: OnDust; - - /// Hook to run before slashing an account. - type OnSlash: OnSlash; - - /// Hook to run before depositing into an account. - type OnDeposit: OnDeposit; - - /// Hook to run before transferring from an account to another. - type OnTransfer: OnTransfer; - - /// Handler for when an account was created - type OnNewTokenAccount: Happened<(Self::AccountId, Self::CurrencyId)>; - - /// Handler for when an account was created - type OnKilledTokenAccount: Happened<(Self::AccountId, Self::CurrencyId)>; + /// Hooks are actions that are executed on certain events. + /// For example: OnDust, OnNewTokenAccount + type CurrencyHooks: CurrencyMutationHooks; #[pallet::constant] type MaxLocks: Get; @@ -765,11 +751,11 @@ impl Pallet { // Ignore the result, because if it failed then there are remaining consumers, // and the account storage in frame_system shouldn't be reaped. let _ = frame_system::Pallet::::dec_providers(who); - T::OnKilledTokenAccount::happened(&(who.clone(), currency_id)); + >::OnKilledTokenAccount::happened(&(who.clone(), currency_id)); } else if !existed && exists { // if new, increase account provider frame_system::Pallet::::inc_providers(who); - T::OnNewTokenAccount::happened(&(who.clone(), currency_id)); + >::OnNewTokenAccount::happened(&(who.clone(), currency_id)); } if let Some(endowed) = maybe_endowed { @@ -783,7 +769,7 @@ impl Pallet { if let Some(dust_amount) = maybe_dust { // `OnDust` maybe get/set storage `Accounts` of `who`, trigger handler here // to avoid some unexpected errors. - T::OnDust::on_dust(who, currency_id, dust_amount); + >::OnDust::on_dust(who, currency_id, dust_amount); Self::deposit_event(Event::DustLost { currency_id, @@ -906,7 +892,12 @@ impl Pallet { return Ok(()); } - T::OnTransfer::on_transfer(currency_id, from, to, amount)?; + >::OnTransfer::on_transfer( + currency_id, + from, + to, + amount, + )?; Self::try_mutate_account(to, currency_id, |to_account, _existed| -> DispatchResult { Self::try_mutate_account(from, currency_id, |from_account, _existed| -> DispatchResult { from_account.free = from_account @@ -1032,7 +1023,11 @@ impl Pallet { return Ok(()); } - T::OnDeposit::on_deposit(currency_id, who, amount)?; + >::OnDeposit::on_deposit( + currency_id, + who, + amount, + )?; Self::try_mutate_account(who, currency_id, |account, existed| -> DispatchResult { if require_existed { ensure!(existed, Error::::DeadAccount); @@ -1128,7 +1123,11 @@ impl MultiCurrency for Pallet { return amount; } - T::OnSlash::on_slash(currency_id, who, amount); + >::OnSlash::on_slash( + currency_id, + who, + amount, + ); let account = Self::accounts(who, currency_id); let free_slashed_amount = account.free.min(amount); // Cannot underflow because free_slashed_amount can never be greater than amount @@ -1295,7 +1294,11 @@ impl MultiReservableCurrency for Pallet { return value; } - T::OnSlash::on_slash(currency_id, who, value); + >::OnSlash::on_slash( + currency_id, + who, + value, + ); let reserved_balance = Self::reserved_balance(currency_id, who); let actual = reserved_balance.min(value); Self::mutate_account(who, currency_id, |account, _| { diff --git a/tokens/src/mock.rs b/tokens/src/mock.rs index 0a4346a27..1b552e0b9 100644 --- a/tokens/src/mock.rs +++ b/tokens/src/mock.rs @@ -228,10 +228,18 @@ thread_local! { pub static KILLED: RefCell> = RefCell::new(vec![]); } -pub struct TrackCreatedAccounts; -impl TrackCreatedAccounts { - pub fn accounts() -> Vec<(AccountId, CurrencyId)> { - CREATED.with(|accounts| accounts.borrow().clone()) +pub struct TrackCreatedAccounts(marker::PhantomData); +impl TrackCreatedAccounts +where + T::AccountId: From + Into, + T::CurrencyId: From + Into, +{ + pub fn accounts() -> Vec<(T::AccountId, T::CurrencyId)> { + CREATED + .with(|accounts| accounts.borrow().clone()) + .iter() + .map(|account| (account.0.clone().into(), account.1.clone().into())) + .collect() } pub fn reset() { @@ -240,18 +248,30 @@ impl TrackCreatedAccounts { }); } } -impl Happened<(AccountId, CurrencyId)> for TrackCreatedAccounts { - fn happened((who, currency): &(AccountId, CurrencyId)) { +impl Happened<(T::AccountId, T::CurrencyId)> for TrackCreatedAccounts +where + T::AccountId: From + Into, + T::CurrencyId: From + Into, +{ + fn happened((who, currency): &(T::AccountId, T::CurrencyId)) { CREATED.with(|accounts| { - accounts.borrow_mut().push((who.clone(), *currency)); + accounts.borrow_mut().push((who.clone().into(), (*currency).into())); }); } } -pub struct TrackKilledAccounts; -impl TrackKilledAccounts { - pub fn accounts() -> Vec<(AccountId, CurrencyId)> { - KILLED.with(|accounts| accounts.borrow().clone()) +pub struct TrackKilledAccounts(marker::PhantomData); +impl TrackKilledAccounts +where + T::AccountId: From + Into, + T::CurrencyId: From + Into, +{ + pub fn accounts() -> Vec<(T::AccountId, T::CurrencyId)> { + KILLED + .with(|accounts| accounts.borrow().clone()) + .iter() + .map(|account| (account.0.clone().into(), account.1.clone().into())) + .collect() } pub fn reset() { @@ -260,10 +280,14 @@ impl TrackKilledAccounts { }); } } -impl Happened<(AccountId, CurrencyId)> for TrackKilledAccounts { - fn happened((who, currency): &(AccountId, CurrencyId)) { +impl Happened<(T::AccountId, T::CurrencyId)> for TrackKilledAccounts +where + T::AccountId: From + Into, + T::CurrencyId: From + Into, +{ + fn happened((who, currency): &(T::AccountId, T::CurrencyId)) { KILLED.with(|accounts| { - accounts.borrow_mut().push((who.clone(), *currency)); + accounts.borrow_mut().push((who.clone().into(), (*currency).into())); }); } } @@ -275,8 +299,8 @@ thread_local! { } pub struct OnSlashHook(marker::PhantomData); -impl OnSlash for OnSlashHook { - fn on_slash(_currency_id: CurrencyId, _account_id: &T::AccountId, _amount: Balance) { +impl OnSlash for OnSlashHook { + fn on_slash(_currency_id: T::CurrencyId, _account_id: &T::AccountId, _amount: T::Balance) { ON_SLASH_CALLS.with(|cell| *cell.borrow_mut() += 1); } } @@ -287,8 +311,8 @@ impl OnSlashHook { } pub struct OnDepositHook(marker::PhantomData); -impl OnDeposit for OnDepositHook { - fn on_deposit(_currency_id: CurrencyId, _account_id: &T::AccountId, _amount: Balance) -> DispatchResult { +impl OnDeposit for OnDepositHook { + fn on_deposit(_currency_id: T::CurrencyId, _account_id: &T::AccountId, _amount: T::Balance) -> DispatchResult { ON_DEPOSIT_CALLS.with(|cell| *cell.borrow_mut() += 1); Ok(()) } @@ -300,12 +324,12 @@ impl OnDepositHook { } pub struct OnTransferHook(marker::PhantomData); -impl OnTransfer for OnTransferHook { +impl OnTransfer for OnTransferHook { fn on_transfer( - _currency_id: CurrencyId, + _currency_id: T::CurrencyId, _from: &T::AccountId, _to: &T::AccountId, - _amount: Balance, + _amount: T::Balance, ) -> DispatchResult { ON_TRANSFER_CALLS.with(|cell| *cell.borrow_mut() += 1); Ok(()) @@ -321,6 +345,20 @@ parameter_types! { pub DustReceiver: AccountId = PalletId(*b"orml/dst").into_account_truncating(); } +pub struct CurrencyHooks(marker::PhantomData); +impl CurrencyMutationHooks for CurrencyHooks +where + T::AccountId: From + Into, + T::CurrencyId: From + Into, +{ + type OnDust = TransferDust; + type OnSlash = OnSlashHook; + type OnDeposit = OnDepositHook; + type OnTransfer = OnTransferHook; + type OnNewTokenAccount = TrackCreatedAccounts; + type OnKilledTokenAccount = TrackKilledAccounts; +} + impl Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = Balance; @@ -328,12 +366,7 @@ impl Config for Runtime { type CurrencyId = CurrencyId; type WeightInfo = (); type ExistentialDeposits = ExistentialDeposits; - type OnDust = TransferDust; - type OnSlash = OnSlashHook; - type OnDeposit = OnDepositHook; - type OnTransfer = OnTransferHook; - type OnNewTokenAccount = TrackCreatedAccounts; - type OnKilledTokenAccount = TrackKilledAccounts; + type CurrencyHooks = CurrencyHooks; type MaxLocks = ConstU32<2>; type MaxReserves = ConstU32<2>; type ReserveIdentifier = ReserveIdentifier; @@ -398,8 +431,8 @@ impl ExtBuilder { .unwrap(); } - TrackCreatedAccounts::reset(); - TrackKilledAccounts::reset(); + TrackCreatedAccounts::::reset(); + TrackKilledAccounts::::reset(); let mut ext = sp_io::TestExternalities::new(t); ext.execute_with(|| System::set_block_number(1)); diff --git a/tokens/src/tests.rs b/tokens/src/tests.rs index 12feb02ea..8d46688f2 100644 --- a/tokens/src/tests.rs +++ b/tokens/src/tests.rs @@ -55,7 +55,7 @@ fn transfer_should_work() { Error::::ExistentialDeposit, ); assert_ok!(Tokens::transfer(Some(ALICE).into(), CHARLIE, DOT, 2)); - assert_eq!(TrackCreatedAccounts::accounts(), vec![(CHARLIE, DOT)]); + assert_eq!(TrackCreatedAccounts::::accounts(), vec![(CHARLIE, DOT)]); // imply AllowDeath assert!(Accounts::::contains_key(ALICE, DOT)); @@ -132,7 +132,7 @@ fn transfer_all_allow_death_should_work() { assert!(Accounts::::contains_key(ALICE, DOT)); assert_eq!(Tokens::free_balance(DOT, &ALICE), 100); assert_ok!(Tokens::transfer_all(Some(ALICE).into(), CHARLIE, DOT, false)); - assert_eq!(TrackCreatedAccounts::accounts(), vec![(CHARLIE, DOT)]); + assert_eq!(TrackCreatedAccounts::::accounts(), vec![(CHARLIE, DOT)]); System::assert_last_event(RuntimeEvent::Tokens(crate::Event::Transfer { currency_id: DOT, from: ALICE, @@ -141,7 +141,7 @@ fn transfer_all_allow_death_should_work() { })); assert!(!Accounts::::contains_key(ALICE, DOT)); assert_eq!(Tokens::free_balance(DOT, &ALICE), 0); - assert_eq!(TrackKilledAccounts::accounts(), vec![(ALICE, DOT)]); + assert_eq!(TrackKilledAccounts::::accounts(), vec![(ALICE, DOT)]); assert_ok!(Tokens::set_lock(ID_1, DOT, &BOB, 50)); assert_eq!(Tokens::accounts(&BOB, DOT).frozen, 50); @@ -179,7 +179,7 @@ fn force_transfer_should_work() { amount: 100, })); assert!(!Accounts::::contains_key(ALICE, DOT)); - assert_eq!(TrackKilledAccounts::accounts(), vec![(ALICE, DOT)]); + assert_eq!(TrackKilledAccounts::::accounts(), vec![(ALICE, DOT)]); assert_eq!(Tokens::free_balance(DOT, &ALICE), 0); assert_eq!(Tokens::free_balance(DOT, &BOB), 200); }); @@ -1154,17 +1154,20 @@ fn exceeding_max_reserves_should_fail() { fn lifecycle_callbacks_are_activated() { ExtBuilder::default().build().execute_with(|| { assert_ok!(Tokens::set_balance(RawOrigin::Root.into(), ALICE, DOT, 200, 0)); - assert_eq!(TrackCreatedAccounts::accounts(), vec![(ALICE, DOT)]); + assert_eq!(TrackCreatedAccounts::::accounts(), vec![(ALICE, DOT)]); assert_ok!(Tokens::set_balance(RawOrigin::Root.into(), ALICE, BTC, 200, 0)); - assert_eq!(TrackCreatedAccounts::accounts(), vec![(ALICE, DOT), (ALICE, BTC)]); + assert_eq!( + TrackCreatedAccounts::::accounts(), + vec![(ALICE, DOT), (ALICE, BTC)] + ); assert_ok!(Tokens::transfer_all(Some(ALICE).into(), CHARLIE, BTC, false)); assert_eq!( - TrackCreatedAccounts::accounts(), + TrackCreatedAccounts::::accounts(), vec![(ALICE, DOT), (ALICE, BTC), (CHARLIE, BTC)] ); - assert_eq!(TrackKilledAccounts::accounts(), vec![(ALICE, BTC)]); + assert_eq!(TrackKilledAccounts::::accounts(), vec![(ALICE, BTC)]); }) } diff --git a/traits/src/currency.rs b/traits/src/currency.rs index ce369c3b4..851824b2b 100644 --- a/traits/src/currency.rs +++ b/traits/src/currency.rs @@ -1,4 +1,4 @@ -use crate::arithmetic; +use crate::{arithmetic, Happened}; use codec::{Codec, FullCodec, MaxEncodedLen}; pub use frame_support::{ traits::{BalanceStatus, LockIdentifier}, @@ -688,3 +688,32 @@ impl OnTransfer Ok(()) } } + +pub trait CurrencyMutationHooks { + /// Handler to burn or transfer account's dust + type OnDust: OnDust; + + /// Hook to run before slashing an account. + type OnSlash: OnSlash; + + /// Hook to run before depositing into an account. + type OnDeposit: OnDeposit; + + /// Hook to run before transferring from an account to another. + type OnTransfer: OnTransfer; + + /// Handler for when an account was created + type OnNewTokenAccount: Happened<(AccountId, CurrencyId)>; + + /// Handler for when an account was created + type OnKilledTokenAccount: Happened<(AccountId, CurrencyId)>; +} + +impl CurrencyMutationHooks for () { + type OnDust = (); + type OnSlash = (); + type OnDeposit = (); + type OnTransfer = (); + type OnNewTokenAccount = (); + type OnKilledTokenAccount = (); +} diff --git a/traits/src/lib.rs b/traits/src/lib.rs index 818d51251..a5894d218 100644 --- a/traits/src/lib.rs +++ b/traits/src/lib.rs @@ -16,7 +16,7 @@ pub use auction::{Auction, AuctionHandler, AuctionInfo, OnNewBidResult}; pub use currency::{ BalanceStatus, BasicCurrency, BasicCurrencyExtended, BasicLockableCurrency, BasicReservableCurrency, LockIdentifier, MultiCurrency, MultiCurrencyExtended, MultiLockableCurrency, MultiReservableCurrency, - NamedBasicReservableCurrency, NamedMultiReservableCurrency, OnDust, + NamedBasicReservableCurrency, NamedMultiReservableCurrency, }; pub use data_provider::{DataFeeder, DataProvider, DataProviderExtended}; pub use get_by_key::GetByKey; diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 830ad48f4..6a91b6185 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -83,16 +83,11 @@ impl orml_tokens::Config for Runtime { type CurrencyId = CurrencyId; type WeightInfo = (); type ExistentialDeposits = ExistentialDeposits; - type OnDust = (); - type OnSlash = (); - type OnDeposit = (); - type OnTransfer = (); + type CurrencyHooks = (); type MaxLocks = ConstU32<50>; type MaxReserves = ConstU32<50>; type ReserveIdentifier = [u8; 8]; type DustRemovalWhitelist = Everything; - type OnNewTokenAccount = (); - type OnKilledTokenAccount = (); } parameter_types! { diff --git a/xtokens/src/mock/para_relative_view.rs b/xtokens/src/mock/para_relative_view.rs index 064cdb0db..357e5cd07 100644 --- a/xtokens/src/mock/para_relative_view.rs +++ b/xtokens/src/mock/para_relative_view.rs @@ -86,16 +86,11 @@ impl orml_tokens::Config for Runtime { type CurrencyId = CurrencyId; type WeightInfo = (); type ExistentialDeposits = ExistentialDeposits; - type OnDust = (); - type OnSlash = (); - type OnDeposit = (); - type OnTransfer = (); + type CurrencyHooks = (); type MaxLocks = ConstU32<50>; type MaxReserves = ConstU32<50>; type ReserveIdentifier = [u8; 8]; type DustRemovalWhitelist = Everything; - type OnNewTokenAccount = (); - type OnKilledTokenAccount = (); } parameter_types! { diff --git a/xtokens/src/mock/para_teleport.rs b/xtokens/src/mock/para_teleport.rs index a9660e07f..01bc013e5 100644 --- a/xtokens/src/mock/para_teleport.rs +++ b/xtokens/src/mock/para_teleport.rs @@ -84,16 +84,11 @@ impl orml_tokens::Config for Runtime { type CurrencyId = CurrencyId; type WeightInfo = (); type ExistentialDeposits = ExistentialDeposits; - type OnDust = (); - type OnSlash = (); - type OnDeposit = (); - type OnTransfer = (); + type CurrencyHooks = (); type MaxLocks = ConstU32<50>; type MaxReserves = ConstU32<50>; type ReserveIdentifier = [u8; 8]; type DustRemovalWhitelist = Everything; - type OnNewTokenAccount = (); - type OnKilledTokenAccount = (); } parameter_types! { From 6c57463f7629511bfcecfdb19c6e7a58f4d27e14 Mon Sep 17 00:00:00 2001 From: Daniel Savu Date: Sat, 5 Nov 2022 15:25:07 +0100 Subject: [PATCH 2/3] feat(tokens): Add post-hooks for Transfer and Deposit --- currencies/src/mock.rs | 6 ++-- tokens/src/lib.rs | 15 ++++++++-- tokens/src/mock.rs | 63 ++++++++++++++++++++++++++++++++---------- tokens/src/tests.rs | 34 +++++++++++++++-------- traits/src/currency.rs | 16 ++++++++--- 5 files changed, 100 insertions(+), 34 deletions(-) diff --git a/currencies/src/mock.rs b/currencies/src/mock.rs index e42804cc2..f7ebb8f4d 100644 --- a/currencies/src/mock.rs +++ b/currencies/src/mock.rs @@ -80,8 +80,10 @@ where { type OnDust = orml_tokens::TransferDust; type OnSlash = (); - type OnDeposit = (); - type OnTransfer = (); + type DepositPreHook = (); + type DepositPostHook = (); + type TransferPreHook = (); + type TransferPostHook = (); type OnNewTokenAccount = (); type OnKilledTokenAccount = (); } diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index 6d0803f63..6df56c187 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -892,7 +892,7 @@ impl Pallet { return Ok(()); } - >::OnTransfer::on_transfer( + >::TransferPreHook::on_transfer( currency_id, from, to, @@ -937,6 +937,12 @@ impl Pallet { Ok(()) })?; + >::TransferPostHook::on_transfer( + currency_id, + from, + to, + amount, + )?; Self::deposit_event(Event::Transfer { currency_id, from: from.clone(), @@ -1023,7 +1029,7 @@ impl Pallet { return Ok(()); } - >::OnDeposit::on_deposit( + >::DepositPreHook::on_deposit( currency_id, who, amount, @@ -1049,6 +1055,11 @@ impl Pallet { } account.free += amount; + >::DepositPostHook::on_deposit( + currency_id, + who, + amount, + )?; Self::deposit_event(Event::Deposited { currency_id, who: who.clone(), diff --git a/tokens/src/mock.rs b/tokens/src/mock.rs index 1b552e0b9..a18cedd0c 100644 --- a/tokens/src/mock.rs +++ b/tokens/src/mock.rs @@ -294,8 +294,10 @@ where thread_local! { pub static ON_SLASH_CALLS: RefCell = RefCell::new(0); - pub static ON_DEPOSIT_CALLS: RefCell = RefCell::new(0); - pub static ON_TRANSFER_CALLS: RefCell = RefCell::new(0); + pub static ON_DEPOSIT_PREHOOK_CALLS: RefCell = RefCell::new(0); + pub static ON_DEPOSIT_POSTHOOK_CALLS: RefCell = RefCell::new(0); + pub static ON_TRANSFER_PREHOOK_CALLS: RefCell = RefCell::new(0); + pub static ON_TRANSFER_POSTHOOK_CALLS: RefCell = RefCell::new(0); } pub struct OnSlashHook(marker::PhantomData); @@ -310,34 +312,65 @@ impl OnSlashHook { } } -pub struct OnDepositHook(marker::PhantomData); -impl OnDeposit for OnDepositHook { +pub struct DepositPreHook(marker::PhantomData); +impl OnDeposit for DepositPreHook { fn on_deposit(_currency_id: T::CurrencyId, _account_id: &T::AccountId, _amount: T::Balance) -> DispatchResult { - ON_DEPOSIT_CALLS.with(|cell| *cell.borrow_mut() += 1); + ON_DEPOSIT_PREHOOK_CALLS.with(|cell| *cell.borrow_mut() += 1); Ok(()) } } -impl OnDepositHook { +impl DepositPreHook { pub fn calls() -> u32 { - ON_DEPOSIT_CALLS.with(|accounts| accounts.borrow().clone()) + ON_DEPOSIT_PREHOOK_CALLS.with(|accounts| accounts.borrow().clone()) } } -pub struct OnTransferHook(marker::PhantomData); -impl OnTransfer for OnTransferHook { +pub struct DepositPostHook(marker::PhantomData); +impl OnDeposit for DepositPostHook { + fn on_deposit(_currency_id: T::CurrencyId, _account_id: &T::AccountId, _amount: T::Balance) -> DispatchResult { + ON_DEPOSIT_POSTHOOK_CALLS.with(|cell| *cell.borrow_mut() += 1); + Ok(()) + } +} +impl DepositPostHook { + pub fn calls() -> u32 { + ON_DEPOSIT_POSTHOOK_CALLS.with(|accounts| accounts.borrow().clone()) + } +} + +pub struct TransferPreHook(marker::PhantomData); +impl OnTransfer for TransferPreHook { + fn on_transfer( + _currency_id: T::CurrencyId, + _from: &T::AccountId, + _to: &T::AccountId, + _amount: T::Balance, + ) -> DispatchResult { + ON_TRANSFER_PREHOOK_CALLS.with(|cell| *cell.borrow_mut() += 1); + Ok(()) + } +} +impl TransferPreHook { + pub fn calls() -> u32 { + ON_TRANSFER_PREHOOK_CALLS.with(|accounts| accounts.borrow().clone()) + } +} + +pub struct TransferPostHook(marker::PhantomData); +impl OnTransfer for TransferPostHook { fn on_transfer( _currency_id: T::CurrencyId, _from: &T::AccountId, _to: &T::AccountId, _amount: T::Balance, ) -> DispatchResult { - ON_TRANSFER_CALLS.with(|cell| *cell.borrow_mut() += 1); + ON_TRANSFER_POSTHOOK_CALLS.with(|cell| *cell.borrow_mut() += 1); Ok(()) } } -impl OnTransferHook { +impl TransferPostHook { pub fn calls() -> u32 { - ON_TRANSFER_CALLS.with(|accounts| accounts.borrow().clone()) + ON_TRANSFER_POSTHOOK_CALLS.with(|accounts| accounts.borrow().clone()) } } @@ -353,8 +386,10 @@ where { type OnDust = TransferDust; type OnSlash = OnSlashHook; - type OnDeposit = OnDepositHook; - type OnTransfer = OnTransferHook; + type DepositPreHook = DepositPreHook; + type DepositPostHook = DepositPostHook; + type TransferPreHook = TransferPreHook; + type TransferPostHook = TransferPostHook; type OnNewTokenAccount = TrackCreatedAccounts; type OnKilledTokenAccount = TrackKilledAccounts; } diff --git a/tokens/src/tests.rs b/tokens/src/tests.rs index 8d46688f2..3d0e89523 100644 --- a/tokens/src/tests.rs +++ b/tokens/src/tests.rs @@ -1177,31 +1177,37 @@ fn lifecycle_callbacks_are_activated() { // ************************************************* #[test] -fn deposit_hook_works() { +fn deposit_hooks_work() { ExtBuilder::default().build().execute_with(|| { - let initial_hook_calls = OnDepositHook::::calls(); + let initial_prehook_calls = DepositPreHook::::calls(); + let initial_posthook_calls = DepositPostHook::::calls(); assert_ok!(Tokens::do_deposit(DOT, &CHARLIE, 0, false, true),); - assert_eq!(OnDepositHook::::calls(), initial_hook_calls); + assert_eq!(DepositPreHook::::calls(), initial_prehook_calls); + assert_eq!(DepositPostHook::::calls(), initial_posthook_calls); assert_ok!(Tokens::do_deposit(DOT, &CHARLIE, 100, false, true),); - assert_eq!(OnDepositHook::::calls(), initial_hook_calls + 1); + assert_eq!(DepositPreHook::::calls(), initial_prehook_calls + 1); + assert_eq!(DepositPostHook::::calls(), initial_posthook_calls + 1); - // The hook must be called even if the actual deposit ends up failing assert_noop!( Tokens::do_deposit(DOT, &BOB, 1, false, true), Error::::ExistentialDeposit ); - assert_eq!(OnDepositHook::::calls(), initial_hook_calls + 2); + // The prehook is called + assert_eq!(DepositPreHook::::calls(), initial_prehook_calls + 2); + // The posthook is not called + assert_eq!(DepositPostHook::::calls(), initial_posthook_calls + 1); }); } #[test] -fn transfer_hook_works() { +fn transfer_hooks_work() { ExtBuilder::default() .balances(vec![(ALICE, DOT, 100)]) .build() .execute_with(|| { - let initial_hook_calls = OnTransferHook::::calls(); + let initial_prehook_calls = TransferPreHook::::calls(); + let initial_posthook_calls = TransferPostHook::::calls(); assert_ok!(Tokens::do_transfer( DOT, &ALICE, @@ -1209,7 +1215,8 @@ fn transfer_hook_works() { 0, ExistenceRequirement::AllowDeath ),); - assert_eq!(OnTransferHook::::calls(), initial_hook_calls); + assert_eq!(TransferPreHook::::calls(), initial_prehook_calls); + assert_eq!(TransferPostHook::::calls(), initial_posthook_calls); assert_ok!(Tokens::do_transfer( DOT, @@ -1218,13 +1225,16 @@ fn transfer_hook_works() { 10, ExistenceRequirement::AllowDeath )); - assert_eq!(OnTransferHook::::calls(), initial_hook_calls + 1); + assert_eq!(TransferPreHook::::calls(), initial_prehook_calls + 1); + assert_eq!(TransferPostHook::::calls(), initial_posthook_calls + 1); - // The hook must be called even if the actual transfer ends up failing assert_noop!( Tokens::do_transfer(DOT, &ALICE, &BOB, 1, ExistenceRequirement::AllowDeath), Error::::ExistentialDeposit ); - assert_eq!(OnTransferHook::::calls(), initial_hook_calls + 2); + // The prehook is called + assert_eq!(TransferPreHook::::calls(), initial_prehook_calls + 2); + // The posthook is not called + assert_eq!(TransferPostHook::::calls(), initial_posthook_calls + 1); }); } diff --git a/traits/src/currency.rs b/traits/src/currency.rs index 851824b2b..cd0f4b66c 100644 --- a/traits/src/currency.rs +++ b/traits/src/currency.rs @@ -697,10 +697,16 @@ pub trait CurrencyMutationHooks { type OnSlash: OnSlash; /// Hook to run before depositing into an account. - type OnDeposit: OnDeposit; + type DepositPreHook: OnDeposit; + + /// Hook to run after depositing into an account. + type DepositPostHook: OnDeposit; /// Hook to run before transferring from an account to another. - type OnTransfer: OnTransfer; + type TransferPreHook: OnTransfer; + + /// Hook to run after transferring from an account to another. + type TransferPostHook: OnTransfer; /// Handler for when an account was created type OnNewTokenAccount: Happened<(AccountId, CurrencyId)>; @@ -712,8 +718,10 @@ pub trait CurrencyMutationHooks { impl CurrencyMutationHooks for () { type OnDust = (); type OnSlash = (); - type OnDeposit = (); - type OnTransfer = (); + type DepositPreHook = (); + type DepositPostHook = (); + type TransferPreHook = (); + type TransferPostHook = (); type OnNewTokenAccount = (); type OnKilledTokenAccount = (); } From 192d2bc00acb27dd47ed53e5717f48d42fae535e Mon Sep 17 00:00:00 2001 From: Daniel Savu Date: Wed, 9 Nov 2022 14:58:42 +0100 Subject: [PATCH 3/3] chore(tokens): improve mutation hook naming --- currencies/src/mock.rs | 12 ++++++------ tokens/src/lib.rs | 24 ++++++++++++------------ tokens/src/mock.rs | 34 +++++++++++++++++----------------- tokens/src/tests.rs | 32 ++++++++++++++++---------------- traits/src/currency.rs | 20 ++++++++++---------- 5 files changed, 61 insertions(+), 61 deletions(-) diff --git a/currencies/src/mock.rs b/currencies/src/mock.rs index 72402650e..cb837a4dd 100644 --- a/currencies/src/mock.rs +++ b/currencies/src/mock.rs @@ -8,7 +8,7 @@ use frame_support::{ traits::{ConstU32, ConstU64, Everything, Nothing}, PalletId, }; -use orml_traits::{currency::CurrencyMutationHooks, parameter_type_with_key}; +use orml_traits::{currency::MutationHooks, parameter_type_with_key}; use sp_core::H256; use sp_runtime::{ testing::Header, @@ -74,16 +74,16 @@ parameter_types! { } pub struct CurrencyHooks(marker::PhantomData); -impl CurrencyMutationHooks for CurrencyHooks +impl MutationHooks for CurrencyHooks where T::AccountId: From, { type OnDust = orml_tokens::TransferDust; type OnSlash = (); - type DepositPreHook = (); - type DepositPostHook = (); - type TransferPreHook = (); - type TransferPostHook = (); + type PreDeposit = (); + type PostDeposit = (); + type PreTransfer = (); + type PostTransfer = (); type OnNewTokenAccount = (); type OnKilledTokenAccount = (); } diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index b27848bce..59be2f68b 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -66,7 +66,7 @@ use sp_std::{cmp, convert::Infallible, marker, prelude::*, vec::Vec}; use orml_traits::{ arithmetic::{self, Signed}, - currency::{CurrencyMutationHooks, OnDeposit, OnDust, OnSlash, OnTransfer, TransferAll}, + currency::{MutationHooks, OnDeposit, OnDust, OnSlash, OnTransfer, TransferAll}, BalanceStatus, GetByKey, Happened, LockIdentifier, MultiCurrency, MultiCurrencyExtended, MultiLockableCurrency, MultiReservableCurrency, NamedMultiReservableCurrency, }; @@ -173,7 +173,7 @@ pub use module::*; #[frame_support::pallet] pub mod module { - use orml_traits::currency::CurrencyMutationHooks; + use orml_traits::currency::MutationHooks; use super::*; @@ -218,7 +218,7 @@ pub mod module { /// Hooks are actions that are executed on certain events. /// For example: OnDust, OnNewTokenAccount - type CurrencyHooks: CurrencyMutationHooks; + type CurrencyHooks: MutationHooks; #[pallet::constant] type MaxLocks: Get; @@ -751,11 +751,11 @@ impl Pallet { // Ignore the result, because if it failed then there are remaining consumers, // and the account storage in frame_system shouldn't be reaped. let _ = frame_system::Pallet::::dec_providers(who); - >::OnKilledTokenAccount::happened(&(who.clone(), currency_id)); + >::OnKilledTokenAccount::happened(&(who.clone(), currency_id)); } else if !existed && exists { // if new, increase account provider frame_system::Pallet::::inc_providers(who); - >::OnNewTokenAccount::happened(&(who.clone(), currency_id)); + >::OnNewTokenAccount::happened(&(who.clone(), currency_id)); } if let Some(endowed) = maybe_endowed { @@ -769,7 +769,7 @@ impl Pallet { if let Some(dust_amount) = maybe_dust { // `OnDust` maybe get/set storage `Accounts` of `who`, trigger handler here // to avoid some unexpected errors. - >::OnDust::on_dust(who, currency_id, dust_amount); + >::OnDust::on_dust(who, currency_id, dust_amount); Self::deposit_event(Event::DustLost { currency_id, @@ -892,7 +892,7 @@ impl Pallet { return Ok(()); } - >::TransferPreHook::on_transfer( + >::PreTransfer::on_transfer( currency_id, from, to, @@ -937,7 +937,7 @@ impl Pallet { Ok(()) })?; - >::TransferPostHook::on_transfer( + >::PostTransfer::on_transfer( currency_id, from, to, @@ -1029,7 +1029,7 @@ impl Pallet { return Ok(()); } - >::DepositPreHook::on_deposit( + >::PreDeposit::on_deposit( currency_id, who, amount, @@ -1055,7 +1055,7 @@ impl Pallet { } account.free = account.free.defensive_saturating_add(amount); - >::DepositPostHook::on_deposit( + >::PostDeposit::on_deposit( currency_id, who, amount, @@ -1134,7 +1134,7 @@ impl MultiCurrency for Pallet { return amount; } - >::OnSlash::on_slash( + >::OnSlash::on_slash( currency_id, who, amount, @@ -1316,7 +1316,7 @@ impl MultiReservableCurrency for Pallet { return value; } - >::OnSlash::on_slash( + >::OnSlash::on_slash( currency_id, who, value, diff --git a/tokens/src/mock.rs b/tokens/src/mock.rs index 9e8f99b53..ca5592c89 100644 --- a/tokens/src/mock.rs +++ b/tokens/src/mock.rs @@ -312,34 +312,34 @@ impl OnSlashHook { } } -pub struct DepositPreHook(marker::PhantomData); -impl OnDeposit for DepositPreHook { +pub struct PreDeposit(marker::PhantomData); +impl OnDeposit for PreDeposit { fn on_deposit(_currency_id: T::CurrencyId, _account_id: &T::AccountId, _amount: T::Balance) -> DispatchResult { ON_DEPOSIT_PREHOOK_CALLS.with(|cell| *cell.borrow_mut() += 1); Ok(()) } } -impl DepositPreHook { +impl PreDeposit { pub fn calls() -> u32 { ON_DEPOSIT_PREHOOK_CALLS.with(|accounts| accounts.borrow().clone()) } } -pub struct DepositPostHook(marker::PhantomData); -impl OnDeposit for DepositPostHook { +pub struct PostDeposit(marker::PhantomData); +impl OnDeposit for PostDeposit { fn on_deposit(_currency_id: T::CurrencyId, _account_id: &T::AccountId, _amount: T::Balance) -> DispatchResult { ON_DEPOSIT_POSTHOOK_CALLS.with(|cell| *cell.borrow_mut() += 1); Ok(()) } } -impl DepositPostHook { +impl PostDeposit { pub fn calls() -> u32 { ON_DEPOSIT_POSTHOOK_CALLS.with(|accounts| accounts.borrow().clone()) } } -pub struct TransferPreHook(marker::PhantomData); -impl OnTransfer for TransferPreHook { +pub struct PreTransfer(marker::PhantomData); +impl OnTransfer for PreTransfer { fn on_transfer( _currency_id: T::CurrencyId, _from: &T::AccountId, @@ -350,14 +350,14 @@ impl OnTransfer for Transfer Ok(()) } } -impl TransferPreHook { +impl PreTransfer { pub fn calls() -> u32 { ON_TRANSFER_PREHOOK_CALLS.with(|accounts| accounts.borrow().clone()) } } -pub struct TransferPostHook(marker::PhantomData); -impl OnTransfer for TransferPostHook { +pub struct PostTransfer(marker::PhantomData); +impl OnTransfer for PostTransfer { fn on_transfer( _currency_id: T::CurrencyId, _from: &T::AccountId, @@ -368,7 +368,7 @@ impl OnTransfer for Transfer Ok(()) } } -impl TransferPostHook { +impl PostTransfer { pub fn calls() -> u32 { ON_TRANSFER_POSTHOOK_CALLS.with(|accounts| accounts.borrow().clone()) } @@ -379,17 +379,17 @@ parameter_types! { } pub struct CurrencyHooks(marker::PhantomData); -impl CurrencyMutationHooks for CurrencyHooks +impl MutationHooks for CurrencyHooks where T::AccountId: From + Into, T::CurrencyId: From + Into, { type OnDust = TransferDust; type OnSlash = OnSlashHook; - type DepositPreHook = DepositPreHook; - type DepositPostHook = DepositPostHook; - type TransferPreHook = TransferPreHook; - type TransferPostHook = TransferPostHook; + type PreDeposit = PreDeposit; + type PostDeposit = PostDeposit; + type PreTransfer = PreTransfer; + type PostTransfer = PostTransfer; type OnNewTokenAccount = TrackCreatedAccounts; type OnKilledTokenAccount = TrackKilledAccounts; } diff --git a/tokens/src/tests.rs b/tokens/src/tests.rs index 3d0e89523..baa96dd69 100644 --- a/tokens/src/tests.rs +++ b/tokens/src/tests.rs @@ -1179,24 +1179,24 @@ fn lifecycle_callbacks_are_activated() { #[test] fn deposit_hooks_work() { ExtBuilder::default().build().execute_with(|| { - let initial_prehook_calls = DepositPreHook::::calls(); - let initial_posthook_calls = DepositPostHook::::calls(); + let initial_prehook_calls = PreDeposit::::calls(); + let initial_posthook_calls = PostDeposit::::calls(); assert_ok!(Tokens::do_deposit(DOT, &CHARLIE, 0, false, true),); - assert_eq!(DepositPreHook::::calls(), initial_prehook_calls); - assert_eq!(DepositPostHook::::calls(), initial_posthook_calls); + assert_eq!(PreDeposit::::calls(), initial_prehook_calls); + assert_eq!(PostDeposit::::calls(), initial_posthook_calls); assert_ok!(Tokens::do_deposit(DOT, &CHARLIE, 100, false, true),); - assert_eq!(DepositPreHook::::calls(), initial_prehook_calls + 1); - assert_eq!(DepositPostHook::::calls(), initial_posthook_calls + 1); + assert_eq!(PreDeposit::::calls(), initial_prehook_calls + 1); + assert_eq!(PostDeposit::::calls(), initial_posthook_calls + 1); assert_noop!( Tokens::do_deposit(DOT, &BOB, 1, false, true), Error::::ExistentialDeposit ); // The prehook is called - assert_eq!(DepositPreHook::::calls(), initial_prehook_calls + 2); + assert_eq!(PreDeposit::::calls(), initial_prehook_calls + 2); // The posthook is not called - assert_eq!(DepositPostHook::::calls(), initial_posthook_calls + 1); + assert_eq!(PostDeposit::::calls(), initial_posthook_calls + 1); }); } @@ -1206,8 +1206,8 @@ fn transfer_hooks_work() { .balances(vec![(ALICE, DOT, 100)]) .build() .execute_with(|| { - let initial_prehook_calls = TransferPreHook::::calls(); - let initial_posthook_calls = TransferPostHook::::calls(); + let initial_prehook_calls = PreTransfer::::calls(); + let initial_posthook_calls = PostTransfer::::calls(); assert_ok!(Tokens::do_transfer( DOT, &ALICE, @@ -1215,8 +1215,8 @@ fn transfer_hooks_work() { 0, ExistenceRequirement::AllowDeath ),); - assert_eq!(TransferPreHook::::calls(), initial_prehook_calls); - assert_eq!(TransferPostHook::::calls(), initial_posthook_calls); + assert_eq!(PreTransfer::::calls(), initial_prehook_calls); + assert_eq!(PostTransfer::::calls(), initial_posthook_calls); assert_ok!(Tokens::do_transfer( DOT, @@ -1225,16 +1225,16 @@ fn transfer_hooks_work() { 10, ExistenceRequirement::AllowDeath )); - assert_eq!(TransferPreHook::::calls(), initial_prehook_calls + 1); - assert_eq!(TransferPostHook::::calls(), initial_posthook_calls + 1); + assert_eq!(PreTransfer::::calls(), initial_prehook_calls + 1); + assert_eq!(PostTransfer::::calls(), initial_posthook_calls + 1); assert_noop!( Tokens::do_transfer(DOT, &ALICE, &BOB, 1, ExistenceRequirement::AllowDeath), Error::::ExistentialDeposit ); // The prehook is called - assert_eq!(TransferPreHook::::calls(), initial_prehook_calls + 2); + assert_eq!(PreTransfer::::calls(), initial_prehook_calls + 2); // The posthook is not called - assert_eq!(TransferPostHook::::calls(), initial_posthook_calls + 1); + assert_eq!(PostTransfer::::calls(), initial_posthook_calls + 1); }); } diff --git a/traits/src/currency.rs b/traits/src/currency.rs index 9be264acf..8c391f604 100644 --- a/traits/src/currency.rs +++ b/traits/src/currency.rs @@ -691,7 +691,7 @@ impl OnTransfer } } -pub trait CurrencyMutationHooks { +pub trait MutationHooks { /// Handler to burn or transfer account's dust type OnDust: OnDust; @@ -699,16 +699,16 @@ pub trait CurrencyMutationHooks { type OnSlash: OnSlash; /// Hook to run before depositing into an account. - type DepositPreHook: OnDeposit; + type PreDeposit: OnDeposit; /// Hook to run after depositing into an account. - type DepositPostHook: OnDeposit; + type PostDeposit: OnDeposit; /// Hook to run before transferring from an account to another. - type TransferPreHook: OnTransfer; + type PreTransfer: OnTransfer; /// Hook to run after transferring from an account to another. - type TransferPostHook: OnTransfer; + type PostTransfer: OnTransfer; /// Handler for when an account was created type OnNewTokenAccount: Happened<(AccountId, CurrencyId)>; @@ -717,13 +717,13 @@ pub trait CurrencyMutationHooks { type OnKilledTokenAccount: Happened<(AccountId, CurrencyId)>; } -impl CurrencyMutationHooks for () { +impl MutationHooks for () { type OnDust = (); type OnSlash = (); - type DepositPreHook = (); - type DepositPostHook = (); - type TransferPreHook = (); - type TransferPostHook = (); + type PreDeposit = (); + type PostDeposit = (); + type PreTransfer = (); + type PostTransfer = (); type OnNewTokenAccount = (); type OnKilledTokenAccount = (); }