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.

Sign up for GitHub

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): clippy warnings #391

Merged
merged 3 commits into from
Nov 22, 2024
Merged

fix(nfts): clippy warnings #391

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a5f17c
fix(nfts): clippy warnings
chungquantin Nov 21, 2024
37ec9ab
fix: clippy warnings
chungquantin Nov 21, 2024
3e886a9
fix: warnings
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
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
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 @@
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 @@ -70,7 +70,7 @@
}

#[cfg(any(test, feature = "runtime-benchmarks"))]
pub fn set_next_id(id: T::CollectionId) {

Check warning on line 73 in pallets/nfts/src/common_functions.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/common_functions.rs:73:2
   |
73 |     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
Open in desktop
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
Open in desktop
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
Open in desktop
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);
},
},
_ => (),
}
}

chungquantin marked this conversation as resolved.
Show resolved Hide resolved
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
);
}
},
},
_ => (),
}
};
}
chungquantin marked this conversation as resolved.
Show resolved Hide resolved

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
Open in desktop
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
Loading