Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Commit 293c1f2

Browse files
jsidorenkojsidorenko
authored
[NFTs] Track item's metadata depositor (#13124)
* Refactor do_mint() * Track the depositor of item's metadata * Revert back the access control * On collection destroy return the metadata deposit * Clear the metadata on item burn returning the deposit * Address comments * Fix clippy * Don't return Ok on non-existing attribute removal
1 parent 2c029fd commit 293c1f2

File tree

8 files changed

+161
-106
lines changed

8 files changed

+161
-106
lines changed

‎frame/nfts/src/features/attributes.rs‎

Lines changed: 54 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -152,65 +152,68 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
152152
namespace: AttributeNamespace<T::AccountId>,
153153
key: BoundedVec<u8, T::KeyLimit>,
154154
) -> DispatchResult {
155-
if let Some((_, deposit)) =
156-
Attribute::<T, I>::take((collection, maybe_item, &namespace, &key))
157-
{
158-
let mut collection_details =
159-
Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
160-
161-
if let Some(check_owner) = &maybe_check_owner {
162-
if deposit.account != maybe_check_owner {
163-
ensure!(
164-
Self::is_valid_namespace(
165-
&check_owner,
166-
&namespace,
167-
&collection,
168-
&collection_details.owner,
169-
&maybe_item,
170-
)?,
171-
Error::<T, I>::NoPermission
172-
);
173-
}
155+
let (_, deposit) = Attribute::<T, I>::take((collection, maybe_item, &namespace, &key))
156+
.ok_or(Error::<T, I>::AttributeNotFound)?;
157+
let mut collection_details =
158+
Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
174159

175-
// can't clear `CollectionOwner` type attributes if the collection/item is locked
176-
match namespace {
177-
AttributeNamespace::CollectionOwner => match maybe_item {
178-
None => {
179-
let collection_config = Self::get_collection_config(&collection)?;
180-
ensure!(
181-
collection_config
182-
.is_setting_enabled(CollectionSetting::UnlockedAttributes),
183-
Error::<T, I>::LockedCollectionAttributes
184-
)
185-
},
186-
Some(item) => {
187-
// NOTE: if the item was previously burned, the ItemConfigOf record
188-
// might not exist. In that case, we allow to clear the attribute.
189-
let maybe_is_locked = Self::get_item_config(&collection, &item)
190-
.map_or(false, |c| {
191-
c.has_disabled_setting(ItemSetting::UnlockedAttributes)
192-
});
193-
ensure!(!maybe_is_locked, Error::<T, I>::LockedItemAttributes);
194-
},
195-
},
196-
_ => (),
197-
};
160+
if let Some(check_owner) = &maybe_check_owner {
161+
// validate the provided namespace when it's not a root call and the caller is not
162+
// the same as the `deposit.account` (e.g. the deposit was paid by different account)
163+
if deposit.account != maybe_check_owner {
164+
ensure!(
165+
Self::is_valid_namespace(
166+
&check_owner,
167+
&namespace,
168+
&collection,
169+
&collection_details.owner,
170+
&maybe_item,
171+
)?,
172+
Error::<T, I>::NoPermission
173+
);
198174
}
199175

200-
collection_details.attributes.saturating_dec();
176+
// can't clear `CollectionOwner` type attributes if the collection/item is locked
201177
match namespace {
202-
AttributeNamespace::CollectionOwner => {
203-
collection_details.owner_deposit.saturating_reduce(deposit.amount);
204-
T::Currency::unreserve(&collection_details.owner, deposit.amount);
178+
AttributeNamespace::CollectionOwner => match maybe_item {
179+
None => {
180+
let collection_config = Self::get_collection_config(&collection)?;
181+
ensure!(
182+
collection_config
183+
.is_setting_enabled(CollectionSetting::UnlockedAttributes),
184+
Error::<T, I>::LockedCollectionAttributes
185+
)
186+
},
187+
Some(item) => {
188+
// NOTE: if the item was previously burned, the ItemConfigOf record
189+
// might not exist. In that case, we allow to clear the attribute.
190+
let maybe_is_locked = Self::get_item_config(&collection, &item)
191+
.map_or(false, |c| {
192+
c.has_disabled_setting(ItemSetting::UnlockedAttributes)
193+
});
194+
ensure!(!maybe_is_locked, Error::<T, I>::LockedItemAttributes);
195+
},
205196
},
206197
_ => (),
207198
};
208-
if let Some(deposit_account) = deposit.account {
209-
T::Currency::unreserve(&deposit_account, deposit.amount);
210-
}
211-
Collection::<T, I>::insert(collection, &collection_details);
212-
Self::deposit_event(Event::AttributeCleared { collection, maybe_item, key, namespace });
213199
}
200+
201+
collection_details.attributes.saturating_dec();
202+
match namespace {
203+
AttributeNamespace::CollectionOwner => {
204+
collection_details.owner_deposit.saturating_reduce(deposit.amount);
205+
T::Currency::unreserve(&collection_details.owner, deposit.amount);
206+
},
207+
_ => (),
208+
};
209+
210+
if let Some(deposit_account) = deposit.account {
211+
T::Currency::unreserve(&deposit_account, deposit.amount);
212+
}
213+
214+
Collection::<T, I>::insert(collection, &collection_details);
215+
Self::deposit_event(Event::AttributeCleared { collection, maybe_item, key, namespace });
216+
214217
Ok(())
215218
}
216219

‎frame/nfts/src/features/create_delete_collection.rs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
8282
Account::<T, I>::remove((&details.owner, &collection, &item));
8383
T::Currency::unreserve(&details.deposit.account, details.deposit.amount);
8484
}
85-
#[allow(deprecated)]
86-
ItemMetadataOf::<T, I>::remove_prefix(&collection, None);
87-
#[allow(deprecated)]
88-
ItemPriceOf::<T, I>::remove_prefix(&collection, None);
89-
#[allow(deprecated)]
90-
PendingSwapOf::<T, I>::remove_prefix(&collection, None);
85+
for (_, metadata) in ItemMetadataOf::<T, I>::drain_prefix(&collection) {
86+
if let Some(depositor) = metadata.deposit.account {
87+
T::Currency::unreserve(&depositor, metadata.deposit.amount);
88+
}
89+
}
90+
let _ = ItemPriceOf::<T, I>::clear_prefix(&collection, witness.items, None);
91+
let _ = PendingSwapOf::<T, I>::clear_prefix(&collection, witness.items, None);
9192
CollectionMetadataOf::<T, I>::remove(&collection);
9293
Self::clear_roles(&collection)?;
9394

‎frame/nfts/src/features/create_delete_item.rs‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
2222
pub fn do_mint(
2323
collection: T::CollectionId,
2424
item: T::ItemId,
25-
depositor: T::AccountId,
25+
maybe_depositor: Option<T::AccountId>,
2626
mint_to: T::AccountId,
2727
item_config: ItemConfig,
28-
deposit_collection_owner: bool,
2928
with_details_and_config: impl FnOnce(
3029
&CollectionDetailsFor<T, I>,
3130
&CollectionConfigFor<T, I>,
@@ -55,9 +54,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
5554
true => T::ItemDeposit::get(),
5655
false => Zero::zero(),
5756
};
58-
let deposit_account = match deposit_collection_owner {
59-
true => collection_details.owner.clone(),
60-
false => depositor,
57+
let deposit_account = match maybe_depositor {
58+
None => collection_details.owner.clone(),
59+
Some(depositor) => depositor,
6160
};
6261

6362
let item_owner = mint_to.clone();
@@ -92,6 +91,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
9291
with_details: impl FnOnce(&ItemDetailsFor<T, I>) -> DispatchResult,
9392
) -> DispatchResult {
9493
ensure!(!T::Locker::is_locked(collection, item), Error::<T, I>::ItemLocked);
94+
let item_config = Self::get_item_config(&collection, &item)?;
9595
let owner = Collection::<T, I>::try_mutate(
9696
&collection,
9797
|maybe_collection_details| -> Result<T::AccountId, DispatchError> {
@@ -104,6 +104,24 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
104104
// Return the deposit.
105105
T::Currency::unreserve(&details.deposit.account, details.deposit.amount);
106106
collection_details.items.saturating_dec();
107+
108+
// Clear the metadata if it's not locked.
109+
if item_config.is_setting_enabled(ItemSetting::UnlockedMetadata) {
110+
if let Some(metadata) = ItemMetadataOf::<T, I>::take(&collection, &item) {
111+
let depositor_account =
112+
metadata.deposit.account.unwrap_or(collection_details.owner.clone());
113+
114+
T::Currency::unreserve(&depositor_account, metadata.deposit.amount);
115+
collection_details.item_metadatas.saturating_dec();
116+
117+
if depositor_account == collection_details.owner {
118+
collection_details
119+
.owner_deposit
120+
.saturating_reduce(metadata.deposit.amount);
121+
}
122+
}
123+
}
124+
107125
Ok(details.owner)
108126
},
109127
)?;
@@ -116,8 +134,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
116134

117135
// NOTE: if item's settings are not empty (e.g. item's metadata is locked)
118136
// then we keep the record and don't remove it
119-
let config = Self::get_item_config(&collection, &item)?;
120-
if !config.has_disabled_settings() {
137+
if !item_config.has_disabled_settings() {
121138
ItemConfigOf::<T, I>::remove(&collection, &item);
122139
}
123140

‎frame/nfts/src/features/metadata.rs‎

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,21 @@ use crate::*;
1919
use frame_support::pallet_prelude::*;
2020

2121
impl<T: Config<I>, I: 'static> Pallet<T, I> {
22+
/// Note: if `maybe_depositor` is None, that means the depositor will be a collection's owner
2223
pub(crate) fn do_set_item_metadata(
2324
maybe_check_owner: Option<T::AccountId>,
2425
collection: T::CollectionId,
2526
item: T::ItemId,
2627
data: BoundedVec<u8, T::StringLimit>,
28+
maybe_depositor: Option<T::AccountId>,
2729
) -> DispatchResult {
30+
let is_root = maybe_check_owner.is_none();
2831
let mut collection_details =
2932
Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
3033

3134
let item_config = Self::get_item_config(&collection, &item)?;
3235
ensure!(
33-
maybe_check_owner.is_none() ||
34-
item_config.is_setting_enabled(ItemSetting::UnlockedMetadata),
36+
is_root || item_config.is_setting_enabled(ItemSetting::UnlockedMetadata),
3537
Error::<T, I>::LockedItemMetadata
3638
);
3739

@@ -45,24 +47,38 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
4547
if metadata.is_none() {
4648
collection_details.item_metadatas.saturating_inc();
4749
}
48-
let old_deposit = metadata.take().map_or(Zero::zero(), |m| m.deposit);
49-
collection_details.owner_deposit.saturating_reduce(old_deposit);
50+
51+
let old_deposit = metadata
52+
.take()
53+
.map_or(ItemMetadataDeposit { account: None, amount: Zero::zero() }, |m| m.deposit);
54+
5055
let mut deposit = Zero::zero();
51-
if collection_config.is_setting_enabled(CollectionSetting::DepositRequired) &&
52-
maybe_check_owner.is_some()
56+
if collection_config.is_setting_enabled(CollectionSetting::DepositRequired) && !is_root
5357
{
5458
deposit = T::DepositPerByte::get()
5559
.saturating_mul(((data.len()) as u32).into())
5660
.saturating_add(T::MetadataDepositBase::get());
5761
}
58-
if deposit > old_deposit {
59-
T::Currency::reserve(&collection_details.owner, deposit - old_deposit)?;
60-
} else if deposit < old_deposit {
61-
T::Currency::unreserve(&collection_details.owner, old_deposit - deposit);
62+
63+
// the previous deposit was taken from the item's owner
64+
if old_deposit.account.is_some() && maybe_depositor.is_none() {
65+
T::Currency::unreserve(&old_deposit.account.unwrap(), old_deposit.amount);
66+
T::Currency::reserve(&collection_details.owner, deposit)?;
67+
} else if deposit > old_deposit.amount {
68+
T::Currency::reserve(&collection_details.owner, deposit - old_deposit.amount)?;
69+
} else if deposit < old_deposit.amount {
70+
T::Currency::unreserve(&collection_details.owner, old_deposit.amount - deposit);
71+
}
72+
73+
if maybe_depositor.is_none() {
74+
collection_details.owner_deposit.saturating_accrue(deposit);
75+
collection_details.owner_deposit.saturating_reduce(old_deposit.amount);
6276
}
63-
collection_details.owner_deposit.saturating_accrue(deposit);
6477

65-
*metadata = Some(ItemMetadata { deposit, data: data.clone() });
78+
*metadata = Some(ItemMetadata {
79+
deposit: ItemMetadataDeposit { account: maybe_depositor, amount: deposit },
80+
data: data.clone(),
81+
});
6682

6783
Collection::<T, I>::insert(&collection, &collection_details);
6884
Self::deposit_event(Event::ItemMetadataSet { collection, item, data });
@@ -75,8 +91,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
7591
collection: T::CollectionId,
7692
item: T::ItemId,
7793
) -> DispatchResult {
94+
let is_root = maybe_check_owner.is_none();
95+
let metadata = ItemMetadataOf::<T, I>::take(collection, item)
96+
.ok_or(Error::<T, I>::MetadataNotFound)?;
7897
let mut collection_details =
7998
Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
99+
let depositor_account =
100+
metadata.deposit.account.unwrap_or(collection_details.owner.clone());
101+
80102
if let Some(check_owner) = &maybe_check_owner {
81103
ensure!(check_owner == &collection_details.owner, Error::<T, I>::NoPermission);
82104
}
@@ -85,20 +107,19 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
85107
let is_locked = Self::get_item_config(&collection, &item)
86108
.map_or(false, |c| c.has_disabled_setting(ItemSetting::UnlockedMetadata));
87109

88-
ensure!(maybe_check_owner.is_none() || !is_locked, Error::<T, I>::LockedItemMetadata);
110+
ensure!(is_root || !is_locked, Error::<T, I>::LockedItemMetadata);
89111

90-
ItemMetadataOf::<T, I>::try_mutate_exists(collection, item, |metadata| {
91-
if metadata.is_some() {
92-
collection_details.item_metadatas.saturating_dec();
93-
}
94-
let deposit = metadata.take().ok_or(Error::<T, I>::UnknownItem)?.deposit;
95-
T::Currency::unreserve(&collection_details.owner, deposit);
96-
collection_details.owner_deposit.saturating_reduce(deposit);
112+
collection_details.item_metadatas.saturating_dec();
113+
T::Currency::unreserve(&depositor_account, metadata.deposit.amount);
97114

98-
Collection::<T, I>::insert(&collection, &collection_details);
99-
Self::deposit_event(Event::ItemMetadataCleared { collection, item });
100-
Ok(())
101-
})
115+
if depositor_account == collection_details.owner {
116+
collection_details.owner_deposit.saturating_reduce(metadata.deposit.amount);
117+
}
118+
119+
Collection::<T, I>::insert(&collection, &collection_details);
120+
Self::deposit_event(Event::ItemMetadataCleared { collection, item });
121+
122+
Ok(())
102123
}
103124

104125
pub(crate) fn do_set_collection_metadata(

‎frame/nfts/src/impl_nonfungibles.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,12 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId, ItemConfig
157157
Self::do_mint(
158158
*collection,
159159
*item,
160-
who.clone(),
160+
match deposit_collection_owner {
161+
true => None,
162+
false => Some(who.clone()),
163+
},
161164
who.clone(),
162165
*item_config,
163-
deposit_collection_owner,
164166
|_, _| Ok(()),
165167
)
166168
}

0 commit comments

Comments
 (0)