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 all 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
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)
}
muharem marked this conversation as resolved.
Show resolved Hide resolved
} 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