Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow Creation of Asset Accounts That Don't Exist Yet and Add Blocked Status #13843

Merged
merged 44 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
b71655a
prevent frozen accounts from receiving assets
joepetrowski Apr 4, 2023
cea71eb
Merge remote-tracking branch 'origin' into joe-asset-freezecreating
joepetrowski Apr 4, 2023
97faa51
refund deposits correctly
joepetrowski Apr 4, 2023
16f6980
plus refund_other
joepetrowski Apr 4, 2023
eaaa623
add benchmarks
joepetrowski Apr 6, 2023
d64fc90
start migration work
joepetrowski Apr 6, 2023
80eac19
docs
joepetrowski Apr 6, 2023
d3c51df
add migration logic
joepetrowski Apr 6, 2023
5bcd359
Merge remote-tracking branch 'origin' into joe-asset-freezecreating
joepetrowski Apr 6, 2023
67c04cc
fix freeze_creating benchmark
joepetrowski Apr 7, 2023
489dea5
support instanced migrations
joepetrowski Apr 7, 2023
6fc0ff0
review
joepetrowski Apr 7, 2023
8fe2e45
correct deposit refund
joepetrowski Apr 7, 2023
c5ece2f
only allow depositor, admin, or account origin to refund deposits
joepetrowski Apr 9, 2023
7634da2
make sure refund actually removes account
joepetrowski Apr 11, 2023
7ec32cf
Merge remote-tracking branch 'origin' into joe-asset-freezecreating
joepetrowski Apr 11, 2023
26efe89
do refund changes
muharem Apr 11, 2023
6d4841d
Asset's account deposit owner (#13874)
muharem Apr 11, 2023
348dfef
storage version back to 1
muharem Apr 11, 2023
5f0a59c
update doc
muharem Apr 11, 2023
237f7ee
fix benches
muharem Apr 11, 2023
b376f2c
update docs
joepetrowski Apr 11, 2023
7d336d0
more tests
joepetrowski Apr 12, 2023
6e43655
Update frame/assets/src/types.rs
joepetrowski Apr 12, 2023
6d10154
refund updating the reason
muharem Apr 18, 2023
90da761
refactor
muharem Apr 18, 2023
423f5aa
separate refund and refund_foreign
muharem Apr 19, 2023
bb2693f
refunds, touch_other, tests
muharem May 1, 2023
be76a98
fixes
muharem May 1, 2023
5256614
Merge remote-tracking branch 'origin/master' into joe-asset-freezecre…
muharem May 1, 2023
f7fc345
fmt
muharem May 1, 2023
301c741
".git/.scripts/commands/bench/bench.sh" pallet dev pallet_assets
May 1, 2023
54efcb4
tests: asserts asset account counts
muharem May 2, 2023
afaadf2
Merge remote-tracking branch 'origin/master' into joe-asset-freezecre…
May 4, 2023
239af28
Account touch trait (#14063)
muharem May 4, 2023
b9d0ef2
Block asset account (#14070)
muharem May 4, 2023
a61c53a
fmt
muharem May 4, 2023
3da5efe
Apply suggestions from code review
muharem May 4, 2023
6919695
rename permissionless to check_depositor
muharem May 4, 2023
c8f5df2
doc fix
muharem May 4, 2023
cca3f57
use account id lookup instead account id
muharem May 5, 2023
1faf270
add benchmark for touch_other
joepetrowski May 5, 2023
57e8719
Merge remote-tracking branch 'origin/master' into joe-asset-freezecre…
May 7, 2023
97fe05b
Merge remote-tracking branch 'origin/master' into joe-asset-freezecre…
May 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1860,6 +1860,7 @@ type Migrations = (
pallet_nomination_pools::migration::v2::MigrateToV2<Runtime>,
pallet_alliance::migration::Migration<Runtime>,
pallet_contracts::Migration<Runtime>,
pallet_assets::migration::v2::MigrateToV2<Runtime>,
);

/// MMR helper types.
Expand Down
68 changes: 68 additions & 0 deletions frame/assets/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,18 @@ benchmarks_instance_pallet! {
assert_last_event::<T, I>(Event::Frozen { asset_id: asset_id.into(), who: caller }.into());
}

freeze_creating {
let (asset_id, asset_owner, _asset_owner_lookup) = create_default_minted_asset::<T, I>(true, 100u32.into());
let target: T::AccountId = account("target", 1, SEED);
let target_lookup = T::Lookup::unlookup(target.clone());
assert_ne!(asset_owner, target);
T::Currency::make_free_balance_be(&asset_owner, DepositBalanceOf::<T, I>::max_value());
assert!(!Account::<T, I>::contains_key(asset_id.into(), &target));
}: _(SystemOrigin::Signed(asset_owner.clone()), asset_id, target_lookup)
verify {
assert_last_event::<T, I>(Event::Frozen { asset_id: asset_id.into(), who: target }.into());
}

thaw {
let (asset_id, caller, caller_lookup) = create_default_minted_asset::<T, I>(true, 100u32.into());
Assets::<T, I>::freeze(
Expand Down Expand Up @@ -482,5 +494,61 @@ benchmarks_instance_pallet! {
assert_last_event::<T, I>(Event::AssetMinBalanceChanged { asset_id: asset_id.into(), new_min_balance: 50u32.into() }.into());
}

touch {
let (asset_id, asset_owner, asset_owner_lookup) = create_default_asset::<T, I>(false);
let new_account: T::AccountId = account("newaccount", 1, SEED);
T::Currency::make_free_balance_be(&new_account, DepositBalanceOf::<T, I>::max_value());
assert_ne!(asset_owner, new_account);
assert!(!Account::<T, I>::contains_key(asset_id.into(), &new_account));
}: _(SystemOrigin::Signed(new_account.clone()), asset_id)
verify {
assert!(Account::<T, I>::contains_key(asset_id.into(), &new_account));
}

refund {
let (asset_id, asset_owner, asset_owner_lookup) = create_default_asset::<T, I>(false);
let new_account: T::AccountId = account("newaccount", 1, SEED);
T::Currency::make_free_balance_be(&new_account, DepositBalanceOf::<T, I>::max_value());
assert_ne!(asset_owner, new_account);
assert!(Assets::<T, I>::touch(
SystemOrigin::Signed(new_account.clone()).into(),
asset_id
).is_ok());
// `touch` should reserve some balance of the caller...
assert!(!T::Currency::reserved_balance(&new_account).is_zero());
// ...and also create an `Account` entry.
assert!(Account::<T, I>::contains_key(asset_id.into(), &new_account));
}: _(SystemOrigin::Signed(new_account.clone()), asset_id, true)
verify {
// `refund`ing should of course repatriate the reserve
assert!(T::Currency::reserved_balance(&new_account).is_zero());
}

refund_other {
let (asset_id, asset_owner, asset_owner_lookup) = create_default_asset::<T, I>(false);
let new_account: T::AccountId = account("newaccount", 1, SEED);
let new_account_lookup = T::Lookup::unlookup(new_account.clone());
T::Currency::make_free_balance_be(&asset_owner, DepositBalanceOf::<T, I>::max_value());
assert_ne!(asset_owner, new_account);
assert!(Assets::<T, I>::freeze_creating(
SystemOrigin::Signed(asset_owner.clone()).into(),
asset_id,
new_account_lookup.clone()
).is_ok());
// `freeze_creating` should reserve balance of the freezer
assert!(!T::Currency::reserved_balance(&asset_owner).is_zero());
assert!(Assets::<T, I>::thaw(
SystemOrigin::Signed(asset_owner.clone()).into(),
asset_id,
new_account_lookup.clone()
).is_ok());
// the account should still exist, just not be frozen
assert!(Account::<T, I>::contains_key(asset_id.into(), &new_account));
}: _(SystemOrigin::Signed(asset_owner.clone()), asset_id, new_account.clone(), true)
verify {
// this should repatriate the reserved balance of the freezer
assert!(T::Currency::reserved_balance(&asset_owner).is_zero());
}

impl_benchmark_test_suite!(Assets, crate::mock::new_test_ext(), crate::mock::Test)
}
77 changes: 63 additions & 14 deletions frame/assets/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
pub(super) fn new_account(
who: &T::AccountId,
d: &mut AssetDetails<T::Balance, T::AccountId, DepositBalanceOf<T, I>>,
maybe_deposit: Option<DepositBalanceOf<T, I>>,
maybe_deposit: Option<(DepositBalanceOf<T, I>, &T::AccountId)>,
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
) -> Result<ExistenceReason<DepositBalanceOf<T, I>>, DispatchError> {
let accounts = d.accounts.checked_add(1).ok_or(ArithmeticError::Overflow)?;
let reason = if let Some(deposit) = maybe_deposit {
let reason = if let Some((deposit, _depositor)) = maybe_deposit {
d.deposit = d.deposit.saturating_add(deposit);
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
ExistenceReason::DepositHeld(deposit)
} else if d.is_sufficient {
frame_system::Pallet::<T>::inc_sufficients(who);
Expand Down Expand Up @@ -123,30 +124,39 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
amount: T::Balance,
increase_supply: bool,
) -> DepositConsequence {
use DepositConsequence::*;
let details = match Asset::<T, I>::get(id) {
Some(details) => details,
None => return DepositConsequence::UnknownAsset,
None => return UnknownAsset,
};
if increase_supply && details.supply.checked_add(&amount).is_none() {
return DepositConsequence::Overflow
return Overflow
}
// We don't allow freezing of accounts that don't already have an `(AssetId, AccountId)`
// entry. The account may have a zero balance provided by a deposit placed by `touch`ing the
// account. However, frozen accounts cannot receive more of the `AssetID`.
if let Some(a) = Account::<T, I>::get(id, who) {
if a.is_frozen {
return Frozen
}
}
if let Some(balance) = Self::maybe_balance(id, who) {
if balance.checked_add(&amount).is_none() {
return DepositConsequence::Overflow
return Overflow
}
} else {
if amount < details.min_balance {
return DepositConsequence::BelowMinimum
return BelowMinimum
}
if !details.is_sufficient && !frame_system::Pallet::<T>::can_inc_consumer(who) {
return DepositConsequence::CannotCreate
return CannotCreate
}
if details.is_sufficient && details.sufficients.checked_add(1).is_none() {
return DepositConsequence::Overflow
return Overflow
}
}

DepositConsequence::Success
Success
}

/// Return the consequence of a withdraw.
Expand Down Expand Up @@ -302,14 +312,18 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Ok((credit, maybe_burn))
}

/// Creates a account for `who` to hold asset `id` with a zero balance and takes a deposit.
pub(super) fn do_touch(id: T::AssetId, who: T::AccountId) -> DispatchResult {
/// Creates an account for `who` to hold asset `id` with a zero balance and takes a deposit.
pub(super) fn do_touch(
id: T::AssetId,
who: &T::AccountId,
depositor: &T::AccountId,
) -> DispatchResult {
ensure!(!Account::<T, I>::contains_key(id, &who), Error::<T, I>::AlreadyExists);
let deposit = T::AssetAccountDeposit::get();
let mut details = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
let reason = Self::new_account(&who, &mut details, Some(deposit))?;
T::Currency::reserve(&who, deposit)?;
let reason = Self::new_account(&who, &mut details, Some((deposit, &depositor)))?;
T::Currency::reserve(&depositor, deposit)?;
Asset::<T, I>::insert(&id, details);
Account::<T, I>::insert(
id,
Expand All @@ -318,6 +332,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
balance: Zero::zero(),
is_frozen: false,
reason,
depositor: Some(depositor.clone()),
extra: T::Extra::default(),
},
);
Expand All @@ -333,7 +348,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
ensure!(account.balance.is_zero() || allow_burn, Error::<T, I>::WouldBurn);
ensure!(!account.is_frozen, Error::<T, I>::Frozen);

T::Currency::unreserve(&who, deposit);
let mut refund_to: T::AccountId = who.clone();
if let Some(depositor) = account.depositor {
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
refund_to = depositor;
}
T::Currency::unreserve(&refund_to, deposit);

if let Remove = Self::dead_account(&who, &mut details, &account.reason, false) {
Account::<T, I>::remove(id, &who);
Expand Down Expand Up @@ -408,6 +427,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
*maybe_account = Some(AssetAccountOf::<T, I> {
balance: amount,
reason: Self::new_account(beneficiary, details, None)?,
depositor: None,
is_frozen: false,
extra: T::Extra::default(),
});
Expand Down Expand Up @@ -604,6 +624,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
balance: credit,
is_frozen: false,
reason: Self::new_account(dest, details, None)?,
depositor: None,
extra: T::Extra::default(),
});
},
Expand Down Expand Up @@ -938,4 +959,32 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
.filter_map(|id| Self::maybe_balance(id, account.clone()).map(|balance| (id, balance)))
.collect::<Vec<_>>()
}

/// Freeze an account `who`, preventing further reception or transfer of asset `id`.
pub fn do_freeze(
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
origin: T::AccountId,
id: T::AssetId,
who: T::AccountId,
create: bool,
) -> DispatchResult {
let d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
ensure!(
d.status == AssetStatus::Live || d.status == AssetStatus::Frozen,
Error::<T, I>::AssetNotLive
);
ensure!(origin == d.freezer, Error::<T, I>::NoPermission);

if create && Account::<T, I>::get(id, &who).is_none() {
// we create the account with zero balance, but the freezer must place a deposit
let _ = Self::do_touch(id, &who, &origin)?;
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
}

Account::<T, I>::try_mutate(id, &who, |maybe_account| -> DispatchResult {
maybe_account.as_mut().ok_or(Error::<T, I>::NoAccount)?.is_frozen = true;
Ok(())
})?;

Self::deposit_event(Event::<T, I>::Frozen { asset_id: id, who });
Ok(())
}
}
84 changes: 63 additions & 21 deletions frame/assets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@
//! * `approve_transfer`: Create or increase an delegated transfer.
//! * `cancel_approval`: Rescind a previous approval.
//! * `transfer_approved`: Transfer third-party's assets to another account.
//! * `touch`: Create an asset account for non-provider assets. Caller must place a deposit.
//! * `refund`: Return the deposit (if any) of the caller's asset account.
//! * `refund_other`: Return the deposit (if any) of a specified asset account.
//!
//! ### Permissioned Functions
//!
Expand All @@ -104,6 +107,8 @@
//! Owner.
//! * `set_metadata`: Set the metadata of an asset class; called by the asset class's Owner.
//! * `clear_metadata`: Remove the metadata of an asset class; called by the asset class's Owner.
//! * `freeze_creating`: Same as `freeze`, but creates the account if necessary; called by the asset
//! class's Freezer.
//!
//! Please refer to the [`Call`] enum and its associated variants for documentation on each
//! function.
Expand Down Expand Up @@ -198,7 +203,7 @@ pub mod pallet {
use frame_system::pallet_prelude::*;

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
Expand Down Expand Up @@ -531,7 +536,7 @@ pub mod pallet {
NoPermission,
/// The given asset ID is unknown.
Unknown,
/// The origin account is frozen.
/// The origin or beneficiary account is frozen.
Frozen,
/// The asset ID is already taken.
InUse,
Expand Down Expand Up @@ -920,7 +925,9 @@ pub mod pallet {
Self::do_transfer(id, &source, &dest, amount, Some(origin), f).map(|_| ())
}

/// Disallow further unprivileged transfers from an account.
/// Disallow further unprivileged transfers of an asset `id` to or from an account `who`.
/// `who` must already exist as an entry in `Account`s of the asset. If you want to freeze
/// an account that does not have an entry, use `freeze_creating` instead.
///
/// Origin must be Signed and the sender should be the Freezer of the asset `id`.
///
Expand All @@ -938,23 +945,10 @@ pub mod pallet {
who: AccountIdLookupOf<T>,
) -> DispatchResult {
let origin = ensure_signed(origin)?;
let id: T::AssetId = id.into();

let d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
ensure!(
d.status == AssetStatus::Live || d.status == AssetStatus::Frozen,
Error::<T, I>::AssetNotLive
);
ensure!(origin == d.freezer, Error::<T, I>::NoPermission);
let who = T::Lookup::lookup(who)?;
let id: T::AssetId = id.into();

Account::<T, I>::try_mutate(id, &who, |maybe_account| -> DispatchResult {
maybe_account.as_mut().ok_or(Error::<T, I>::NoAccount)?.is_frozen = true;
Ok(())
})?;

Self::deposit_event(Event::<T, I>::Frozen { asset_id: id, who });
Ok(())
Self::do_freeze(origin, id, who, false)
}

/// Allow unprivileged transfers from an account again.
Expand Down Expand Up @@ -1493,10 +1487,11 @@ pub mod pallet {
///
/// Emits `Touched` event when successful.
#[pallet::call_index(26)]
#[pallet::weight(T::WeightInfo::mint())]
#[pallet::weight(T::WeightInfo::touch())]
pub fn touch(origin: OriginFor<T>, id: T::AssetIdParameter) -> DispatchResult {
let who = ensure_signed(origin)?;
let id: T::AssetId = id.into();
Self::do_touch(id, ensure_signed(origin)?)
Self::do_touch(id, &who, &who)
}

/// Return the deposit (if any) of an asset account.
Expand All @@ -1508,7 +1503,7 @@ pub mod pallet {
///
/// Emits `Refunded` event when successful.
#[pallet::call_index(27)]
#[pallet::weight(T::WeightInfo::mint())]
#[pallet::weight(T::WeightInfo::refund())]
pub fn refund(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -1564,6 +1559,53 @@ pub mod pallet {
});
Ok(())
}

/// Disallow further unprivileged transfers to or from an account. Creates the account and
/// takes a deposit if it does not already exist in `Account`.
///
/// Origin must be Signed and the sender should be the Freezer of the asset `id`.
///
/// - `id`: The identifier of the asset to be frozen.
/// - `who`: The account to be frozen.
///
/// Emits `Frozen`.
///
/// Weight: `O(1)`
#[pallet::call_index(29)]
#[pallet::weight(T::WeightInfo::freeze_creating())]
pub fn freeze_creating(
origin: OriginFor<T>,
id: T::AssetIdParameter,
who: AccountIdLookupOf<T>,
) -> DispatchResult {
let origin = ensure_signed(origin)?;
let who = T::Lookup::lookup(who)?;
let id: T::AssetId = id.into();

Self::do_freeze(origin, id, who, true)
}

/// Return the deposit (if any) of a target asset account. Useful if you are the depositor.
///
/// The origin must be Signed.
///
/// - `id`: The identifier of the asset for the account to be created.
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
/// - `who`: The account to refund.
/// - `allow_burn`: If `true` then assets may be destroyed in order to complete the refund.
///
/// Emits `Refunded` event when successful.
#[pallet::call_index(30)]
#[pallet::weight(T::WeightInfo::refund_other())]
pub fn refund_other(
muharem marked this conversation as resolved.
Show resolved Hide resolved
origin: OriginFor<T>,
id: T::AssetIdParameter,
who: T::AccountId,
allow_burn: bool,
) -> DispatchResult {
let _ = ensure_signed(origin)?;
let id: T::AssetId = id.into();
Self::do_refund(id, who, allow_burn)
}
}
}

Expand Down
Loading