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 1 commit
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
Prev Previous commit
Next Next commit
only allow depositor, admin, or account origin to refund deposits
  • Loading branch information
joepetrowski committed Apr 9, 2023
commit c5ece2f031cc738dabb02ce60ec134ce3dd64299
28 changes: 20 additions & 8 deletions frame/assets/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ 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>, &T::AccountId)>,
maybe_deposit: Option<DepositBalanceOf<T, I>>,
) -> Result<ExistenceReason<DepositBalanceOf<T, I>>, DispatchError> {
let accounts = d.accounts.checked_add(1).ok_or(ArithmeticError::Overflow)?;
let reason = if let Some((deposit, _depositor)) = maybe_deposit {
let reason = if let Some(deposit) = maybe_deposit {
ExistenceReason::DepositHeld(deposit)
} else if d.is_sufficient {
frame_system::Pallet::<T>::inc_sufficients(who);
Expand Down Expand Up @@ -321,7 +321,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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, &depositor)))?;
let reason = Self::new_account(&who, &mut details, Some(deposit))?;
T::Currency::reserve(&depositor, deposit)?;
Asset::<T, I>::insert(&id, details);
Account::<T, I>::insert(
Expand All @@ -339,7 +339,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

/// 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)?;
Expand All @@ -349,6 +354,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

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);
Expand Down Expand Up @@ -963,8 +975,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

/// Freeze an account `who`, preventing further reception or transfer of asset `id`.
pub fn do_freeze(
maybe_freezer: T::AccountId,
pub(super) fn do_freeze(
check_freezer: T::AccountId,
id: T::AssetId,
who: T::AccountId,
create: bool,
Expand All @@ -974,11 +986,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
d.status == AssetStatus::Live || d.status == AssetStatus::Frozen,
Error::<T, I>::AssetNotLive
);
ensure!(maybe_freezer == d.freezer, Error::<T, I>::NoPermission);
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, &maybe_freezer)?;
let _ = Self::do_touch(id, &who, &check_freezer)?;
}

Account::<T, I>::try_mutate(id, &who, |maybe_account| -> DispatchResult {
Expand Down
12 changes: 7 additions & 5 deletions frame/assets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,8 @@ pub mod pallet {
///
/// The origin must be Signed.
///
/// - `id`: The identifier of the asset for the account to be created.
/// - `id`: The identifier of the asset for which the caller would like the deposit
/// refunded.
/// - `allow_burn`: If `true` then assets may be destroyed in order to complete the refund.
///
/// Emits `Refunded` event when successful.
Expand All @@ -1509,8 +1510,9 @@ pub mod pallet {
id: T::AssetIdParameter,
allow_burn: bool,
) -> DispatchResult {
let origin = ensure_signed(origin)?;
let id: T::AssetId = id.into();
Self::do_refund(id, ensure_signed(origin)?, allow_burn)
Self::do_refund(&origin, id, &origin, allow_burn)
}

/// Sets the minimum balance of an asset.
Expand Down Expand Up @@ -1589,7 +1591,7 @@ pub mod pallet {
///
/// The origin must be Signed.
///
/// - `id`: The identifier of the asset for the account to be created.
/// - `id`: The identifier of the asset for the account holding a deposit.
/// - `who`: The account to refund.
/// - `allow_burn`: If `true` then assets may be destroyed in order to complete the refund.
///
Expand All @@ -1602,9 +1604,9 @@ pub mod pallet {
who: T::AccountId,
allow_burn: bool,
) -> DispatchResult {
let _ = ensure_signed(origin)?;
let origin = ensure_signed(origin)?;
let id: T::AssetId = id.into();
Self::do_refund(id, who, allow_burn)
Self::do_refund(&origin, id, &who, allow_burn)
}
}
}
Expand Down
39 changes: 39 additions & 0 deletions frame/assets/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,45 @@ fn freeze_creating_works() {
});
}

#[test]
fn cannot_refund_other_account_with_balance() {
new_test_ext().execute_with(|| {
// 1 will be the asset admin
// 2 will `touch` and exist without balance
// 3 will be created via freeze
Balances::make_free_balance_be(&1, 100);
Balances::make_free_balance_be(&2, 100);
Balances::make_free_balance_be(&3, 0);
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_eq!(Assets::balance(0, 2), 0);
assert_eq!(Assets::balance(0, 3), 0);

assert_ok!(Assets::touch(RuntimeOrigin::signed(2), 0));
assert!(Account::<Test>::contains_key(0, &2));
assert_ok!(Assets::transfer(RuntimeOrigin::signed(1), 0, 2, 50));
assert_ok!(Assets::transfer(RuntimeOrigin::signed(2), 0, 1, 50));
assert_eq!(Assets::balance(0, 2), 0);
assert!(Account::<Test>::contains_key(0, &2));
// 4 is not the depositor, account holder, or admin
assert_noop!(
Assets::refund_other(RuntimeOrigin::signed(4), 0, 2, true),
Error::<Test>::NoPermission
);
// but 1 is the asset admin, ok.
assert_ok!(Assets::refund_other(RuntimeOrigin::signed(1), 0, 2, true));

assert_ok!(Assets::freeze_creating(RuntimeOrigin::signed(1), 0, 3));
assert_eq!(Assets::balance(0, 3), 0);
assert!(Account::<Test>::contains_key(0, &3));
assert_noop!(
Assets::refund_other(RuntimeOrigin::signed(1), 0, 3, true),
Error::<Test>::Frozen
);
})
}

#[test]
fn transferring_amount_more_than_available_balance_should_not_work() {
new_test_ext().execute_with(|| {
Expand Down