From 3c447b9e3afe6a263015fb68e2f91739adfb031c Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 3 May 2023 16:34:58 +0200 Subject: [PATCH 1/4] replace is_fronzen flag by status enum --- frame/assets/src/functions.rs | 12 ++++++------ frame/assets/src/lib.rs | 6 ++++-- frame/assets/src/types.rs | 27 +++++++++++++++++++++++++-- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/frame/assets/src/functions.rs b/frame/assets/src/functions.rs index 65213f66cc402..ade463fcc9c29 100644 --- a/frame/assets/src/functions.rs +++ b/frame/assets/src/functions.rs @@ -179,7 +179,7 @@ impl, I: 'static> Pallet { Some(a) => a, None => return BalanceLow, }; - if account.is_frozen { + if account.status.is_frozen() { return Frozen } if let Some(rest) = account.balance.checked_sub(&amount) { @@ -220,7 +220,7 @@ impl, I: 'static> Pallet { ensure!(details.status == AssetStatus::Live, Error::::AssetNotLive); let account = Account::::get(id, who).ok_or(Error::::NoAccount)?; - ensure!(!account.is_frozen, Error::::Frozen); + ensure!(!account.status.is_frozen(), Error::::Frozen); let amount = if let Some(frozen) = T::Freezer::frozen_balance(id, who) { // Frozen balance: account CANNOT be deleted @@ -329,7 +329,7 @@ impl, I: 'static> Pallet { &who, AssetAccountOf:: { balance: Zero::zero(), - is_frozen: false, + status: AccountStatus::Liquid, reason, extra: T::Extra::default(), }, @@ -378,7 +378,7 @@ impl, I: 'static> Pallet { account.reason.take_deposit_from().ok_or(Error::::NoDeposit)?; let mut details = Asset::::get(&id).ok_or(Error::::Unknown)?; ensure!(details.status == AssetStatus::Live, Error::::AssetNotLive); - ensure!(!account.is_frozen, Error::::Frozen); + ensure!(!account.status.is_frozen(), Error::::Frozen); ensure!(caller == &depositor || caller == &details.admin, Error::::NoPermission); ensure!(account.balance.is_zero(), Error::::WouldBurn); @@ -460,7 +460,7 @@ impl, I: 'static> Pallet { *maybe_account = Some(AssetAccountOf:: { balance: amount, reason: Self::new_account(beneficiary, details, None)?, - is_frozen: false, + status: AccountStatus::Liquid, extra: T::Extra::default(), }); }, @@ -654,7 +654,7 @@ impl, I: 'static> Pallet { maybe_account @ None => { *maybe_account = Some(AssetAccountOf:: { balance: credit, - is_frozen: false, + status: AccountStatus::Liquid, reason: Self::new_account(dest, details, None)?, extra: T::Extra::default(), }); diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs index c2b8481f56d70..9c476528666b7 100644 --- a/frame/assets/src/lib.rs +++ b/frame/assets/src/lib.rs @@ -949,7 +949,8 @@ pub mod pallet { let who = T::Lookup::lookup(who)?; Account::::try_mutate(id, &who, |maybe_account| -> DispatchResult { - maybe_account.as_mut().ok_or(Error::::NoAccount)?.is_frozen = true; + maybe_account.as_mut().ok_or(Error::::NoAccount)?.status = + AccountStatus::Frozen; Ok(()) })?; @@ -985,7 +986,8 @@ pub mod pallet { let who = T::Lookup::lookup(who)?; Account::::try_mutate(id, &who, |maybe_account| -> DispatchResult { - maybe_account.as_mut().ok_or(Error::::NoAccount)?.is_frozen = false; + maybe_account.as_mut().ok_or(Error::::NoAccount)?.status = + AccountStatus::Liquid; Ok(()) })?; diff --git a/frame/assets/src/types.rs b/frame/assets/src/types.rs index 0b919bb010630..4e9046b82e5c0 100644 --- a/frame/assets/src/types.rs +++ b/frame/assets/src/types.rs @@ -145,12 +145,35 @@ where } } +#[test] +fn ensure_bool_decodes_to_liquid_or_frozen() { + assert_eq!(false.encode(), AccountStatus::Liquid.encode()); + assert_eq!(true.encode(), AccountStatus::Frozen.encode()); +} + +/// The status of an asset account. +#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen, TypeInfo)] +pub enum AccountStatus { + /// Asset account can receive and transfer the assets. + Liquid, + /// Asset account cannot transfer the assets. + Frozen, + /// Asset account cannot receive and transfer the assets. + Blocked, +} +impl AccountStatus { + /// Returns `true` if frozen or blocked. + pub(crate) fn is_frozen(&self) -> bool { + matches!(self, AccountStatus::Frozen | AccountStatus::Blocked) + } +} + #[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen, TypeInfo)] pub struct AssetAccount { /// The balance. pub(super) balance: Balance, - /// Whether the account is frozen. - pub(super) is_frozen: bool, + /// The status of the account. + pub(super) status: AccountStatus, /// The reason for the existence of the account. pub(super) reason: ExistenceReason, /// Additional "sidecar" data, in case some other pallet wants to use this storage item. From c2f8f8ea5e54041d76d556e68d5c166bc8fb86a2 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 3 May 2023 17:51:54 +0200 Subject: [PATCH 2/4] block asset account --- frame/assets/src/benchmarking.rs | 7 ++++ frame/assets/src/functions.rs | 7 ++-- frame/assets/src/lib.rs | 47 +++++++++++++++++++++++-- frame/assets/src/tests.rs | 31 ++++++++++++++++ frame/assets/src/types.rs | 4 +++ frame/assets/src/weights.rs | 27 ++++++++++++++ frame/support/src/traits/tokens/misc.rs | 3 ++ primitives/runtime/src/lib.rs | 3 ++ 8 files changed, 124 insertions(+), 5 deletions(-) diff --git a/frame/assets/src/benchmarking.rs b/frame/assets/src/benchmarking.rs index ab92f0e107d7f..9685bc2b8ac97 100644 --- a/frame/assets/src/benchmarking.rs +++ b/frame/assets/src/benchmarking.rs @@ -532,5 +532,12 @@ benchmarks_instance_pallet! { assert!(T::Currency::reserved_balance(&asset_owner).is_zero()); } + block { + let (asset_id, caller, caller_lookup) = create_default_minted_asset::(true, 100u32.into()); + }: _(SystemOrigin::Signed(caller.clone()), asset_id, caller_lookup) + verify { + assert_last_event::(Event::Blocked { asset_id: asset_id.into(), who: caller }.into()); + } + impl_benchmark_test_suite!(Assets, crate::mock::new_test_ext(), crate::mock::Test) } diff --git a/frame/assets/src/functions.rs b/frame/assets/src/functions.rs index ade463fcc9c29..4dc0c988166b1 100644 --- a/frame/assets/src/functions.rs +++ b/frame/assets/src/functions.rs @@ -135,8 +135,11 @@ impl, I: 'static> Pallet { if increase_supply && details.supply.checked_add(&amount).is_none() { return DepositConsequence::Overflow } - if let Some(balance) = Self::maybe_balance(id, who) { - if balance.checked_add(&amount).is_none() { + if let Some(account) = Account::::get(id, who) { + if account.status.is_blocked() { + return DepositConsequence::Blocked + } + if account.balance.checked_add(&amount).is_none() { return DepositConsequence::Overflow } } else { diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs index 9c476528666b7..5060818767ade 100644 --- a/frame/assets/src/lib.rs +++ b/frame/assets/src/lib.rs @@ -102,7 +102,7 @@ //! * `burn`: Decreases the asset balance of an account; called by the asset class's Admin. //! * `force_transfer`: Transfers between arbitrary accounts; called by the asset class's Admin. //! * `freeze`: Disallows further `transfer`s from an account; called by the asset class's Freezer. -//! * `thaw`: Allows further `transfer`s from an account; called by the asset class's Admin. +//! * `thaw`: Allows further `transfer`s to and from an account; called by the asset class's Admin. //! * `transfer_ownership`: Changes an asset class's Owner; called by the asset class's Owner. //! * `set_team`: Changes an asset class's Admin, Freezer and Issuer; called by the asset class's //! Owner. @@ -110,6 +110,8 @@ //! * `clear_metadata`: Remove the metadata of an asset class; called by the asset class's Owner. //! * `touch_other`: Create an asset account for specified account. Caller must place a deposit; //! called by the asset class's Freezer or Admin. +//! * `block`: Disallows further `transfer`s to and from an account; called by the asset class's +//! Freezer. //! //! Please refer to the [`Call`] enum and its associated variants for documentation on each //! function. @@ -197,7 +199,7 @@ pub trait AssetsCallback { /// Empty implementation in case no callbacks are required. impl AssetsCallback for () {} -#[frame_support::pallet] +#[frame_support::pallet()] pub mod pallet { use super::*; use frame_support::pallet_prelude::*; @@ -527,6 +529,8 @@ pub mod pallet { AssetMinBalanceChanged { asset_id: T::AssetId, new_min_balance: T::Balance }, /// Some account `who` was created with a deposit from `depositor`. Touched { asset_id: T::AssetId, who: T::AccountId, depositor: T::AccountId }, + /// Some account `who` was blocked. + Blocked { asset_id: T::AssetId, who: T::AccountId }, } #[pallet::error] @@ -958,7 +962,7 @@ pub mod pallet { Ok(()) } - /// Allow unprivileged transfers from an account again. + /// Allow unprivileged transfers to and from an account again. /// /// Origin must be Signed and the sender should be the Admin of the asset `id`. /// @@ -1602,6 +1606,43 @@ pub mod pallet { let id: T::AssetId = id.into(); Self::do_refund_other(id, &who, &origin) } + + /// Disallow further unprivileged transfers of an asset `id` to and from an account `who`. + /// + /// Origin must be Signed and the sender should be the Freezer of the asset `id`. + /// + /// - `id`: The identifier of the account's asset. + /// - `who`: The account to be unblocked. + /// + /// Emits `Unblocked`. + /// + /// Weight: `O(1)` + #[pallet::call_index(31)] + pub fn block( + origin: OriginFor, + id: T::AssetIdParameter, + who: AccountIdLookupOf, + ) -> DispatchResult { + let origin = ensure_signed(origin)?; + let id: T::AssetId = id.into(); + + let d = Asset::::get(id).ok_or(Error::::Unknown)?; + ensure!( + d.status == AssetStatus::Live || d.status == AssetStatus::Frozen, + Error::::AssetNotLive + ); + ensure!(origin == d.freezer, Error::::NoPermission); + let who = T::Lookup::lookup(who)?; + + Account::::try_mutate(id, &who, |maybe_account| -> DispatchResult { + maybe_account.as_mut().ok_or(Error::::NoAccount)?.status = + AccountStatus::Blocked; + Ok(()) + })?; + + Self::deposit_event(Event::::Blocked { asset_id: id, who }); + Ok(()) + } } } diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs index 44fa9f7616de3..009d0180f36bc 100644 --- a/frame/assets/src/tests.rs +++ b/frame/assets/src/tests.rs @@ -738,6 +738,37 @@ fn approve_transfer_frozen_asset_should_not_work() { }); } +#[test] +fn transferring_from_blocked_account_should_not_work() { + new_test_ext().execute_with(|| { + assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100)); + assert_eq!(Assets::balance(0, 1), 100); + assert_ok!(Assets::block(RuntimeOrigin::signed(1), 0, 1)); + // behaves as frozen when transferring from blocked + assert_noop!(Assets::transfer(RuntimeOrigin::signed(1), 0, 2, 50), Error::::Frozen); + assert_ok!(Assets::thaw(RuntimeOrigin::signed(1), 0, 1)); + assert_ok!(Assets::transfer(RuntimeOrigin::signed(1), 0, 2, 50)); + assert_ok!(Assets::transfer(RuntimeOrigin::signed(2), 0, 1, 50)); + }); +} + +#[test] +fn transferring_to_blocked_account_should_not_work() { + new_test_ext().execute_with(|| { + assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, 1, true, 1)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 2, 100)); + assert_eq!(Assets::balance(0, 1), 100); + assert_eq!(Assets::balance(0, 2), 100); + assert_ok!(Assets::block(RuntimeOrigin::signed(1), 0, 1)); + assert_noop!(Assets::transfer(RuntimeOrigin::signed(2), 0, 1, 50), TokenError::Blocked); + assert_ok!(Assets::thaw(RuntimeOrigin::signed(1), 0, 1)); + assert_ok!(Assets::transfer(RuntimeOrigin::signed(2), 0, 1, 50)); + assert_ok!(Assets::transfer(RuntimeOrigin::signed(1), 0, 2, 50)); + }); +} + #[test] fn origin_guards_should_work() { new_test_ext().execute_with(|| { diff --git a/frame/assets/src/types.rs b/frame/assets/src/types.rs index 4e9046b82e5c0..559afccb946c5 100644 --- a/frame/assets/src/types.rs +++ b/frame/assets/src/types.rs @@ -166,6 +166,10 @@ impl AccountStatus { pub(crate) fn is_frozen(&self) -> bool { matches!(self, AccountStatus::Frozen | AccountStatus::Blocked) } + /// Returns `true` if blocked. + pub(crate) fn is_blocked(&self) -> bool { + matches!(self, AccountStatus::Blocked) + } } #[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, MaxEncodedLen, TypeInfo)] diff --git a/frame/assets/src/weights.rs b/frame/assets/src/weights.rs index fee30dae80aa5..d0aa9adb5deb6 100644 --- a/frame/assets/src/weights.rs +++ b/frame/assets/src/weights.rs @@ -80,6 +80,7 @@ pub trait WeightInfo { fn touch() -> Weight; fn refund() -> Weight; fn refund_other() -> Weight; + fn block() -> Weight; } /// Weights for pallet_assets using the Substrate node and recommended hardware. @@ -501,6 +502,19 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } + /// Storage: Assets Asset (r:1 w:0) + /// Proof: Assets Asset (max_values: None, max_size: Some(210), added: 2685, mode: MaxEncodedLen) + /// Storage: Assets Account (r:1 w:1) + /// Proof: Assets Account (max_values: None, max_size: Some(134), added: 2609, mode: MaxEncodedLen) + fn block() -> Weight { + // Proof Size summary in bytes: + // Measured: `459` + // Estimated: `3675` + // Minimum execution time: 115_000_000 picoseconds. + Weight::from_parts(163_000_000, 3675) + .saturating_add(T::DbWeight::get().reads(2_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } // For backwards compatibility and tests @@ -921,4 +935,17 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } + /// Storage: Assets Asset (r:1 w:0) + /// Proof: Assets Asset (max_values: None, max_size: Some(210), added: 2685, mode: MaxEncodedLen) + /// Storage: Assets Account (r:1 w:1) + /// Proof: Assets Account (max_values: None, max_size: Some(134), added: 2609, mode: MaxEncodedLen) + fn block() -> Weight { + // Proof Size summary in bytes: + // Measured: `459` + // Estimated: `3675` + // Minimum execution time: 115_000_000 picoseconds. + Weight::from_parts(163_000_000, 3675) + .saturating_add(RocksDbWeight::get().reads(2_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) + } } diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index 75aef0e04ea65..0ba900e95f9b8 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -141,6 +141,8 @@ pub enum DepositConsequence { Overflow, /// Account continued in existence. Success, + /// Account cannot receive the assets. + Blocked, } impl DepositConsequence { @@ -152,6 +154,7 @@ impl DepositConsequence { CannotCreate => TokenError::CannotCreate.into(), UnknownAsset => TokenError::UnknownAsset.into(), Overflow => ArithmeticError::Overflow.into(), + Blocked => TokenError::Blocked.into(), Success => return Ok(()), }) } diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 0994dc21b31dd..d17338bc37707 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -624,6 +624,8 @@ pub enum TokenError { CannotCreateHold, /// Withdrawal would cause unwanted loss of account. NotExpendable, + /// Account cannot receive the assets. + Blocked, } impl From for &'static str { @@ -639,6 +641,7 @@ impl From for &'static str { TokenError::CannotCreateHold => "Account cannot be created for recording amount on hold", TokenError::NotExpendable => "Account that is desired to remain would die", + TokenError::Blocked => "Account cannot receive the assets", } } } From c1a1931e2062c765d8a353daaa87fe3e18068bf8 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 3 May 2023 17:55:13 +0200 Subject: [PATCH 3/4] remove redundant brackets --- frame/assets/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs index 5060818767ade..34fa48778fd1e 100644 --- a/frame/assets/src/lib.rs +++ b/frame/assets/src/lib.rs @@ -199,7 +199,7 @@ pub trait AssetsCallback { /// Empty implementation in case no callbacks are required. impl AssetsCallback for () {} -#[frame_support::pallet()] +#[frame_support::pallet] pub mod pallet { use super::*; use frame_support::pallet_prelude::*; From 330201ed884a8bc54e32c08170e3f96826231a81 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 3 May 2023 18:45:51 +0200 Subject: [PATCH 4/4] fix typo --- frame/assets/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs index 34fa48778fd1e..c300b10b5d243 100644 --- a/frame/assets/src/lib.rs +++ b/frame/assets/src/lib.rs @@ -1614,7 +1614,7 @@ pub mod pallet { /// - `id`: The identifier of the account's asset. /// - `who`: The account to be unblocked. /// - /// Emits `Unblocked`. + /// Emits `Blocked`. /// /// Weight: `O(1)` #[pallet::call_index(31)]