Insufficient asset quota and deposits (#10382)

* Allow asset accounts to exist by deposit * Place limit on consumers (and therefore freebie asset accounts) * Maximum number of assets * Fixes * Fixes * Formatting * Docs * Formatting * Destroyed assets are properly tidied * Update frame/assets/src/types.rs Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> * Docs * Docs * Formatting * Docs * Docs * Fixes * Fixes Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
2026-05-01 15:57:55 +00:00 · 2021-12-09 13:22:54 +01:00
parent 73bc2f420e
commit 3dd8f83a3a
84 changed files with 513 additions and 154 deletions
@@ -20,6 +20,14 @@
 use super::*;
 use frame_support::{traits::Get, BoundedVec};

+#[must_use]
+pub(super) enum DeadConsequence {
+	Remove,
+	Keep,
+}
+
+use DeadConsequence::*;
+
 // The main implementation block for the module.
 impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	// Public immutables
@@ -32,9 +40,17 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		ExtraMutator::maybe_new(id, who)
 	}

-	/// Get the asset `id` balance of `who`.
+	/// Get the asset `id` balance of `who`, or zero if the asset-account doesn't exist.
 	pub fn balance(id: T::AssetId, who: impl sp_std::borrow::Borrow<T::AccountId>) -> T::Balance {
-		Account::<T, I>::get(id, who.borrow()).balance
+		Self::maybe_balance(id, who).unwrap_or_default()
+	}
+
+	/// Get the asset `id` balance of `who` if the asset-account exists.
+	pub fn maybe_balance(
+		id: T::AssetId,
+		who: impl sp_std::borrow::Borrow<T::AccountId>,
+	) -> Option<T::Balance> {
+		Account::<T, I>::get(id, who.borrow()).map(|a| a.balance)
 	}

 	/// Get the total supply of an asset `id`.
@@ -45,34 +61,44 @@ 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>>,
-	) -> Result<bool, DispatchError> {
+		maybe_deposit: Option<DepositBalanceOf<T, I>>,
+	) -> Result<ExistenceReason<DepositBalanceOf<T, I>>, DispatchError> {
 		let accounts = d.accounts.checked_add(1).ok_or(ArithmeticError::Overflow)?;
-		let is_sufficient = if d.is_sufficient {
+		let reason = if let Some(deposit) = maybe_deposit {
+			ExistenceReason::DepositHeld(deposit)
+		} else if d.is_sufficient {
 			frame_system::Pallet::<T>::inc_sufficients(who);
 			d.sufficients += 1;
-			true
+			ExistenceReason::Sufficient
 		} else {
 			frame_system::Pallet::<T>::inc_consumers(who).map_err(|_| Error::<T, I>::NoProvider)?;
-			false
+			ExistenceReason::Consumer
 		};
 		d.accounts = accounts;
-		Ok(is_sufficient)
+		Ok(reason)
 	}

 	pub(super) fn dead_account(
 		what: T::AssetId,
 		who: &T::AccountId,
 		d: &mut AssetDetails<T::Balance, T::AccountId, DepositBalanceOf<T, I>>,
-		sufficient: bool,
-	) {
-		if sufficient {
-			d.sufficients = d.sufficients.saturating_sub(1);
-			frame_system::Pallet::<T>::dec_sufficients(who);
-		} else {
-			frame_system::Pallet::<T>::dec_consumers(who);
+		reason: &ExistenceReason<DepositBalanceOf<T, I>>,
+		force: bool,
+	) -> DeadConsequence {
+		let mut result = Remove;
+		match *reason {
+			ExistenceReason::Consumer => frame_system::Pallet::<T>::dec_consumers(who),
+			ExistenceReason::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(_) => result = Keep,
 		}
 		d.accounts = d.accounts.saturating_sub(1);
-		T::Freezer::died(what, who)
+		T::Freezer::died(what, who);
+		result
 	}

 	pub(super) fn can_increase(
@@ -87,15 +113,15 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		if details.supply.checked_add(&amount).is_none() {
 			return DepositConsequence::Overflow
 		}
-		let account = Account::<T, I>::get(id, who);
-		if account.balance.checked_add(&amount).is_none() {
-			return DepositConsequence::Overflow
-		}
-		if account.balance.is_zero() {
+		if let Some(balance) = Self::maybe_balance(id, who) {
+			if balance.checked_add(&amount).is_none() {
+				return DepositConsequence::Overflow
+			}
+		} else {
 			if amount < details.min_balance {
 				return DepositConsequence::BelowMinimum
 			}
-			if !details.is_sufficient && frame_system::Pallet::<T>::providers(who) == 0 {
+			if !details.is_sufficient && !frame_system::Pallet::<T>::can_inc_consumer(who) {
 				return DepositConsequence::CannotCreate
 			}
 			if details.is_sufficient && details.sufficients.checked_add(1).is_none() {
@@ -124,7 +150,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		if details.is_frozen {
 			return Frozen
 		}
-		let account = Account::<T, I>::get(id, who);
+		if amount.is_zero() {
+			return Success
+		}
+		let account = match Account::<T, I>::get(id, who) {
+			Some(a) => a,
+			None => return NoFunds,
+		};
 		if account.is_frozen {
 			return Frozen
 		}
@@ -165,7 +197,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let details = Asset::<T, I>::get(id).ok_or_else(|| Error::<T, I>::Unknown)?;
 		ensure!(!details.is_frozen, Error::<T, I>::Frozen);

-		let account = Account::<T, I>::get(id, who);
+		let account = Account::<T, I>::get(id, who).ok_or(Error::<T, I>::NoAccount)?;
 		ensure!(!account.is_frozen, Error::<T, I>::Frozen);

 		let amount = if let Some(frozen) = T::Freezer::frozen_balance(id, who) {
@@ -253,6 +285,48 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Ok((credit, maybe_burn))
 	}

+	/// Creates a account for `who` to hold asset `id` with a zero balance and takes a deposit.
+	pub(super) fn do_touch(id: T::AssetId, who: T::AccountId) -> DispatchResult {
+		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)?;
+		let reason = Self::new_account(&who, &mut details, Some(deposit))?;
+		T::Currency::reserve(&who, deposit)?;
+		Asset::<T, I>::insert(&id, details);
+		Account::<T, I>::insert(
+			id,
+			&who,
+			AssetAccountOf::<T, I> {
+				balance: Zero::zero(),
+				is_frozen: false,
+				reason,
+				extra: T::Extra::default(),
+			},
+		);
+		Ok(())
+	}
+
+	/// Returns a deposit, destroying an asset-account.
+	pub(super) fn do_refund(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)?;
+
+		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);
+
+		if let Remove = Self::dead_account(id, &who, &mut details, &account.reason, false) {
+			Account::<T, I>::remove(id, &who);
+		} else {
+			debug_assert!(false, "refund did not result in dead account?!");
+		}
+		Asset::<T, I>::insert(&id, details);
+		Ok(())
+	}
+
 	/// Increases the asset `id` balance of `beneficiary` by `amount`.
 	///
 	/// This alters the registered supply of the asset and emits an event.
@@ -307,13 +381,22 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

 			check(details)?;

-			Account::<T, I>::try_mutate(id, beneficiary, |t| -> DispatchResult {
-				let new_balance = t.balance.saturating_add(amount);
-				ensure!(new_balance >= details.min_balance, TokenError::BelowMinimum);
-				if t.balance.is_zero() {
-					t.sufficient = Self::new_account(beneficiary, details)?;
+			Account::<T, I>::try_mutate(id, beneficiary, |maybe_account| -> DispatchResult {
+				match maybe_account {
+					Some(ref mut account) => {
+						account.balance.saturating_accrue(amount);
+					},
+					maybe_account @ None => {
+						// Note this should never fail as it's already checked by `can_increase`.
+						ensure!(amount >= details.min_balance, TokenError::BelowMinimum);
+						*maybe_account = Some(AssetAccountOf::<T, I> {
+							balance: amount,
+							reason: Self::new_account(beneficiary, details, None)?,
+							is_frozen: false,
+							extra: T::Extra::default(),
+						});
+					},
 				}
-				t.balance = new_balance;
 				Ok(())
 			})?;
 			Ok(())
@@ -375,23 +458,25 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let actual = Self::prep_debit(id, target, amount, f)?;

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

 			check(actual, details)?;

-			Account::<T, I>::try_mutate_exists(id, target, |maybe_account| -> DispatchResult {
-				let mut account = maybe_account.take().unwrap_or_default();
+			Account::<T, I>::try_mutate(id, target, |maybe_account| -> DispatchResult {
+				let mut account = maybe_account.take().ok_or(Error::<T, I>::NoAccount)?;
 				debug_assert!(account.balance >= actual, "checked in prep; qed");

 				// Make the debit.
 				account.balance = account.balance.saturating_sub(actual);
-				*maybe_account = if account.balance < details.min_balance {
+				if account.balance < details.min_balance {
 					debug_assert!(account.balance.is_zero(), "checked in prep; qed");
-					Self::dead_account(id, target, details, account.sufficient);
-					None
-				} else {
-					Some(account)
+					if let Remove =
+						Self::dead_account(id, target, &mut details, &account.reason, false)
+					{
+						return Ok(())
+					}
 				};
+				*maybe_account = Some(account);
 				Ok(())
 			})?;

@@ -432,7 +517,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let debit = Self::prep_debit(id, &source, amount, f.into())?;
 		let (credit, maybe_burn) = Self::prep_credit(id, &dest, amount, debit, f.burn_dust)?;

-		let mut source_account = Account::<T, I>::get(id, &source);
+		let mut source_account =
+			Account::<T, I>::get(id, &source).ok_or(Error::<T, I>::NoAccount)?;

 		Asset::<T, I>::try_mutate(id, |maybe_details| -> DispatchResult {
 			let details = maybe_details.as_mut().ok_or(Error::<T, I>::Unknown)?;
@@ -459,29 +545,40 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			debug_assert!(source_account.balance >= debit, "checked in prep; qed");
 			source_account.balance = source_account.balance.saturating_sub(debit);

-			Account::<T, I>::try_mutate(id, &dest, |a| -> DispatchResult {
-				// Calculate new balance; this will not saturate since it's already checked in prep.
-				debug_assert!(a.balance.checked_add(&credit).is_some(), "checked in prep; qed");
-				let new_balance = a.balance.saturating_add(credit);
-
-				// Create a new account if there wasn't one already.
-				if a.balance.is_zero() {
-					a.sufficient = Self::new_account(&dest, details)?;
+			Account::<T, I>::try_mutate(id, &dest, |maybe_account| -> DispatchResult {
+				match maybe_account {
+					Some(ref mut account) => {
+						// Calculate new balance; this will not saturate since it's already checked
+						// in prep.
+						debug_assert!(
+							account.balance.checked_add(&credit).is_some(),
+							"checked in prep; qed"
+						);
+						account.balance.saturating_accrue(credit);
+					},
+					maybe_account @ None => {
+						*maybe_account = Some(AssetAccountOf::<T, I> {
+							balance: credit,
+							is_frozen: false,
+							reason: Self::new_account(&dest, details, None)?,
+							extra: T::Extra::default(),
+						});
+					},
 				}
-
-				a.balance = new_balance;
 				Ok(())
 			})?;

 			// Remove source account if it's now dead.
 			if source_account.balance < details.min_balance {
 				debug_assert!(source_account.balance.is_zero(), "checked in prep; qed");
-				Self::dead_account(id, &source, details, source_account.sufficient);
-				Account::<T, I>::remove(id, &source);
-			} else {
-				Account::<T, I>::insert(id, &source, &source_account)
+				if let Remove =
+					Self::dead_account(id, &source, details, &source_account.reason, false)
+				{
+					Account::<T, I>::remove(id, &source);
+					return Ok(())
+				}
 			}
-
+			Account::<T, I>::insert(id, &source, &source_account);
 			Ok(())
 		})?;

@@ -554,7 +651,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			ensure!(details.approvals <= witness.approvals, Error::<T, I>::BadWitness);

 			for (who, v) in Account::<T, I>::drain_prefix(id) {
-				Self::dead_account(id, &who, &mut details, v.sufficient);
+				// 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(id, &who, &mut details, &v.reason, true);
 			}
 			debug_assert_eq!(details.accounts, 0);
 			debug_assert_eq!(details.sufficients, 0);