Skip to content

Commit

Permalink
fix(nfts): clippy warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
@chungquantin
chungquantin committed Nov 21, 2024
1 parent aff6408 commit 4a5f17c
Show file tree
Hide file tree
Showing 16 changed files with 145 additions and 151 deletions.
16 changes: 7 additions & 9 deletions pallets/nfts/src/benchmarking.rs
View file
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 @@ -74,8 +72,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 @@ -682,7 +680,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 @@ -788,7 +786,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 @@ -823,14 +821,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 @@ -866,7 +864,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
4 changes: 2 additions & 2 deletions pallets/nfts/src/common_functions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
signature: &T::OffchainSignature,
signer: &T::AccountId,
) -> DispatchResult {
if signature.verify(&**data, &signer) {
if signature.verify(&**data, signer) {
return Ok(())
}

Expand All @@ -58,7 +58,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 Down
14 changes: 6 additions & 8 deletions pallets/nfts/src/features/approvals.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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 @@ -73,7 +72,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
.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,
Expand Down Expand Up @@ -106,8 +105,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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 @@ -125,7 +123,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

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 @@ -156,14 +154,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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
22 changes: 11 additions & 11 deletions pallets/nfts/src/features/atomic_swap.rs
View file
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
29 changes: 14 additions & 15 deletions pallets/nfts/src/features/attributes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

Check warning on line 87 in pallets/nfts/src/features/attributes.rs

View workflow job for this annotation

GitHub Actions / clippy

you seem to be trying to use `match` for an equality check. Consider using `if` 

warning: you seem to be trying to use `match` for an equality check. Consider using `if`
  --> pallets/nfts/src/features/attributes.rs:72:3
   |
72 | /         match namespace {
73 | |             AttributeNamespace::CollectionOwner => match maybe_item {
74 | |                 None => {
75 | |                     ensure!(
...  |
86 | |             _ => (),
87 | |         }
   | |_________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match
   = note: `#[warn(clippy::single_match)]` on by default
help: try
   |
72 ~         if namespace == AttributeNamespace::CollectionOwner { match maybe_item {
73 ~                   None => {
74 ~                       ensure!(
75 ~                           collection_config.is_setting_enabled(CollectionSetting::UnlockedAttributes),
76 ~                           Error::<T, I>::LockedCollectionAttributes
77 ~                       )
78 ~                   },
79 ~                   Some(item) => {
80 ~                       let maybe_is_locked = Self::get_item_config(&collection, &item)
81 ~                           .map(|c| c.has_disabled_setting(ItemSetting::UnlockedAttributes))?;
82 ~                       ensure!(!maybe_is_locked, Error::<T, I>::LockedItemAttributes);
83 ~                   },
84 ~               } }
   |

let mut collection_details =
Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
Collection::<T, I>::get(collection).ok_or(Error::<T, I>::UnknownCollection)?;

let attribute = Attribute::<T, I>::get((collection, maybe_item, &namespace, &key));
let attribute_exists = attribute.is_some();
Expand Down Expand Up @@ -183,7 +183,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
value: BoundedVec<u8, T::ValueLimit>,
) -> DispatchResult {
let mut collection_details =
Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
Collection::<T, I>::get(collection).ok_or(Error::<T, I>::UnknownCollection)?;

let attribute = Attribute::<T, I>::get((collection, maybe_item, &namespace, &key));
if let Some((_, deposit)) = attribute {
Expand Down Expand Up @@ -229,8 +229,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let now = frame_system::Pallet::<T>::block_number();
ensure!(deadline >= now, Error::<T, I>::DeadlineExpired);

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

// Only the CollectionOwner and Account() namespaces could be updated in this way.
Expand All @@ -239,7 +238,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
AttributeNamespace::CollectionOwner => {},
AttributeNamespace::Account(account) => {
ensure!(account == &signer, Error::<T, I>::NoPermission);
let approvals = ItemAttributesApprovalsOf::<T, I>::get(&collection, &item);
let approvals = ItemAttributesApprovalsOf::<T, I>::get(collection, item);
if !approvals.contains(account) {
Self::do_approve_item_attributes(
origin.clone(),
Expand Down Expand Up @@ -274,7 +273,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// `depositor` account. The deposit associated with the attribute, if any, will be unreserved.
///
/// - `maybe_check_origin`: An optional account that acts as an additional security check when
/// clearing the attribute. This can be `None` if no additional check is required.
/// clearing the attribute. This can be `None` if no additional check is required.
/// - `collection`: The identifier of the collection to which the item belongs, or the
/// collection itself if clearing a collection attribute.
/// - `maybe_item`: The identifier of the item to which the attribute belongs, or `None` if
Expand All @@ -298,7 +297,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
// the same as the `deposit.account` (e.g. the deposit was paid by different account)
if deposit.account != maybe_check_origin {
ensure!(
Self::is_valid_namespace(&check_origin, &namespace, &collection, &maybe_item)?,
Self::is_valid_namespace(check_origin, &namespace, &collection, &maybe_item)?,
Error::<T, I>::NoPermission
);
}
Expand Down Expand Up @@ -327,7 +326,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
// e.g. in off-chain mints, the attribute's depositor will be the item's
// owner, that's why we need to do this extra check.
ensure!(
Self::has_role(&collection, &check_origin, CollectionRole::Admin),
Self::has_role(&collection, check_origin, CollectionRole::Admin),
Error::<T, I>::NoPermission
);
}
Expand All @@ -338,7 +337,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

let mut collection_details =
Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
Collection::<T, I>::get(collection).ok_or(Error::<T, I>::UnknownCollection)?;

collection_details.attributes.saturating_dec();

Expand Down Expand Up @@ -381,7 +380,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Error::<T, I>::MethodDisabled
);

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

ItemAttributesApprovalsOf::<T, I>::try_mutate(collection, item, |approvals| {
Expand Down Expand Up @@ -422,7 +421,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Error::<T, I>::MethodDisabled
);

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

ItemAttributesApprovalsOf::<T, I>::try_mutate(collection, item, |approvals| {
Expand Down Expand Up @@ -463,17 +462,17 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let mut result = false;
match namespace {
AttributeNamespace::CollectionOwner =>
result = Self::has_role(&collection, &origin, CollectionRole::Admin),
result = Self::has_role(collection, origin, CollectionRole::Admin),
AttributeNamespace::ItemOwner =>
if let Some(item) = maybe_item {
let item_details =
Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownItem)?;
Item::<T, I>::get(collection, item).ok_or(Error::<T, I>::UnknownItem)?;
result = origin == &item_details.owner
},
AttributeNamespace::Account(account_id) =>
if let Some(item) = maybe_item {
let approvals = ItemAttributesApprovalsOf::<T, I>::get(&collection, &item);
result = account_id == origin && approvals.contains(&origin)
let approvals = ItemAttributesApprovalsOf::<T, I>::get(collection, item);
result = account_id == origin && approvals.contains(origin)
},
_ => (),
};
Expand Down
10 changes: 5 additions & 5 deletions pallets/nfts/src/features/buy_sell.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Error::<T, I>::MethodDisabled
);

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

let collection_config = Self::get_collection_config(&collection)?;
Expand All @@ -98,15 +98,15 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
);

if let Some(ref price) = price {
ItemPriceOf::<T, I>::insert(&collection, &item, (price, whitelisted_buyer.clone()));
ItemPriceOf::<T, I>::insert(collection, item, (price, whitelisted_buyer.clone()));
Self::deposit_event(Event::ItemPriceSet {
collection,
item,
price: *price,
whitelisted_buyer,
});
} else {
ItemPriceOf::<T, I>::remove(&collection, &item);
ItemPriceOf::<T, I>::remove(collection, item);
Self::deposit_event(Event::ItemPriceRemoved { collection, item });
}

Expand Down Expand Up @@ -137,11 +137,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Error::<T, I>::MethodDisabled
);

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

let price_info =
ItemPriceOf::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::NotForSale)?;
ItemPriceOf::<T, I>::get(collection, item).ok_or(Error::<T, I>::NotForSale)?;

ensure!(bid_price >= price_info.0, Error::<T, I>::BidTooLow);

Expand Down
14 changes: 7 additions & 7 deletions pallets/nfts/src/features/create_delete_collection.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
),
);

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 @@ -120,13 +120,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
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 @@ -137,10 +137,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}
}

CollectionAccount::<T, I>::remove(&collection_details.owner, &collection);
CollectionAccount::<T, I>::remove(&collection_details.owner, collection);
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

0 comments on commit 4a5f17c

Please sign in to comment.