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 14 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)
}
87 changes: 75 additions & 12 deletions frame/assets/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,30 +123,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 +311,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)?;
T::Currency::reserve(&depositor, deposit)?;
Asset::<T, I>::insert(&id, details);
Account::<T, I>::insert(
id,
Expand All @@ -318,26 +331,46 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
balance: Zero::zero(),
is_frozen: false,
reason,
depositor: Some(depositor.clone()),
extra: T::Extra::default(),
},
);
Ok(())
}

/// Returns a deposit, destroying an asset-account.
pub(super) fn do_refund(id: T::AssetId, who: T::AccountId, allow_burn: bool) -> DispatchResult {
pub(super) fn do_refund(
caller: &T::AccountId,
id: T::AssetId,
who: &T::AccountId,
allow_burn: bool,
) -> DispatchResult {
let mut account = Account::<T, I>::get(id, &who).ok_or(Error::<T, I>::NoDeposit)?;
let deposit = account.reason.take_deposit().ok_or(Error::<T, I>::NoDeposit)?;
let mut details = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
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.take() {
// Only the account owner (`who`), the depositor, or the asset admin can remove an
// account. That is, a depositor cannot force another's account to exist with zero
// balance.
ensure!(
caller == &depositor || caller == who || caller == &details.admin,
Error::<T, I>::NoPermission
);
refund_to = depositor;
}
T::Currency::unreserve(&refund_to, deposit);
account.depositor = None;
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved

if let Remove = Self::dead_account(&who, &mut details, &account.reason, false) {
Account::<T, I>::remove(id, &who);
} else {
// deposit has been refunded, need to update `Account`
Account::<T, I>::insert(id, &who, account);
NachoPal marked this conversation as resolved.
Show resolved Hide resolved
debug_assert!(false, "refund did not result in dead account?!");
}
Asset::<T, I>::insert(&id, details);
Expand Down Expand Up @@ -408,6 +441,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 +638,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 +973,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(super) fn do_freeze(
check_freezer: 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!(check_freezer == 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, &check_freezer)?;
}

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(())
}
}
Loading