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

Asset's account deposit owner #13874

Merged
merged 6 commits into from
Apr 11, 2023
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
1 change: 0 additions & 1 deletion bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1860,7 +1860,6 @@ type Migrations = (
pallet_nomination_pools::migration::v2::MigrateToV2<Runtime>,
pallet_alliance::migration::Migration<Runtime>,
pallet_contracts::Migration<Runtime>,
pallet_assets::migration::v2::MigrateToV2<Runtime>,
);

/// MMR helper types.
Expand Down
67 changes: 30 additions & 37 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)
}
} 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 @@ -123,39 +128,30 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
amount: T::Balance,
increase_supply: bool,
) -> DepositConsequence {
use DepositConsequence::*;
let details = match Asset::<T, I>::get(id) {
Some(details) => details,
None => return UnknownAsset,
None => return DepositConsequence::UnknownAsset,
};
if increase_supply && details.supply.checked_add(&amount).is_none() {
return Overflow
}
// We don't allow freezing of accounts that don't already have an `(AssetId, AccountId)`
// entry. The account may have a zero balance provided by a deposit placed by `touch`ing the
// account. However, frozen accounts cannot receive more of the `AssetID`.
if let Some(a) = Account::<T, I>::get(id, who) {
if a.is_frozen {
return Frozen
}
return DepositConsequence::Overflow
}
if let Some(balance) = Self::maybe_balance(id, who) {
if balance.checked_add(&amount).is_none() {
return Overflow
return DepositConsequence::Overflow
}
} else {
if amount < details.min_balance {
return BelowMinimum
return DepositConsequence::BelowMinimum
}
if !details.is_sufficient && !frame_system::Pallet::<T>::can_inc_consumer(who) {
return CannotCreate
return DepositConsequence::CannotCreate
}
if details.is_sufficient && details.sufficients.checked_add(1).is_none() {
return Overflow
return DepositConsequence::Overflow
}
}

Success
DepositConsequence::Success
}

/// Return the consequence of a withdraw.
Expand Down Expand Up @@ -321,7 +317,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))?;
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(
Expand All @@ -331,7 +327,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
balance: Zero::zero(),
is_frozen: false,
reason,
depositor: Some(depositor.clone()),
extra: T::Extra::default(),
},
);
Expand All @@ -346,8 +341,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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 depositor = account.depositor.take().unwrap_or(who.clone());
let (depositor, deposit) =
account.reason.take_deposit(&who).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);
Expand All @@ -362,9 +357,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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 has been refunded, need to update `Account`
Account::<T, I>::insert(id, &who, account);
debug_assert!(false, "refund did not result in dead account?!");
}
Asset::<T, I>::insert(&id, details);
// Executing a hook here is safe, since it is not in a `mutate`.
Expand Down Expand Up @@ -434,7 +429,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
*maybe_account = Some(AssetAccountOf::<T, I> {
balance: amount,
reason: Self::new_account(beneficiary, details, None)?,
depositor: None,
is_frozen: false,
extra: T::Extra::default(),
});
Expand Down Expand Up @@ -631,7 +625,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
balance: credit,
is_frozen: false,
reason: Self::new_account(dest, details, None)?,
depositor: None,
extra: T::Extra::default(),
});
},
Expand Down Expand Up @@ -969,7 +962,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

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

Account::<T, I>::try_mutate(id, &who, |maybe_account| -> DispatchResult {
Expand Down
4 changes: 1 addition & 3 deletions frame/assets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1593,7 +1593,6 @@ pub mod pallet {
///
/// - `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.
///
/// Emits `Refunded` event when successful.
#[pallet::call_index(30)]
Expand All @@ -1602,11 +1601,10 @@ pub mod pallet {
origin: OriginFor<T>,
id: T::AssetIdParameter,
who: T::AccountId,
allow_burn: bool,
) -> DispatchResult {
let origin = ensure_signed(origin)?;
let id: T::AssetId = id.into();
Self::do_refund(&origin, id, &who, allow_burn)
Self::do_refund(&origin, id, &who, false)
}
}
}
Expand Down
126 changes: 0 additions & 126 deletions frame/assets/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,129 +131,3 @@ pub mod v1 {
}
}
}

pub mod v2 {
use frame_support::{pallet_prelude::*, weights::Weight};

use super::*;

#[derive(Decode)]
pub struct OldAssetAccount<Balance, DepositBalance, Extra, AccountId> {
pub(super) balance: Balance,
pub(super) is_frozen: bool,
pub(super) reason: ExistenceReason<DepositBalance>,
pub(super) extra: Extra,
_phantom: sp_std::marker::PhantomData<AccountId>,
}

impl<Balance, DepositBalance, Extra, AccountId>
OldAssetAccount<Balance, DepositBalance, Extra, AccountId>
{
fn migrate_to_v2(
self,
who: AccountId,
) -> AssetAccount<Balance, DepositBalance, Extra, AccountId> {
let depositor_account = match self.reason {
// any accounts that exist via deposit will have placed it themselves
ExistenceReason::DepositHeld(_) => Some(who),
_ => None,
};

AssetAccount {
balance: self.balance,
is_frozen: self.is_frozen,
reason: self.reason,
depositor: depositor_account,
extra: self.extra,
}
}
}

// Migration to V2 possible from both V0 and V1.
pub struct MigrateToV2<T, I = ()>(sp_std::marker::PhantomData<(T, I)>);
impl<T: Config<I>, I: 'static> OnRuntimeUpgrade for MigrateToV2<T, I> {
fn on_runtime_upgrade() -> Weight {
// `current_version` is what we are migrating to
let current_version = Pallet::<T, I>::current_storage_version();
// `onchain_version` is what is already in place
let onchain_version = Pallet::<T, I>::on_chain_storage_version();

// `Account`s are the same in v0 and v1, so we can migrate to v2 from either one.
// Of course the v1 migration should be applied for `Asset`s.
if (onchain_version == 0 || onchain_version == 1) && current_version == 2 {
let mut translated = 0u64;

Account::<T, I>::translate::<
OldAssetAccount<T::Balance, DepositBalanceOf<T, I>, T::Extra, T::AccountId>,
_,
>(|_asset_id, account, old_value| {
translated.saturating_inc();
Some(old_value.migrate_to_v2(account))
});

// update the storage version of the pallet
StorageVersion::new(2).put::<Pallet<T, I>>();
log::info!(
target: LOG_TARGET,
"Upgraded info for {} accounts, storage to version {:?}",
translated,
current_version
);
T::DbWeight::get().reads_writes(translated + 1, translated + 1)
} else {
log::info!(
target: LOG_TARGET,
"Migration did not execute. This probably should be removed"
);
T::DbWeight::get().reads(1)
}
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
frame_support::ensure!(
Pallet::<T, I>::on_chain_storage_version() < 2,
"should not execute otherwise"
);
let prev_count = Account::<T, I>::iter().count();
Ok((prev_count as u32).encode())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(prev_count: Vec<u8>) -> Result<(), &'static str> {
let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect(
"the state parameter should be something that was generated by pre_upgrade",
);
let post_count = Account::<T, I>::iter().count() as u32;
assert_eq!(
prev_count, post_count,
"the number of asset accounts before and after the migration should be the same"
);

let current_version = Pallet::<T, I>::current_storage_version();
let onchain_version = Pallet::<T, I>::on_chain_storage_version();

frame_support::ensure!(current_version == 2, "must_upgrade");
assert_eq!(
current_version, onchain_version,
"after migration, the current_version and onchain_version should be the same"
);

Account::<T, I>::iter().for_each(|(_asset, account, account_data)| match account_data
.reason
{
ExistenceReason::DepositHeld(_) => {
assert_eq!(
account_data.depositor,
Some(account),
"account depositor should be the account"
);
},
_ => {
assert_eq!(account_data.depositor, None, "account should have no depositor");
},
});
Ok(())
}
}
}
Loading