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

Pallet Assets: move is frozen to asset status #12547

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 10 additions & 15 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,8 +205,8 @@ 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.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
ensure!(!details.is_frozen, Error::<T, I>::Frozen);
ensure!(details.status != AssetStatus::Destroying, Error::<T, I>::AssetNotLive);
ensure!(details.status != AssetStatus::Frozen, Error::<T, I>::Frozen);

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 @@ -325,7 +325,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let mut details = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;

ensure!(account.balance.is_zero() || allow_burn, Error::<T, I>::WouldBurn);
ensure!(!details.is_frozen, Error::<T, I>::Frozen);
ensure!(details.status != AssetStatus::Frozen, Error::<T, I>::Frozen);
ensure!(!account.is_frozen, Error::<T, I>::Frozen);

T::Currency::unreserve(&who, deposit);
Expand Down Expand Up @@ -392,7 +392,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);
ensure!(details.status != AssetStatus::Destroying, Error::<T, I>::AssetNotLive);
check(details)?;

Account::<T, I>::try_mutate(id, beneficiary, |maybe_account| -> DispatchResult {
Expand Down Expand Up @@ -433,7 +433,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
f: DebitFlags,
) -> Result<T::Balance, DispatchError> {
let details = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
ensure!(details.status != AssetStatus::Destroying, Error::<T, I>::AssetNotLive);

let actual = Self::decrease_balance(id, target, amount, f, |actual, details| {
// Check admin rights.
Expand Down Expand Up @@ -660,7 +660,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
accounts: 0,
sufficients: 0,
approvals: 0,
is_frozen: false,
Copy link
Member

Choose a reason for hiding this comment

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

So this should rather merge into the other MR, since otherwise there would be two version 1 migrations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, exactly. This is based off the other PR. But i wanted it to be separate so it can be reviewed easily before I merged it. Otherwise there would just be too much going on in theo other PR

status: AssetStatus::Live,
},
);
Expand All @@ -679,7 +678,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
if let Some(check_owner) = maybe_check_owner {
ensure!(details.owner == check_owner, Error::<T, I>::NoPermission);
}
ensure!(details.is_frozen, Error::<T, I>::NotFrozen);
details.status = AssetStatus::Destroying;

Self::deposit_event(Event::DestructionStarted { asset_id: id });
Expand All @@ -700,8 +698,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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)?;
ensure!(details.is_frozen, Error::<T, I>::NotFrozen);
// Should only destroy accounts while the asset is being destroyed
// Should only destroy accounts while the asset is in a destroying state
ensure!(details.status == AssetStatus::Destroying, Error::<T, I>::LiveAsset);

for (who, v) in Account::<T, I>::drain_prefix(id) {
Expand Down Expand Up @@ -740,8 +737,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Asset::<T, I>::try_mutate_exists(id, |maybe_details| -> Result<(), DispatchError> {
let mut details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;

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

for ((owner, _), approval) in Approvals::<T, I>::drain_prefix((id,)) {
Expand All @@ -768,7 +764,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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.is_frozen, Error::<T, I>::NotFrozen);
ensure!(details.status == AssetStatus::Destroying, Error::<T, I>::LiveAsset);
ensure!(details.accounts == 0, Error::<T, I>::InUse);
ensure!(details.approvals == 0, Error::<T, I>::InUse);
Expand All @@ -795,8 +790,8 @@ 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.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
ensure!(!d.is_frozen, Error::<T, I>::Frozen);
ensure!(d.status != AssetStatus::Destroying, Error::<T, I>::AssetNotLive);
ensure!(d.status != AssetStatus::Frozen, Error::<T, I>::Frozen);
Approvals::<T, I>::try_mutate(
(id, &owner, &delegate),
|maybe_approved| -> DispatchResult {
Expand Down
15 changes: 9 additions & 6 deletions frame/assets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,6 @@ pub mod pallet {
accounts: 0,
sufficients: 0,
approvals: 0,
is_frozen: false,
status: AssetStatus::Live,
},
);
Expand Down Expand Up @@ -562,7 +561,6 @@ pub mod pallet {
accounts: 0,
sufficients: 0,
approvals: 0,
is_frozen: false,
status: AssetStatus::Live,
},
);
Expand Down Expand Up @@ -925,7 +923,7 @@ pub mod pallet {
let d = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
ensure!(origin == d.freezer, Error::<T, I>::NoPermission);

d.is_frozen = true;
d.status = AssetStatus::Frozen;

Self::deposit_event(Event::<T, I>::AssetFrozen { asset_id: id });
Ok(())
Expand All @@ -951,8 +949,9 @@ pub mod pallet {
Asset::<T, I>::try_mutate(id, |maybe_details| {
let d = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
ensure!(origin == d.admin, Error::<T, I>::NoPermission);
ensure!(d.status != AssetStatus::Destroying, Error::<T, I>::AssetNotLive);

d.is_frozen = false;
d.status = AssetStatus::Live;

Self::deposit_event(Event::<T, I>::AssetThawed { asset_id: id });
Ok(())
Expand Down Expand Up @@ -1211,14 +1210,18 @@ pub mod pallet {

Asset::<T, I>::try_mutate(id, |maybe_asset| {
let mut asset = maybe_asset.take().ok_or(Error::<T, I>::Unknown)?;
ensure!(asset.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);
ensure!(asset.status != AssetStatus::Destroying, Error::<T, I>::AssetNotLive);
asset.owner = T::Lookup::lookup(owner)?;
asset.issuer = T::Lookup::lookup(issuer)?;
asset.admin = T::Lookup::lookup(admin)?;
asset.freezer = T::Lookup::lookup(freezer)?;
asset.min_balance = min_balance;
asset.is_sufficient = is_sufficient;
asset.is_frozen = is_frozen;
if is_frozen {
asset.status = AssetStatus::Frozen;
} else {
asset.status = AssetStatus::Live;
}
*maybe_asset = Some(asset);

Self::deposit_event(Event::AssetStatusChanged { asset_id: id });
Expand Down
5 changes: 3 additions & 2 deletions frame/assets/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ pub mod v1 {

impl<Balance, AccountId, DepositBalance> OldAssetDetails<Balance, AccountId, DepositBalance> {
fn migrate_to_v1(self) -> AssetDetails<Balance, AccountId, DepositBalance> {
let status = if self.is_frozen { AssetStatus::Frozen } else { AssetStatus::Live };

AssetDetails {
owner: self.owner,
issuer: self.issuer,
Expand All @@ -53,8 +55,7 @@ pub mod v1 {
accounts: self.accounts,
sufficients: self.sufficients,
approvals: self.approvals,
is_frozen: self.is_frozen,
status: AssetStatus::Live,
status,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions frame/assets/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub(super) type AssetAccountOf<T, I> =
pub(super) enum AssetStatus {
/// The asset is active and able to be used.
Live,
/// Whether the asset is frozen for non-admin transfers.
Frozen,
/// The asset is currently being destroyed, and all actions are no longer permitted on the
/// asset. Once set to `Destroying`, the asset can never transition back to a `Live` state.
Destroying,
Expand Down Expand Up @@ -65,8 +67,6 @@ pub struct AssetDetails<Balance, AccountId, DepositBalance> {
pub(super) sufficients: u32,
/// The total number of approvals.
pub(super) approvals: u32,
/// Whether the asset is frozen for non-admin transfers.
pub(super) is_frozen: bool,
/// The status of the asset
pub(super) status: AssetStatus,
}
Expand Down