From ce2329de934aa9b2af05a13836d866a84cdf1470 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 10 Jun 2021 11:02:14 +0800 Subject: [PATCH 1/9] implement fungible and fungibles for tokens --- tokens/src/lib.rs | 354 +++++++++++++++++++++++++++++++++++++++++++- tokens/src/tests.rs | 104 +++++++++++++ 2 files changed, 457 insertions(+), 1 deletion(-) diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index c757c0e2e..ee3991f55 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -43,6 +43,7 @@ use frame_support::{ ensure, log, pallet_prelude::*, traits::{ + tokens::{fungible as PalletFungible, fungibles as PalletFungibles, DepositConsequence, WithdrawConsequence}, BalanceStatus as Status, Currency as PalletCurrency, ExistenceRequirement, Get, Imbalance, LockableCurrency as PalletLockableCurrency, MaxEncodedLen, ReservableCurrency as PalletReservableCurrency, SignedImbalance, WithdrawReasons, @@ -171,7 +172,7 @@ pub mod module { + MaybeSerializeDeserialize; /// The currency ID type - type CurrencyId: Parameter + Member + Copy + MaybeSerializeDeserialize + Ord; + type CurrencyId: Parameter + Member + Copy + MaybeSerializeDeserialize + Ord + Default; /// Weight information for extrinsics in this module. type WeightInfo: WeightInfo; @@ -346,6 +347,83 @@ impl Pallet { PalletId::try_from_account(account_id).is_some() } + pub(crate) fn deposit_consequence( + _who: &T::AccountId, + currency_id: T::CurrencyId, + amount: T::Balance, + account: &AccountData, + ) -> DepositConsequence { + if amount.is_zero() { + return DepositConsequence::Success; + } + + if TotalIssuance::::get(currency_id).checked_add(&amount).is_none() { + return DepositConsequence::Overflow; + } + + let new_total_balance = match account.total().checked_add(&amount) { + Some(x) => x, + None => return DepositConsequence::Overflow, + }; + + if new_total_balance < T::ExistentialDeposits::get(¤cy_id) { + return DepositConsequence::BelowMinimum; + } + + // NOTE: We assume that we are a provider, so don't need to do any checks in the + // case of account creation. + + DepositConsequence::Success + } + + pub(crate) fn withdraw_consequence( + who: &T::AccountId, + currency_id: T::CurrencyId, + amount: T::Balance, + account: &AccountData, + ) -> WithdrawConsequence { + if amount.is_zero() { + return WithdrawConsequence::Success; + } + + if TotalIssuance::::get(currency_id).checked_sub(&amount).is_none() { + return WithdrawConsequence::Underflow; + } + + let new_total_balance = match account.total().checked_sub(&amount) { + Some(x) => x, + None => return WithdrawConsequence::NoFunds, + }; + + // Provider restriction - total account balance cannot be reduced to zero if it + // cannot sustain the loss of a provider reference. + // NOTE: This assumes that the pallet is a provider (which is true). Is this + // ever changes, then this will need to adapt accordingly. + let ed = T::ExistentialDeposits::get(¤cy_id); + let success = if new_total_balance < ed { + if frame_system::Pallet::::can_dec_provider(who) { + WithdrawConsequence::ReducedToZero(new_total_balance) + } else { + return WithdrawConsequence::WouldDie; + } + } else { + WithdrawConsequence::Success + }; + + // Enough free funds to have them be reduced. + let new_free_balance = match account.free.checked_sub(&amount) { + Some(b) => b, + None => return WithdrawConsequence::NoFunds, + }; + + // Eventual free funds must be no less than the frozen balance. + if new_free_balance < account.frozen() { + return WithdrawConsequence::Frozen; + } + + success + } + pub(crate) fn try_mutate_account( who: &T::AccountId, currency_id: T::CurrencyId, @@ -812,6 +890,170 @@ impl MultiReservableCurrency for Pallet { } } +impl PalletFungibles::Inspect for Pallet { + type AssetId = T::CurrencyId; + type Balance = T::Balance; + + fn total_issuance(asset_id: Self::AssetId) -> Self::Balance { + Pallet::::total_issuance(asset_id) + } + fn minimum_balance(asset_id: Self::AssetId) -> Self::Balance { + >::minimum_balance(asset_id) + } + fn balance(asset_id: Self::AssetId, who: &T::AccountId) -> Self::Balance { + Pallet::::total_balance(asset_id, who) + } + fn reducible_balance(asset_id: Self::AssetId, who: &T::AccountId, keep_alive: bool) -> Self::Balance { + let a = Pallet::::accounts(who, asset_id); + // Liquid balance is what is neither reserved nor locked/frozen. + let liquid = a.free.saturating_sub(a.frozen); + if frame_system::Pallet::::can_dec_provider(who) && !keep_alive { + liquid + } else { + // `must_remain_to_exist` is the part of liquid balance which must remain to + // keep total over ED. + let must_remain_to_exist = T::ExistentialDeposits::get(&asset_id).saturating_sub(a.total() - liquid); + liquid.saturating_sub(must_remain_to_exist) + } + } + fn can_deposit(asset_id: Self::AssetId, who: &T::AccountId, amount: Self::Balance) -> DepositConsequence { + Pallet::::deposit_consequence(who, asset_id, amount, &Pallet::::accounts(who, asset_id)) + } + fn can_withdraw( + asset_id: Self::AssetId, + who: &T::AccountId, + amount: Self::Balance, + ) -> WithdrawConsequence { + Pallet::::withdraw_consequence(who, asset_id, amount, &Pallet::::accounts(who, asset_id)) + } +} + +impl PalletFungibles::Mutate for Pallet { + fn mint_into(asset_id: Self::AssetId, who: &T::AccountId, amount: Self::Balance) -> DispatchResult { + if amount.is_zero() { + return Ok(()); + } + Pallet::::try_mutate_account(who, asset_id, |account, _is_new| -> DispatchResult { + Pallet::::deposit_consequence(who, asset_id, amount, &account).into_result()?; + account.free += amount; + Ok(()) + })?; + >::mutate(asset_id, |t| *t += amount); + Ok(()) + } + + fn burn_from( + asset_id: Self::AssetId, + who: &T::AccountId, + amount: Self::Balance, + ) -> Result { + if amount.is_zero() { + return Ok(Self::Balance::zero()); + } + let actual = + Pallet::::try_mutate_account(who, asset_id, |account, _is_new| -> Result { + let extra = Pallet::::withdraw_consequence(who, asset_id, amount, &account).into_result()?; + let actual = amount + extra; + account.free -= actual; + Ok(actual) + })?; + >::mutate(asset_id, |t| *t -= actual); + Ok(actual) + } +} + +impl PalletFungibles::Transfer for Pallet { + fn transfer( + asset_id: Self::AssetId, + source: &T::AccountId, + dest: &T::AccountId, + amount: T::Balance, + _keep_alive: bool, + ) -> Result { + as MultiCurrency>::transfer(asset_id, source, dest, amount).map(|_| amount) + } +} + +impl PalletFungibles::Unbalanced for Pallet { + fn set_balance(asset_id: Self::AssetId, who: &T::AccountId, amount: Self::Balance) -> DispatchResult { + Pallet::::mutate_account(who, asset_id, |account, _| account.free = amount); + Ok(()) + } + + fn set_total_issuance(asset_id: Self::AssetId, amount: Self::Balance) { + >::mutate(asset_id, |t| *t = amount); + } +} + +impl PalletFungibles::InspectHold for Pallet { + fn balance_on_hold(asset_id: Self::AssetId, who: &T::AccountId) -> T::Balance { + Pallet::::accounts(who, asset_id).reserved + } + fn can_hold(asset_id: Self::AssetId, who: &T::AccountId, amount: T::Balance) -> bool { + let a = Pallet::::accounts(who, asset_id); + let min_balance = T::ExistentialDeposits::get(&asset_id).max(a.frozen); + 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 + } +} + +impl PalletFungibles::MutateHold for Pallet { + fn hold(asset_id: Self::AssetId, who: &T::AccountId, amount: Self::Balance) -> DispatchResult { + if amount.is_zero() { + return Ok(()); + } + ensure!( + Pallet::::can_reserve(asset_id, who, amount), + Error::::BalanceTooLow + ); + Pallet::::mutate_account(who, asset_id, |a, _| { + a.free -= amount; + a.reserved += amount; + }); + Ok(()) + } + fn release( + asset_id: Self::AssetId, + who: &T::AccountId, + amount: Self::Balance, + best_effort: bool, + ) -> Result { + if amount.is_zero() { + return Ok(amount); + } + // Done on a best-effort basis. + Pallet::::try_mutate_account(who, asset_id, |a, _| { + let new_free = a.free.saturating_add(amount.min(a.reserved)); + let actual = new_free - a.free; + // Guaranteed to be <= amount and <= a.reserved + ensure!(best_effort || actual == amount, Error::::BalanceTooLow); + a.free = new_free; + a.reserved = a.reserved.saturating_sub(actual.clone()); + Ok(actual) + }) + } + fn transfer_held( + asset_id: Self::AssetId, + source: &T::AccountId, + dest: &T::AccountId, + amount: Self::Balance, + _best_effort: bool, + on_hold: bool, + ) -> Result { + let status = if on_hold { Status::Reserved } else { Status::Free }; + Pallet::::repatriate_reserved(asset_id, source, dest, amount, status) + } +} + pub struct CurrencyAdapter(marker::PhantomData<(T, GetCurrencyId)>); impl PalletCurrency for CurrencyAdapter @@ -1049,3 +1291,113 @@ impl TransferAll for Pallet { }) } } + +impl PalletFungible::Inspect for CurrencyAdapter +where + T: Config, + GetCurrencyId: Get, +{ + type Balance = T::Balance; + + fn total_issuance() -> Self::Balance { + as PalletFungibles::Inspect<_>>::total_issuance(GetCurrencyId::get()) + } + fn minimum_balance() -> Self::Balance { + as PalletFungibles::Inspect<_>>::minimum_balance(GetCurrencyId::get()) + } + fn balance(who: &T::AccountId) -> Self::Balance { + as PalletFungibles::Inspect<_>>::balance(GetCurrencyId::get(), who) + } + fn reducible_balance(who: &T::AccountId, keep_alive: bool) -> Self::Balance { + as PalletFungibles::Inspect<_>>::reducible_balance(GetCurrencyId::get(), who, keep_alive) + } + fn can_deposit(who: &T::AccountId, amount: Self::Balance) -> DepositConsequence { + as PalletFungibles::Inspect<_>>::can_deposit(GetCurrencyId::get(), who, amount) + } + fn can_withdraw(who: &T::AccountId, amount: Self::Balance) -> WithdrawConsequence { + as PalletFungibles::Inspect<_>>::can_withdraw(GetCurrencyId::get(), who, amount) + } +} + +impl PalletFungible::Mutate for CurrencyAdapter +where + T: Config, + GetCurrencyId: Get, +{ + fn mint_into(who: &T::AccountId, amount: Self::Balance) -> DispatchResult { + as PalletFungibles::Mutate<_>>::mint_into(GetCurrencyId::get(), who, amount) + } + fn burn_from(who: &T::AccountId, amount: Self::Balance) -> Result { + as PalletFungibles::Mutate<_>>::burn_from(GetCurrencyId::get(), who, amount) + } +} + +impl PalletFungible::Transfer for CurrencyAdapter +where + T: Config, + GetCurrencyId: Get, +{ + fn transfer( + source: &T::AccountId, + dest: &T::AccountId, + amount: T::Balance, + keep_alive: bool, + ) -> Result { + as PalletFungibles::Transfer<_>>::transfer(GetCurrencyId::get(), source, dest, amount, keep_alive) + } +} + +impl PalletFungible::Unbalanced for CurrencyAdapter +where + T: Config, + GetCurrencyId: Get, +{ + fn set_balance(who: &T::AccountId, amount: Self::Balance) -> DispatchResult { + as PalletFungibles::Unbalanced<_>>::set_balance(GetCurrencyId::get(), who, amount) + } + fn set_total_issuance(amount: Self::Balance) { + as PalletFungibles::Unbalanced<_>>::set_total_issuance(GetCurrencyId::get(), amount) + } +} + +impl PalletFungible::InspectHold for CurrencyAdapter +where + T: Config, + GetCurrencyId: Get, +{ + fn balance_on_hold(who: &T::AccountId) -> T::Balance { + as PalletFungibles::InspectHold<_>>::balance_on_hold(GetCurrencyId::get(), who) + } + fn can_hold(who: &T::AccountId, amount: T::Balance) -> bool { + as PalletFungibles::InspectHold<_>>::can_hold(GetCurrencyId::get(), who, amount) + } +} + +impl PalletFungible::MutateHold for CurrencyAdapter +where + T: Config, + GetCurrencyId: Get, +{ + fn hold(who: &T::AccountId, amount: Self::Balance) -> DispatchResult { + as PalletFungibles::MutateHold<_>>::hold(GetCurrencyId::get(), who, amount) + } + fn release(who: &T::AccountId, amount: Self::Balance, best_effort: bool) -> Result { + as PalletFungibles::MutateHold<_>>::release(GetCurrencyId::get(), who, amount, best_effort) + } + fn transfer_held( + source: &T::AccountId, + dest: &T::AccountId, + amount: Self::Balance, + best_effort: bool, + on_hold: bool, + ) -> Result { + as PalletFungibles::MutateHold<_>>::transfer_held( + GetCurrencyId::get(), + source, + dest, + amount, + best_effort, + on_hold, + ) + } +} diff --git a/tokens/src/tests.rs b/tokens/src/tests.rs index 0e027068a..70076b266 100644 --- a/tokens/src/tests.rs +++ b/tokens/src/tests.rs @@ -986,3 +986,107 @@ fn exceeding_max_locks_should_fail() { assert_eq!(Tokens::locks(ALICE, DOT).len(), 2); }); } + +#[test] +fn fungibles_inspect_trait_should_work() { + ExtBuilder::default() + .one_hundred_for_alice_n_bob() + .build() + .execute_with(|| { + assert_eq!(>::total_issuance(DOT), 200); + assert_eq!(>::minimum_balance(DOT), 2); + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_eq!( + >::reducible_balance(DOT, &ALICE, true), + 98 + ); + assert_ok!(>::can_deposit(DOT, &ALICE, 1).into_result()); + assert_ok!(>::can_withdraw(DOT, &ALICE, 1).into_result()); + }); +} + +#[test] +fn fungibles_mutate_trait_should_work() { + ExtBuilder::default() + .one_hundred_for_alice_n_bob() + .build() + .execute_with(|| { + assert_ok!(>::mint_into(DOT, &ALICE, 10)); + assert_eq!(>::burn_from(DOT, &ALICE, 8), Ok(8)); + }); +} + +#[test] +fn fungibles_transfer_trait_should_work() { + ExtBuilder::default() + .one_hundred_for_alice_n_bob() + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_eq!(>::balance(DOT, &BOB), 100); + assert_ok!(>::transfer( + DOT, &ALICE, &BOB, 10, true + )); + assert_eq!(>::balance(DOT, &ALICE), 90); + assert_eq!(>::balance(DOT, &BOB), 110); + }); +} + +#[test] +fn fungibles_unbalanced_trait_should_work() { + ExtBuilder::default() + .one_hundred_for_alice_n_bob() + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_ok!(>::set_balance(DOT, &ALICE, 10)); + assert_eq!(>::balance(DOT, &ALICE), 10); + + assert_eq!(>::total_issuance(DOT), 200); + >::set_total_issuance(DOT, 10); + assert_eq!(>::total_issuance(DOT), 10); + }); +} + +#[test] +fn fungibles_inspect_hold_trait_should_work() { + ExtBuilder::default() + .one_hundred_for_alice_n_bob() + .build() + .execute_with(|| { + assert_eq!( + >::balance_on_hold(DOT, &ALICE), + 0 + ); + assert_eq!( + >::can_hold(DOT, &ALICE, 50), + true + ); + assert_eq!( + >::can_hold(DOT, &ALICE, 100), + false + ); + }); +} + +#[test] +fn fungibles_mutate_hold_trait_should_work() { + ExtBuilder::default() + .one_hundred_for_alice_n_bob() + .build() + .execute_with(|| { + assert_noop!( + >::hold(DOT, &ALICE, 200), + Error::::BalanceTooLow + ); + assert_ok!(>::hold(DOT, &ALICE, 100)); + assert_eq!( + >::release(DOT, &ALICE, 50, true), + Ok(50) + ); + assert_eq!( + >::transfer_held(DOT, &ALICE, &BOB, 100, true, true), + Ok(50) + ); + }); +} From 2fd417facd8d2058063581a16607949986271b8e Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Thu, 10 Jun 2021 11:16:56 +0800 Subject: [PATCH 2/9] fix clippy --- tokens/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index ee3991f55..c66f9731f 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -1037,7 +1037,7 @@ impl PalletFungibles::MutateHold for Pallet { // Guaranteed to be <= amount and <= a.reserved ensure!(best_effort || actual == amount, Error::::BalanceTooLow); a.free = new_free; - a.reserved = a.reserved.saturating_sub(actual.clone()); + a.reserved = a.reserved.saturating_sub(actual); Ok(actual) }) } From b1ffbc4f212b4f8fb8f5de453e9115bfb9e6d6fa Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Fri, 11 Jun 2021 11:46:05 +0800 Subject: [PATCH 3/9] Apply review suggestions --- tokens/src/lib.rs | 66 +++++++++++++++++++++++++-------------------- tokens/src/tests.rs | 61 ++++++++++++++++++----------------------- 2 files changed, 63 insertions(+), 64 deletions(-) diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index c66f9731f..f5b54bb5f 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -43,7 +43,7 @@ use frame_support::{ ensure, log, pallet_prelude::*, traits::{ - tokens::{fungible as PalletFungible, fungibles as PalletFungibles, DepositConsequence, WithdrawConsequence}, + tokens::{fungible, fungibles, DepositConsequence, WithdrawConsequence}, BalanceStatus as Status, Currency as PalletCurrency, ExistenceRequirement, Get, Imbalance, LockableCurrency as PalletLockableCurrency, MaxEncodedLen, ReservableCurrency as PalletReservableCurrency, SignedImbalance, WithdrawReasons, @@ -890,7 +890,7 @@ impl MultiReservableCurrency for Pallet { } } -impl PalletFungibles::Inspect for Pallet { +impl fungibles::Inspect for Pallet { type AssetId = T::CurrencyId; type Balance = T::Balance; @@ -928,16 +928,18 @@ impl PalletFungibles::Inspect for Pallet { } } -impl PalletFungibles::Mutate for Pallet { +impl fungibles::Mutate for Pallet { fn mint_into(asset_id: Self::AssetId, who: &T::AccountId, amount: Self::Balance) -> DispatchResult { if amount.is_zero() { return Ok(()); } Pallet::::try_mutate_account(who, asset_id, |account, _is_new| -> DispatchResult { Pallet::::deposit_consequence(who, asset_id, amount, &account).into_result()?; + // deposit_consequence already did overflow checking account.free += amount; Ok(()) })?; + // deposit_consequence already did overflow checking >::mutate(asset_id, |t| *t += amount); Ok(()) } @@ -953,16 +955,18 @@ impl PalletFungibles::Mutate for Pallet { let actual = Pallet::::try_mutate_account(who, asset_id, |account, _is_new| -> Result { let extra = Pallet::::withdraw_consequence(who, asset_id, amount, &account).into_result()?; + // withdraw_consequence already did underflow checking let actual = amount + extra; account.free -= actual; Ok(actual) })?; + // withdraw_consequence already did underflow checking >::mutate(asset_id, |t| *t -= actual); Ok(actual) } } -impl PalletFungibles::Transfer for Pallet { +impl fungibles::Transfer for Pallet { fn transfer( asset_id: Self::AssetId, source: &T::AccountId, @@ -974,18 +978,20 @@ impl PalletFungibles::Transfer for Pallet { } } -impl PalletFungibles::Unbalanced for Pallet { +impl fungibles::Unbalanced for Pallet { fn set_balance(asset_id: Self::AssetId, who: &T::AccountId, amount: Self::Balance) -> DispatchResult { + // Balance is the same type and will not overflow Pallet::::mutate_account(who, asset_id, |account, _| account.free = amount); Ok(()) } fn set_total_issuance(asset_id: Self::AssetId, amount: Self::Balance) { + // Balance is the same type and will not overflow >::mutate(asset_id, |t| *t = amount); } } -impl PalletFungibles::InspectHold for Pallet { +impl fungibles::InspectHold for Pallet { fn balance_on_hold(asset_id: Self::AssetId, who: &T::AccountId) -> T::Balance { Pallet::::accounts(who, asset_id).reserved } @@ -1006,7 +1012,7 @@ impl PalletFungibles::InspectHold for Pallet { } } -impl PalletFungibles::MutateHold for Pallet { +impl fungibles::MutateHold for Pallet { fn hold(asset_id: Self::AssetId, who: &T::AccountId, amount: Self::Balance) -> DispatchResult { if amount.is_zero() { return Ok(()); @@ -1016,7 +1022,9 @@ impl PalletFungibles::MutateHold for Pallet { Error::::BalanceTooLow ); Pallet::::mutate_account(who, asset_id, |a, _| { + // `can_reserve` has did underflow checking a.free -= amount; + // Cannot overflow as `amount` is from `a.free` a.reserved += amount; }); Ok(()) @@ -1292,7 +1300,7 @@ impl TransferAll for Pallet { } } -impl PalletFungible::Inspect for CurrencyAdapter +impl fungible::Inspect for CurrencyAdapter where T: Config, GetCurrencyId: Get, @@ -1300,39 +1308,39 @@ where type Balance = T::Balance; fn total_issuance() -> Self::Balance { - as PalletFungibles::Inspect<_>>::total_issuance(GetCurrencyId::get()) + as fungibles::Inspect<_>>::total_issuance(GetCurrencyId::get()) } fn minimum_balance() -> Self::Balance { - as PalletFungibles::Inspect<_>>::minimum_balance(GetCurrencyId::get()) + as fungibles::Inspect<_>>::minimum_balance(GetCurrencyId::get()) } fn balance(who: &T::AccountId) -> Self::Balance { - as PalletFungibles::Inspect<_>>::balance(GetCurrencyId::get(), who) + as fungibles::Inspect<_>>::balance(GetCurrencyId::get(), who) } fn reducible_balance(who: &T::AccountId, keep_alive: bool) -> Self::Balance { - as PalletFungibles::Inspect<_>>::reducible_balance(GetCurrencyId::get(), who, keep_alive) + as fungibles::Inspect<_>>::reducible_balance(GetCurrencyId::get(), who, keep_alive) } fn can_deposit(who: &T::AccountId, amount: Self::Balance) -> DepositConsequence { - as PalletFungibles::Inspect<_>>::can_deposit(GetCurrencyId::get(), who, amount) + as fungibles::Inspect<_>>::can_deposit(GetCurrencyId::get(), who, amount) } fn can_withdraw(who: &T::AccountId, amount: Self::Balance) -> WithdrawConsequence { - as PalletFungibles::Inspect<_>>::can_withdraw(GetCurrencyId::get(), who, amount) + as fungibles::Inspect<_>>::can_withdraw(GetCurrencyId::get(), who, amount) } } -impl PalletFungible::Mutate for CurrencyAdapter +impl fungible::Mutate for CurrencyAdapter where T: Config, GetCurrencyId: Get, { fn mint_into(who: &T::AccountId, amount: Self::Balance) -> DispatchResult { - as PalletFungibles::Mutate<_>>::mint_into(GetCurrencyId::get(), who, amount) + as fungibles::Mutate<_>>::mint_into(GetCurrencyId::get(), who, amount) } fn burn_from(who: &T::AccountId, amount: Self::Balance) -> Result { - as PalletFungibles::Mutate<_>>::burn_from(GetCurrencyId::get(), who, amount) + as fungibles::Mutate<_>>::burn_from(GetCurrencyId::get(), who, amount) } } -impl PalletFungible::Transfer for CurrencyAdapter +impl fungible::Transfer for CurrencyAdapter where T: Config, GetCurrencyId: Get, @@ -1343,46 +1351,46 @@ where amount: T::Balance, keep_alive: bool, ) -> Result { - as PalletFungibles::Transfer<_>>::transfer(GetCurrencyId::get(), source, dest, amount, keep_alive) + as fungibles::Transfer<_>>::transfer(GetCurrencyId::get(), source, dest, amount, keep_alive) } } -impl PalletFungible::Unbalanced for CurrencyAdapter +impl fungible::Unbalanced for CurrencyAdapter where T: Config, GetCurrencyId: Get, { fn set_balance(who: &T::AccountId, amount: Self::Balance) -> DispatchResult { - as PalletFungibles::Unbalanced<_>>::set_balance(GetCurrencyId::get(), who, amount) + as fungibles::Unbalanced<_>>::set_balance(GetCurrencyId::get(), who, amount) } fn set_total_issuance(amount: Self::Balance) { - as PalletFungibles::Unbalanced<_>>::set_total_issuance(GetCurrencyId::get(), amount) + as fungibles::Unbalanced<_>>::set_total_issuance(GetCurrencyId::get(), amount) } } -impl PalletFungible::InspectHold for CurrencyAdapter +impl fungible::InspectHold for CurrencyAdapter where T: Config, GetCurrencyId: Get, { fn balance_on_hold(who: &T::AccountId) -> T::Balance { - as PalletFungibles::InspectHold<_>>::balance_on_hold(GetCurrencyId::get(), who) + as fungibles::InspectHold<_>>::balance_on_hold(GetCurrencyId::get(), who) } fn can_hold(who: &T::AccountId, amount: T::Balance) -> bool { - as PalletFungibles::InspectHold<_>>::can_hold(GetCurrencyId::get(), who, amount) + as fungibles::InspectHold<_>>::can_hold(GetCurrencyId::get(), who, amount) } } -impl PalletFungible::MutateHold for CurrencyAdapter +impl fungible::MutateHold for CurrencyAdapter where T: Config, GetCurrencyId: Get, { fn hold(who: &T::AccountId, amount: Self::Balance) -> DispatchResult { - as PalletFungibles::MutateHold<_>>::hold(GetCurrencyId::get(), who, amount) + as fungibles::MutateHold<_>>::hold(GetCurrencyId::get(), who, amount) } fn release(who: &T::AccountId, amount: Self::Balance, best_effort: bool) -> Result { - as PalletFungibles::MutateHold<_>>::release(GetCurrencyId::get(), who, amount, best_effort) + as fungibles::MutateHold<_>>::release(GetCurrencyId::get(), who, amount, best_effort) } fn transfer_held( source: &T::AccountId, @@ -1391,7 +1399,7 @@ where best_effort: bool, on_hold: bool, ) -> Result { - as PalletFungibles::MutateHold<_>>::transfer_held( + as fungibles::MutateHold<_>>::transfer_held( GetCurrencyId::get(), source, dest, diff --git a/tokens/src/tests.rs b/tokens/src/tests.rs index 70076b266..7b40cc39e 100644 --- a/tokens/src/tests.rs +++ b/tokens/src/tests.rs @@ -993,15 +993,15 @@ fn fungibles_inspect_trait_should_work() { .one_hundred_for_alice_n_bob() .build() .execute_with(|| { - assert_eq!(>::total_issuance(DOT), 200); - assert_eq!(>::minimum_balance(DOT), 2); - assert_eq!(>::balance(DOT, &ALICE), 100); + assert_eq!(>::total_issuance(DOT), 200); + assert_eq!(>::minimum_balance(DOT), 2); + assert_eq!(>::balance(DOT, &ALICE), 100); assert_eq!( - >::reducible_balance(DOT, &ALICE, true), + >::reducible_balance(DOT, &ALICE, true), 98 ); - assert_ok!(>::can_deposit(DOT, &ALICE, 1).into_result()); - assert_ok!(>::can_withdraw(DOT, &ALICE, 1).into_result()); + assert_ok!(>::can_deposit(DOT, &ALICE, 1).into_result()); + assert_ok!(>::can_withdraw(DOT, &ALICE, 1).into_result()); }); } @@ -1011,8 +1011,8 @@ fn fungibles_mutate_trait_should_work() { .one_hundred_for_alice_n_bob() .build() .execute_with(|| { - assert_ok!(>::mint_into(DOT, &ALICE, 10)); - assert_eq!(>::burn_from(DOT, &ALICE, 8), Ok(8)); + assert_ok!(>::mint_into(DOT, &ALICE, 10)); + assert_eq!(>::burn_from(DOT, &ALICE, 8), Ok(8)); }); } @@ -1022,13 +1022,13 @@ fn fungibles_transfer_trait_should_work() { .one_hundred_for_alice_n_bob() .build() .execute_with(|| { - assert_eq!(>::balance(DOT, &ALICE), 100); - assert_eq!(>::balance(DOT, &BOB), 100); - assert_ok!(>::transfer( + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_eq!(>::balance(DOT, &BOB), 100); + assert_ok!(>::transfer( DOT, &ALICE, &BOB, 10, true )); - assert_eq!(>::balance(DOT, &ALICE), 90); - assert_eq!(>::balance(DOT, &BOB), 110); + assert_eq!(>::balance(DOT, &ALICE), 90); + assert_eq!(>::balance(DOT, &BOB), 110); }); } @@ -1038,13 +1038,13 @@ fn fungibles_unbalanced_trait_should_work() { .one_hundred_for_alice_n_bob() .build() .execute_with(|| { - assert_eq!(>::balance(DOT, &ALICE), 100); - assert_ok!(>::set_balance(DOT, &ALICE, 10)); - assert_eq!(>::balance(DOT, &ALICE), 10); + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_ok!(>::set_balance(DOT, &ALICE, 10)); + assert_eq!(>::balance(DOT, &ALICE), 10); - assert_eq!(>::total_issuance(DOT), 200); - >::set_total_issuance(DOT, 10); - assert_eq!(>::total_issuance(DOT), 10); + assert_eq!(>::total_issuance(DOT), 200); + >::set_total_issuance(DOT, 10); + assert_eq!(>::total_issuance(DOT), 10); }); } @@ -1054,18 +1054,9 @@ fn fungibles_inspect_hold_trait_should_work() { .one_hundred_for_alice_n_bob() .build() .execute_with(|| { - assert_eq!( - >::balance_on_hold(DOT, &ALICE), - 0 - ); - assert_eq!( - >::can_hold(DOT, &ALICE, 50), - true - ); - assert_eq!( - >::can_hold(DOT, &ALICE, 100), - false - ); + assert_eq!(>::balance_on_hold(DOT, &ALICE), 0); + assert_eq!(>::can_hold(DOT, &ALICE, 50), true); + assert_eq!(>::can_hold(DOT, &ALICE, 100), false); }); } @@ -1076,16 +1067,16 @@ fn fungibles_mutate_hold_trait_should_work() { .build() .execute_with(|| { assert_noop!( - >::hold(DOT, &ALICE, 200), + >::hold(DOT, &ALICE, 200), Error::::BalanceTooLow ); - assert_ok!(>::hold(DOT, &ALICE, 100)); + assert_ok!(>::hold(DOT, &ALICE, 100)); assert_eq!( - >::release(DOT, &ALICE, 50, true), + >::release(DOT, &ALICE, 50, true), Ok(50) ); assert_eq!( - >::transfer_held(DOT, &ALICE, &BOB, 100, true, true), + >::transfer_held(DOT, &ALICE, &BOB, 100, true, true), Ok(50) ); }); From eb46df0ee6d568e014b5805cff529bdbad318481 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Fri, 11 Jun 2021 11:51:09 +0800 Subject: [PATCH 4/9] add TODO --- tokens/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index f5b54bb5f..561496372 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -172,6 +172,7 @@ pub mod module { + MaybeSerializeDeserialize; /// The currency ID type + // TODO: remove `Default` after https://github.com/paritytech/substrate/pull/9062 type CurrencyId: Parameter + Member + Copy + MaybeSerializeDeserialize + Ord + Default; /// Weight information for extrinsics in this module. From bdf5ed2ae0cede5bb5ca08d71014b0b21efd8adf Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Fri, 11 Jun 2021 14:45:42 +0800 Subject: [PATCH 5/9] support ExistenceRequirement --- tokens/src/lib.rs | 92 +++++++++++++++++++++++++++++++++--------- tokens/src/tests.rs | 10 ++++- traits/src/currency.rs | 3 +- 3 files changed, 82 insertions(+), 23 deletions(-) diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index 561496372..12937924f 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -87,7 +87,13 @@ where fn on_dust(who: &T::AccountId, currency_id: T::CurrencyId, amount: T::Balance) { // transfer the dust to treasury account, ignore the result, // if failed will leave some dust which still could be recycled. - let _ = as MultiCurrency>::transfer(currency_id, who, &GetAccountId::get(), amount); + let _ = as MultiCurrency>::transfer( + currency_id, + who, + &GetAccountId::get(), + amount, + ExistenceRequirement::AllowDeath, + ); } } @@ -197,6 +203,8 @@ pub mod module { LiquidityRestrictions, /// Failed because the maximum locks was exceeded MaxLocksExceeded, + /// Transfer/payment would kill account + KeepAlive, } #[pallet::event] @@ -315,7 +323,7 @@ pub mod module { ) -> DispatchResultWithPostInfo { let from = ensure_signed(origin)?; let to = T::Lookup::lookup(dest)?; - >::transfer(currency_id, &from, &to, amount)?; + >::transfer(currency_id, &from, &to, amount, ExistenceRequirement::AllowDeath)?; Self::deposit_event(Event::Transferred(currency_id, from, to, amount)); Ok(().into()) @@ -334,7 +342,13 @@ pub mod module { let from = ensure_signed(origin)?; let to = T::Lookup::lookup(dest)?; let balance = >::free_balance(currency_id, &from); - >::transfer(currency_id, &from, &to, balance)?; + >::transfer( + currency_id, + &from, + &to, + balance, + ExistenceRequirement::AllowDeath, + )?; Self::deposit_event(Event::Transferred(currency_id, from, to, balance)); Ok(().into()) @@ -594,21 +608,31 @@ impl MultiCurrency for Pallet { from: &T::AccountId, to: &T::AccountId, amount: Self::Balance, + existence_requirement: ExistenceRequirement, ) -> DispatchResult { if amount.is_zero() || from == to { return Ok(()); } - Self::ensure_can_withdraw(currency_id, from, amount)?; - let from_balance = Self::free_balance(currency_id, from); - let to_balance = Self::free_balance(currency_id, to) - .checked_add(&amount) - .ok_or(ArithmeticError::Overflow)?; - // Cannot underflow because ensure_can_withdraw check - Self::set_free_balance(currency_id, from, from_balance - amount); - Self::set_free_balance(currency_id, to, to_balance); + Pallet::::try_mutate_account(to, currency_id, |to_account, _is_new| -> DispatchResult { + Pallet::::try_mutate_account(from, currency_id, |from_account, _is_new| -> DispatchResult { + from_account.free = from_account + .free + .checked_sub(&amount) + .ok_or(Error::::BalanceTooLow)?; + to_account.free = to_account.free.checked_add(&amount).ok_or(ArithmeticError::Overflow)?; - Ok(()) + Self::ensure_can_withdraw(currency_id, from, amount)?; + + let ed = T::ExistentialDeposits::get(¤cy_id); + let allow_death = existence_requirement == ExistenceRequirement::AllowDeath; + let allow_death = allow_death && !frame_system::Pallet::::is_provider_required(from); + ensure!(allow_death || from_account.total() >= ed, Error::::KeepAlive); + + Ok(()) + })?; + Ok(()) + }) } /// Deposit some `amount` into the free balance of account `who`. @@ -973,9 +997,14 @@ impl fungibles::Transfer for Pallet { source: &T::AccountId, dest: &T::AccountId, amount: T::Balance, - _keep_alive: bool, + keep_alive: bool, ) -> Result { - as MultiCurrency>::transfer(asset_id, source, dest, amount).map(|_| amount) + let er = if keep_alive { + ExistenceRequirement::KeepAlive + } else { + ExistenceRequirement::AllowDeath + }; + as MultiCurrency>::transfer(asset_id, source, dest, amount, er).map(|_| amount) } } @@ -1133,9 +1162,15 @@ where source: &T::AccountId, dest: &T::AccountId, value: Self::Balance, - _existence_requirement: ExistenceRequirement, + existence_requirement: ExistenceRequirement, ) -> DispatchResult { - as MultiCurrency>::transfer(GetCurrencyId::get(), &source, &dest, value) + as MultiCurrency>::transfer( + GetCurrencyId::get(), + &source, + &dest, + value, + existence_requirement, + ) } fn slash(who: &T::AccountId, value: Self::Balance) -> (Self::NegativeImbalance, Self::Balance) { @@ -1191,14 +1226,25 @@ where who: &T::AccountId, value: Self::Balance, _reasons: WithdrawReasons, - _liveness: ExistenceRequirement, + liveness: ExistenceRequirement, ) -> sp_std::result::Result { if value.is_zero() { return Ok(Self::NegativeImbalance::zero()); } + let currency_id = GetCurrencyId::get(); - Pallet::::ensure_can_withdraw(currency_id, who, value)?; - Pallet::::set_free_balance(currency_id, who, Pallet::::free_balance(currency_id, who) - value); + Pallet::::try_mutate_account(who, currency_id, |account, _is_new| -> DispatchResult { + account.free = account.free.checked_sub(&value).ok_or(Error::::BalanceTooLow)?; + + Pallet::::ensure_can_withdraw(currency_id, who, value)?; + + let ed = T::ExistentialDeposits::get(¤cy_id); + let allow_death = liveness == ExistenceRequirement::AllowDeath; + let allow_death = allow_death && !frame_system::Pallet::::is_provider_required(who); + ensure!(allow_death || account.total() >= ed, Error::::KeepAlive); + + Ok(()) + })?; Ok(Self::NegativeImbalance::new(value)) } @@ -1296,7 +1342,13 @@ impl TransferAll for Pallet { #[transactional] fn transfer_all(source: &T::AccountId, dest: &T::AccountId) -> DispatchResult { Accounts::::iter_prefix(source).try_for_each(|(currency_id, account_data)| -> DispatchResult { - >::transfer(currency_id, source, dest, account_data.free) + >::transfer( + currency_id, + source, + dest, + account_data.free, + ExistenceRequirement::AllowDeath, + ) }) } } diff --git a/tokens/src/tests.rs b/tokens/src/tests.rs index 7b40cc39e..afdd9dc03 100644 --- a/tokens/src/tests.rs +++ b/tokens/src/tests.rs @@ -125,11 +125,17 @@ fn frozen_can_limit_liquidity() { .execute_with(|| { assert_ok!(Tokens::set_lock(ID_1, DOT, &ALICE, 90)); assert_noop!( - >::transfer(DOT, &ALICE, &BOB, 11), + >::transfer(DOT, &ALICE, &BOB, 11, ExistenceRequirement::AllowDeath), Error::::LiquidityRestrictions, ); assert_ok!(Tokens::set_lock(ID_1, DOT, &ALICE, 10)); - assert_ok!(>::transfer(DOT, &ALICE, &BOB, 11),); + assert_ok!(>::transfer( + DOT, + &ALICE, + &BOB, + 11, + ExistenceRequirement::AllowDeath + ),); }); } diff --git a/traits/src/currency.rs b/traits/src/currency.rs index c104368ff..497c6a52e 100644 --- a/traits/src/currency.rs +++ b/traits/src/currency.rs @@ -1,7 +1,7 @@ use crate::arithmetic; use codec::{Codec, FullCodec}; pub use frame_support::{ - traits::{BalanceStatus, LockIdentifier}, + traits::{BalanceStatus, ExistenceRequirement, LockIdentifier}, transactional, }; use sp_runtime::{ @@ -49,6 +49,7 @@ pub trait MultiCurrency { from: &AccountId, to: &AccountId, amount: Self::Balance, + existence_requirement: ExistenceRequirement, ) -> DispatchResult; /// Add `amount` to the balance of `who` under `currency_id` and increase From 8925c320e1513b14f74b7e245a40c7ba1f87018f Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Fri, 11 Jun 2021 14:57:41 +0800 Subject: [PATCH 6/9] currencies support ExistenceRequirement --- currencies/src/lib.rs | 46 +++++++++++++++++++++++++++++++++-------- currencies/src/tests.rs | 27 ++++++++++++++++++++---- traits/src/currency.rs | 7 ++++++- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/currencies/src/lib.rs b/currencies/src/lib.rs index cec25c405..f631bdad3 100644 --- a/currencies/src/lib.rs +++ b/currencies/src/lib.rs @@ -145,7 +145,13 @@ pub mod module { ) -> DispatchResultWithPostInfo { let from = ensure_signed(origin)?; let to = T::Lookup::lookup(dest)?; - >::transfer(currency_id, &from, &to, amount)?; + >::transfer( + currency_id, + &from, + &to, + amount, + ExistenceRequirement::AllowDeath, + )?; Ok(().into()) } @@ -161,7 +167,7 @@ pub mod module { ) -> DispatchResultWithPostInfo { let from = ensure_signed(origin)?; let to = T::Lookup::lookup(dest)?; - T::NativeCurrency::transfer(&from, &to, amount)?; + T::NativeCurrency::transfer(&from, &to, amount, ExistenceRequirement::AllowDeath)?; Self::deposit_event(Event::Transferred(T::GetNativeCurrencyId::get(), from, to, amount)); Ok(().into()) @@ -234,14 +240,15 @@ impl MultiCurrency for Pallet { from: &T::AccountId, to: &T::AccountId, amount: Self::Balance, + existence_requirement: ExistenceRequirement, ) -> DispatchResult { if amount.is_zero() || from == to { return Ok(()); } if currency_id == T::GetNativeCurrencyId::get() { - T::NativeCurrency::transfer(from, to, amount)?; + T::NativeCurrency::transfer(from, to, amount, existence_requirement)?; } else { - T::MultiCurrency::transfer(currency_id, from, to, amount)?; + T::MultiCurrency::transfer(currency_id, from, to, amount, existence_requirement)?; } Self::deposit_event(Event::Transferred(currency_id, from.clone(), to.clone(), amount)); Ok(()) @@ -427,8 +434,19 @@ where >::ensure_can_withdraw(GetCurrencyId::get(), who, amount) } - fn transfer(from: &T::AccountId, to: &T::AccountId, amount: Self::Balance) -> DispatchResult { - as MultiCurrency>::transfer(GetCurrencyId::get(), from, to, amount) + fn transfer( + from: &T::AccountId, + to: &T::AccountId, + amount: Self::Balance, + existence_requirement: ExistenceRequirement, + ) -> DispatchResult { + as MultiCurrency>::transfer( + GetCurrencyId::get(), + from, + to, + amount, + existence_requirement, + ) } fn deposit(who: &T::AccountId, amount: Self::Balance) -> DispatchResult { @@ -561,8 +579,13 @@ where Currency::ensure_can_withdraw(who, amount, WithdrawReasons::all(), new_balance) } - fn transfer(from: &AccountId, to: &AccountId, amount: Self::Balance) -> DispatchResult { - Currency::transfer(from, to, amount, ExistenceRequirement::AllowDeath) + fn transfer( + from: &AccountId, + to: &AccountId, + amount: Self::Balance, + existence_requirement: ExistenceRequirement, + ) -> DispatchResult { + Currency::transfer(from, to, amount, existence_requirement) } fn deposit(who: &AccountId, amount: Self::Balance) -> DispatchResult { @@ -685,7 +708,12 @@ impl TransferAll for Pallet { T::MultiCurrency::transfer_all(source, dest)?; // transfer all free to dest - T::NativeCurrency::transfer(source, dest, T::NativeCurrency::free_balance(source)) + T::NativeCurrency::transfer( + source, + dest, + T::NativeCurrency::free_balance(source), + ExistenceRequirement::AllowDeath, + ) }) } } diff --git a/currencies/src/tests.rs b/currencies/src/tests.rs index 0221d22e0..b1d693b1b 100644 --- a/currencies/src/tests.rs +++ b/currencies/src/tests.rs @@ -121,7 +121,12 @@ fn native_currency_should_work() { assert_eq!(NativeCurrency::free_balance(&ALICE), 50); assert_eq!(NativeCurrency::free_balance(&BOB), 150); - assert_ok!(NativeCurrency::transfer(&ALICE, &BOB, 10)); + assert_ok!(NativeCurrency::transfer( + &ALICE, + &BOB, + 10, + ExistenceRequirement::AllowDeath + )); assert_eq!(NativeCurrency::free_balance(&ALICE), 40); assert_eq!(NativeCurrency::free_balance(&BOB), 160); @@ -155,12 +160,22 @@ fn basic_currency_adapting_pallet_balances_transfer() { .one_hundred_for_alice_n_bob() .build() .execute_with(|| { - assert_ok!(AdaptedBasicCurrency::transfer(&ALICE, &BOB, 50)); + assert_ok!(AdaptedBasicCurrency::transfer( + &ALICE, + &BOB, + 50, + ExistenceRequirement::AllowDeath + )); assert_eq!(PalletBalances::total_balance(&ALICE), 50); assert_eq!(PalletBalances::total_balance(&BOB), 150); // creation fee - assert_ok!(AdaptedBasicCurrency::transfer(&ALICE, &EVA, 10)); + assert_ok!(AdaptedBasicCurrency::transfer( + &ALICE, + &EVA, + 10, + ExistenceRequirement::AllowDeath + )); assert_eq!(PalletBalances::total_balance(&ALICE), 40); assert_eq!(PalletBalances::total_balance(&EVA), 10); }); @@ -257,7 +272,11 @@ fn call_event_should_work() { System::assert_last_event(Event::currencies(crate::Event::Transferred(X_TOKEN_ID, ALICE, BOB, 50))); assert_ok!(>::transfer( - X_TOKEN_ID, &ALICE, &BOB, 10 + X_TOKEN_ID, + &ALICE, + &BOB, + 10, + ExistenceRequirement::AllowDeath )); assert_eq!(Currencies::free_balance(X_TOKEN_ID, &ALICE), 40); assert_eq!(Currencies::free_balance(X_TOKEN_ID, &BOB), 160); diff --git a/traits/src/currency.rs b/traits/src/currency.rs index 497c6a52e..336788893 100644 --- a/traits/src/currency.rs +++ b/traits/src/currency.rs @@ -213,7 +213,12 @@ pub trait BasicCurrency { // Public mutables /// Transfer some amount from one account to another. - fn transfer(from: &AccountId, to: &AccountId, amount: Self::Balance) -> DispatchResult; + fn transfer( + from: &AccountId, + to: &AccountId, + amount: Self::Balance, + existence_requirement: ExistenceRequirement, + ) -> DispatchResult; /// Add `amount` to the balance of `who` and increase total issuance. fn deposit(who: &AccountId, amount: Self::Balance) -> DispatchResult; From 2d3d8d9b6192c5f59698dc11790ddb4a023ba3c5 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Mon, 14 Jun 2021 09:43:59 +0800 Subject: [PATCH 7/9] revert BasicCurrency ExistenceRequirement --- currencies/src/lib.rs | 29 +++++++---------------------- currencies/src/tests.rs | 21 +++------------------ traits/src/currency.rs | 7 +------ 3 files changed, 11 insertions(+), 46 deletions(-) diff --git a/currencies/src/lib.rs b/currencies/src/lib.rs index f631bdad3..13d90c460 100644 --- a/currencies/src/lib.rs +++ b/currencies/src/lib.rs @@ -167,7 +167,7 @@ pub mod module { ) -> DispatchResultWithPostInfo { let from = ensure_signed(origin)?; let to = T::Lookup::lookup(dest)?; - T::NativeCurrency::transfer(&from, &to, amount, ExistenceRequirement::AllowDeath)?; + T::NativeCurrency::transfer(&from, &to, amount)?; Self::deposit_event(Event::Transferred(T::GetNativeCurrencyId::get(), from, to, amount)); Ok(().into()) @@ -246,7 +246,7 @@ impl MultiCurrency for Pallet { return Ok(()); } if currency_id == T::GetNativeCurrencyId::get() { - T::NativeCurrency::transfer(from, to, amount, existence_requirement)?; + T::NativeCurrency::transfer(from, to, amount)?; } else { T::MultiCurrency::transfer(currency_id, from, to, amount, existence_requirement)?; } @@ -434,18 +434,13 @@ where >::ensure_can_withdraw(GetCurrencyId::get(), who, amount) } - fn transfer( - from: &T::AccountId, - to: &T::AccountId, - amount: Self::Balance, - existence_requirement: ExistenceRequirement, - ) -> DispatchResult { + fn transfer(from: &T::AccountId, to: &T::AccountId, amount: Self::Balance) -> DispatchResult { as MultiCurrency>::transfer( GetCurrencyId::get(), from, to, amount, - existence_requirement, + ExistenceRequirement::AllowDeath, ) } @@ -579,13 +574,8 @@ where Currency::ensure_can_withdraw(who, amount, WithdrawReasons::all(), new_balance) } - fn transfer( - from: &AccountId, - to: &AccountId, - amount: Self::Balance, - existence_requirement: ExistenceRequirement, - ) -> DispatchResult { - Currency::transfer(from, to, amount, existence_requirement) + fn transfer(from: &AccountId, to: &AccountId, amount: Self::Balance) -> DispatchResult { + Currency::transfer(from, to, amount, ExistenceRequirement::AllowDeath) } fn deposit(who: &AccountId, amount: Self::Balance) -> DispatchResult { @@ -708,12 +698,7 @@ impl TransferAll for Pallet { T::MultiCurrency::transfer_all(source, dest)?; // transfer all free to dest - T::NativeCurrency::transfer( - source, - dest, - T::NativeCurrency::free_balance(source), - ExistenceRequirement::AllowDeath, - ) + T::NativeCurrency::transfer(source, dest, T::NativeCurrency::free_balance(source)) }) } } diff --git a/currencies/src/tests.rs b/currencies/src/tests.rs index b1d693b1b..503633d17 100644 --- a/currencies/src/tests.rs +++ b/currencies/src/tests.rs @@ -121,12 +121,7 @@ fn native_currency_should_work() { assert_eq!(NativeCurrency::free_balance(&ALICE), 50); assert_eq!(NativeCurrency::free_balance(&BOB), 150); - assert_ok!(NativeCurrency::transfer( - &ALICE, - &BOB, - 10, - ExistenceRequirement::AllowDeath - )); + assert_ok!(NativeCurrency::transfer(&ALICE, &BOB, 10,)); assert_eq!(NativeCurrency::free_balance(&ALICE), 40); assert_eq!(NativeCurrency::free_balance(&BOB), 160); @@ -160,22 +155,12 @@ fn basic_currency_adapting_pallet_balances_transfer() { .one_hundred_for_alice_n_bob() .build() .execute_with(|| { - assert_ok!(AdaptedBasicCurrency::transfer( - &ALICE, - &BOB, - 50, - ExistenceRequirement::AllowDeath - )); + assert_ok!(AdaptedBasicCurrency::transfer(&ALICE, &BOB, 50,)); assert_eq!(PalletBalances::total_balance(&ALICE), 50); assert_eq!(PalletBalances::total_balance(&BOB), 150); // creation fee - assert_ok!(AdaptedBasicCurrency::transfer( - &ALICE, - &EVA, - 10, - ExistenceRequirement::AllowDeath - )); + assert_ok!(AdaptedBasicCurrency::transfer(&ALICE, &EVA, 10,)); assert_eq!(PalletBalances::total_balance(&ALICE), 40); assert_eq!(PalletBalances::total_balance(&EVA), 10); }); diff --git a/traits/src/currency.rs b/traits/src/currency.rs index 336788893..497c6a52e 100644 --- a/traits/src/currency.rs +++ b/traits/src/currency.rs @@ -213,12 +213,7 @@ pub trait BasicCurrency { // Public mutables /// Transfer some amount from one account to another. - fn transfer( - from: &AccountId, - to: &AccountId, - amount: Self::Balance, - existence_requirement: ExistenceRequirement, - ) -> DispatchResult; + fn transfer(from: &AccountId, to: &AccountId, amount: Self::Balance) -> DispatchResult; /// Add `amount` to the balance of `who` and increase total issuance. fn deposit(who: &AccountId, amount: Self::Balance) -> DispatchResult; From 295feaa6acf3bdcd662695367494799f1759983d Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Mon, 14 Jun 2021 10:58:59 +0800 Subject: [PATCH 8/9] add helper do_transfer --- currencies/src/lib.rs | 19 ++------ currencies/src/tests.rs | 12 ++---- tokens/src/lib.rs | 96 ++++++++++++++++++----------------------- tokens/src/tests.rs | 12 ++---- traits/src/currency.rs | 3 +- 5 files changed, 53 insertions(+), 89 deletions(-) diff --git a/currencies/src/lib.rs b/currencies/src/lib.rs index 13d90c460..cec25c405 100644 --- a/currencies/src/lib.rs +++ b/currencies/src/lib.rs @@ -145,13 +145,7 @@ pub mod module { ) -> DispatchResultWithPostInfo { let from = ensure_signed(origin)?; let to = T::Lookup::lookup(dest)?; - >::transfer( - currency_id, - &from, - &to, - amount, - ExistenceRequirement::AllowDeath, - )?; + >::transfer(currency_id, &from, &to, amount)?; Ok(().into()) } @@ -240,7 +234,6 @@ impl MultiCurrency for Pallet { from: &T::AccountId, to: &T::AccountId, amount: Self::Balance, - existence_requirement: ExistenceRequirement, ) -> DispatchResult { if amount.is_zero() || from == to { return Ok(()); @@ -248,7 +241,7 @@ impl MultiCurrency for Pallet { if currency_id == T::GetNativeCurrencyId::get() { T::NativeCurrency::transfer(from, to, amount)?; } else { - T::MultiCurrency::transfer(currency_id, from, to, amount, existence_requirement)?; + T::MultiCurrency::transfer(currency_id, from, to, amount)?; } Self::deposit_event(Event::Transferred(currency_id, from.clone(), to.clone(), amount)); Ok(()) @@ -435,13 +428,7 @@ where } fn transfer(from: &T::AccountId, to: &T::AccountId, amount: Self::Balance) -> DispatchResult { - as MultiCurrency>::transfer( - GetCurrencyId::get(), - from, - to, - amount, - ExistenceRequirement::AllowDeath, - ) + as MultiCurrency>::transfer(GetCurrencyId::get(), from, to, amount) } fn deposit(who: &T::AccountId, amount: Self::Balance) -> DispatchResult { diff --git a/currencies/src/tests.rs b/currencies/src/tests.rs index 503633d17..0221d22e0 100644 --- a/currencies/src/tests.rs +++ b/currencies/src/tests.rs @@ -121,7 +121,7 @@ fn native_currency_should_work() { assert_eq!(NativeCurrency::free_balance(&ALICE), 50); assert_eq!(NativeCurrency::free_balance(&BOB), 150); - assert_ok!(NativeCurrency::transfer(&ALICE, &BOB, 10,)); + assert_ok!(NativeCurrency::transfer(&ALICE, &BOB, 10)); assert_eq!(NativeCurrency::free_balance(&ALICE), 40); assert_eq!(NativeCurrency::free_balance(&BOB), 160); @@ -155,12 +155,12 @@ fn basic_currency_adapting_pallet_balances_transfer() { .one_hundred_for_alice_n_bob() .build() .execute_with(|| { - assert_ok!(AdaptedBasicCurrency::transfer(&ALICE, &BOB, 50,)); + assert_ok!(AdaptedBasicCurrency::transfer(&ALICE, &BOB, 50)); assert_eq!(PalletBalances::total_balance(&ALICE), 50); assert_eq!(PalletBalances::total_balance(&BOB), 150); // creation fee - assert_ok!(AdaptedBasicCurrency::transfer(&ALICE, &EVA, 10,)); + assert_ok!(AdaptedBasicCurrency::transfer(&ALICE, &EVA, 10)); assert_eq!(PalletBalances::total_balance(&ALICE), 40); assert_eq!(PalletBalances::total_balance(&EVA), 10); }); @@ -257,11 +257,7 @@ fn call_event_should_work() { System::assert_last_event(Event::currencies(crate::Event::Transferred(X_TOKEN_ID, ALICE, BOB, 50))); assert_ok!(>::transfer( - X_TOKEN_ID, - &ALICE, - &BOB, - 10, - ExistenceRequirement::AllowDeath + X_TOKEN_ID, &ALICE, &BOB, 10 )); assert_eq!(Currencies::free_balance(X_TOKEN_ID, &ALICE), 40); assert_eq!(Currencies::free_balance(X_TOKEN_ID, &BOB), 160); diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index 12937924f..a3dae56ad 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -87,13 +87,7 @@ where fn on_dust(who: &T::AccountId, currency_id: T::CurrencyId, amount: T::Balance) { // transfer the dust to treasury account, ignore the result, // if failed will leave some dust which still could be recycled. - let _ = as MultiCurrency>::transfer( - currency_id, - who, - &GetAccountId::get(), - amount, - ExistenceRequirement::AllowDeath, - ); + let _ = as MultiCurrency>::transfer(currency_id, who, &GetAccountId::get(), amount); } } @@ -323,7 +317,7 @@ pub mod module { ) -> DispatchResultWithPostInfo { let from = ensure_signed(origin)?; let to = T::Lookup::lookup(dest)?; - >::transfer(currency_id, &from, &to, amount, ExistenceRequirement::AllowDeath)?; + >::transfer(currency_id, &from, &to, amount)?; Self::deposit_event(Event::Transferred(currency_id, from, to, amount)); Ok(().into()) @@ -342,13 +336,7 @@ pub mod module { let from = ensure_signed(origin)?; let to = T::Lookup::lookup(dest)?; let balance = >::free_balance(currency_id, &from); - >::transfer( - currency_id, - &from, - &to, - balance, - ExistenceRequirement::AllowDeath, - )?; + >::transfer(currency_id, &from, &to, balance)?; Self::deposit_event(Event::Transferred(currency_id, from, to, balance)); Ok(().into()) @@ -560,6 +548,41 @@ impl Pallet { Ok(()) } + + /// Transfer some free balance from `from` to `to`. + /// Is a no-op if value to be transferred is zero or the `from` is the + /// same as `to`. + pub(crate) fn do_transfer( + currency_id: T::CurrencyId, + from: &T::AccountId, + to: &T::AccountId, + amount: T::Balance, + existence_requirement: ExistenceRequirement, + ) -> DispatchResult { + if amount.is_zero() || from == to { + return Ok(()); + } + + Pallet::::try_mutate_account(to, currency_id, |to_account, _is_new| -> DispatchResult { + Pallet::::try_mutate_account(from, currency_id, |from_account, _is_new| -> DispatchResult { + from_account.free = from_account + .free + .checked_sub(&amount) + .ok_or(Error::::BalanceTooLow)?; + to_account.free = to_account.free.checked_add(&amount).ok_or(ArithmeticError::Overflow)?; + + Self::ensure_can_withdraw(currency_id, from, amount)?; + + let ed = T::ExistentialDeposits::get(¤cy_id); + let allow_death = existence_requirement == ExistenceRequirement::AllowDeath; + let allow_death = allow_death && !frame_system::Pallet::::is_provider_required(from); + ensure!(allow_death || from_account.total() >= ed, Error::::KeepAlive); + + Ok(()) + })?; + Ok(()) + }) + } } impl MultiCurrency for Pallet { @@ -608,31 +631,8 @@ impl MultiCurrency for Pallet { from: &T::AccountId, to: &T::AccountId, amount: Self::Balance, - existence_requirement: ExistenceRequirement, ) -> DispatchResult { - if amount.is_zero() || from == to { - return Ok(()); - } - - Pallet::::try_mutate_account(to, currency_id, |to_account, _is_new| -> DispatchResult { - Pallet::::try_mutate_account(from, currency_id, |from_account, _is_new| -> DispatchResult { - from_account.free = from_account - .free - .checked_sub(&amount) - .ok_or(Error::::BalanceTooLow)?; - to_account.free = to_account.free.checked_add(&amount).ok_or(ArithmeticError::Overflow)?; - - Self::ensure_can_withdraw(currency_id, from, amount)?; - - let ed = T::ExistentialDeposits::get(¤cy_id); - let allow_death = existence_requirement == ExistenceRequirement::AllowDeath; - let allow_death = allow_death && !frame_system::Pallet::::is_provider_required(from); - ensure!(allow_death || from_account.total() >= ed, Error::::KeepAlive); - - Ok(()) - })?; - Ok(()) - }) + Self::do_transfer(currency_id, from, to, amount, ExistenceRequirement::AllowDeath) } /// Deposit some `amount` into the free balance of account `who`. @@ -1004,7 +1004,7 @@ impl fungibles::Transfer for Pallet { } else { ExistenceRequirement::AllowDeath }; - as MultiCurrency>::transfer(asset_id, source, dest, amount, er).map(|_| amount) + Self::do_transfer(asset_id, source, dest, amount, er).map(|_| amount) } } @@ -1164,13 +1164,7 @@ where value: Self::Balance, existence_requirement: ExistenceRequirement, ) -> DispatchResult { - as MultiCurrency>::transfer( - GetCurrencyId::get(), - &source, - &dest, - value, - existence_requirement, - ) + Self::do_transfer(GetCurrencyId::get(), &source, &dest, value, existence_requirement) } fn slash(who: &T::AccountId, value: Self::Balance) -> (Self::NegativeImbalance, Self::Balance) { @@ -1342,13 +1336,7 @@ impl TransferAll for Pallet { #[transactional] fn transfer_all(source: &T::AccountId, dest: &T::AccountId) -> DispatchResult { Accounts::::iter_prefix(source).try_for_each(|(currency_id, account_data)| -> DispatchResult { - >::transfer( - currency_id, - source, - dest, - account_data.free, - ExistenceRequirement::AllowDeath, - ) + >::transfer(currency_id, source, dest, account_data.free) }) } } diff --git a/tokens/src/tests.rs b/tokens/src/tests.rs index afdd9dc03..afb065408 100644 --- a/tokens/src/tests.rs +++ b/tokens/src/tests.rs @@ -125,17 +125,11 @@ fn frozen_can_limit_liquidity() { .execute_with(|| { assert_ok!(Tokens::set_lock(ID_1, DOT, &ALICE, 90)); assert_noop!( - >::transfer(DOT, &ALICE, &BOB, 11, ExistenceRequirement::AllowDeath), + >::transfer(DOT, &ALICE, &BOB, 11), Error::::LiquidityRestrictions, ); assert_ok!(Tokens::set_lock(ID_1, DOT, &ALICE, 10)); - assert_ok!(>::transfer( - DOT, - &ALICE, - &BOB, - 11, - ExistenceRequirement::AllowDeath - ),); + assert_ok!(>::transfer(DOT, &ALICE, &BOB, 11)); }); } @@ -157,7 +151,7 @@ fn reserve_should_work() { .one_hundred_for_alice_n_bob() .build() .execute_with(|| { - assert_noop!(Tokens::reserve(DOT, &ALICE, 101), Error::::BalanceTooLow,); + assert_noop!(Tokens::reserve(DOT, &ALICE, 101), Error::::BalanceTooLow); assert_ok!(Tokens::reserve(DOT, &ALICE, 0)); assert_eq!(Tokens::free_balance(DOT, &ALICE), 100); assert_eq!(Tokens::reserved_balance(DOT, &ALICE), 0); diff --git a/traits/src/currency.rs b/traits/src/currency.rs index 497c6a52e..c104368ff 100644 --- a/traits/src/currency.rs +++ b/traits/src/currency.rs @@ -1,7 +1,7 @@ use crate::arithmetic; use codec::{Codec, FullCodec}; pub use frame_support::{ - traits::{BalanceStatus, ExistenceRequirement, LockIdentifier}, + traits::{BalanceStatus, LockIdentifier}, transactional, }; use sp_runtime::{ @@ -49,7 +49,6 @@ pub trait MultiCurrency { from: &AccountId, to: &AccountId, amount: Self::Balance, - existence_requirement: ExistenceRequirement, ) -> DispatchResult; /// Add `amount` to the balance of `who` under `currency_id` and increase From 967ff55aed83fc7757f8acb29b7bd08db4122112 Mon Sep 17 00:00:00 2001 From: zjb0807 Date: Mon, 14 Jun 2021 11:03:53 +0800 Subject: [PATCH 9/9] fix clippy --- tokens/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index a3dae56ad..a72cd2134 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -1164,7 +1164,7 @@ where value: Self::Balance, existence_requirement: ExistenceRequirement, ) -> DispatchResult { - Self::do_transfer(GetCurrencyId::get(), &source, &dest, value, existence_requirement) + Pallet::::do_transfer(GetCurrencyId::get(), &source, &dest, value, existence_requirement) } fn slash(who: &T::AccountId, value: Self::Balance) -> (Self::NegativeImbalance, Self::Balance) {