diff --git a/substrate/frame/nfts/src/features/attributes.rs b/substrate/frame/nfts/src/features/attributes.rs
index da663d39a4ef5dbe49cb7dfe07ef8de549f0cccf..b25f2a60cd62d8b2e48140169f33154906413179 100644
--- a/substrate/frame/nfts/src/features/attributes.rs
+++ b/substrate/frame/nfts/src/features/attributes.rs
@@ -152,65 +152,68 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
namespace: AttributeNamespace<T::AccountId>,
key: BoundedVec<u8, T::KeyLimit>,
) -> DispatchResult {
- if let Some((_, deposit)) =
- Attribute::<T, I>::take((collection, maybe_item, &namespace, &key))
- {
- let mut collection_details =
- Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
-
- if let Some(check_owner) = &maybe_check_owner {
- if deposit.account != maybe_check_owner {
- ensure!(
- Self::is_valid_namespace(
- &check_owner,
- &namespace,
- &collection,
- &collection_details.owner,
- &maybe_item,
- )?,
- Error::<T, I>::NoPermission
- );
- }
+ let (_, deposit) = Attribute::<T, I>::take((collection, maybe_item, &namespace, &key))
+ .ok_or(Error::<T, I>::AttributeNotFound)?;
+ let mut collection_details =
+ Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
- // can't clear `CollectionOwner` type attributes if the collection/item is locked
- match namespace {
- AttributeNamespace::CollectionOwner => match maybe_item {
- None => {
- let collection_config = Self::get_collection_config(&collection)?;
- ensure!(
- collection_config
- .is_setting_enabled(CollectionSetting::UnlockedAttributes),
- Error::<T, I>::LockedCollectionAttributes
- )
- },
- Some(item) => {
- // NOTE: if the item was previously burned, the ItemConfigOf record
- // might not exist. In that case, we allow to clear the attribute.
- let maybe_is_locked = Self::get_item_config(&collection, &item)
- .map_or(false, |c| {
- c.has_disabled_setting(ItemSetting::UnlockedAttributes)
- });
- ensure!(!maybe_is_locked, Error::<T, I>::LockedItemAttributes);
- },
- },
- _ => (),
- };
+ if let Some(check_owner) = &maybe_check_owner {
+ // validate the provided namespace when it's not a root call and the caller is not
+ // the same as the `deposit.account` (e.g. the deposit was paid by different account)
+ if deposit.account != maybe_check_owner {
+ ensure!(
+ Self::is_valid_namespace(
+ &check_owner,
+ &namespace,
+ &collection,
+ &collection_details.owner,
+ &maybe_item,
+ )?,
+ Error::<T, I>::NoPermission
+ );
}
- collection_details.attributes.saturating_dec();
+ // can't clear `CollectionOwner` type attributes if the collection/item is locked
match namespace {
- AttributeNamespace::CollectionOwner => {
- collection_details.owner_deposit.saturating_reduce(deposit.amount);
- T::Currency::unreserve(&collection_details.owner, deposit.amount);
+ AttributeNamespace::CollectionOwner => match maybe_item {
+ None => {
+ let collection_config = Self::get_collection_config(&collection)?;
+ ensure!(
+ collection_config
+ .is_setting_enabled(CollectionSetting::UnlockedAttributes),
+ Error::<T, I>::LockedCollectionAttributes
+ )
+ },
+ Some(item) => {
+ // NOTE: if the item was previously burned, the ItemConfigOf record
+ // might not exist. In that case, we allow to clear the attribute.
+ let maybe_is_locked = Self::get_item_config(&collection, &item)
+ .map_or(false, |c| {
+ c.has_disabled_setting(ItemSetting::UnlockedAttributes)
+ });
+ ensure!(!maybe_is_locked, Error::<T, I>::LockedItemAttributes);
+ },
},
_ => (),
};
- if let Some(deposit_account) = deposit.account {
- T::Currency::unreserve(&deposit_account, deposit.amount);
- }
- Collection::<T, I>::insert(collection, &collection_details);
- Self::deposit_event(Event::AttributeCleared { collection, maybe_item, key, namespace });
}
+
+ collection_details.attributes.saturating_dec();
+ match namespace {
+ AttributeNamespace::CollectionOwner => {
+ collection_details.owner_deposit.saturating_reduce(deposit.amount);
+ T::Currency::unreserve(&collection_details.owner, deposit.amount);
+ },
+ _ => (),
+ };
+
+ if let Some(deposit_account) = deposit.account {
+ T::Currency::unreserve(&deposit_account, deposit.amount);
+ }
+
+ Collection::<T, I>::insert(collection, &collection_details);
+ Self::deposit_event(Event::AttributeCleared { collection, maybe_item, key, namespace });
+
Ok(())
}
diff --git a/substrate/frame/nfts/src/features/create_delete_collection.rs b/substrate/frame/nfts/src/features/create_delete_collection.rs
index 86625bf49efb29a680510f26045448a3a09c4438..a2caa04bc8dc3d162b565a33216b500ea4f65352 100644
--- a/substrate/frame/nfts/src/features/create_delete_collection.rs
+++ b/substrate/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)?;
diff --git a/substrate/frame/nfts/src/features/create_delete_item.rs b/substrate/frame/nfts/src/features/create_delete_item.rs
index 8d6c8a280956c3f6f07799fc9b57d8126e1ee75c..f724fe5c63b4373f33f6bfd6612a04de9531d225 100644
--- a/substrate/frame/nfts/src/features/create_delete_item.rs
+++ b/substrate/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);
+ 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);
}
diff --git a/substrate/frame/nfts/src/features/metadata.rs b/substrate/frame/nfts/src/features/metadata.rs
index 942f377141a3344d54f06b366a533e965be83cab..272b2247426d6586d289f017d8bd7a59edcd606c 100644
--- a/substrate/frame/nfts/src/features/metadata.rs
+++ b/substrate/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);
- 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(
diff --git a/substrate/frame/nfts/src/impl_nonfungibles.rs b/substrate/frame/nfts/src/impl_nonfungibles.rs
index ab4d7a16ec215a621979cfbd60c6550f62281ca6..86d82b4021517f88d66da6dab47fc9df119419ce 100644
--- a/substrate/frame/nfts/src/impl_nonfungibles.rs
+++ b/substrate/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(()),
)
}
diff --git a/substrate/frame/nfts/src/lib.rs b/substrate/frame/nfts/src/lib.rs
index 9a6242143aedb2161a0966d14a831a3d1350c90b..8f24c8dcd6e982992646dea2ce8c27309318435b 100644
--- a/substrate/frame/nfts/src/lib.rs
+++ b/substrate/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,10 @@ pub mod pallet {
UnknownItem,
/// Swap doesn't exist.
UnknownSwap,
+ /// The given item has no metadata set.
+ MetadataNotFound,
+ /// The provided attribute can't be found.
+ AttributeNotFound,
/// Item is not for sale.
NotForSale,
/// The provided bid is too low.
@@ -746,10 +750,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 +852,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 +1363,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 +1465,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.
diff --git a/substrate/frame/nfts/src/tests.rs b/substrate/frame/nfts/src/tests.rs
index f8187b3777ebd29c2bf67ab378353e4d48a1e84d..ebbba33b04fa2880343caedce5be8a517b56b195 100644
--- a/substrate/frame/nfts/src/tests.rs
+++ b/substrate/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()));
diff --git a/substrate/frame/nfts/src/types.rs b/substrate/frame/nfts/src/types.rs
index 9b89fb964b316b906166dee3d71932bb2c4d2863..d8938aab4377aa35acb496803efa0c7ed4c293f8 100644
--- a/substrate/frame/nfts/src/types.rs
+++ b/substrate/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 {