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

Asset Pallet: Support repeated destroys to safely destroy large assets #12310

Merged
merged 73 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
6a9d9d1
Support repeated destroys to safely destroy large assets
tonyalaribe Sep 20, 2022
7db1fd5
require freezing accounts before destroying
tonyalaribe Sep 20, 2022
cbe14b2
support only deleting asset as final stage when there's no assets left
tonyalaribe Sep 21, 2022
001eaef
Merge branch 'master' into aa/safely-destroy-large-assets
tonyalaribe Sep 21, 2022
1beae07
pre: introduce the RemoveKeyLimit config parameter
tonyalaribe Sep 22, 2022
c9ce171
debug_ensure empty account in the right if block
tonyalaribe Sep 22, 2022
3067e38
update to having separate max values for accounts and approvals
tonyalaribe Sep 22, 2022
7c4f610
add tests and use RemoveKeyLimit constant
tonyalaribe Sep 26, 2022
aca497f
add useful comments to the extrinsics, and calculate returned weight
tonyalaribe Sep 27, 2022
634345a
Merge branch 'master' of github.com:paritytech/substrate into aa/safe…
tonyalaribe Sep 27, 2022
93d886d
add benchmarking for start_destroy and finish destroy
tonyalaribe Sep 27, 2022
2d3b650
push failing benchmark logic
tonyalaribe Sep 27, 2022
5a612d1
add benchmark tests for new functions
tonyalaribe Sep 27, 2022
b961255
update weights via local benchmarks
tonyalaribe Sep 27, 2022
91e3c18
remove extra weight file
tonyalaribe Sep 27, 2022
60f954a
Update frame/assets/src/lib.rs
tonyalaribe Sep 28, 2022
ce00b5b
Update frame/assets/src/types.rs
tonyalaribe Sep 28, 2022
5492b6d
Update frame/assets/src/lib.rs
tonyalaribe Sep 28, 2022
bc20a6c
effect some changes from codereview
tonyalaribe Sep 28, 2022
6ceb592
use NotFrozen error
tonyalaribe Sep 29, 2022
2edac52
remove origin checks, as anyone can complete destruction after owner …
tonyalaribe Sep 29, 2022
aa2aecf
fix comments about Origin behaviour
tonyalaribe Sep 29, 2022
10fdd90
add AssetStatus docs
tonyalaribe Sep 29, 2022
139d2e9
modularize logic to allow calling logic in on_idle and on_initialize …
tonyalaribe Oct 5, 2022
597f532
introduce simple migration for assets details
tonyalaribe Oct 6, 2022
a09e762
Merge remote-tracking branch 'origin/master' into aa/safely-destroy-l…
Oct 6, 2022
70930ac
reintroduce logging in the migrations
tonyalaribe Oct 7, 2022
dc4a243
move deposit_Event out of the mutate block
tonyalaribe Oct 12, 2022
05d778d
Update frame/assets/src/functions.rs
tonyalaribe Oct 12, 2022
62d4f13
Update frame/assets/src/migration.rs
tonyalaribe Oct 12, 2022
e9e108b
move AssetNotLive checkout out of the mutate blocks
tonyalaribe Oct 12, 2022
5df4a62
Merge branch 'aa/safely-destroy-large-assets' of github.com:paritytec…
tonyalaribe Oct 12, 2022
544ac52
rename RemoveKeysLimit to RemoveItemsLimit
tonyalaribe Oct 13, 2022
9206acf
update docs
tonyalaribe Oct 13, 2022
29c7745
fix event name in benchmark
tonyalaribe Oct 14, 2022
2986f29
fix cargo fmt.
tonyalaribe Oct 14, 2022
6ac84a1
fix lint in benchmarking
tonyalaribe Oct 14, 2022
1f2930f
Merge branch 'master' of github.com:paritytech/substrate into aa/safe…
tonyalaribe Oct 14, 2022
791aa7f
Empty commit to trigger CI
tonyalaribe Oct 14, 2022
450a698
Update frame/assets/src/lib.rs
tonyalaribe Oct 14, 2022
3daef79
Update frame/assets/src/lib.rs
tonyalaribe Oct 14, 2022
ea34cf5
Update frame/assets/src/functions.rs
tonyalaribe Oct 14, 2022
84cbb47
Update frame/assets/src/functions.rs
tonyalaribe Oct 14, 2022
082a10c
Update frame/assets/src/functions.rs
tonyalaribe Oct 14, 2022
e84d5ae
Update frame/assets/src/lib.rs
tonyalaribe Oct 14, 2022
78cde15
Update frame/assets/src/functions.rs
tonyalaribe Oct 14, 2022
c6470fb
effect change suggested during code review
tonyalaribe Oct 14, 2022
85e50b9
move limit to a single location
tonyalaribe Oct 14, 2022
c97e850
Merge branch 'master' of github.com:paritytech/substrate into aa/safe…
tonyalaribe Oct 17, 2022
df27e0b
Update frame/assets/src/functions.rs
tonyalaribe Oct 18, 2022
ca9fa2e
rename events
tonyalaribe Oct 18, 2022
9609f13
fix weight typo, using rocksdb instead of T::DbWeight. Pending genera…
tonyalaribe Oct 18, 2022
26d27cc
switch to using dead_account.len()
tonyalaribe Oct 18, 2022
72ec8f8
rename event in the benchmarks
tonyalaribe Oct 20, 2022
ee30cf8
empty to retrigger CI
tonyalaribe Oct 20, 2022
87a7877
Merge branch 'master' of github.com:paritytech/substrate into aa/safe…
tonyalaribe Oct 20, 2022
773212e
trigger CI to check cumulus dependency
tonyalaribe Oct 20, 2022
a4be57f
trigger CI for dependent cumulus
tonyalaribe Oct 20, 2022
111da0e
Merge branch 'master' into aa/safely-destroy-large-assets
tonyalaribe Oct 22, 2022
9c06bb8
Update frame/assets/src/migration.rs
tonyalaribe Oct 24, 2022
4eef069
move is-frozen to the assetStatus enum (#12547)
tonyalaribe Oct 26, 2022
486814f
add pre and post migration hooks
tonyalaribe Oct 26, 2022
c185cb0
update do_transfer logic to add new assert for more correct error mes…
tonyalaribe Oct 26, 2022
91095af
trigger CI
tonyalaribe Oct 26, 2022
9ef3c2b
switch checking AssetStatus from checking Destroying state to checkin…
tonyalaribe Oct 27, 2022
6165576
fix error type in tests from Frozen to AssetNotLive
tonyalaribe Oct 27, 2022
7811b62
trigger CI
tonyalaribe Oct 27, 2022
cd3e28d
change ensure check for fn reducible_balance()
tonyalaribe Oct 27, 2022
aee596e
change the error type to Error:<T,I>::IncorrectStatus to be clearer
tonyalaribe Nov 7, 2022
30814df
Trigger CI
tonyalaribe Nov 7, 2022
9b653a5
Merge branch 'master' into aa/safely-destroy-large-assets
tonyalaribe Nov 7, 2022
57fe747
Merge branch 'master' into aa/safely-destroy-large-assets
tonyalaribe Nov 10, 2022
7ba1ef5
Merge branch 'master' into aa/safely-destroy-large-assets
tonyalaribe Nov 14, 2022
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 @@ -1451,6 +1451,7 @@ impl pallet_assets::Config for Runtime {
type Freezer = ();
type Extra = ();
type WeightInfo = pallet_assets::weights::SubstrateWeight<Runtime>;
type RemoveItemsLimit = ConstU32<1000>;
}

parameter_types! {
Expand Down
85 changes: 57 additions & 28 deletions frame/assets/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,25 +78,6 @@ fn swap_is_sufficient<T: Config<I>, I: 'static>(s: &mut bool) {
});
}

fn add_consumers<T: Config<I>, I: 'static>(minter: T::AccountId, n: u32) {
let origin = SystemOrigin::Signed(minter);
let mut s = false;
swap_is_sufficient::<T, I>(&mut s);
for i in 0..n {
let target = account("consumer", i, SEED);
T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance());
let target_lookup = T::Lookup::unlookup(target);
assert!(Assets::<T, I>::mint(
origin.clone().into(),
Default::default(),
target_lookup,
100u32.into()
)
.is_ok());
}
swap_is_sufficient::<T, I>(&mut s);
}

fn add_sufficients<T: Config<I>, I: 'static>(minter: T::AccountId, n: u32) {
let origin = SystemOrigin::Signed(minter);
let mut s = true;
Expand Down Expand Up @@ -168,18 +149,66 @@ benchmarks_instance_pallet! {
assert_last_event::<T, I>(Event::ForceCreated { asset_id: Default::default(), owner: caller }.into());
}

destroy {
let c in 0 .. 5_000;
let s in 0 .. 5_000;
let a in 0 .. 5_00;
start_destroy {
let (caller, caller_lookup) = create_default_minted_asset::<T, I>(true, 100u32.into());
Assets::<T, I>::freeze_asset(
SystemOrigin::Signed(caller.clone()).into(),
Default::default(),
)?;
}:_(SystemOrigin::Signed(caller), Default::default())
verify {
assert_last_event::<T, I>(Event::DestructionStarted { asset_id: Default::default() }.into());
}

destroy_accounts {
let c in 0 .. T::RemoveItemsLimit::get();
let (caller, _) = create_default_asset::<T, I>(true);
add_consumers::<T, I>(caller.clone(), c);
add_sufficients::<T, I>(caller.clone(), s);
add_sufficients::<T, I>(caller.clone(), c);
Assets::<T, I>::freeze_asset(
SystemOrigin::Signed(caller.clone()).into(),
Default::default(),
)?;
Assets::<T,I>::start_destroy(SystemOrigin::Signed(caller.clone()).into(), Default::default())?;
}:_(SystemOrigin::Signed(caller), Default::default())
verify {
assert_last_event::<T, I>(Event::AccountsDestroyed {
asset_id: Default::default() ,
accounts_destroyed: c,
accounts_remaining: 0,
}.into());
}

destroy_approvals {
let a in 0 .. T::RemoveItemsLimit::get();
let (caller, _) = create_default_minted_asset::<T, I>(true, 100u32.into());
add_approvals::<T, I>(caller.clone(), a);
let witness = Asset::<T, I>::get(T::AssetId::default()).unwrap().destroy_witness();
}: _(SystemOrigin::Signed(caller), Default::default(), witness)
Assets::<T, I>::freeze_asset(
SystemOrigin::Signed(caller.clone()).into(),
Default::default(),
)?;
Assets::<T,I>::start_destroy(SystemOrigin::Signed(caller.clone()).into(), Default::default())?;
}:_(SystemOrigin::Signed(caller), Default::default())
verify {
assert_last_event::<T, I>(Event::Destroyed { asset_id: Default::default() }.into());
assert_last_event::<T, I>(Event::ApprovalsDestroyed {
asset_id: Default::default() ,
approvals_destroyed: a,
approvals_remaining: 0,
}.into());
}

finish_destroy {
let (caller, caller_lookup) = create_default_asset::<T, I>(true);
Assets::<T, I>::freeze_asset(
SystemOrigin::Signed(caller.clone()).into(),
Default::default(),
)?;
Assets::<T,I>::start_destroy(SystemOrigin::Signed(caller.clone()).into(), Default::default())?;
}:_(SystemOrigin::Signed(caller), Default::default())
verify {
assert_last_event::<T, I>(Event::Destroyed {
asset_id: Default::default() ,
}.into()
);
}

mint {
Expand Down
171 changes: 118 additions & 53 deletions frame/assets/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
if details.supply.checked_sub(&amount).is_none() {
return Underflow
}
if details.is_frozen {
if details.status == AssetStatus::Frozen {
return Frozen
}
if amount.is_zero() {
Expand Down Expand Up @@ -205,7 +205,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
keep_alive: bool,
) -> Result<T::Balance, DispatchError> {
let details = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
ensure!(!details.is_frozen, Error::<T, I>::Frozen);
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);
Expand Down Expand Up @@ -300,6 +300,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It the is_frozen depends on the asset status, then this is superfluous.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think they depend on each other, since they have different meanings. But in a future PR, we might also make is_frozen an asset status.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's probably simpler to do it with current changes, the migration already in place.
If not, let's have an issue.

let reason = Self::new_account(&who, &mut details, Some(deposit))?;
T::Currency::reserve(&who, deposit)?;
Asset::<T, I>::insert(&id, details);
Expand All @@ -321,9 +322,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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!(!details.is_frozen, Error::<T, I>::Frozen);
ensure!(!account.is_frozen, Error::<T, I>::Frozen);

T::Currency::unreserve(&who, deposit);
Expand Down Expand Up @@ -390,7 +390,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Self::can_increase(id, beneficiary, amount, true).into_result()?;
Asset::<T, I>::try_mutate(id, |maybe_details| -> DispatchResult {
let details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;

ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
muharem marked this conversation as resolved.
Show resolved Hide resolved
check(details)?;

Account::<T, I>::try_mutate(id, beneficiary, |maybe_account| -> DispatchResult {
Expand Down Expand Up @@ -430,6 +430,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
maybe_check_admin: Option<T::AccountId>,
f: DebitFlags,
) -> Result<T::Balance, DispatchError> {
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
tonyalaribe marked this conversation as resolved.
Show resolved Hide resolved
);

let actual = Self::decrease_balance(id, target, amount, f, |actual, details| {
// Check admin rights.
if let Some(check_admin) = maybe_check_admin {
Expand Down Expand Up @@ -467,12 +473,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
return Ok(amount)
}

let details = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could have it's own ensure_live(details)?; function as it comes up a lot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, but this logic is in a lot of places, and I don't want to add more things to this PR. I would rather as a different task


let actual = Self::prep_debit(id, target, amount, f)?;
let mut target_died: Option<DeadConsequence> = None;

Asset::<T, I>::try_mutate(id, |maybe_details| -> DispatchResult {
let details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;

check(actual, details)?;

Account::<T, I>::try_mutate(id, target, |maybe_account| -> DispatchResult {
Expand Down Expand Up @@ -540,6 +548,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
if amount.is_zero() {
return Ok((amount, None))
}
let details = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);

// Figure out the debit and credit, together with side-effects.
let debit = Self::prep_debit(id, source, amount, f.into())?;
Expand Down Expand Up @@ -651,72 +661,123 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
accounts: 0,
sufficients: 0,
approvals: 0,
is_frozen: false,
status: AssetStatus::Live,
},
);
Self::deposit_event(Event::ForceCreated { asset_id: id, owner });
Ok(())
}

/// Destroy an existing asset.
///
/// * `id`: The asset you want to destroy.
muharem marked this conversation as resolved.
Show resolved Hide resolved
/// * `witness`: Witness data needed about the current state of the asset, used to confirm
/// complexity of the operation.
/// * `maybe_check_owner`: An optional check before destroying the asset, if the provided
/// account is the owner of that asset. Can be used for authorization checks.
pub(super) fn do_destroy(
/// Start the process of destroying an asset, by setting the asset status to `Destroying`, and
/// emitting the `DestructionStarted` event.
pub(super) fn do_start_destroy(
id: T::AssetId,
witness: DestroyWitness,
maybe_check_owner: Option<T::AccountId>,
) -> Result<DestroyWitness, DispatchError> {
let mut dead_accounts: Vec<T::AccountId> = vec![];
) -> DispatchResult {
Asset::<T, I>::try_mutate_exists(id, |maybe_details| -> Result<(), DispatchError> {
let mut details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
if let Some(check_owner) = maybe_check_owner {
ensure!(details.owner == check_owner, Error::<T, I>::NoPermission);
}
details.status = AssetStatus::Destroying;

let result_witness: DestroyWitness = Asset::<T, I>::try_mutate_exists(
id,
|maybe_details| -> Result<DestroyWitness, DispatchError> {
let mut details = maybe_details.take().ok_or(Error::<T, I>::Unknown)?;
if let Some(check_owner) = maybe_check_owner {
ensure!(details.owner == check_owner, Error::<T, I>::NoPermission);
}
ensure!(details.accounts <= witness.accounts, Error::<T, I>::BadWitness);
ensure!(details.sufficients <= witness.sufficients, Error::<T, I>::BadWitness);
ensure!(details.approvals <= witness.approvals, Error::<T, I>::BadWitness);
Self::deposit_event(Event::DestructionStarted { asset_id: id });
Ok(())
})
}

/// Destroy accounts associated with a given asset up to the max (T::RemoveItemsLimit).
///
/// Each call emits the `Event::DestroyedAccounts` event.
/// Returns the number of destroyed accounts.
pub(super) fn do_destroy_accounts(
id: T::AssetId,
max_items: u32,
) -> Result<u32, DispatchError> {
let mut dead_accounts: Vec<T::AccountId> = vec![];
let mut remaining_accounts = 0;
let _ =
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
Asset::<T, I>::try_mutate_exists(id, |maybe_details| -> Result<(), DispatchError> {
let mut details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
// Should only destroy accounts while the asset is in a destroying state
ensure!(details.status == AssetStatus::Destroying, Error::<T, I>::IncorrectStatus);

for (who, v) in Account::<T, I>::drain_prefix(id) {
// We have to force this as it's destroying the entire asset class.
// This could mean that some accounts now have irreversibly reserved
// funds.
let _ = Self::dead_account(&who, &mut details, &v.reason, true);
dead_accounts.push(who);
if dead_accounts.len() >= (max_items as usize) {
break
}
}
debug_assert_eq!(details.accounts, 0);
debug_assert_eq!(details.sufficients, 0);
remaining_accounts = details.accounts;
Ok(())
})?;

for who in &dead_accounts {
T::Freezer::died(id, &who);
}

let metadata = Metadata::<T, I>::take(&id);
T::Currency::unreserve(
&details.owner,
details.deposit.saturating_add(metadata.deposit),
);
Self::deposit_event(Event::AccountsDestroyed {
asset_id: id,
accounts_destroyed: dead_accounts.len() as u32,
accounts_remaining: remaining_accounts as u32,
});
Ok(dead_accounts.len() as u32)
}

for ((owner, _), approval) in Approvals::<T, I>::drain_prefix((&id,)) {
/// Destroy approvals associated with a given asset up to the max (T::RemoveItemsLimit).
///
/// Each call emits the `Event::DestroyedApprovals` event
tonyalaribe marked this conversation as resolved.
Show resolved Hide resolved
/// Returns the number of destroyed approvals.
pub(super) fn do_destroy_approvals(
id: T::AssetId,
max_items: u32,
) -> Result<u32, DispatchError> {
let mut removed_approvals = 0;
let _ =
Asset::<T, I>::try_mutate_exists(id, |maybe_details| -> Result<(), DispatchError> {
let mut details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;

// Should only destroy accounts while the asset is in a destroying state.
ensure!(details.status == AssetStatus::Destroying, Error::<T, I>::IncorrectStatus);

for ((owner, _), approval) in Approvals::<T, I>::drain_prefix((id,)) {
T::Currency::unreserve(&owner, approval.deposit);
removed_approvals = removed_approvals.saturating_add(1);
details.approvals = details.approvals.saturating_sub(1);
if removed_approvals >= max_items {
break
}
}
Self::deposit_event(Event::Destroyed { asset_id: id });
Self::deposit_event(Event::ApprovalsDestroyed {
asset_id: id,
approvals_destroyed: removed_approvals as u32,
approvals_remaining: details.approvals as u32,
});
Ok(())
})?;
Ok(removed_approvals)
}

Ok(DestroyWitness {
accounts: details.accounts,
sufficients: details.sufficients,
approvals: details.approvals,
})
},
)?;
/// Complete destroying an asset and unreserve the deposit.
///
/// On success, the `Event::Destroyed` event is emitted.
pub(super) fn do_finish_destroy(id: T::AssetId) -> DispatchResult {
Asset::<T, I>::try_mutate_exists(id, |maybe_details| -> Result<(), DispatchError> {
let details = maybe_details.take().ok_or(Error::<T, I>::Unknown)?;
ensure!(details.status == AssetStatus::Destroying, Error::<T, I>::IncorrectStatus);
ensure!(details.accounts == 0, Error::<T, I>::InUse);
ensure!(details.approvals == 0, Error::<T, I>::InUse);

let metadata = Metadata::<T, I>::take(&id);
T::Currency::unreserve(
&details.owner,
details.deposit.saturating_add(metadata.deposit),
);
Self::deposit_event(Event::Destroyed { asset_id: id });

// Execute hooks outside of `mutate`.
for who in dead_accounts {
T::Freezer::died(id, &who);
}
Ok(result_witness)
Ok(())
})
}

/// Creates an approval from `owner` to spend `amount` of asset `id` tokens by 'delegate'
Expand All @@ -730,7 +791,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
amount: T::Balance,
) -> DispatchResult {
let mut d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
ensure!(!d.is_frozen, Error::<T, I>::Frozen);
ensure!(d.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
Approvals::<T, I>::try_mutate(
(id, &owner, &delegate),
|maybe_approved| -> DispatchResult {
Expand Down Expand Up @@ -780,6 +841,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
) -> DispatchResult {
let mut owner_died: Option<DeadConsequence> = None;

let d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
ensure!(d.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);

Approvals::<T, I>::try_mutate_exists(
(id, &owner, delegate),
|maybe_approved| -> DispatchResult {
Expand Down Expand Up @@ -826,6 +890,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
symbol.clone().try_into().map_err(|_| Error::<T, I>::BadMetadata)?;

let d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
ensure!(d.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
ensure!(from == &d.owner, Error::<T, I>::NoPermission);

Metadata::<T, I>::try_mutate_exists(id, |metadata| {
Expand Down
Loading