From b5b0ef592e515b12b7b51b931d8699acbc02f1d6 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Tue, 23 Mar 2021 14:10:36 +0100 Subject: [PATCH] Fungibles trait and impl for Assets pallet (#8425) * Fungibles trait and impl for Assets pallet * Comment & whitespace * Fixes * Fix up CI/CD for the new labels. * New labels. * Fix labels * Fix labels * Whitespace * Bump impl version. * Fix accidental change * Fixes * Questionable fix. * Better benchmark --- substrate/bin/node/runtime/src/lib.rs | 2 +- substrate/frame/assets/src/benchmarking.rs | 11 +- substrate/frame/assets/src/lib.rs | 263 ++++++++++++------- substrate/frame/staking/reward-fn/src/lib.rs | 7 +- substrate/frame/support/src/traits.rs | 16 ++ 5 files changed, 192 insertions(+), 107 deletions(-) diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index fd25c900da..5f5a3cc663 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -114,7 +114,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. spec_version: 265, - impl_version: 0, + impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 2, }; diff --git a/substrate/frame/assets/src/benchmarking.rs b/substrate/frame/assets/src/benchmarking.rs index 37300bf221..227d45623d 100644 --- a/substrate/frame/assets/src/benchmarking.rs +++ b/substrate/frame/assets/src/benchmarking.rs @@ -127,6 +127,14 @@ fn assert_last_event(generic_event: ::Event) { assert_eq!(event, &system_event); } +fn assert_event(generic_event: ::Event) { + let system_event: ::Event = generic_event.into(); + let events = frame_system::Pallet::::events(); + assert!(events.iter().any(|event_record| { + matches!(&event_record, frame_system::EventRecord { event, .. } if &system_event == event) + })); +} + benchmarks! { create { let caller: T::AccountId = whitelisted_caller(); @@ -383,7 +391,8 @@ benchmarks! { let dest_lookup = T::Lookup::unlookup(dest.clone()); }: _(SystemOrigin::Signed(delegate.clone()), id, owner_lookup, dest_lookup, amount) verify { - assert_last_event::(Event::TransferredApproved(id, owner, delegate, dest, amount).into()); + assert!(T::Currency::reserved_balance(&owner).is_zero()); + assert_event::(Event::Transferred(id, owner, dest, amount).into()); } cancel_approval { diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs index b8d436f106..e9f5445dd8 100644 --- a/substrate/frame/assets/src/lib.rs +++ b/substrate/frame/assets/src/lib.rs @@ -138,16 +138,49 @@ use sp_runtime::{ } }; use codec::{Encode, Decode, HasCompact}; -use frame_support::{ - ensure, - traits::{Currency, ReservableCurrency, BalanceStatus::Reserved}, - dispatch::{DispatchError, DispatchResult}, -}; +use frame_support::{ensure, dispatch::{DispatchError, DispatchResult}}; +use frame_support::traits::{Currency, ReservableCurrency, BalanceStatus::Reserved, Fungibles}; +use frame_system::Config as SystemConfig; pub use weights::WeightInfo; - pub use pallet::*; -type DepositBalanceOf = <::Currency as Currency<::AccountId>>::Balance; +impl Fungibles<::AccountId> for Pallet { + type AssetId = T::AssetId; + type Balance = T::Balance; + + fn balance( + asset: Self::AssetId, + who: &::AccountId, + ) -> Self::Balance { + Pallet::::balance(asset, who) + } + + fn can_deposit( + asset: Self::AssetId, + who: &::AccountId, + amount: Self::Balance, + ) -> bool { + Pallet::::can_deposit(asset, who, amount) + } + + fn deposit( + asset: Self::AssetId, + who: ::AccountId, + amount: Self::Balance, + ) -> DispatchResult { + Pallet::::increase_balance(asset, who, amount, None) + } + + fn withdraw( + asset: Self::AssetId, + who: ::AccountId, + amount: Self::Balance, + ) -> DispatchResult { + Pallet::::reduce_balance(asset, who, amount, None) + } +} + +type DepositBalanceOf = <::Currency as Currency<::AccountId>>::Balance; #[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug)] pub struct AssetDetails< @@ -273,7 +306,7 @@ pub mod pallet { /// The units in which we record balances. type Balance: Member + Parameter + AtLeast32BitUnsigned + Default + Copy; - /// The arithmetic type of asset identifier. + /// Identifier for the class of asset. type AssetId: Member + Parameter + Default + Copy + HasCompact; /// The currency mechanism. @@ -435,8 +468,7 @@ pub mod pallet { /// /// The origin must be Signed and the sender must have sufficient funds free. /// - /// Funds of sender are reserved according to the formula: - /// `AssetDepositBase + AssetDepositPerZombie * max_zombies`. + /// Funds of sender are reserved by `AssetDeposit`. /// /// Parameters: /// - `id`: The identifier of the new asset. This must not be currently in use to identify @@ -611,25 +643,7 @@ pub mod pallet { ) -> DispatchResult { let origin = ensure_signed(origin)?; let beneficiary = T::Lookup::lookup(beneficiary)?; - - Asset::::try_mutate(id, |maybe_details| { - let details = maybe_details.as_mut().ok_or(Error::::Unknown)?; - - ensure!(&origin == &details.issuer, Error::::NoPermission); - details.supply = details.supply.checked_add(&amount).ok_or(Error::::Overflow)?; - - Account::::try_mutate(id, &beneficiary, |t| -> DispatchResult { - let new_balance = t.balance.saturating_add(amount); - ensure!(new_balance >= details.min_balance, Error::::BalanceLow); - if t.balance.is_zero() { - t.sufficient = Self::new_account(&beneficiary, details)?; - } - t.balance = new_balance; - Ok(()) - })?; - Self::deposit_event(Event::Issued(id, beneficiary, amount)); - Ok(()) - }) + Self::increase_balance(id, beneficiary, amount, Some(origin)) } /// Reduce the balance of `who` by as much as possible up to `amount` assets of `id`. @@ -657,33 +671,7 @@ pub mod pallet { let origin = ensure_signed(origin)?; let who = T::Lookup::lookup(who)?; - Asset::::try_mutate(id, |maybe_details| { - let d = maybe_details.as_mut().ok_or(Error::::Unknown)?; - ensure!(&origin == &d.admin, Error::::NoPermission); - - let burned = Account::::try_mutate_exists( - id, - &who, - |maybe_account| -> Result { - let mut account = maybe_account.take().ok_or(Error::::BalanceZero)?; - let mut burned = amount.min(account.balance); - account.balance -= burned; - *maybe_account = if account.balance < d.min_balance { - burned += account.balance; - Self::dead_account(&who, d, account.sufficient); - None - } else { - Some(account) - }; - Ok(burned) - } - )?; - - d.supply = d.supply.saturating_sub(burned); - - Self::deposit_event(Event::Burned(id, who, burned)); - Ok(()) - }) + Self::reduce_balance(id, who, amount, Some(origin)) } /// Move some assets from the sender account to another. @@ -714,9 +702,7 @@ pub mod pallet { let origin = ensure_signed(origin)?; let dest = T::Lookup::lookup(target)?; - Self::do_transfer(id, &origin, &dest, amount, None, false)?; - Self::deposit_event(Event::Transferred(id, origin, dest, amount)); - Ok(()) + Self::do_transfer(id, origin, dest, amount, None, false) } /// Move some assets from the sender account to another, keeping the sender account alive. @@ -747,9 +733,7 @@ pub mod pallet { let origin = ensure_signed(origin)?; let dest = T::Lookup::lookup(target)?; - Self::do_transfer(id, &origin, &dest, amount, None, true)?; - Self::deposit_event(Event::Transferred(id, origin, dest, amount)); - Ok(()) + Self::do_transfer(id, origin, dest, amount, None, true) } /// Move some assets from one account to another. @@ -783,9 +767,7 @@ pub mod pallet { let source = T::Lookup::lookup(source)?; let dest = T::Lookup::lookup(dest)?; - Self::do_transfer(id, &source, &dest, amount, Some(origin), false)?; - Self::deposit_event(Event::Transferred(id, source, dest, amount)); - Ok(()) + Self::do_transfer(id, source, dest, amount, Some(origin), false) } /// Disallow further unprivileged transfers from an account. @@ -1338,7 +1320,7 @@ pub mod pallet { let mut approved = maybe_approved.take().ok_or(Error::::Unapproved)?; let remaining = approved.amount.checked_sub(&amount).ok_or(Error::::Unapproved)?; - Self::do_transfer(id, &key.owner, &destination, amount, None, false)?; + Self::do_transfer(id, key.owner.clone(), destination, amount, None, false)?; if remaining.is_zero() { T::Currency::unreserve(&key.owner, approved.deposit); @@ -1348,8 +1330,6 @@ pub mod pallet { } Ok(()) })?; - let event = Event::TransferredApproved(id, key.owner, key.delegate, destination, amount); - Self::deposit_event(event); Ok(()) } } @@ -1360,8 +1340,8 @@ impl Pallet { // Public immutables /// Get the asset `id` balance of `who`. - pub fn balance(id: T::AssetId, who: T::AccountId) -> T::Balance { - Account::::get(id, who).balance + pub fn balance(id: T::AssetId, who: impl sp_std::borrow::Borrow) -> T::Balance { + Account::::get(id, who.borrow()).balance } /// Get the total supply of an asset `id`. @@ -1400,15 +1380,97 @@ impl Pallet { d.accounts = d.accounts.saturating_sub(1); } + fn can_deposit(id: T::AssetId, who: &T::AccountId, amount: T::Balance) -> bool { + let details = match Asset::::get(id) { + Some(details) => details, + None => return false, + }; + if details.supply.checked_add(&amount).is_none() { return false } + let account = Account::::get(id, who); + if account.balance.checked_add(&amount).is_none() { return false } + if account.balance.is_zero() { + if amount < details.min_balance { return false } + if !details.is_sufficient && frame_system::Pallet::::providers(who) == 0 { return false } + if details.is_sufficient && details.sufficients.checked_add(1).is_none() { return false } + } + + true + } + + fn increase_balance( + id: T::AssetId, + beneficiary: T::AccountId, + amount: T::Balance, + maybe_check_issuer: Option, + ) -> DispatchResult { + Asset::::try_mutate(id, |maybe_details| { + let details = maybe_details.as_mut().ok_or(Error::::Unknown)?; + + if let Some(check_issuer) = maybe_check_issuer { + ensure!(&check_issuer == &details.issuer, Error::::NoPermission); + } + details.supply = details.supply.checked_add(&amount).ok_or(Error::::Overflow)?; + + Account::::try_mutate(id, &beneficiary, |t| -> DispatchResult { + let new_balance = t.balance.saturating_add(amount); + ensure!(new_balance >= details.min_balance, Error::::BalanceLow); + if t.balance.is_zero() { + t.sufficient = Self::new_account(&beneficiary, details)?; + } + t.balance = new_balance; + Ok(()) + })?; + Self::deposit_event(Event::Issued(id, beneficiary, amount)); + Ok(()) + }) + } + + fn reduce_balance( + id: T::AssetId, + target: T::AccountId, + amount: T::Balance, + maybe_check_admin: Option, + ) -> DispatchResult { + Asset::::try_mutate(id, |maybe_details| { + let d = maybe_details.as_mut().ok_or(Error::::Unknown)?; + if let Some(check_admin) = maybe_check_admin { + ensure!(&check_admin == &d.admin, Error::::NoPermission); + } + + let burned = Account::::try_mutate_exists( + id, + &target, + |maybe_account| -> Result { + let mut account = maybe_account.take().ok_or(Error::::BalanceZero)?; + let mut burned = amount.min(account.balance); + account.balance -= burned; + *maybe_account = if account.balance < d.min_balance { + burned += account.balance; + Self::dead_account(&target, d, account.sufficient); + None + } else { + Some(account) + }; + Ok(burned) + } + )?; + + d.supply = d.supply.saturating_sub(burned); + + Self::deposit_event(Event::Burned(id, target, burned)); + Ok(()) + }) + } + fn do_transfer( id: T::AssetId, - source: &T::AccountId, - dest: &T::AccountId, + source: T::AccountId, + dest: T::AccountId, amount: T::Balance, maybe_need_admin: Option, keep_alive: bool, ) -> DispatchResult { - let mut source_account = Account::::get(id, source); + let mut source_account = Account::::get(id, &source); ensure!(!source_account.is_frozen, Error::::Frozen); source_account.balance = source_account.balance.checked_sub(&amount) @@ -1422,38 +1484,37 @@ impl Pallet { ensure!(&need_admin == &details.admin, Error::::NoPermission); } - if dest == source || amount.is_zero() { - return Ok(()) - } - - let mut amount = amount; - if source_account.balance < details.min_balance { - ensure!(!keep_alive, Error::::WouldDie); - amount += source_account.balance; - source_account.balance = Zero::zero(); - } - - Account::::try_mutate(id, dest, |a| -> DispatchResult { - let new_balance = a.balance.saturating_add(amount); - - // This is impossible since `new_balance > amount > min_balance`, but we can - // handle it, so we do. - ensure!(new_balance >= details.min_balance, Error::::BalanceLow); - - if a.balance.is_zero() { - a.sufficient = Self::new_account(dest, details)?; + if dest != source && !amount.is_zero() { + let mut amount = amount; + if source_account.balance < details.min_balance { + ensure!(!keep_alive, Error::::WouldDie); + amount += source_account.balance; + source_account.balance = Zero::zero(); } - a.balance = new_balance; - Ok(()) - })?; - if source_account.balance.is_zero() { - Self::dead_account(source, details, source_account.sufficient); - Account::::remove(id, source); - } else { - Account::::insert(id, source, &source_account) + Account::::try_mutate(id, &dest, |a| -> DispatchResult { + let new_balance = a.balance.saturating_add(amount); + + // This is impossible since `new_balance > amount > min_balance`, but we can + // handle it, so we do. + ensure!(new_balance >= details.min_balance, Error::::BalanceLow); + + if a.balance.is_zero() { + a.sufficient = Self::new_account(&dest, details)?; + } + a.balance = new_balance; + Ok(()) + })?; + + if source_account.balance.is_zero() { + Self::dead_account(&source, details, source_account.sufficient); + Account::::remove(id, &source); + } else { + Account::::insert(id, &source, &source_account) + } } + Self::deposit_event(Event::Transferred(id, source, dest, amount)); Ok(()) }) } diff --git a/substrate/frame/staking/reward-fn/src/lib.rs b/substrate/frame/staking/reward-fn/src/lib.rs index 6c54fec923..205f020767 100644 --- a/substrate/frame/staking/reward-fn/src/lib.rs +++ b/substrate/frame/staking/reward-fn/src/lib.rs @@ -79,7 +79,9 @@ pub fn compute_inflation( falloff.lstrip(); let ln2 = { - let ln2 = P::from_rational(LN2.deconstruct().into(), Perquintill::ACCURACY.into()); + /// `ln(2)` expressed in as perquintillionth. + const LN2: u64 = 0_693_147_180_559_945_309; + let ln2 = P::from_rational(LN2.into(), Perquintill::ACCURACY.into()); BigUint::from(ln2.deconstruct().into()) }; @@ -119,9 +121,6 @@ struct INPoSParam { accuracy: BigUint, } -/// `ln(2)` expressed in as perquintillionth. -const LN2: Perquintill = Perquintill::from_parts(0_693_147_180_559_945_309); - /// Compute `2^((x_ideal - x) / d)` using taylor serie. /// /// x must be strictly more than x_ideal. diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 286c545d30..9f8afdf7c7 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -1128,6 +1128,22 @@ pub trait Currency { ) -> SignedImbalance; } +/// Trait for providing an ERC-20 style set of named fungible assets. +pub trait Fungibles { + /// Means of identifying one asset class from another. + type AssetId: FullCodec + Copy + Default; + /// Scalar type for storing balance of an account. + type Balance: AtLeast32BitUnsigned + FullCodec + Copy + Default; + /// Get the `asset` balance of `who`. + fn balance(asset: Self::AssetId, who: &AccountId) -> Self::Balance; + /// Returns `true` if the `asset` balance of `who` may be increased by `amount`. + fn can_deposit(asset: Self::AssetId, who: &AccountId, amount: Self::Balance) -> bool; + /// Increase the `asset` balance of `who` by `amount`. + fn deposit(asset: Self::AssetId, who: AccountId, amount: Self::Balance) -> DispatchResult; + /// Attempt to reduce the `asset` balance of `who` by `amount`. + fn withdraw(asset: Self::AssetId, who: AccountId, amount: Self::Balance) -> DispatchResult; +} + /// Status of funds. #[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug)] pub enum BalanceStatus {