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

Commit

Permalink
Allow Creation of Asset Accounts That Don't Exist Yet and Add `Blocke…
Browse files Browse the repository at this point in the history
…d` Status (#13843)

* prevent frozen accounts from receiving assets

* refund deposits correctly

* plus refund_other

* add benchmarks

* start migration work

* docs

* add migration logic

* fix freeze_creating benchmark

* support instanced migrations

* review

* correct deposit refund

* only allow depositor, admin, or account origin to refund deposits

* make sure refund actually removes account

* do refund changes

* Asset's account deposit owner (#13874)

* assets deposit owner

* doc typo

* remove migration

* empty commit

* can transfer to frozen account

* remove allow_burn from refund_other

* storage version back to 1

* update doc

* fix benches

* update docs

* more tests

* Update frame/assets/src/types.rs

* refund updating the reason

* refactor

* separate refund and refund_foreign

* refunds, touch_other, tests

* fixes

* fmt

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_assets

* tests: asserts asset account counts

* Account touch trait (#14063)

* assets touch trait

* docs

* move touch trait into support/traits

* permissionless flag for do_touch

* Apply suggestions from code review

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* move trait to misc, drop option

* Apply suggestions from code review

Co-authored-by: Gavin Wood <gavin@parity.io>

* correct doc

* Update frame/assets/src/functions.rs

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Gavin Wood <gavin@parity.io>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Block asset account (#14070)

* replace is_fronzen flag by status enum

* block asset account

* remove redundant brackets

* fix typo

* fmt

* Apply suggestions from code review

Co-authored-by: Jegor Sidorenko <5252494+jsidorenko@users.noreply.github.com>

* rename permissionless to check_depositor

* doc fix

* use account id lookup instead account id

* add benchmark for touch_other

---------

Co-authored-by: muharem <ismailov.m.h@gmail.com>
Co-authored-by: command-bot <>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Gavin Wood <gavin@parity.io>
Co-authored-by: Jegor Sidorenko <5252494+jsidorenko@users.noreply.github.com>
  • Loading branch information
5 people authored and EgorPopelyaev committed May 11, 2023
1 parent 3bb3882 commit d315f41
Show file tree
Hide file tree
Showing 11 changed files with 1,040 additions and 219 deletions.
69 changes: 69 additions & 0 deletions frame/assets/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,5 +482,74 @@ 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));
}

touch_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!(!Account::<T, I>::contains_key(asset_id.into(), &new_account));
}: _(SystemOrigin::Signed(asset_owner.clone()), asset_id, new_account_lookup)
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>::touch_other(
SystemOrigin::Signed(asset_owner.clone()).into(),
asset_id,
new_account_lookup.clone()
).is_ok());
// `touch_other` should reserve balance of the freezer
assert!(!T::Currency::reserved_balance(&asset_owner).is_zero());
assert!(Account::<T, I>::contains_key(asset_id.into(), &new_account));
}: _(SystemOrigin::Signed(asset_owner.clone()), asset_id, new_account_lookup.clone())
verify {
// this should repatriate the reserved balance of the freezer
assert!(T::Currency::reserved_balance(&asset_owner).is_zero());
}

block {
let (asset_id, caller, caller_lookup) = create_default_minted_asset::<T, I>(true, 100u32.into());
}: _(SystemOrigin::Signed(caller.clone()), asset_id, caller_lookup)
verify {
assert_last_event::<T, I>(Event::Blocked { asset_id: asset_id.into(), who: caller }.into());
}

impl_benchmark_test_suite!(Assets, crate::mock::new_test_ext(), crate::mock::Test)
}
111 changes: 85 additions & 26 deletions frame/assets/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,15 @@ 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>>,
) -> Result<ExistenceReason<DepositBalanceOf<T, I>>, DispatchError> {
maybe_deposit: Option<(&T::AccountId, DepositBalanceOf<T, I>)>,
) -> Result<ExistenceReasonOf<T, I>, DispatchError> {
let accounts = d.accounts.checked_add(1).ok_or(ArithmeticError::Overflow)?;
let reason = if let Some(deposit) = maybe_deposit {
ExistenceReason::DepositHeld(deposit)
let reason = if let Some((depositor, deposit)) = maybe_deposit {
if depositor == who {
ExistenceReason::DepositHeld(deposit)
} else {
ExistenceReason::DepositFrom(depositor.clone(), deposit)
}
} else if d.is_sufficient {
frame_system::Pallet::<T>::inc_sufficients(who);
d.sufficients += 1;
Expand All @@ -93,18 +97,19 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
pub(super) fn dead_account(
who: &T::AccountId,
d: &mut AssetDetails<T::Balance, T::AccountId, DepositBalanceOf<T, I>>,
reason: &ExistenceReason<DepositBalanceOf<T, I>>,
reason: &ExistenceReasonOf<T, I>,
force: bool,
) -> DeadConsequence {
use ExistenceReason::*;
match *reason {
ExistenceReason::Consumer => frame_system::Pallet::<T>::dec_consumers(who),
ExistenceReason::Sufficient => {
Consumer => frame_system::Pallet::<T>::dec_consumers(who),
Sufficient => {
d.sufficients = d.sufficients.saturating_sub(1);
frame_system::Pallet::<T>::dec_sufficients(who);
},
ExistenceReason::DepositRefunded => {},
ExistenceReason::DepositHeld(_) if !force => return Keep,
ExistenceReason::DepositHeld(_) => {},
DepositRefunded => {},
DepositHeld(_) | DepositFrom(..) if !force => return Keep,
DepositHeld(_) | DepositFrom(..) => {},
}
d.accounts = d.accounts.saturating_sub(1);
Remove
Expand All @@ -130,8 +135,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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::<T, I>::get(id, who) {
if account.status.is_blocked() {
return DepositConsequence::Blocked
}
if account.balance.checked_add(&amount).is_none() {
return DepositConsequence::Overflow
}
} else {
Expand Down Expand Up @@ -174,7 +182,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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) {
Expand Down Expand Up @@ -215,7 +223,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);

let account = Account::<T, I>::get(id, who).ok_or(Error::<T, I>::NoAccount)?;
ensure!(!account.is_frozen, Error::<T, I>::Frozen);
ensure!(!account.status.is_frozen(), Error::<T, I>::Frozen);

let amount = if let Some(frozen) = T::Freezer::frozen_balance(id, who) {
// Frozen balance: account CANNOT be deleted
Expand Down Expand Up @@ -302,50 +310,101 @@ 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.
///
/// When `check_depositor` is set to true, the depositor must be either the asset's Admin or
/// Freezer, otherwise the depositor can be any account.
pub(super) fn do_touch(
id: T::AssetId,
who: T::AccountId,
depositor: T::AccountId,
check_depositor: bool,
) -> 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)?;
ensure!(
!check_depositor || &depositor == &details.admin || &depositor == &details.freezer,
Error::<T, I>::NoPermission
);
let reason = Self::new_account(&who, &mut details, Some((&depositor, deposit)))?;
T::Currency::reserve(&depositor, deposit)?;
Asset::<T, I>::insert(&id, details);
Account::<T, I>::insert(
id,
&who,
AssetAccountOf::<T, I> {
balance: Zero::zero(),
is_frozen: false,
status: AccountStatus::Liquid,
reason,
extra: T::Extra::default(),
},
);
Self::deposit_event(Event::Touched { asset_id: id, who, depositor });
Ok(())
}

/// Returns a deposit, destroying an asset-account.
/// Returns a deposit or a consumer reference, destroying an asset-account.
/// Non-zero balance accounts refunded and destroyed only if `allow_burn` is true.
pub(super) fn do_refund(id: T::AssetId, who: T::AccountId, allow_burn: bool) -> DispatchResult {
use AssetStatus::*;
use ExistenceReason::*;
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)?;
ensure!(matches!(account.reason, Consumer | DepositHeld(..)), 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!(matches!(details.status, Live | Frozen), Error::<T, I>::IncorrectStatus);
ensure!(account.balance.is_zero() || allow_burn, Error::<T, I>::WouldBurn);
ensure!(!account.is_frozen, Error::<T, I>::Frozen);

T::Currency::unreserve(&who, deposit);
if let Some(deposit) = account.reason.take_deposit() {
T::Currency::unreserve(&who, deposit);
}

if let Remove = Self::dead_account(&who, &mut details, &account.reason, false) {
Account::<T, I>::remove(id, &who);
} else {
debug_assert!(false, "refund did not result in dead account?!");
// deposit may have been refunded, need to update `Account`
Account::<T, I>::insert(id, &who, account);
return Ok(())
}
Asset::<T, I>::insert(&id, details);
// Executing a hook here is safe, since it is not in a `mutate`.
T::Freezer::died(id, &who);
Ok(())
}

/// Returns a `DepositFrom` of an account only if balance is zero.
pub(super) fn do_refund_other(
id: T::AssetId,
who: &T::AccountId,
caller: &T::AccountId,
) -> DispatchResult {
let mut account = Account::<T, I>::get(id, &who).ok_or(Error::<T, I>::NoDeposit)?;
let (depositor, deposit) =
account.reason.take_deposit_from().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.status.is_frozen(), Error::<T, I>::Frozen);
ensure!(caller == &depositor || caller == &details.admin, Error::<T, I>::NoPermission);
ensure!(account.balance.is_zero(), Error::<T, I>::WouldBurn);

T::Currency::unreserve(&depositor, deposit);

if let Remove = Self::dead_account(&who, &mut details, &account.reason, false) {
Account::<T, I>::remove(id, &who);
} else {
debug_assert!(false, "refund did not result in dead account?!");
// deposit may have been refunded, need to update `Account`
Account::<T, I>::insert(id, &who, account);
return Ok(())
}
Asset::<T, I>::insert(&id, details);
// Executing a hook here is safe, since it is not in a `mutate`.
T::Freezer::died(id, &who);
return Ok(())
}

/// Increases the asset `id` balance of `beneficiary` by `amount`.
///
/// This alters the registered supply of the asset and emits an event.
Expand Down Expand Up @@ -408,7 +467,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)?,
is_frozen: false,
status: AccountStatus::Liquid,
extra: T::Extra::default(),
});
},
Expand Down Expand Up @@ -602,7 +661,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
maybe_account @ None => {
*maybe_account = Some(AssetAccountOf::<T, I> {
balance: credit,
is_frozen: false,
status: AccountStatus::Liquid,
reason: Self::new_account(dest, details, None)?,
extra: T::Extra::default(),
});
Expand Down
Loading

0 comments on commit d315f41

Please sign in to comment.