Skip to content

Commit

Permalink
chore: fork pallet nfts (#382)
Browse files Browse the repository at this point in the history
  • Loading branch information
chungquantin committed Nov 19, 2024
1 parent dad89cb commit 6ef968f
Show file tree
Hide file tree
Showing 11 changed files with 627 additions and 1,097 deletions.
64 changes: 8 additions & 56 deletions pallets/nfts/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,27 +64,6 @@ fn add_collection_metadata<T: Config<I>, I: 'static>() -> (T::AccountId, Account
(caller, caller_lookup)
}

fn approve_collection<T: Config<I>, I: 'static>(
index: u32,
) -> (T::AccountId, AccountIdLookupOf<T>) {
let caller = Collection::<T, I>::get(T::Helper::collection(0)).unwrap().owner;
if caller != whitelisted_caller() {
whitelist_account!(caller);
}
let caller_lookup = T::Lookup::unlookup(caller.clone());
let delegate: T::AccountId = account("delegate", 0, SEED + index);
let delegate_lookup = T::Lookup::unlookup(delegate.clone());
let deadline = BlockNumberFor::<T>::max_value();
assert_ok!(Nfts::<T, I>::approve_transfer(
SystemOrigin::Signed(caller.clone()).into(),
T::Helper::collection(0),
None,
delegate_lookup.clone(),
Some(deadline),
));
(caller, caller_lookup)
}

fn mint_item<T: Config<I>, I: 'static>(
index: u16,
) -> (T::ItemId, T::AccountId, AccountIdLookupOf<T>) {
Expand All @@ -98,7 +77,7 @@ fn mint_item<T: Config<I>, I: 'static>(
let item_exists = Item::<T, I>::contains_key(&collection, &item);

Check warning on line 77 in pallets/nfts/src/benchmarking.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/benchmarking.rs:77:60 | 77 | let item_exists = Item::<T, I>::contains_key(&collection, &item); | ^^^^^ help: change this to: `item` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args

Check warning on line 77 in pallets/nfts/src/benchmarking.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/benchmarking.rs:77:47 | 77 | let item_exists = Item::<T, I>::contains_key(&collection, &item); | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args = note: `#[warn(clippy::needless_borrows_for_generic_args)]` on by default
let item_config = ItemConfigOf::<T, I>::get(&collection, &item);

Check warning on line 78 in pallets/nfts/src/benchmarking.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/benchmarking.rs:78:59 | 78 | let item_config = ItemConfigOf::<T, I>::get(&collection, &item); | ^^^^^ help: change this to: `item` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args

Check warning on line 78 in pallets/nfts/src/benchmarking.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/benchmarking.rs:78:46 | 78 | let item_config = ItemConfigOf::<T, I>::get(&collection, &item); | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
if item_exists {
return (item, caller, caller_lookup);
return (item, caller, caller_lookup)
} else if let Some(item_config) = item_config {
assert_ok!(Nfts::<T, I>::force_mint(
SystemOrigin::Signed(caller.clone()).into(),
Expand Down Expand Up @@ -271,8 +250,6 @@ benchmarks_instance_pallet! {
let m in 0 .. 1_000;
let c in 0 .. 1_000;
let a in 0 .. 1_000;
let h in 0 .. 1_000;
let l in 0 .. 1_000;

let (collection, caller, _) = create_collection::<T, I>();
add_collection_metadata::<T, I>();
Expand All @@ -290,13 +267,6 @@ benchmarks_instance_pallet! {
for i in 0..a {
add_collection_attribute::<T, I>(i as u16);
}
for i in 0..h {
mint_item::<T, I>(i as u16);
burn_item::<T, I>(i as u16);
}
for i in 0..l {
approve_collection::<T, I>(i);
}
let witness = Collection::<T, I>::get(collection).unwrap().destroy_witness();
}: _(SystemOrigin::Signed(caller), collection, witness)
verify {
Expand Down Expand Up @@ -603,45 +573,27 @@ benchmarks_instance_pallet! {
}

approve_transfer {
let i in 0..1;

let (collection, caller, _) = create_collection::<T, I>();
let (item, ..) = mint_item::<T, I>(0);
let delegate: T::AccountId = account("delegate", 0, SEED);
let delegate_lookup = T::Lookup::unlookup(delegate.clone());
let maybe_deadline = if i == 0 {
None
} else {
Some(BlockNumberFor::<T>::max_value())
};
let maybe_item = if i == 0 {
None
} else {
Some(item)
};
}: _(SystemOrigin::Signed(caller.clone()), collection, maybe_item, delegate_lookup, maybe_deadline)
let deadline = BlockNumberFor::<T>::max_value();
}: _(SystemOrigin::Signed(caller.clone()), collection, item, delegate_lookup, Some(deadline))
verify {
assert_last_event::<T, I>(Event::TransferApproved { collection, item: maybe_item, owner: caller, delegate, deadline: maybe_deadline }.into());
assert_last_event::<T, I>(Event::TransferApproved { collection, item, owner: caller, delegate, deadline: Some(deadline) }.into());
}

cancel_approval {
let i in 0..1;

let (collection, caller, _) = create_collection::<T, I>();
let (item, ..) = mint_item::<T, I>(0);
let delegate: T::AccountId = account("delegate", 0, SEED);
let delegate_lookup = T::Lookup::unlookup(delegate.clone());
let origin = SystemOrigin::Signed(caller.clone()).into();
let deadline = BlockNumberFor::<T>::max_value();
let maybe_item = if i == 0 {
None
} else {
Some(item)
};
Nfts::<T, I>::approve_transfer(origin, collection, maybe_item, delegate_lookup.clone(), Some(deadline))?;
}: _(SystemOrigin::Signed(caller.clone()), collection, maybe_item, delegate_lookup)
Nfts::<T, I>::approve_transfer(origin, collection, item, delegate_lookup.clone(), Some(deadline))?;
}: _(SystemOrigin::Signed(caller.clone()), collection, item, delegate_lookup)
verify {
assert_last_event::<T, I>(Event::ApprovalCancelled { collection, item: maybe_item, owner: caller, delegate }.into());
assert_last_event::<T, I>(Event::ApprovalCancelled { collection, item, owner: caller, delegate }.into());
}

clear_all_transfer_approvals {
Expand All @@ -651,7 +603,7 @@ benchmarks_instance_pallet! {
let delegate_lookup = T::Lookup::unlookup(delegate.clone());
let origin = SystemOrigin::Signed(caller.clone()).into();
let deadline = BlockNumberFor::<T>::max_value();
Nfts::<T, I>::approve_transfer(origin, collection, Some(item), delegate_lookup.clone(), Some(deadline))?;
Nfts::<T, I>::approve_transfer(origin, collection, item, delegate_lookup.clone(), Some(deadline))?;
}: _(SystemOrigin::Signed(caller.clone()), collection, item)
verify {
assert_last_event::<T, I>(Event::AllApprovalsCancelled {collection, item, owner: caller}.into());
Expand Down
20 changes: 1 addition & 19 deletions pallets/nfts/src/common_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Collection::<T, I>::get(collection).map(|i| i.owner)
}

/// Get the total number of items in the collection, if the collection exists.
pub fn collection_items(collection: T::CollectionId) -> Option<u32> {
Collection::<T, I>::get(collection).map(|i| i.items)
}

/// Get the allowances to spend items within the collection.
pub fn collection_allowances(collection: T::CollectionId) -> Option<u32> {
Collection::<T, I>::get(collection).map(|i| i.allowances)
}

/// Get the metadata of the collection item.
pub fn item_metadata(
collection: T::CollectionId,
item: T::ItemId,
) -> Option<BoundedVec<u8, T::StringLimit>> {
ItemMetadataOf::<T, I>::get(collection, item).map(|metadata| metadata.data)
}

/// Validates the signature of the given data with the provided signer's account ID.
///
/// # Errors
Expand All @@ -64,7 +46,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
signer: &T::AccountId,
) -> DispatchResult {
if signature.verify(&**data, &signer) {

Check warning on line 48 in pallets/nfts/src/common_functions.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler

warning: this expression creates a reference which is immediately dereferenced by the compiler --> pallets/nfts/src/common_functions.rs:48:32 | 48 | if signature.verify(&**data, &signer) { | ^^^^^^^ help: change this to: `signer` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow = note: `#[warn(clippy::needless_borrow)]` on by default
return Ok(());
return Ok(())
}

// NOTE: for security reasons modern UIs implicitly wrap the data requested to sign into
Expand Down
105 changes: 5 additions & 100 deletions pallets/nfts/src/features/approvals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
if let Some(check_origin) = maybe_check_origin {
ensure!(check_origin == details.owner, Error::<T, I>::NoPermission);
}

let now = frame_system::Pallet::<T>::block_number();
let deadline = maybe_deadline.map(|d| d.saturating_add(now));

Expand All @@ -73,13 +74,15 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
.try_insert(delegate.clone(), deadline)
.map_err(|_| Error::<T, I>::ReachedApprovalLimit)?;
Item::<T, I>::insert(&collection, &item, &details);

Check warning on line 76 in pallets/nfts/src/features/approvals.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/approvals.rs:76:37 | 76 | Item::<T, I>::insert(&collection, &item, &details); | ^^^^^ help: change this to: `item` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args

Check warning on line 76 in pallets/nfts/src/features/approvals.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/approvals.rs:76:24 | 76 | Item::<T, I>::insert(&collection, &item, &details); | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args

Self::deposit_event(Event::TransferApproved {
collection,
item: Some(item),
item,
owner: details.owner,
delegate,
deadline,
});

Ok(())
}

Expand Down Expand Up @@ -126,7 +129,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

Self::deposit_event(Event::ApprovalCancelled {
collection,
item: Some(item),
item,
owner: details.owner,
delegate,
});
Expand Down Expand Up @@ -170,102 +173,4 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

Ok(())
}

pub(crate) fn do_approve_collection(
maybe_check_origin: Option<T::AccountId>,
collection: T::CollectionId,
delegate: T::AccountId,
) -> DispatchResult {
ensure!(
Self::is_pallet_feature_enabled(PalletFeature::Approvals),
Error::<T, I>::MethodDisabled
);
let owner = Self::collection_owner(collection).ok_or(Error::<T, I>::UnknownCollection)?;

let collection_config = Self::get_collection_config(&collection)?;
ensure!(
collection_config.is_setting_enabled(CollectionSetting::TransferableItems),
Error::<T, I>::ItemsNonTransferable
);

if let Some(check_origin) = maybe_check_origin {
ensure!(check_origin == owner, Error::<T, I>::NoPermission);
}

Allowances::<T, I>::mutate((&collection, &owner, &delegate), |allowance| {
*allowance = true;
});
Collection::<T, I>::try_mutate(
&collection,
|maybe_collection_details| -> Result<(), DispatchError> {
let collection_details =
maybe_collection_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
collection_details.allowances.saturating_inc();
Ok(())
},
)?;

Self::deposit_event(Event::TransferApproved {
collection,
item: None,
owner,
delegate,
deadline: None,
});
Ok(())
}

pub(crate) fn do_cancel_collection(
maybe_check_origin: Option<T::AccountId>,
collection: T::CollectionId,
delegate: T::AccountId,
) -> DispatchResult {
let owner = Self::collection_owner(collection).ok_or(Error::<T, I>::UnknownCollection)?;

if let Some(check_origin) = maybe_check_origin {
ensure!(check_origin == owner, Error::<T, I>::NoPermission);
}
Allowances::<T, I>::remove((&collection, &owner, &delegate));
Collection::<T, I>::try_mutate(
&collection,
|maybe_collection_details| -> Result<(), DispatchError> {
let collection_details =
maybe_collection_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
collection_details.allowances.saturating_dec();
Ok(())
},
)?;

Self::deposit_event(Event::ApprovalCancelled { collection, owner, item: None, delegate });

Ok(())
}

pub fn check_allowance(
collection: &T::CollectionId,
item: &Option<T::ItemId>,
owner: &T::AccountId,
delegate: &T::AccountId,
) -> Result<(), DispatchError> {
// Check if a `delegate` has a permission to spend the collection.
if Allowances::<T, I>::get((&collection, &owner, &delegate)) {
if let Some(item) = item {
Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownItem)?;
};
return Ok(());
}
// Check if a `delegate` has a permission to spend the collection item.
if let Some(item) = item {
let details =
Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownItem)?;

let deadline = details.approvals.get(&delegate).ok_or(Error::<T, I>::NoPermission)?;
if let Some(d) = deadline {
let block_number = frame_system::Pallet::<T>::block_number();
ensure!(block_number <= *d, Error::<T, I>::ApprovalExpired);
}
return Ok(());
};
Err(Error::<T, I>::NoPermission.into())
}
}
12 changes: 0 additions & 12 deletions pallets/nfts/src/features/create_delete_collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
items: 0,
item_metadatas: 0,
item_configs: 0,
item_holders: 0,
attributes: 0,
allowances: 0,
},
);
CollectionRoleOf::<T, I>::insert(
Expand Down Expand Up @@ -121,11 +119,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
collection_details.item_configs == witness.item_configs,
Error::<T, I>::BadWitness
);
ensure!(
collection_details.item_holders == witness.item_holders,
Error::<T, I>::BadWitness
);
ensure!(collection_details.allowances == witness.allowances, Error::<T, I>::BadWitness);

for (_, metadata) in ItemMetadataOf::<T, I>::drain_prefix(&collection) {

Check warning on line 123 in pallets/nfts/src/features/create_delete_collection.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_collection.rs:123:62 | 123 | for (_, metadata) in ItemMetadataOf::<T, I>::drain_prefix(&collection) { | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
if let Some(depositor) = metadata.deposit.account {
Expand All @@ -144,9 +137,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}
}

let _ =
AccountBalance::<T, I>::clear_prefix(collection, collection_details.items, None);
let _ = Allowances::<T, I>::clear_prefix((collection,), collection_details.items, None);
CollectionAccount::<T, I>::remove(&collection_details.owner, &collection);

Check warning on line 140 in pallets/nfts/src/features/create_delete_collection.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_collection.rs:140:65 | 140 | CollectionAccount::<T, I>::remove(&collection_details.owner, &collection); | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
T::Currency::unreserve(&collection_details.owner, collection_details.owner_deposit);
CollectionConfigOf::<T, I>::remove(&collection);

Check warning on line 142 in pallets/nfts/src/features/create_delete_collection.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_collection.rs:142:39 | 142 | CollectionConfigOf::<T, I>::remove(&collection); | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
Expand All @@ -157,9 +147,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Ok(DestroyWitness {
item_metadatas: collection_details.item_metadatas,
item_configs: collection_details.item_configs,
item_holders: collection_details.item_holders,
attributes: collection_details.attributes,
allowances: collection_details.allowances,
})
})
}
Expand Down
16 changes: 0 additions & 16 deletions pallets/nfts/src/features/create_delete_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

collection_details.items.saturating_inc();

let account_balance =
AccountBalance::<T, I>::mutate(collection, &mint_to, |balance| -> u32 {
balance.saturating_inc();
*balance
});
if account_balance == 1 {
collection_details.item_holders.saturating_inc();
}

let collection_config = Self::get_collection_config(&collection)?;
let deposit_amount = match collection_config
.is_setting_enabled(CollectionSetting::DepositRequired)
Expand Down Expand Up @@ -263,10 +254,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}
}

if AccountBalance::<T, I>::get(collection, &details.owner) == 1 {
collection_details.item_holders.saturating_dec();
}

Ok(details.owner)
},
)?;
Expand All @@ -276,9 +263,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
ItemPriceOf::<T, I>::remove(&collection, &item);

Check warning on line 263 in pallets/nfts/src/features/create_delete_item.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_item.rs:263:44 | 263 | ItemPriceOf::<T, I>::remove(&collection, &item); | ^^^^^ help: change this to: `item` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args

Check warning on line 263 in pallets/nfts/src/features/create_delete_item.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_item.rs:263:31 | 263 | ItemPriceOf::<T, I>::remove(&collection, &item); | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
PendingSwapOf::<T, I>::remove(&collection, &item);

Check warning on line 264 in pallets/nfts/src/features/create_delete_item.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_item.rs:264:46 | 264 | PendingSwapOf::<T, I>::remove(&collection, &item); | ^^^^^ help: change this to: `item` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args

Check warning on line 264 in pallets/nfts/src/features/create_delete_item.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_item.rs:264:33 | 264 | PendingSwapOf::<T, I>::remove(&collection, &item); | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
ItemAttributesApprovalsOf::<T, I>::remove(&collection, &item);

Check warning on line 265 in pallets/nfts/src/features/create_delete_item.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_item.rs:265:58 | 265 | ItemAttributesApprovalsOf::<T, I>::remove(&collection, &item); | ^^^^^ help: change this to: `item` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args

Check warning on line 265 in pallets/nfts/src/features/create_delete_item.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_item.rs:265:45 | 265 | ItemAttributesApprovalsOf::<T, I>::remove(&collection, &item); | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
AccountBalance::<T, I>::mutate(collection, &owner, |balance| {
balance.saturating_dec();
});

if remove_config {
ItemConfigOf::<T, I>::remove(&collection, &item);

Check warning on line 268 in pallets/nfts/src/features/create_delete_item.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_item.rs:268:46 | 268 | ItemConfigOf::<T, I>::remove(&collection, &item); | ^^^^^ help: change this to: `item` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args

Check warning on line 268 in pallets/nfts/src/features/create_delete_item.rs

View workflow job for this annotation

GitHub Actions / clippy

the borrowed expression implements the required traits

warning: the borrowed expression implements the required traits --> pallets/nfts/src/features/create_delete_item.rs:268:33 | 268 | ItemConfigOf::<T, I>::remove(&collection, &item); | ^^^^^^^^^^^ help: change this to: `collection` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
Expand Down
Loading

0 comments on commit 6ef968f

Please sign in to comment.