[NFTs] Track item's metadata depositor

frame/nfts/src/features/attributes.rs

@@ -159,6 +159,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
 
 			if let Some(check_owner) = &maybe_check_owner {
+				// deposit.account.is_none(); maybe_check_owner.is_some()
 				if deposit.account != maybe_check_owner {
 					ensure!(
 						Self::is_valid_namespace(

frame/nfts/src/features/create_delete_collection.rs

@@ -82,12 +82,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				Account::<T, I>::remove((&details.owner, &collection, &item));
 				T::Currency::unreserve(&details.deposit.account, details.deposit.amount);
 			}
-			#[allow(deprecated)]
-			ItemMetadataOf::<T, I>::remove_prefix(&collection, None);
-			#[allow(deprecated)]
-			ItemPriceOf::<T, I>::remove_prefix(&collection, None);
-			#[allow(deprecated)]
-			PendingSwapOf::<T, I>::remove_prefix(&collection, None);
+			for (_, metadata) in ItemMetadataOf::<T, I>::drain_prefix(&collection) {
+				if let Some(depositor) = metadata.deposit.account {
+					T::Currency::unreserve(&depositor, metadata.deposit.amount);
+				}
+			}
+			let _ = ItemPriceOf::<T, I>::clear_prefix(&collection, witness.items, None);
+			let _ = PendingSwapOf::<T, I>::clear_prefix(&collection, witness.items, None);
 			CollectionMetadataOf::<T, I>::remove(&collection);
 			Self::clear_roles(&collection)?;
 

frame/nfts/src/features/create_delete_item.rs

@@ -22,10 +22,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	pub fn do_mint(
 		collection: T::CollectionId,
 		item: T::ItemId,
-		depositor: T::AccountId,
+		maybe_depositor: Option<T::AccountId>,
 		mint_to: T::AccountId,
 		item_config: ItemConfig,
-		deposit_collection_owner: bool,
 		with_details_and_config: impl FnOnce(
 			&CollectionDetailsFor<T, I>,
 			&CollectionConfigFor<T, I>,
@@ -55,9 +54,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 					true => T::ItemDeposit::get(),
 					false => Zero::zero(),
 				};
-				let deposit_account = match deposit_collection_owner {
-					true => collection_details.owner.clone(),
-					false => depositor,
+				let deposit_account = match maybe_depositor {
+					None => collection_details.owner.clone(),
+					Some(depositor) => depositor,
 				};
 
 				let item_owner = mint_to.clone();
@@ -92,6 +91,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		with_details: impl FnOnce(&ItemDetailsFor<T, I>) -> DispatchResult,
 	) -> DispatchResult {
 		ensure!(!T::Locker::is_locked(collection, item), Error::<T, I>::ItemLocked);
+		let item_config = Self::get_item_config(&collection, &item)?;
 		let owner = Collection::<T, I>::try_mutate(
 			&collection,
 			|maybe_collection_details| -> Result<T::AccountId, DispatchError> {
@@ -104,6 +104,24 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				// Return the deposit.
 				T::Currency::unreserve(&details.deposit.account, details.deposit.amount);
 				collection_details.items.saturating_dec();
+
+				// Clear the metadata if it's not locked.
+				if item_config.is_setting_enabled(ItemSetting::UnlockedMetadata) {
+					if let Some(metadata) = ItemMetadataOf::<T, I>::take(&collection, &item) {
+						let depositor_account =
+							metadata.deposit.account.unwrap_or(collection_details.owner.clone());
+
+						T::Currency::unreserve(&depositor_account, metadata.deposit.amount.clone());
+						collection_details.item_metadatas.saturating_dec();
+
+						if depositor_account == collection_details.owner {
+							collection_details
+								.owner_deposit
+								.saturating_reduce(metadata.deposit.amount);
+						}
+					}
+				}
+
 				Ok(details.owner)
 			},
 		)?;
@@ -116,8 +134,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		// NOTE: if item's settings are not empty (e.g. item's metadata is locked)
 		// then we keep the record and don't remove it
-		let config = Self::get_item_config(&collection, &item)?;
-		if !config.has_disabled_settings() {
+		if !item_config.has_disabled_settings() {
 			ItemConfigOf::<T, I>::remove(&collection, &item);
 		}
 

frame/nfts/src/features/metadata.rs

@@ -19,19 +19,21 @@ use crate::*;
 use frame_support::pallet_prelude::*;
 
 impl<T: Config<I>, I: 'static> Pallet<T, I> {
+	/// Note: if `maybe_depositor` is None, that means the depositor will be a collection's owner
 	pub(crate) fn do_set_item_metadata(
 		maybe_check_owner: Option<T::AccountId>,
 		collection: T::CollectionId,
 		item: T::ItemId,
 		data: BoundedVec<u8, T::StringLimit>,
+		maybe_depositor: Option<T::AccountId>,
 	) -> DispatchResult {
+		let is_root = maybe_check_owner.is_none();
 		let mut collection_details =
 			Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
 
 		let item_config = Self::get_item_config(&collection, &item)?;
 		ensure!(
-			maybe_check_owner.is_none() ||
-				item_config.is_setting_enabled(ItemSetting::UnlockedMetadata),
+			is_root || item_config.is_setting_enabled(ItemSetting::UnlockedMetadata),
 			Error::<T, I>::LockedItemMetadata
 		);
 
@@ -45,24 +47,38 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			if metadata.is_none() {
 				collection_details.item_metadatas.saturating_inc();
 			}
-			let old_deposit = metadata.take().map_or(Zero::zero(), |m| m.deposit);
-			collection_details.owner_deposit.saturating_reduce(old_deposit);
+
+			let old_deposit = metadata
+				.take()
+				.map_or(ItemMetadataDeposit { account: None, amount: Zero::zero() }, |m| m.deposit);
+
 			let mut deposit = Zero::zero();
-			if collection_config.is_setting_enabled(CollectionSetting::DepositRequired) &&
-				maybe_check_owner.is_some()
+			if collection_config.is_setting_enabled(CollectionSetting::DepositRequired) && !is_root
 			{
 				deposit = T::DepositPerByte::get()
 					.saturating_mul(((data.len()) as u32).into())
 					.saturating_add(T::MetadataDepositBase::get());
 			}
-			if deposit > old_deposit {
-				T::Currency::reserve(&collection_details.owner, deposit - old_deposit)?;
-			} else if deposit < old_deposit {
-				T::Currency::unreserve(&collection_details.owner, old_deposit - deposit);
+
+			// the previous deposit was taken from the item's owner
+			if old_deposit.account.is_some() && maybe_depositor.is_none() {
+				T::Currency::unreserve(&old_deposit.account.unwrap(), old_deposit.amount);
+				T::Currency::reserve(&collection_details.owner, deposit)?;
+			} else if deposit > old_deposit.amount {
+				T::Currency::reserve(&collection_details.owner, deposit - old_deposit.amount)?;
+			} else if deposit < old_deposit.amount {
+				T::Currency::unreserve(&collection_details.owner, old_deposit.amount - deposit);
+			}
+
+			if maybe_depositor.is_none() {
+				collection_details.owner_deposit.saturating_accrue(deposit);
+				collection_details.owner_deposit.saturating_reduce(old_deposit.amount);
 			}
-			collection_details.owner_deposit.saturating_accrue(deposit);
 
-			*metadata = Some(ItemMetadata { deposit, data: data.clone() });
+			*metadata = Some(ItemMetadata {
+				deposit: ItemMetadataDeposit { account: maybe_depositor, amount: deposit },
+				data: data.clone(),
+			});
 
 			Collection::<T, I>::insert(&collection, &collection_details);
 			Self::deposit_event(Event::ItemMetadataSet { collection, item, data });
@@ -75,8 +91,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		collection: T::CollectionId,
 		item: T::ItemId,
 	) -> DispatchResult {
+		let is_root = maybe_check_owner.is_none();
+		let metadata = ItemMetadataOf::<T, I>::take(collection, item)
+			.ok_or(Error::<T, I>::MetadataNotFound)?;
 		let mut collection_details =
 			Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
+		let depositor_account =
+			metadata.deposit.account.unwrap_or(collection_details.owner.clone());
+
 		if let Some(check_owner) = &maybe_check_owner {
 			ensure!(check_owner == &collection_details.owner, Error::<T, I>::NoPermission);
 		}
@@ -85,20 +107,19 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let is_locked = Self::get_item_config(&collection, &item)
 			.map_or(false, |c| c.has_disabled_setting(ItemSetting::UnlockedMetadata));
 
-		ensure!(maybe_check_owner.is_none() || !is_locked, Error::<T, I>::LockedItemMetadata);
+		ensure!(is_root || !is_locked, Error::<T, I>::LockedItemMetadata);
 
-		ItemMetadataOf::<T, I>::try_mutate_exists(collection, item, |metadata| {
-			if metadata.is_some() {
-				collection_details.item_metadatas.saturating_dec();
-			}
-			let deposit = metadata.take().ok_or(Error::<T, I>::UnknownItem)?.deposit;
-			T::Currency::unreserve(&collection_details.owner, deposit);
-			collection_details.owner_deposit.saturating_reduce(deposit);
+		collection_details.item_metadatas.saturating_dec();
+		T::Currency::unreserve(&depositor_account, metadata.deposit.amount.clone());
 
-			Collection::<T, I>::insert(&collection, &collection_details);
-			Self::deposit_event(Event::ItemMetadataCleared { collection, item });
-			Ok(())
-		})
+		if depositor_account == collection_details.owner {
+			collection_details.owner_deposit.saturating_reduce(metadata.deposit.amount);
+		}
+
+		Collection::<T, I>::insert(&collection, &collection_details);
+		Self::deposit_event(Event::ItemMetadataCleared { collection, item });
+
+		Ok(())
 	}
 
 	pub(crate) fn do_set_collection_metadata(

frame/nfts/src/impl_nonfungibles.rs

@@ -157,10 +157,12 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId, ItemConfig
 		Self::do_mint(
 			*collection,
 			*item,
-			who.clone(),
+			match deposit_collection_owner {
+				true => None,
+				false => Some(who.clone()),
+			},
 			who.clone(),
 			*item_config,
-			deposit_collection_owner,
 			|_, _| Ok(()),
 		)
 	}

frame/nfts/src/lib.rs

@@ -263,7 +263,7 @@ pub mod pallet {
 		T::CollectionId,
 		Blake2_128Concat,
 		T::ItemId,
-		ItemMetadata<DepositBalanceOf<T, I>, T::StringLimit>,
+		ItemMetadata<ItemMetadataDepositOf<T, I>, T::StringLimit>,
 		OptionQuery,
 	>;
 
@@ -559,6 +559,8 @@ pub mod pallet {
 		UnknownItem,
 		/// Swap doesn't exist.
 		UnknownSwap,
+		/// The given item has no metadata set.
+		MetadataNotFound,
 		/// Item is not for sale.
 		NotForSale,
 		/// The provided bid is too low.
@@ -746,10 +748,9 @@ pub mod pallet {
 			Self::do_mint(
 				collection,
 				item,
-				caller.clone(),
+				Some(caller.clone()),
 				mint_to.clone(),
 				item_config,
-				false,
 				|collection_details, collection_config| {
 					// Issuer can mint regardless of mint settings
 					if Self::has_role(&collection, &caller, CollectionRole::Issuer) {
@@ -849,9 +850,7 @@ pub mod pallet {
 					Error::<T, I>::NoPermission
 				);
 			}
-			Self::do_mint(collection, item, mint_to.clone(), mint_to, item_config, true, |_, _| {
-				Ok(())
-			})
+			Self::do_mint(collection, item, None, mint_to, item_config, |_, _| Ok(()))
 		}
 
 		/// Destroy a single item.
@@ -1362,7 +1361,7 @@ pub mod pallet {
 		/// Clear an attribute for a collection or item.
 		///
 		/// Origin must be either `ForceOrigin` or Signed and the sender should be the Owner of the
-		/// `collection`.
+		/// attribute.
 		///
 		/// Any deposit is freed for the collection's owner.
 		///
@@ -1464,7 +1463,7 @@ pub mod pallet {
 			let maybe_check_owner = T::ForceOrigin::try_origin(origin)
 				.map(|_| None)
 				.or_else(|origin| ensure_signed(origin).map(Some).map_err(DispatchError::from))?;
-			Self::do_set_item_metadata(maybe_check_owner, collection, item, data)
+			Self::do_set_item_metadata(maybe_check_owner, collection, item, data, None)
 		}
 
 		/// Clear the metadata for an item.

frame/nfts/src/tests.rs

@@ -608,7 +608,7 @@ fn set_item_metadata_should_work() {
 		);
 		assert_noop!(
 			Nfts::clear_metadata(RuntimeOrigin::signed(1), 1, 42),
-			Error::<Test>::UnknownCollection,
+			Error::<Test>::MetadataNotFound,
 		);
 		assert_ok!(Nfts::clear_metadata(RuntimeOrigin::root(), 0, 42));
 		assert!(!ItemMetadataOf::<Test>::contains_key(0, 42));
@@ -1267,7 +1267,7 @@ fn burn_works() {
 
 		assert_noop!(
 			Nfts::burn(RuntimeOrigin::signed(5), 0, 42, Some(5)),
-			Error::<Test>::UnknownCollection
+			Error::<Test>::UnknownItem
 		);
 
 		assert_ok!(Nfts::force_mint(RuntimeOrigin::signed(2), 0, 42, 5, default_item_config()));

frame/nfts/src/types.rs

@@ -43,6 +43,8 @@ pub(super) type ItemDepositOf<T, I> =
 	ItemDeposit<DepositBalanceOf<T, I>, <T as SystemConfig>::AccountId>;
 pub(super) type AttributeDepositOf<T, I> =
 	AttributeDeposit<DepositBalanceOf<T, I>, <T as SystemConfig>::AccountId>;
+pub(super) type ItemMetadataDepositOf<T, I> =
+	ItemMetadataDeposit<DepositBalanceOf<T, I>, <T as SystemConfig>::AccountId>;
 pub(super) type ItemDetailsFor<T, I> =
 	ItemDetails<<T as SystemConfig>::AccountId, ItemDepositOf<T, I>, ApprovalsOf<T, I>>;
 pub(super) type BalanceOf<T, I = ()> =
@@ -137,12 +139,12 @@ pub struct ItemDeposit<DepositBalance, AccountId> {
 /// Information about the collection's metadata.
 #[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, Default, TypeInfo, MaxEncodedLen)]
 #[scale_info(skip_type_params(StringLimit))]
-#[codec(mel_bound(DepositBalance: MaxEncodedLen))]
-pub struct CollectionMetadata<DepositBalance, StringLimit: Get<u32>> {
+#[codec(mel_bound(Deposit: MaxEncodedLen))]
+pub struct CollectionMetadata<Deposit, StringLimit: Get<u32>> {
 	/// The balance deposited for this metadata.
 	///
 	/// This pays for the data stored in this struct.
-	pub(super) deposit: DepositBalance,
+	pub(super) deposit: Deposit,
 	/// General information concerning this collection. Limited in length by `StringLimit`. This
 	/// will generally be either a JSON dump or the hash of some JSON which can be found on a
 	/// hash-addressable global publication system such as IPFS.
@@ -152,12 +154,11 @@ pub struct CollectionMetadata<DepositBalance, StringLimit: Get<u32>> {
 /// Information about the item's metadata.
 #[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, Default, TypeInfo, MaxEncodedLen)]
 #[scale_info(skip_type_params(StringLimit))]
-#[codec(mel_bound(DepositBalance: MaxEncodedLen))]
-pub struct ItemMetadata<DepositBalance, StringLimit: Get<u32>> {
+pub struct ItemMetadata<Deposit, StringLimit: Get<u32>> {
 	/// The balance deposited for this metadata.
 	///
 	/// This pays for the data stored in this struct.
-	pub(super) deposit: DepositBalance,
+	pub(super) deposit: Deposit,
 	/// General information concerning this item. Limited in length by `StringLimit`. This will
 	/// generally be either a JSON dump or the hash of some JSON which can be found on a
 	/// hash-addressable global publication system such as IPFS.
@@ -199,6 +200,15 @@ pub struct AttributeDeposit<DepositBalance, AccountId> {
 	pub(super) amount: DepositBalance,
 }
 
+/// Information about the reserved item's metadata deposit.
+#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
+pub struct ItemMetadataDeposit<DepositBalance, AccountId> {
+	/// A depositor account, None means the deposit is collection's owner.
+	pub(super) account: Option<AccountId>,
+	/// An amount that gets reserved.
+	pub(super) amount: DepositBalance,
+}
+
 /// Specifies whether the tokens will be sent or received.
 #[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
 pub enum PriceDirection {