Skip to content

Commit

Permalink
Assets: can_decrease/increase for destroying asset is not successful (#…
Browse files Browse the repository at this point in the history
…3286)

Functions `can_decrease` and `can_increase` do not return successful
consequence results for assets undergoing destruction; instead, they
return the `UnknownAsset` consequence variant.

This update aligns their behavior with similar functions, such as
`reducible_balance`, `increase_balance`, `decrease_balance`, and `burn`,
which return an `AssetNotLive` error for assets in the process of being
destroyed.
  • Loading branch information
muharem authored Jul 7, 2024
1 parent d3cdfc4 commit f7dd85d
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 1 deletion.
16 changes: 16 additions & 0 deletions prdoc/pr_3286.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: "Assets: can_decrease/increase for destroying asset is not successful"

doc:
- audience: Runtime Dev
description: |
Functions `can_decrease` and `can_increase` do not return successful consequence results
for assets undergoing destruction; instead, they return the `UnknownAsset` consequence variant.
This update aligns their behavior with similar functions, such as `reducible_balance`,
`increase_balance`, `decrease_balance`, and `burn`, which return an `AssetNotLive` error
for assets in the process of being destroyed.

crates:
- name: pallet-assets
6 changes: 6 additions & 0 deletions substrate/frame/assets/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Some(details) => details,
None => return DepositConsequence::UnknownAsset,
};
if details.status == AssetStatus::Destroying {
return DepositConsequence::UnknownAsset
}
if increase_supply && details.supply.checked_add(&amount).is_none() {
return DepositConsequence::Overflow
}
Expand Down Expand Up @@ -175,6 +178,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
if details.status == AssetStatus::Frozen {
return Frozen
}
if details.status == AssetStatus::Destroying {
return UnknownAsset
}
if amount.is_zero() {
return Success
}
Expand Down
35 changes: 34 additions & 1 deletion substrate/frame/assets/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ use crate::{mock::*, Error};
use frame_support::{
assert_noop, assert_ok,
dispatch::GetDispatchInfo,
traits::{fungibles::InspectEnumerable, tokens::Preservation::Protect, Currency},
traits::{
fungibles::InspectEnumerable,
tokens::{Preservation::Protect, Provenance},
Currency,
},
};
use pallet_balances::Error as BalancesError;
use sp_io::storage;
Expand Down Expand Up @@ -1778,6 +1782,35 @@ fn asset_destroy_refund_existence_deposit() {
});
}

#[test]
fn increasing_or_decreasing_destroying_asset_should_not_work() {
new_test_ext().execute_with(|| {
use frame_support::traits::fungibles::Inspect;

let admin = 1;
let admin_origin = RuntimeOrigin::signed(admin);

assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, admin, true, 1));
assert_ok!(Assets::mint(RuntimeOrigin::signed(1), 0, 1, 100));
assert_eq!(Assets::balance(0, 1), 100);

assert_eq!(Assets::can_deposit(0, &1, 10, Provenance::Extant), DepositConsequence::Success);
assert_eq!(Assets::can_withdraw(0, &1, 10), WithdrawConsequence::<_>::Success);
assert_eq!(Assets::can_increase(0, &1, 10, false), DepositConsequence::Success);
assert_eq!(Assets::can_decrease(0, &1, 10, false), WithdrawConsequence::<_>::Success);

assert_ok!(Assets::start_destroy(admin_origin, 0));

assert_eq!(
Assets::can_deposit(0, &1, 10, Provenance::Extant),
DepositConsequence::UnknownAsset
);
assert_eq!(Assets::can_withdraw(0, &1, 10), WithdrawConsequence::<_>::UnknownAsset);
assert_eq!(Assets::can_increase(0, &1, 10, false), DepositConsequence::UnknownAsset);
assert_eq!(Assets::can_decrease(0, &1, 10, false), WithdrawConsequence::<_>::UnknownAsset);
});
}

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

0 comments on commit f7dd85d

Please sign in to comment.