Remove the Copy bound on AssetId (#14158)

* Remove the `Copy` bound on `AssetId` * Also relax the `Copy` bound in the assets pallet * Fix warnings on the newest nightly Rust * Remove some unnecessary `clone()`s * Try to satisfy clippy * Remove some more unnecessary `clone()`s * Add more `.clone()`s for newly merged code * Also add `clone()`s in the benchmarks --------- Co-authored-by: parity-processbot <>
2026-04-26 16:57:58 +00:00 · 2023-05-23 20:34:04 +09:00
parent 06865d4c08
commit 194c9edd4a
13 changed files with 223 additions and 187 deletions
@@ -128,7 +128,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		amount: T::Balance,
 		increase_supply: bool,
 	) -> DepositConsequence {
-		let details = match Asset::<T, I>::get(id) {
+		let details = match Asset::<T, I>::get(&id) {
 			Some(details) => details,
 			None => return DepositConsequence::UnknownAsset,
 		};
@@ -165,7 +165,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		keep_alive: bool,
 	) -> WithdrawConsequence<T::Balance> {
 		use WithdrawConsequence::*;
-		let details = match Asset::<T, I>::get(id) {
+		let details = match Asset::<T, I>::get(&id) {
 			Some(details) => details,
 			None => return UnknownAsset,
 		};
@@ -178,7 +178,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		if amount.is_zero() {
 			return Success
 		}
-		let account = match Account::<T, I>::get(id, who) {
+		let account = match Account::<T, I>::get(&id, who) {
 			Some(a) => a,
 			None => return BalanceLow,
 		};
@@ -186,7 +186,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			return Frozen
 		}
 		if let Some(rest) = account.balance.checked_sub(&amount) {
-			if let Some(frozen) = T::Freezer::frozen_balance(id, who) {
+			if let Some(frozen) = T::Freezer::frozen_balance(id.clone(), who) {
 				match frozen.checked_add(&details.min_balance) {
 					Some(required) if rest < required => return Frozen,
 					None => return Overflow,
@@ -219,10 +219,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		who: &T::AccountId,
 		keep_alive: bool,
 	) -> Result<T::Balance, DispatchError> {
-		let details = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
+		let details = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
 		ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);

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

 		let amount = if let Some(frozen) = T::Freezer::frozen_balance(id, who) {
@@ -265,7 +265,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		amount: T::Balance,
 		f: DebitFlags,
 	) -> Result<T::Balance, DispatchError> {
-		let actual = Self::reducible_balance(id, target, f.keep_alive)?.min(amount);
+		let actual = Self::reducible_balance(id.clone(), target, f.keep_alive)?.min(amount);
 		ensure!(f.best_effort || actual >= amount, Error::<T, I>::BalanceLow);

 		let conseq = Self::can_decrease(id, target, actual, f.keep_alive);
@@ -320,7 +320,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		depositor: T::AccountId,
 		check_depositor: bool,
 	) -> DispatchResult {
-		ensure!(!Account::<T, I>::contains_key(id, &who), Error::<T, I>::AlreadyExists);
+		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);
@@ -332,7 +332,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		T::Currency::reserve(&depositor, deposit)?;
 		Asset::<T, I>::insert(&id, details);
 		Account::<T, I>::insert(
-			id,
+			&id,
 			&who,
 			AssetAccountOf::<T, I> {
 				balance: Zero::zero(),
@@ -350,7 +350,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	pub(super) fn do_refund(id: T::AssetId, who: T::AccountId, allow_burn: bool) -> DispatchResult {
 		use AssetStatus::*;
 		use ExistenceReason::*;
-		let mut account = Account::<T, I>::get(id, &who).ok_or(Error::<T, I>::NoDeposit)?;
+		let mut account = Account::<T, I>::get(&id, &who).ok_or(Error::<T, I>::NoDeposit)?;
 		ensure!(matches!(account.reason, Consumer | DepositHeld(..)), Error::<T, I>::NoDeposit);
 		let mut details = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
 		ensure!(matches!(details.status, Live | Frozen), Error::<T, I>::IncorrectStatus);
@@ -361,7 +361,7 @@ 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);
+			Account::<T, I>::remove(&id, &who);
 		} else {
 			debug_assert!(false, "refund did not result in dead account?!");
 			// deposit may have been refunded, need to update `Account`
@@ -380,7 +380,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		who: &T::AccountId,
 		caller: &T::AccountId,
 	) -> DispatchResult {
-		let mut account = Account::<T, I>::get(id, &who).ok_or(Error::<T, I>::NoDeposit)?;
+		let mut account = Account::<T, I>::get(&id, &who).ok_or(Error::<T, I>::NoDeposit)?;
 		let (depositor, deposit) =
 			account.reason.take_deposit_from().ok_or(Error::<T, I>::NoDeposit)?;
 		let mut details = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
@@ -392,11 +392,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		T::Currency::unreserve(&depositor, deposit);

 		if let Remove = Self::dead_account(&who, &mut details, &account.reason, false) {
-			Account::<T, I>::remove(id, &who);
+			Account::<T, I>::remove(&id, &who);
 		} else {
 			debug_assert!(false, "refund did not result in dead account?!");
 			// deposit may have been refunded, need to update `Account`
-			Account::<T, I>::insert(id, &who, account);
+			Account::<T, I>::insert(&id, &who, account);
 			return Ok(())
 		}
 		Asset::<T, I>::insert(&id, details);
@@ -416,7 +416,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		amount: T::Balance,
 		maybe_check_issuer: Option<T::AccountId>,
 	) -> DispatchResult {
-		Self::increase_balance(id, beneficiary, amount, |details| -> DispatchResult {
+		Self::increase_balance(id.clone(), beneficiary, amount, |details| -> DispatchResult {
 			if let Some(check_issuer) = maybe_check_issuer {
 				ensure!(check_issuer == details.issuer, Error::<T, I>::NoPermission);
 			}
@@ -450,19 +450,20 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			return Ok(())
 		}

-		Self::can_increase(id, beneficiary, amount, true).into_result()?;
-		Asset::<T, I>::try_mutate(id, |maybe_details| -> DispatchResult {
+		Self::can_increase(id.clone(), 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);
 			check(details)?;

-			Account::<T, I>::try_mutate(id, beneficiary, |maybe_account| -> DispatchResult {
+			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`.
+						// 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,
@@ -493,13 +494,13 @@ 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)?;
+		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
 		);

-		let actual = Self::decrease_balance(id, target, amount, f, |actual, details| {
+		let actual = Self::decrease_balance(id.clone(), target, amount, f, |actual, details| {
 			// Check admin rights.
 			if let Some(check_admin) = maybe_check_admin {
 				ensure!(check_admin == details.admin, Error::<T, I>::NoPermission);
@@ -536,17 +537,17 @@ 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)?;
+		let details = Asset::<T, I>::get(&id).ok_or(Error::<T, I>::Unknown)?;
 		ensure!(details.status == AssetStatus::Live, Error::<T, I>::AssetNotLive);

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

-		Asset::<T, I>::try_mutate(id, |maybe_details| -> DispatchResult {
+		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 {
+			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");

@@ -590,7 +591,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		f: TransferFlags,
 	) -> Result<T::Balance, DispatchError> {
 		let (balance, died) =
-			Self::transfer_and_die(id, source, dest, amount, maybe_need_admin, f)?;
+			Self::transfer_and_die(id.clone(), source, dest, amount, maybe_need_admin, f)?;
 		if let Some(Remove) = died {
 			T::Freezer::died(id, source);
 		}
@@ -611,18 +612,18 @@ 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)?;
+		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())?;
-		let (credit, maybe_burn) = Self::prep_credit(id, dest, amount, debit, f.burn_dust)?;
+		let debit = Self::prep_debit(id.clone(), source, amount, f.into())?;
+		let (credit, maybe_burn) = Self::prep_credit(id.clone(), dest, amount, debit, f.burn_dust)?;

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

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

 			// Check admin rights.
@@ -647,7 +648,7 @@ 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, |maybe_account| -> DispatchResult {
+			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
@@ -676,11 +677,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				source_died =
 					Some(Self::dead_account(source, details, &source_account.reason, false));
 				if let Some(Remove) = source_died {
-					Account::<T, I>::remove(id, &source);
+					Account::<T, I>::remove(&id, &source);
 					return Ok(())
 				}
 			}
-			Account::<T, I>::insert(id, &source, &source_account);
+			Account::<T, I>::insert(&id, &source, &source_account);
 			Ok(())
 		})?;

@@ -707,11 +708,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		is_sufficient: bool,
 		min_balance: T::Balance,
 	) -> DispatchResult {
-		ensure!(!Asset::<T, I>::contains_key(id), Error::<T, I>::InUse);
+		ensure!(!Asset::<T, I>::contains_key(&id), Error::<T, I>::InUse);
 		ensure!(!min_balance.is_zero(), Error::<T, I>::MinBalanceZero);

 		Asset::<T, I>::insert(
-			id,
+			&id,
 			AssetDetails {
 				owner: owner.clone(),
 				issuer: owner.clone(),
@@ -738,8 +739,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		id: T::AssetId,
 		maybe_check_owner: Option<T::AccountId>,
 	) -> 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)?;
+		Asset::<T, I>::try_mutate_exists(id.clone(), |maybe_details| -> Result<(), DispatchError> {
+			let 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);
 			}
@@ -761,12 +762,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let mut dead_accounts: Vec<T::AccountId> = vec![];
 		let mut remaining_accounts = 0;
 		let _ =
-			Asset::<T, I>::try_mutate_exists(id, |maybe_details| -> Result<(), DispatchError> {
+			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) {
+				for (who, v) in Account::<T, I>::drain_prefix(&id) {
 					let _ = Self::dead_account(&who, &mut details, &v.reason, true);
 					dead_accounts.push(who);
 					if dead_accounts.len() >= (max_items as usize) {
@@ -778,7 +779,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			})?;

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

 		Self::deposit_event(Event::AccountsDestroyed {
@@ -798,14 +799,15 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		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)?;
+		let _ = Asset::<T, I>::try_mutate_exists(
+			id.clone(),
+			|maybe_details| -> Result<(), DispatchError> {
+				let 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,)) {
+				for ((owner, _), approval) in Approvals::<T, I>::drain_prefix((id.clone(),)) {
 					T::Currency::unreserve(&owner, approval.deposit);
 					removed_approvals = removed_approvals.saturating_add(1);
 					details.approvals = details.approvals.saturating_sub(1);
@@ -819,7 +821,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 					approvals_remaining: details.approvals as u32,
 				});
 				Ok(())
-			})?;
+			},
+		)?;
 		Ok(removed_approvals)
 	}

@@ -827,7 +830,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	///
 	/// 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> {
+		Asset::<T, I>::try_mutate_exists(id.clone(), |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);
@@ -855,10 +858,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		delegate: &T::AccountId,
 		amount: T::Balance,
 	) -> DispatchResult {
-		let mut d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
+		let mut 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(
-			(id, &owner, &delegate),
+			(id.clone(), &owner, &delegate),
 			|maybe_approved| -> DispatchResult {
 				let mut approved = match maybe_approved.take() {
 					// an approval already exists and is being updated
@@ -879,7 +882,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				Ok(())
 			},
 		)?;
-		Asset::<T, I>::insert(id, d);
+		Asset::<T, I>::insert(&id, d);
 		Self::deposit_event(Event::ApprovedTransfer {
 			asset_id: id,
 			source: owner.clone(),
@@ -906,22 +909,23 @@ 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)?;
+		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),
+			(id.clone(), &owner, delegate),
 			|maybe_approved| -> DispatchResult {
 				let mut approved = maybe_approved.take().ok_or(Error::<T, I>::Unapproved)?;
 				let remaining =
 					approved.amount.checked_sub(&amount).ok_or(Error::<T, I>::Unapproved)?;

 				let f = TransferFlags { keep_alive: false, best_effort: false, burn_dust: false };
-				owner_died = Self::transfer_and_die(id, owner, destination, amount, None, f)?.1;
+				owner_died =
+					Self::transfer_and_die(id.clone(), owner, destination, amount, None, f)?.1;

 				if remaining.is_zero() {
 					T::Currency::unreserve(owner, approved.deposit);
-					Asset::<T, I>::mutate(id, |maybe_details| {
+					Asset::<T, I>::mutate(id.clone(), |maybe_details| {
 						if let Some(details) = maybe_details {
 							details.approvals.saturating_dec();
 						}
@@ -954,11 +958,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let bounded_symbol: BoundedVec<u8, T::StringLimit> =
 			symbol.clone().try_into().map_err(|_| Error::<T, I>::BadMetadata)?;

-		let d = Asset::<T, I>::get(id).ok_or(Error::<T, I>::Unknown)?;
+		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| {
+		Metadata::<T, I>::try_mutate_exists(id.clone(), |metadata| {
 			ensure!(metadata.as_ref().map_or(true, |m| !m.is_frozen), Error::<T, I>::NoPermission);

 			let old_deposit = metadata.take().map_or(Zero::zero(), |m| m.deposit);
@@ -999,7 +1003,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	/// Returns all the non-zero balances for all assets of the given `account`.
 	pub fn account_balances(account: T::AccountId) -> Vec<(T::AssetId, T::Balance)> {
 		Asset::<T, I>::iter_keys()
-			.filter_map(|id| Self::maybe_balance(id, account.clone()).map(|balance| (id, balance)))
+			.filter_map(|id| {
+				Self::maybe_balance(id.clone(), account.clone()).map(|balance| (id, balance))
+			})
 			.collect::<Vec<_>>()
 	}
 }