Skip to content

Commit

Permalink
fix(nfts): clippy warnings (#391)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chungquantin
chungquantin authored Nov 22, 2024
1 parent aff6408 commit 3476994
Show file tree
Hide file tree
Showing 16 changed files with 185 additions and 183 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
5 changes: 3 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 All @@ -69,6 +69,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
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
43 changes: 20 additions & 23 deletions pallets/nfts/src/features/attributes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

let collection_config = Self::get_collection_config(&collection)?;
// for the `CollectionOwner` namespace we need to check if the collection/item is not locked
match namespace {
AttributeNamespace::CollectionOwner => match maybe_item {
if namespace == AttributeNamespace::CollectionOwner {
match maybe_item {
None => {
ensure!(
collection_config.is_setting_enabled(CollectionSetting::UnlockedAttributes),
Expand All @@ -82,12 +82,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
.map(|c| c.has_disabled_setting(ItemSetting::UnlockedAttributes))?;
ensure!(!maybe_is_locked, Error::<T, I>::LockedItemAttributes);
},
},
_ => (),
}
}

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 +182,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 +228,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 +237,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 +272,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,14 +296,14 @@ 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
);
}

// can't clear `CollectionOwner` type attributes if the collection/item is locked
match namespace {
AttributeNamespace::CollectionOwner => match maybe_item {
if namespace == AttributeNamespace::CollectionOwner {
match maybe_item {
None => {
let collection_config = Self::get_collection_config(&collection)?;
ensure!(
Expand All @@ -327,18 +325,17 @@ 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
);
}
},
},
_ => (),
}
};
}

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 +378,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 +419,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 +460,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
Loading

0 comments on commit 3476994

Please sign in to comment.