Remove the Copy bound on CollectionId in the uniques pallet (#14111)

* Remove the `Copy` bound on `CollectionId` in the uniques pallet

* Also add `clone`s in benchmarks

substrate/frame/uniques/src/benchmarking.rs

@@ -44,7 +44,7 @@ fn create_collection<T: Config<I>, I: 'static>(
 	T::Currency::make_free_balance_be(&caller, DepositBalanceOf::<T, I>::max_value());
 	assert!(Uniques::<T, I>::force_create(
 		SystemOrigin::Root.into(),
-		collection,
+		collection.clone(),
 		caller_lookup.clone(),
 		false,
 	)
@@ -173,26 +173,26 @@ benchmarks_instance_pallet! {
 		for i in 0..a {
 			add_item_attribute::<T, I>(T::Helper::item(i as u16));
 		}
-		let witness = Collection::<T, I>::get(collection).unwrap().destroy_witness();
-	}: _(SystemOrigin::Signed(caller), collection, witness)
+		let witness = Collection::<T, I>::get(collection.clone()).unwrap().destroy_witness();
+	}: _(SystemOrigin::Signed(caller), collection.clone(), witness)
 	verify {
-		assert_last_event::<T, I>(Event::Destroyed { collection }.into());
+		assert_last_event::<T, I>(Event::Destroyed { collection: collection.clone() }.into());
 	}
 
 	mint {
 		let (collection, caller, caller_lookup) = create_collection::<T, I>();
 		let item = T::Helper::item(0);
-	}: _(SystemOrigin::Signed(caller.clone()), collection, item, caller_lookup)
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone(), item, caller_lookup)
 	verify {
-		assert_last_event::<T, I>(Event::Issued { collection, item, owner: caller }.into());
+		assert_last_event::<T, I>(Event::Issued { collection: collection.clone(), item, owner: caller }.into());
 	}
 
 	burn {
 		let (collection, caller, caller_lookup) = create_collection::<T, I>();
 		let (item, ..) = mint_item::<T, I>(0);
-	}: _(SystemOrigin::Signed(caller.clone()), collection, item, Some(caller_lookup))
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone(), item, Some(caller_lookup))
 	verify {
-		assert_last_event::<T, I>(Event::Burned { collection, item, owner: caller }.into());
+		assert_last_event::<T, I>(Event::Burned { collection: collection.clone(), item, owner: caller }.into());
 	}
 
 	transfer {
@@ -201,9 +201,9 @@ benchmarks_instance_pallet! {
 
 		let target: T::AccountId = account("target", 0, SEED);
 		let target_lookup = T::Lookup::unlookup(target.clone());
-	}: _(SystemOrigin::Signed(caller.clone()), collection, item, target_lookup)
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone(), item, target_lookup)
 	verify {
-		assert_last_event::<T, I>(Event::Transferred { collection, item, from: caller, to: target }.into());
+		assert_last_event::<T, I>(Event::Transferred { collection: collection.clone(), item, from: caller, to: target }.into());
 	}
 
 	redeposit {
@@ -212,7 +212,7 @@ benchmarks_instance_pallet! {
 		let items = (0..i).map(|x| mint_item::<T, I>(x as u16).0).collect::<Vec<_>>();
 		Uniques::<T, I>::force_item_status(
 			SystemOrigin::Root.into(),
-			collection,
+			collection.clone(),
 			caller_lookup.clone(),
 			caller_lookup.clone(),
 			caller_lookup.clone(),
@@ -220,9 +220,9 @@ benchmarks_instance_pallet! {
 			true,
 			false,
 		)?;
-	}: _(SystemOrigin::Signed(caller.clone()), collection, items.clone())
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone(), items.clone())
 	verify {
-		assert_last_event::<T, I>(Event::Redeposited { collection, successful_items: items }.into());
+		assert_last_event::<T, I>(Event::Redeposited { collection: collection.clone(), successful_items: items }.into());
 	}
 
 	freeze {
@@ -238,28 +238,28 @@ benchmarks_instance_pallet! {
 		let (item, ..) = mint_item::<T, I>(0);
 		Uniques::<T, I>::freeze(
 			SystemOrigin::Signed(caller.clone()).into(),
-			collection,
+			collection.clone(),
 			item,
 		)?;
-	}: _(SystemOrigin::Signed(caller.clone()), collection, item)
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone(), item)
 	verify {
-		assert_last_event::<T, I>(Event::Thawed { collection, item }.into());
+		assert_last_event::<T, I>(Event::Thawed { collection: collection.clone(), item }.into());
 	}
 
 	freeze_collection {
 		let (collection, caller, caller_lookup) = create_collection::<T, I>();
-	}: _(SystemOrigin::Signed(caller.clone()), collection)
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone())
 	verify {
-		assert_last_event::<T, I>(Event::CollectionFrozen { collection }.into());
+		assert_last_event::<T, I>(Event::CollectionFrozen { collection: collection.clone() }.into());
 	}
 
 	thaw_collection {
 		let (collection, caller, caller_lookup) = create_collection::<T, I>();
 		let origin = SystemOrigin::Signed(caller.clone()).into();
-		Uniques::<T, I>::freeze_collection(origin, collection)?;
-	}: _(SystemOrigin::Signed(caller.clone()), collection)
+		Uniques::<T, I>::freeze_collection(origin, collection.clone())?;
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone())
 	verify {
-		assert_last_event::<T, I>(Event::CollectionThawed { collection }.into());
+		assert_last_event::<T, I>(Event::CollectionThawed { collection: collection.clone() }.into());
 	}
 
 	transfer_ownership {
@@ -268,10 +268,10 @@ benchmarks_instance_pallet! {
 		let target_lookup = T::Lookup::unlookup(target.clone());
 		T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance());
 		let origin = SystemOrigin::Signed(target.clone()).into();
-		Uniques::<T, I>::set_accept_ownership(origin, Some(collection))?;
-	}: _(SystemOrigin::Signed(caller), collection, target_lookup)
+		Uniques::<T, I>::set_accept_ownership(origin, Some(collection.clone()))?;
+	}: _(SystemOrigin::Signed(caller), collection.clone(), target_lookup)
 	verify {
-		assert_last_event::<T, I>(Event::OwnerChanged { collection, new_owner: target }.into());
+		assert_last_event::<T, I>(Event::OwnerChanged { collection: collection.clone(), new_owner: target }.into());
 	}
 
 	set_team {
@@ -279,10 +279,10 @@ benchmarks_instance_pallet! {
 		let target0 = T::Lookup::unlookup(account("target", 0, SEED));
 		let target1 = T::Lookup::unlookup(account("target", 1, SEED));
 		let target2 = T::Lookup::unlookup(account("target", 2, SEED));
-	}: _(SystemOrigin::Signed(caller), collection, target0, target1, target2)
+	}: _(SystemOrigin::Signed(caller), collection.clone(), target0, target1, target2)
 	verify {
 		assert_last_event::<T, I>(Event::TeamChanged{
-			collection,
+			collection: collection.clone(),
 			issuer: account("target", 0, SEED),
 			admin: account("target", 1, SEED),
 			freezer: account("target", 2, SEED),
@@ -294,7 +294,7 @@ benchmarks_instance_pallet! {
 		let origin =
 			T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
 		let call = Call::<T, I>::force_item_status {
-			collection,
+			collection: collection.clone(),
 			owner: caller_lookup.clone(),
 			issuer: caller_lookup.clone(),
 			admin: caller_lookup.clone(),
@@ -304,7 +304,7 @@ benchmarks_instance_pallet! {
 		};
 	}: { call.dispatch_bypass_filter(origin)? }
 	verify {
-		assert_last_event::<T, I>(Event::ItemStatusChanged { collection }.into());
+		assert_last_event::<T, I>(Event::ItemStatusChanged { collection: collection.clone() }.into());
 	}
 
 	set_attribute {
@@ -314,9 +314,9 @@ benchmarks_instance_pallet! {
 		let (collection, caller, _) = create_collection::<T, I>();
 		let (item, ..) = mint_item::<T, I>(0);
 		add_item_metadata::<T, I>(item);
-	}: _(SystemOrigin::Signed(caller), collection, Some(item), key.clone(), value.clone())
+	}: _(SystemOrigin::Signed(caller), collection.clone(), Some(item), key.clone(), value.clone())
 	verify {
-		assert_last_event::<T, I>(Event::AttributeSet { collection, maybe_item: Some(item), key, value }.into());
+		assert_last_event::<T, I>(Event::AttributeSet { collection: collection.clone(), maybe_item: Some(item), key, value }.into());
 	}
 
 	clear_attribute {
@@ -324,9 +324,9 @@ benchmarks_instance_pallet! {
 		let (item, ..) = mint_item::<T, I>(0);
 		add_item_metadata::<T, I>(item);
 		let (key, ..) = add_item_attribute::<T, I>(item);
-	}: _(SystemOrigin::Signed(caller), collection, Some(item), key.clone())
+	}: _(SystemOrigin::Signed(caller), collection.clone(), Some(item), key.clone())
 	verify {
-		assert_last_event::<T, I>(Event::AttributeCleared { collection, maybe_item: Some(item), key }.into());
+		assert_last_event::<T, I>(Event::AttributeCleared { collection: collection.clone(), maybe_item: Some(item), key }.into());
 	}
 
 	set_metadata {
@@ -334,35 +334,35 @@ benchmarks_instance_pallet! {
 
 		let (collection, caller, _) = create_collection::<T, I>();
 		let (item, ..) = mint_item::<T, I>(0);
-	}: _(SystemOrigin::Signed(caller), collection, item, data.clone(), false)
+	}: _(SystemOrigin::Signed(caller), collection.clone(), item, data.clone(), false)
 	verify {
-		assert_last_event::<T, I>(Event::MetadataSet { collection, item, data, is_frozen: false }.into());
+		assert_last_event::<T, I>(Event::MetadataSet { collection: collection.clone(), item, data, is_frozen: false }.into());
 	}
 
 	clear_metadata {
 		let (collection, caller, _) = create_collection::<T, I>();
 		let (item, ..) = mint_item::<T, I>(0);
 		add_item_metadata::<T, I>(item);
-	}: _(SystemOrigin::Signed(caller), collection, item)
+	}: _(SystemOrigin::Signed(caller), collection.clone(), item)
 	verify {
-		assert_last_event::<T, I>(Event::MetadataCleared { collection, item }.into());
+		assert_last_event::<T, I>(Event::MetadataCleared { collection: collection.clone(), item }.into());
 	}
 
 	set_collection_metadata {
 		let data: BoundedVec<_, _> = vec![0u8; T::StringLimit::get() as usize].try_into().unwrap();
 
 		let (collection, caller, _) = create_collection::<T, I>();
-	}: _(SystemOrigin::Signed(caller), collection, data.clone(), false)
+	}: _(SystemOrigin::Signed(caller), collection.clone(), data.clone(), false)
 	verify {
-		assert_last_event::<T, I>(Event::CollectionMetadataSet { collection, data, is_frozen: false }.into());
+		assert_last_event::<T, I>(Event::CollectionMetadataSet { collection: collection.clone(), data, is_frozen: false }.into());
 	}
 
 	clear_collection_metadata {
 		let (collection, caller, _) = create_collection::<T, I>();
 		add_collection_metadata::<T, I>();
-	}: _(SystemOrigin::Signed(caller), collection)
+	}: _(SystemOrigin::Signed(caller), collection.clone())
 	verify {
-		assert_last_event::<T, I>(Event::CollectionMetadataCleared { collection }.into());
+		assert_last_event::<T, I>(Event::CollectionMetadataCleared { collection: collection.clone() }.into());
 	}
 
 	approve_transfer {
@@ -370,9 +370,9 @@ benchmarks_instance_pallet! {
 		let (item, ..) = mint_item::<T, I>(0);
 		let delegate: T::AccountId = account("delegate", 0, SEED);
 		let delegate_lookup = T::Lookup::unlookup(delegate.clone());
-	}: _(SystemOrigin::Signed(caller.clone()), collection, item, delegate_lookup)
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone(), item, delegate_lookup)
 	verify {
-		assert_last_event::<T, I>(Event::ApprovedTransfer { collection, item, owner: caller, delegate }.into());
+		assert_last_event::<T, I>(Event::ApprovedTransfer { collection: collection.clone(), item, owner: caller, delegate }.into());
 	}
 
 	cancel_approval {
@@ -381,17 +381,17 @@ benchmarks_instance_pallet! {
 		let delegate: T::AccountId = account("delegate", 0, SEED);
 		let delegate_lookup = T::Lookup::unlookup(delegate.clone());
 		let origin = SystemOrigin::Signed(caller.clone()).into();
-		Uniques::<T, I>::approve_transfer(origin, collection, item, delegate_lookup.clone())?;
-	}: _(SystemOrigin::Signed(caller.clone()), collection, item, Some(delegate_lookup))
+		Uniques::<T, I>::approve_transfer(origin, collection.clone(), item, delegate_lookup.clone())?;
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone(), item, Some(delegate_lookup))
 	verify {
-		assert_last_event::<T, I>(Event::ApprovalCancelled { collection, item, owner: caller, delegate }.into());
+		assert_last_event::<T, I>(Event::ApprovalCancelled { collection: collection.clone(), item, owner: caller, delegate }.into());
 	}
 
 	set_accept_ownership {
 		let caller: T::AccountId = whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller, DepositBalanceOf::<T, I>::max_value());
 		let collection = T::Helper::collection(0);
-	}: _(SystemOrigin::Signed(caller.clone()), Some(collection))
+	}: _(SystemOrigin::Signed(caller.clone()), Some(collection.clone()))
 	verify {
 		assert_last_event::<T, I>(Event::OwnershipAcceptanceChanged {
 			who: caller,
@@ -401,10 +401,10 @@ benchmarks_instance_pallet! {
 
 	set_collection_max_supply {
 		let (collection, caller, _) = create_collection::<T, I>();
-	}: _(SystemOrigin::Signed(caller.clone()), collection, u32::MAX)
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone(), u32::MAX)
 	verify {
 		assert_last_event::<T, I>(Event::CollectionMaxSupplySet {
-			collection,
+			collection: collection.clone(),
 			max_supply: u32::MAX,
 		}.into());
 	}
@@ -415,10 +415,10 @@ benchmarks_instance_pallet! {
 		let delegate: T::AccountId = account("delegate", 0, SEED);
 		let delegate_lookup = T::Lookup::unlookup(delegate.clone());
 		let price = ItemPrice::<T, I>::from(100u32);
-	}: _(SystemOrigin::Signed(caller.clone()), collection, item, Some(price), Some(delegate_lookup))
+	}: _(SystemOrigin::Signed(caller.clone()), collection.clone(), item, Some(price), Some(delegate_lookup))
 	verify {
 		assert_last_event::<T, I>(Event::ItemPriceSet {
-			collection,
+			collection: collection.clone(),
 			item,
 			price,
 			whitelisted_buyer: Some(delegate),
@@ -432,12 +432,12 @@ benchmarks_instance_pallet! {
 		let buyer_lookup = T::Lookup::unlookup(buyer.clone());
 		let price = ItemPrice::<T, I>::from(0u32);
 		let origin = SystemOrigin::Signed(seller.clone()).into();
-		Uniques::<T, I>::set_price(origin, collection, item, Some(price.clone()), Some(buyer_lookup))?;
+		Uniques::<T, I>::set_price(origin, collection.clone(), item, Some(price.clone()), Some(buyer_lookup))?;
 		T::Currency::make_free_balance_be(&buyer, DepositBalanceOf::<T, I>::max_value());
-	}: _(SystemOrigin::Signed(buyer.clone()), collection, item, price.clone())
+	}: _(SystemOrigin::Signed(buyer.clone()), collection.clone(), item, price.clone())
 	verify {
 		assert_last_event::<T, I>(Event::ItemBought {
-			collection,
+			collection: collection.clone(),
 			item,
 			price,
 			seller,

substrate/frame/uniques/src/functions.rs

@@ -37,7 +37,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let collection_details =
 			Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
 		ensure!(!collection_details.is_frozen, Error::<T, I>::Frozen);
-		ensure!(!T::Locker::is_locked(collection, item), Error::<T, I>::Locked);
+		ensure!(!T::Locker::is_locked(collection.clone(), item), Error::<T, I>::Locked);
 
 		let mut details =
 			Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownCollection)?;
@@ -74,12 +74,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		free_holding: bool,
 		event: Event<T, I>,
 	) -> DispatchResult {
-		ensure!(!Collection::<T, I>::contains_key(collection), Error::<T, I>::InUse);
+		ensure!(!Collection::<T, I>::contains_key(collection.clone()), Error::<T, I>::InUse);
 
 		T::Currency::reserve(&owner, deposit)?;
 
 		Collection::<T, I>::insert(
-			collection,
+			collection.clone(),
 			CollectionDetails {
 				owner: owner.clone(),
 				issuer: admin.clone(),
@@ -104,7 +104,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		witness: DestroyWitness,
 		maybe_check_owner: Option<T::AccountId>,
 	) -> Result<DestroyWitness, DispatchError> {
-		Collection::<T, I>::try_mutate_exists(collection, |maybe_details| {
+		Collection::<T, I>::try_mutate_exists(collection.clone(), |maybe_details| {
 			let collection_details =
 				maybe_details.take().ok_or(Error::<T, I>::UnknownCollection)?;
 			if let Some(check_owner) = maybe_check_owner {
@@ -147,7 +147,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		owner: T::AccountId,
 		with_details: impl FnOnce(&CollectionDetailsFor<T, I>) -> DispatchResult,
 	) -> DispatchResult {
-		ensure!(!Item::<T, I>::contains_key(collection, item), Error::<T, I>::AlreadyExists);
+		ensure!(
+			!Item::<T, I>::contains_key(collection.clone(), item),
+			Error::<T, I>::AlreadyExists
+		);
 
 		Collection::<T, I>::try_mutate(
 			&collection,
@@ -189,7 +192,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		item: T::ItemId,
 		with_details: impl FnOnce(&CollectionDetailsFor<T, I>, &ItemDetailsFor<T, I>) -> DispatchResult,
 	) -> DispatchResult {
-		ensure!(!T::Locker::is_locked(collection, item), Error::<T, I>::Locked);
+		ensure!(!T::Locker::is_locked(collection.clone(), item), Error::<T, I>::Locked);
 		let owner = Collection::<T, I>::try_mutate(
 			&collection,
 			|maybe_collection_details| -> Result<T::AccountId, DispatchError> {
@@ -268,7 +271,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		let old_owner = details.owner.clone();
 
-		Self::do_transfer(collection, item, buyer.clone(), |_, _| Ok(()))?;
+		Self::do_transfer(collection.clone(), item, buyer.clone(), |_, _| Ok(()))?;
 
 		Self::deposit_event(Event::ItemBought {
 			collection,

substrate/frame/uniques/src/impl_nonfungibles.rs

@@ -94,12 +94,16 @@ impl<T: Config<I>, I: 'static> Create<<T as SystemConfig>::AccountId> for Pallet
 		admin: &T::AccountId,
 	) -> DispatchResult {
 		Self::do_create_collection(
-			*collection,
+			collection.clone(),
 			who.clone(),
 			admin.clone(),
 			T::CollectionDeposit::get(),
 			false,
-			Event::Created { collection: *collection, creator: who.clone(), owner: admin.clone() },
+			Event::Created {
+				collection: collection.clone(),
+				creator: who.clone(),
+				owner: admin.clone(),
+			},
 		)
 	}
 }
@@ -126,7 +130,7 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId> for Pallet
 		item: &Self::ItemId,
 		who: &T::AccountId,
 	) -> DispatchResult {
-		Self::do_mint(*collection, *item, who.clone(), |_| Ok(()))
+		Self::do_mint(collection.clone(), *item, who.clone(), |_| Ok(()))
 	}
 
 	fn burn(
@@ -134,7 +138,7 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId> for Pallet
 		item: &Self::ItemId,
 		maybe_check_owner: Option<&T::AccountId>,
 	) -> DispatchResult {
-		Self::do_burn(*collection, *item, |_, d| {
+		Self::do_burn(collection.clone(), *item, |_, d| {
 			if let Some(check_owner) = maybe_check_owner {
 				if &d.owner != check_owner {
 					return Err(Error::<T, I>::NoPermission.into())
@@ -151,7 +155,7 @@ impl<T: Config<I>, I: 'static> Transfer<T::AccountId> for Pallet<T, I> {
 		item: &Self::ItemId,
 		destination: &T::AccountId,
 	) -> DispatchResult {
-		Self::do_transfer(*collection, *item, destination.clone(), |_, _| Ok(()))
+		Self::do_transfer(collection.clone(), *item, destination.clone(), |_, _| Ok(()))
 	}
 }
 

substrate/frame/uniques/src/lib.rs

@@ -96,7 +96,7 @@ pub mod pallet {
 			+ IsType<<Self as frame_system::Config>::RuntimeEvent>;
 
 		/// Identifier for the collection of item.
-		type CollectionId: Member + Parameter + MaxEncodedLen + Copy;
+		type CollectionId: Member + Parameter + MaxEncodedLen;
 
 		/// The type used to identify a unique item within a collection.
 		type ItemId: Member + Parameter + MaxEncodedLen + Copy;
@@ -461,7 +461,7 @@ pub mod pallet {
 			let admin = T::Lookup::lookup(admin)?;
 
 			Self::do_create_collection(
-				collection,
+				collection.clone(),
 				owner.clone(),
 				admin.clone(),
 				T::CollectionDeposit::get(),
@@ -499,7 +499,7 @@ pub mod pallet {
 			let owner = T::Lookup::lookup(owner)?;
 
 			Self::do_create_collection(
-				collection,
+				collection.clone(),
 				owner.clone(),
 				owner.clone(),
 				Zero::zero(),
@@ -799,7 +799,7 @@ pub mod pallet {
 		) -> DispatchResult {
 			let origin = ensure_signed(origin)?;
 
-			Collection::<T, I>::try_mutate(collection, |maybe_details| {
+			Collection::<T, I>::try_mutate(collection.clone(), |maybe_details| {
 				let details = maybe_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
 				ensure!(origin == details.freezer, Error::<T, I>::NoPermission);
 
@@ -827,7 +827,7 @@ pub mod pallet {
 		) -> DispatchResult {
 			let origin = ensure_signed(origin)?;
 
-			Collection::<T, I>::try_mutate(collection, |maybe_details| {
+			Collection::<T, I>::try_mutate(collection.clone(), |maybe_details| {
 				let details = maybe_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
 				ensure!(origin == details.admin, Error::<T, I>::NoPermission);
 
@@ -862,7 +862,7 @@ pub mod pallet {
 			let acceptable_collection = OwnershipAcceptance::<T, I>::get(&owner);
 			ensure!(acceptable_collection.as_ref() == Some(&collection), Error::<T, I>::Unaccepted);
 
-			Collection::<T, I>::try_mutate(collection, |maybe_details| {
+			Collection::<T, I>::try_mutate(collection.clone(), |maybe_details| {
 				let details = maybe_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
 				ensure!(origin == details.owner, Error::<T, I>::NoPermission);
 				if details.owner == owner {
@@ -912,7 +912,7 @@ pub mod pallet {
 			let admin = T::Lookup::lookup(admin)?;
 			let freezer = T::Lookup::lookup(freezer)?;
 
-			Collection::<T, I>::try_mutate(collection, |maybe_details| {
+			Collection::<T, I>::try_mutate(collection.clone(), |maybe_details| {
 				let details = maybe_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
 				ensure!(origin == details.owner, Error::<T, I>::NoPermission);
 
@@ -1060,7 +1060,7 @@ pub mod pallet {
 		) -> DispatchResult {
 			T::ForceOrigin::ensure_origin(origin)?;
 
-			Collection::<T, I>::try_mutate(collection, |maybe_item| {
+			Collection::<T, I>::try_mutate(collection.clone(), |maybe_item| {
 				let mut item = maybe_item.take().ok_or(Error::<T, I>::UnknownCollection)?;
 				let old_owner = item.owner;
 				let new_owner = T::Lookup::lookup(owner)?;
@@ -1115,12 +1115,13 @@ pub mod pallet {
 				ensure!(check_owner == &collection_details.owner, Error::<T, I>::NoPermission);
 			}
 			let maybe_is_frozen = match maybe_item {
-				None => CollectionMetadataOf::<T, I>::get(collection).map(|v| v.is_frozen),
-				Some(item) => ItemMetadataOf::<T, I>::get(collection, item).map(|v| v.is_frozen),
+				None => CollectionMetadataOf::<T, I>::get(collection.clone()).map(|v| v.is_frozen),
+				Some(item) =>
+					ItemMetadataOf::<T, I>::get(collection.clone(), item).map(|v| v.is_frozen),
 			};
 			ensure!(!maybe_is_frozen.unwrap_or(false), Error::<T, I>::Frozen);
 
-			let attribute = Attribute::<T, I>::get((collection, maybe_item, &key));
+			let attribute = Attribute::<T, I>::get((collection.clone(), maybe_item, &key));
 			if attribute.is_none() {
 				collection_details.attributes.saturating_inc();
 			}
@@ -1140,7 +1141,7 @@ pub mod pallet {
 			}
 
 			Attribute::<T, I>::insert((&collection, maybe_item, &key), (&value, deposit));
-			Collection::<T, I>::insert(collection, &collection_details);
+			Collection::<T, I>::insert(collection.clone(), &collection_details);
 			Self::deposit_event(Event::AttributeSet { collection, maybe_item, key, value });
 			Ok(())
 		}
@@ -1177,16 +1178,19 @@ pub mod pallet {
 				ensure!(check_owner == &collection_details.owner, Error::<T, I>::NoPermission);
 			}
 			let maybe_is_frozen = match maybe_item {
-				None => CollectionMetadataOf::<T, I>::get(collection).map(|v| v.is_frozen),
-				Some(item) => ItemMetadataOf::<T, I>::get(collection, item).map(|v| v.is_frozen),
+				None => CollectionMetadataOf::<T, I>::get(collection.clone()).map(|v| v.is_frozen),
+				Some(item) =>
+					ItemMetadataOf::<T, I>::get(collection.clone(), item).map(|v| v.is_frozen),
 			};
 			ensure!(!maybe_is_frozen.unwrap_or(false), Error::<T, I>::Frozen);
 
-			if let Some((_, deposit)) = Attribute::<T, I>::take((collection, maybe_item, &key)) {
+			if let Some((_, deposit)) =
+				Attribute::<T, I>::take((collection.clone(), maybe_item, &key))
+			{
 				collection_details.attributes.saturating_dec();
 				collection_details.total_deposit.saturating_reduce(deposit);
 				T::Currency::unreserve(&collection_details.owner, deposit);
-				Collection::<T, I>::insert(collection, &collection_details);
+				Collection::<T, I>::insert(collection.clone(), &collection_details);
 				Self::deposit_event(Event::AttributeCleared { collection, maybe_item, key });
 			}
 			Ok(())
@@ -1229,7 +1233,7 @@ pub mod pallet {
 				ensure!(check_owner == &collection_details.owner, Error::<T, I>::NoPermission);
 			}
 
-			ItemMetadataOf::<T, I>::try_mutate_exists(collection, item, |metadata| {
+			ItemMetadataOf::<T, I>::try_mutate_exists(collection.clone(), item, |metadata| {
 				let was_frozen = metadata.as_ref().map_or(false, |m| m.is_frozen);
 				ensure!(maybe_check_owner.is_none() || !was_frozen, Error::<T, I>::Frozen);
 
@@ -1289,7 +1293,7 @@ pub mod pallet {
 				ensure!(check_owner == &collection_details.owner, Error::<T, I>::NoPermission);
 			}
 
-			ItemMetadataOf::<T, I>::try_mutate_exists(collection, item, |metadata| {
+			ItemMetadataOf::<T, I>::try_mutate_exists(collection.clone(), item, |metadata| {
 				let was_frozen = metadata.as_ref().map_or(false, |m| m.is_frozen);
 				ensure!(maybe_check_owner.is_none() || !was_frozen, Error::<T, I>::Frozen);
 
@@ -1340,7 +1344,7 @@ pub mod pallet {
 				ensure!(check_owner == &details.owner, Error::<T, I>::NoPermission);
 			}
 
-			CollectionMetadataOf::<T, I>::try_mutate_exists(collection, |metadata| {
+			CollectionMetadataOf::<T, I>::try_mutate_exists(collection.clone(), |metadata| {
 				let was_frozen = metadata.as_ref().map_or(false, |m| m.is_frozen);
 				ensure!(maybe_check_owner.is_none() || !was_frozen, Error::<T, I>::Frozen);
 
@@ -1396,7 +1400,7 @@ pub mod pallet {
 				ensure!(check_owner == &details.owner, Error::<T, I>::NoPermission);
 			}
 
-			CollectionMetadataOf::<T, I>::try_mutate_exists(collection, |metadata| {
+			CollectionMetadataOf::<T, I>::try_mutate_exists(collection.clone(), |metadata| {
 				let was_frozen = metadata.as_ref().map_or(false, |m| m.is_frozen);
 				ensure!(maybe_check_owner.is_none() || !was_frozen, Error::<T, I>::Frozen);