Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(nfts): rebase feat-nfts branch to fix clippy warnings #392

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
4a5f17c
fix(nfts): clippy warnings
chungquantin Nov 21, 2024
5ed6292
chore: fork pallet nfts
chungquantin Nov 13, 2024
38d56d7
chore: resolve feedback
chungquantin Nov 16, 2024
45eb281
feat: nfts pallet optimization
chungquantin Nov 19, 2024
e53484d
chore: fork pallet nfts (#382)
chungquantin Nov 18, 2024
136d26d
chore: fork pallet nfts
chungquantin Nov 13, 2024
65a863c
chore: resolve feedback
chungquantin Nov 16, 2024
4c3d279
feat: nfts pallet optimization
chungquantin Nov 19, 2024
01ecc61
chore: fork pallet nfts (#382)
chungquantin Nov 18, 2024
4b96b3b
feat: add new storage items and optimize
chungquantin Nov 19, 2024
0d43dff
chore: resolve feedback
chungquantin Nov 19, 2024
d1cc5fd
fix: formatting
chungquantin Nov 21, 2024
97409f3
Merge remote-tracking branch 'refs/remotes/origin/chungquantin/feat-n…
chungquantin Nov 21, 2024
483ddae
fix: formatting
chungquantin Nov 21, 2024
37ec9ab
fix: clippy warnings
chungquantin Nov 21, 2024
3e886a9
fix: warnings
chungquantin Nov 22, 2024
2e02651
chore: fork pallet nfts
chungquantin Nov 13, 2024
52a9f27
chore: resolve feedback
chungquantin Nov 16, 2024
1134365
feat: nfts pallet optimization
chungquantin Nov 19, 2024
7d95985
chore: fork pallet nfts (#382)
chungquantin Nov 18, 2024
285a035
chore: fork pallet nfts
chungquantin Nov 13, 2024
fc0323a
chore: resolve feedback
chungquantin Nov 16, 2024
d764cf8
feat: nfts pallet optimization
chungquantin Nov 19, 2024
b79c095
chore: fork pallet nfts (#382)
chungquantin Nov 18, 2024
2841d0d
feat: add new storage items and optimize
chungquantin Nov 19, 2024
da6dca9
chore: resolve feedback
chungquantin Nov 19, 2024
8036ca2
fix: formatting
chungquantin Nov 21, 2024
5eeb3be
chore: fork pallet nfts
chungquantin Nov 13, 2024
94c2806
chore: resolve feedback
chungquantin Nov 16, 2024
a217fd6
feat: nfts pallet optimization
chungquantin Nov 19, 2024
619a4d8
chore: fork pallet nfts (#382)
chungquantin Nov 18, 2024
18704f1
chore: fork pallet nfts
chungquantin Nov 13, 2024
87049f1
feat: nfts pallet optimization
chungquantin Nov 19, 2024
c8cf3e8
chore: fork pallet nfts (#382)
chungquantin Nov 18, 2024
a59f420
feat: add new storage items and optimize
chungquantin Nov 19, 2024
3cec0cd
Merge remote-tracking branch 'refs/remotes/origin/chungquantin/feat-n…
chungquantin Nov 22, 2024
a6b0c98
fix: reformatting
chungquantin Nov 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions pallets/nfts/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

//! Nfts pallet benchmarking.

#![cfg(feature = "runtime-benchmarks")]

use enumflags2::{BitFlag, BitFlags};
use frame_benchmarking::v1::{
account, benchmarks_instance_pallet, whitelist_account, whitelisted_caller, BenchmarkError,
Expand Down Expand Up @@ -95,8 +93,8 @@ fn mint_item<T: Config<I>, I: 'static>(
whitelist_account!(caller);
}
let caller_lookup = T::Lookup::unlookup(caller.clone());
let item_exists = Item::<T, I>::contains_key(&collection, &item);
let item_config = ItemConfigOf::<T, I>::get(&collection, &item);
let item_exists = Item::<T, I>::contains_key(collection, item);
let item_config = ItemConfigOf::<T, I>::get(collection, item);
if item_exists {
return (item, caller, caller_lookup);
} else if let Some(item_config) = item_config {
Expand Down Expand Up @@ -730,7 +728,7 @@ benchmarks_instance_pallet! {
}

pay_tips {
let n in 0 .. T::MaxTips::get() as u32;
let n in 0 .. T::MaxTips::get();
let amount = BalanceOf::<T, I>::from(100u32);
let caller: T::AccountId = whitelisted_caller();
let collection = T::Helper::collection(0);
Expand Down Expand Up @@ -836,7 +834,7 @@ benchmarks_instance_pallet! {
}

mint_pre_signed {
let n in 0 .. T::MaxAttributesPerCall::get() as u32;
let n in 0 .. T::MaxAttributesPerCall::get();
let (caller_public, caller) = T::Helper::signer();
T::Currency::make_free_balance_be(&caller, DepositBalanceOf::<T, I>::max_value());
let caller_lookup = T::Lookup::unlookup(caller.clone());
Expand Down Expand Up @@ -871,14 +869,14 @@ benchmarks_instance_pallet! {
let target: T::AccountId = account("target", 0, SEED);
T::Currency::make_free_balance_be(&target, DepositBalanceOf::<T, I>::max_value());
frame_system::Pallet::<T>::set_block_number(One::one());
}: _(SystemOrigin::Signed(target.clone()), Box::new(mint_data), signature.into(), caller)
}: _(SystemOrigin::Signed(target.clone()), Box::new(mint_data), signature, caller)
verify {
let metadata: BoundedVec<_, _> = metadata.try_into().unwrap();
assert_last_event::<T, I>(Event::ItemMetadataSet { collection, item, data: metadata }.into());
}

set_attributes_pre_signed {
let n in 0 .. T::MaxAttributesPerCall::get() as u32;
let n in 0 .. T::MaxAttributesPerCall::get();
let (collection, _, _) = create_collection::<T, I>();

let item_owner: T::AccountId = account("item_owner", 0, SEED);
Expand Down Expand Up @@ -914,7 +912,7 @@ benchmarks_instance_pallet! {
let signature = T::Helper::sign(&signer_public, &message);

frame_system::Pallet::<T>::set_block_number(One::one());
}: _(SystemOrigin::Signed(item_owner.clone()), pre_signed_data, signature.into(), signer.clone())
}: _(SystemOrigin::Signed(item_owner.clone()), pre_signed_data, signature, signer.clone())
verify {
assert_last_event::<T, I>(
Event::PreSignedAttributesSet {
Expand Down
7 changes: 4 additions & 3 deletions pallets/nfts/src/common_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
signature: &T::OffchainSignature,
signer: &T::AccountId,
) -> DispatchResult {
if signature.verify(&**data, &signer) {
return Ok(())
if signature.verify(&**data, signer) {
return Ok(());
}

// NOTE: for security reasons modern UIs implicitly wrap the data requested to sign into
Expand All @@ -76,7 +76,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
wrapped.extend(data);
wrapped.extend(suffix);

ensure!(signature.verify(&*wrapped, &signer), Error::<T, I>::WrongSignature);
ensure!(signature.verify(&*wrapped, signer), Error::<T, I>::WrongSignature);

Ok(())
}
Expand All @@ -87,6 +87,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Self::deposit_event(Event::NextCollectionIdIncremented { next_id });
}

#[allow(missing_docs)]
#[cfg(any(test, feature = "runtime-benchmarks"))]
pub fn set_next_id(id: T::CollectionId) {
NextCollectionId::<T, I>::set(Some(id));
Expand Down
14 changes: 6 additions & 8 deletions pallets/nfts/src/features/approvals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@
Self::is_pallet_feature_enabled(PalletFeature::Approvals),
Error::<T, I>::MethodDisabled
);
let mut details =
Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownItem)?;
let mut details = Item::<T, I>::get(collection, item).ok_or(Error::<T, I>::UnknownItem)?;

let collection_config = Self::get_collection_config(&collection)?;
ensure!(
Expand All @@ -72,7 +71,7 @@
.approvals
.try_insert(delegate.clone(), deadline)
.map_err(|_| Error::<T, I>::ReachedApprovalLimit)?;
Item::<T, I>::insert(&collection, &item, &details);
Item::<T, I>::insert(collection, item, &details);
Self::deposit_event(Event::TransferApproved {
collection,
item: Some(item),
Expand Down Expand Up @@ -103,8 +102,7 @@
item: T::ItemId,
delegate: T::AccountId,
) -> DispatchResult {
let mut details =
Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownItem)?;
let mut details = Item::<T, I>::get(collection, item).ok_or(Error::<T, I>::UnknownItem)?;

let maybe_deadline = details.approvals.get(&delegate).ok_or(Error::<T, I>::NotDelegate)?;

Expand All @@ -122,7 +120,7 @@
}

details.approvals.remove(&delegate);
Item::<T, I>::insert(&collection, &item, &details);
Item::<T, I>::insert(collection, item, &details);

Self::deposit_event(Event::ApprovalCancelled {
collection,
Expand Down Expand Up @@ -153,14 +151,14 @@
item: T::ItemId,
) -> DispatchResult {
let mut details =
Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownCollection)?;
Item::<T, I>::get(collection, item).ok_or(Error::<T, I>::UnknownCollection)?;

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

details.approvals.clear();
Item::<T, I>::insert(&collection, &item, &details);
Item::<T, I>::insert(collection, item, &details);

Self::deposit_event(Event::AllApprovalsCancelled {
collection,
Expand Down Expand Up @@ -188,7 +186,7 @@
);

let owner = Collection::<T, I>::try_mutate(
&collection,

Check warning on line 189 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:189:4 | 189 | &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 = note: `#[warn(clippy::needless_borrows_for_generic_args)]` on by default
|maybe_collection_details| -> Result<T::AccountId, DispatchError> {
let collection_details =
maybe_collection_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
Expand Down Expand Up @@ -227,7 +225,7 @@
}
Allowances::<T, I>::remove((&collection, &owner, &delegate));
Collection::<T, I>::try_mutate(
&collection,

Check warning on line 228 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:228:4 | 228 | &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
|maybe_collection_details| -> Result<(), DispatchError> {
let collection_details =
maybe_collection_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
Expand All @@ -241,25 +239,25 @@
Ok(())
}

pub fn check_allowance(
collection: &T::CollectionId,
item: &Option<T::ItemId>,
owner: &T::AccountId,
delegate: &T::AccountId,
) -> Result<(), DispatchError> {

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

View workflow job for this annotation

GitHub Actions / clippy

missing documentation for an associated function

warning: missing documentation for an associated function --> pallets/nfts/src/features/approvals.rs:242:2 | 242 | / pub fn check_allowance( 243 | | collection: &T::CollectionId, 244 | | item: &Option<T::ItemId>, 245 | | owner: &T::AccountId, 246 | | delegate: &T::AccountId, 247 | | ) -> Result<(), DispatchError> { | |__________________________________^ | = note: requested on the command line with `-W missing-docs`
// 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)?;

Check warning on line 251 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:251:36 | 251 | Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownItem)?; | ^^^^^ 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 251 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:251:23 | 251 | Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownItem)?; | ^^^^^^^^^^^ 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
};
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)?;

Check warning on line 258 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:258:36 | 258 | Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownItem)?; | ^^^^^ 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 258 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:258:23 | 258 | Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownItem)?; | ^^^^^^^^^^^ 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

let deadline = details.approvals.get(&delegate).ok_or(Error::<T, I>::NoPermission)?;

Check warning on line 260 in pallets/nfts/src/features/approvals.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/features/approvals.rs:260:41 | 260 | let deadline = details.approvals.get(&delegate).ok_or(Error::<T, I>::NoPermission)?; | ^^^^^^^^^ help: change this to: `delegate` | = 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
if let Some(d) = deadline {
let block_number = frame_system::Pallet::<T>::block_number();
ensure!(block_number <= *d, Error::<T, I>::ApprovalExpired);
Expand Down
22 changes: 11 additions & 11 deletions pallets/nfts/src/features/atomic_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,17 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
);
ensure!(duration <= T::MaxDeadlineDuration::get(), Error::<T, I>::WrongDuration);

let item = Item::<T, I>::get(&offered_collection_id, &offered_item_id)
let item = Item::<T, I>::get(offered_collection_id, offered_item_id)
.ok_or(Error::<T, I>::UnknownItem)?;
ensure!(item.owner == caller, Error::<T, I>::NoPermission);

match maybe_desired_item_id {
Some(desired_item_id) => ensure!(
Item::<T, I>::contains_key(&desired_collection_id, &desired_item_id),
Item::<T, I>::contains_key(desired_collection_id, desired_item_id),
Error::<T, I>::UnknownItem
),
None => ensure!(
Collection::<T, I>::contains_key(&desired_collection_id),
Collection::<T, I>::contains_key(desired_collection_id),
Error::<T, I>::UnknownCollection
),
};
Expand All @@ -81,8 +81,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let deadline = duration.saturating_add(now);

PendingSwapOf::<T, I>::insert(
&offered_collection_id,
&offered_item_id,
offered_collection_id,
offered_item_id,
PendingSwap {
desired_collection: desired_collection_id,
desired_item: maybe_desired_item_id,
Expand Down Expand Up @@ -118,17 +118,17 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
offered_collection_id: T::CollectionId,
offered_item_id: T::ItemId,
) -> DispatchResult {
let swap = PendingSwapOf::<T, I>::get(&offered_collection_id, &offered_item_id)
let swap = PendingSwapOf::<T, I>::get(offered_collection_id, offered_item_id)
.ok_or(Error::<T, I>::UnknownSwap)?;

let now = frame_system::Pallet::<T>::block_number();
if swap.deadline > now {
let item = Item::<T, I>::get(&offered_collection_id, &offered_item_id)
let item = Item::<T, I>::get(offered_collection_id, offered_item_id)
.ok_or(Error::<T, I>::UnknownItem)?;
ensure!(item.owner == caller, Error::<T, I>::NoPermission);
}

PendingSwapOf::<T, I>::remove(&offered_collection_id, &offered_item_id);
PendingSwapOf::<T, I>::remove(offered_collection_id, offered_item_id);

Self::deposit_event(Event::SwapCancelled {
offered_collection: offered_collection_id,
Expand Down Expand Up @@ -172,11 +172,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Error::<T, I>::MethodDisabled
);

let send_item = Item::<T, I>::get(&send_collection_id, &send_item_id)
let send_item = Item::<T, I>::get(send_collection_id, send_item_id)
.ok_or(Error::<T, I>::UnknownItem)?;
let receive_item = Item::<T, I>::get(&receive_collection_id, &receive_item_id)
let receive_item = Item::<T, I>::get(receive_collection_id, receive_item_id)
.ok_or(Error::<T, I>::UnknownItem)?;
let swap = PendingSwapOf::<T, I>::get(&receive_collection_id, &receive_item_id)
let swap = PendingSwapOf::<T, I>::get(receive_collection_id, receive_item_id)
.ok_or(Error::<T, I>::UnknownSwap)?;

ensure!(send_item.owner == caller, Error::<T, I>::NoPermission);
Expand Down
12 changes: 6 additions & 6 deletions pallets/nfts/src/features/create_delete_collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@
),
);

CollectionConfigOf::<T, I>::insert(&collection, config);
CollectionAccount::<T, I>::insert(&owner, &collection, ());
CollectionConfigOf::<T, I>::insert(collection, config);
CollectionAccount::<T, I>::insert(&owner, collection, ());

Self::deposit_event(event);

Expand Down Expand Up @@ -127,13 +127,13 @@
);
ensure!(collection_details.allowances == witness.allowances, Error::<T, I>::BadWitness);

for (_, metadata) in ItemMetadataOf::<T, I>::drain_prefix(&collection) {
for (_, metadata) in ItemMetadataOf::<T, I>::drain_prefix(collection) {
if let Some(depositor) = metadata.deposit.account {
T::Currency::unreserve(&depositor, metadata.deposit.amount);
}
}

CollectionMetadataOf::<T, I>::remove(&collection);
CollectionMetadataOf::<T, I>::remove(collection);
Self::clear_roles(&collection)?;

for (_, (_, deposit)) in Attribute::<T, I>::drain_prefix((&collection,)) {
Expand All @@ -147,10 +147,10 @@
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 150 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:150:65 | 150 | 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);
let _ = ItemConfigOf::<T, I>::clear_prefix(&collection, witness.item_configs, None);
CollectionConfigOf::<T, I>::remove(collection);
let _ = ItemConfigOf::<T, I>::clear_prefix(collection, witness.item_configs, None);

Self::deposit_event(Event::Destroyed { collection });

Expand Down
Loading
Loading