diff --git a/pallets/nfts/src/benchmarking.rs b/pallets/nfts/src/benchmarking.rs index d8876d52..9533bade 100644 --- a/pallets/nfts/src/benchmarking.rs +++ b/pallets/nfts/src/benchmarking.rs @@ -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, @@ -95,8 +93,8 @@ fn mint_item, I: 'static>( whitelist_account!(caller); } let caller_lookup = T::Lookup::unlookup(caller.clone()); - let item_exists = Item::::contains_key(&collection, &item); - let item_config = ItemConfigOf::::get(&collection, &item); + let item_exists = Item::::contains_key(collection, item); + let item_config = ItemConfigOf::::get(collection, item); if item_exists { return (item, caller, caller_lookup); } else if let Some(item_config) = item_config { @@ -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::::from(100u32); let caller: T::AccountId = whitelisted_caller(); let collection = T::Helper::collection(0); @@ -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::::max_value()); let caller_lookup = T::Lookup::unlookup(caller.clone()); @@ -871,14 +869,14 @@ benchmarks_instance_pallet! { let target: T::AccountId = account("target", 0, SEED); T::Currency::make_free_balance_be(&target, DepositBalanceOf::::max_value()); frame_system::Pallet::::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::(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::(); let item_owner: T::AccountId = account("item_owner", 0, SEED); @@ -914,7 +912,7 @@ benchmarks_instance_pallet! { let signature = T::Helper::sign(&signer_public, &message); frame_system::Pallet::::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::( Event::PreSignedAttributesSet { diff --git a/pallets/nfts/src/common_functions.rs b/pallets/nfts/src/common_functions.rs index 6158eab8..38a488b9 100644 --- a/pallets/nfts/src/common_functions.rs +++ b/pallets/nfts/src/common_functions.rs @@ -63,8 +63,8 @@ impl, I: 'static> Pallet { 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 @@ -76,7 +76,7 @@ impl, I: 'static> Pallet { wrapped.extend(data); wrapped.extend(suffix); - ensure!(signature.verify(&*wrapped, &signer), Error::::WrongSignature); + ensure!(signature.verify(&*wrapped, signer), Error::::WrongSignature); Ok(()) } @@ -87,6 +87,7 @@ impl, I: 'static> Pallet { 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::::set(Some(id)); diff --git a/pallets/nfts/src/features/approvals.rs b/pallets/nfts/src/features/approvals.rs index 3e3d5e63..a75ec57f 100644 --- a/pallets/nfts/src/features/approvals.rs +++ b/pallets/nfts/src/features/approvals.rs @@ -53,8 +53,7 @@ impl, I: 'static> Pallet { Self::is_pallet_feature_enabled(PalletFeature::Approvals), Error::::MethodDisabled ); - let mut details = - Item::::get(&collection, &item).ok_or(Error::::UnknownItem)?; + let mut details = Item::::get(collection, item).ok_or(Error::::UnknownItem)?; let collection_config = Self::get_collection_config(&collection)?; ensure!( @@ -72,7 +71,7 @@ impl, I: 'static> Pallet { .approvals .try_insert(delegate.clone(), deadline) .map_err(|_| Error::::ReachedApprovalLimit)?; - Item::::insert(&collection, &item, &details); + Item::::insert(collection, item, &details); Self::deposit_event(Event::TransferApproved { collection, item: Some(item), @@ -103,8 +102,7 @@ impl, I: 'static> Pallet { item: T::ItemId, delegate: T::AccountId, ) -> DispatchResult { - let mut details = - Item::::get(&collection, &item).ok_or(Error::::UnknownItem)?; + let mut details = Item::::get(collection, item).ok_or(Error::::UnknownItem)?; let maybe_deadline = details.approvals.get(&delegate).ok_or(Error::::NotDelegate)?; @@ -122,7 +120,7 @@ impl, I: 'static> Pallet { } details.approvals.remove(&delegate); - Item::::insert(&collection, &item, &details); + Item::::insert(collection, item, &details); Self::deposit_event(Event::ApprovalCancelled { collection, @@ -153,14 +151,14 @@ impl, I: 'static> Pallet { item: T::ItemId, ) -> DispatchResult { let mut details = - Item::::get(&collection, &item).ok_or(Error::::UnknownCollection)?; + Item::::get(collection, item).ok_or(Error::::UnknownCollection)?; if let Some(check_origin) = maybe_check_origin { ensure!(check_origin == details.owner, Error::::NoPermission); } details.approvals.clear(); - Item::::insert(&collection, &item, &details); + Item::::insert(collection, item, &details); Self::deposit_event(Event::AllApprovalsCancelled { collection, diff --git a/pallets/nfts/src/features/atomic_swap.rs b/pallets/nfts/src/features/atomic_swap.rs index 31c93fba..6c15f15a 100644 --- a/pallets/nfts/src/features/atomic_swap.rs +++ b/pallets/nfts/src/features/atomic_swap.rs @@ -62,17 +62,17 @@ impl, I: 'static> Pallet { ); ensure!(duration <= T::MaxDeadlineDuration::get(), Error::::WrongDuration); - let item = Item::::get(&offered_collection_id, &offered_item_id) + let item = Item::::get(offered_collection_id, offered_item_id) .ok_or(Error::::UnknownItem)?; ensure!(item.owner == caller, Error::::NoPermission); match maybe_desired_item_id { Some(desired_item_id) => ensure!( - Item::::contains_key(&desired_collection_id, &desired_item_id), + Item::::contains_key(desired_collection_id, desired_item_id), Error::::UnknownItem ), None => ensure!( - Collection::::contains_key(&desired_collection_id), + Collection::::contains_key(desired_collection_id), Error::::UnknownCollection ), }; @@ -81,8 +81,8 @@ impl, I: 'static> Pallet { let deadline = duration.saturating_add(now); PendingSwapOf::::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, @@ -118,17 +118,17 @@ impl, I: 'static> Pallet { offered_collection_id: T::CollectionId, offered_item_id: T::ItemId, ) -> DispatchResult { - let swap = PendingSwapOf::::get(&offered_collection_id, &offered_item_id) + let swap = PendingSwapOf::::get(offered_collection_id, offered_item_id) .ok_or(Error::::UnknownSwap)?; let now = frame_system::Pallet::::block_number(); if swap.deadline > now { - let item = Item::::get(&offered_collection_id, &offered_item_id) + let item = Item::::get(offered_collection_id, offered_item_id) .ok_or(Error::::UnknownItem)?; ensure!(item.owner == caller, Error::::NoPermission); } - PendingSwapOf::::remove(&offered_collection_id, &offered_item_id); + PendingSwapOf::::remove(offered_collection_id, offered_item_id); Self::deposit_event(Event::SwapCancelled { offered_collection: offered_collection_id, @@ -172,11 +172,11 @@ impl, I: 'static> Pallet { Error::::MethodDisabled ); - let send_item = Item::::get(&send_collection_id, &send_item_id) + let send_item = Item::::get(send_collection_id, send_item_id) .ok_or(Error::::UnknownItem)?; - let receive_item = Item::::get(&receive_collection_id, &receive_item_id) + let receive_item = Item::::get(receive_collection_id, receive_item_id) .ok_or(Error::::UnknownItem)?; - let swap = PendingSwapOf::::get(&receive_collection_id, &receive_item_id) + let swap = PendingSwapOf::::get(receive_collection_id, receive_item_id) .ok_or(Error::::UnknownSwap)?; ensure!(send_item.owner == caller, Error::::NoPermission); diff --git a/pallets/nfts/src/features/create_delete_collection.rs b/pallets/nfts/src/features/create_delete_collection.rs index 1d08cd1b..9be73a3b 100644 --- a/pallets/nfts/src/features/create_delete_collection.rs +++ b/pallets/nfts/src/features/create_delete_collection.rs @@ -67,8 +67,8 @@ impl, I: 'static> Pallet { ), ); - CollectionConfigOf::::insert(&collection, config); - CollectionAccount::::insert(&owner, &collection, ()); + CollectionConfigOf::::insert(collection, config); + CollectionAccount::::insert(&owner, collection, ()); Self::deposit_event(event); @@ -127,13 +127,13 @@ impl, I: 'static> Pallet { ); ensure!(collection_details.allowances == witness.allowances, Error::::BadWitness); - for (_, metadata) in ItemMetadataOf::::drain_prefix(&collection) { + for (_, metadata) in ItemMetadataOf::::drain_prefix(collection) { if let Some(depositor) = metadata.deposit.account { T::Currency::unreserve(&depositor, metadata.deposit.amount); } } - CollectionMetadataOf::::remove(&collection); + CollectionMetadataOf::::remove(collection); Self::clear_roles(&collection)?; for (_, (_, deposit)) in Attribute::::drain_prefix((&collection,)) { @@ -149,8 +149,8 @@ impl, I: 'static> Pallet { let _ = Allowances::::clear_prefix((collection,), collection_details.items, None); CollectionAccount::::remove(&collection_details.owner, &collection); T::Currency::unreserve(&collection_details.owner, collection_details.owner_deposit); - CollectionConfigOf::::remove(&collection); - let _ = ItemConfigOf::::clear_prefix(&collection, witness.item_configs, None); + CollectionConfigOf::::remove(collection); + let _ = ItemConfigOf::::clear_prefix(collection, witness.item_configs, None); Self::deposit_event(Event::Destroyed { collection }); diff --git a/pallets/nfts/src/features/create_delete_item.rs b/pallets/nfts/src/features/create_delete_item.rs index 08cf5f95..aed2c204 100644 --- a/pallets/nfts/src/features/create_delete_item.rs +++ b/pallets/nfts/src/features/create_delete_item.rs @@ -55,64 +55,60 @@ impl, I: 'static> Pallet { ) -> DispatchResult { ensure!(!Item::::contains_key(collection, item), Error::::AlreadyExists); - Collection::::try_mutate( - &collection, - |maybe_collection_details| -> DispatchResult { - let collection_details = - maybe_collection_details.as_mut().ok_or(Error::::UnknownCollection)?; + Collection::::try_mutate(collection, |maybe_collection_details| -> DispatchResult { + let collection_details = + maybe_collection_details.as_mut().ok_or(Error::::UnknownCollection)?; - let collection_config = Self::get_collection_config(&collection)?; - with_details_and_config(collection_details, &collection_config)?; + let collection_config = Self::get_collection_config(&collection)?; + with_details_and_config(collection_details, &collection_config)?; - if let Some(max_supply) = collection_config.max_supply { - ensure!(collection_details.items < max_supply, Error::::MaxSupplyReached); - } + if let Some(max_supply) = collection_config.max_supply { + ensure!(collection_details.items < max_supply, Error::::MaxSupplyReached); + } - collection_details.items.saturating_inc(); + collection_details.items.saturating_inc(); - let account_balance = - AccountBalance::::mutate(collection, &mint_to, |balance| -> u32 { - balance.saturating_inc(); - *balance - }); - if account_balance == 1 { - collection_details.item_holders.saturating_inc(); - } + let account_balance = + AccountBalance::::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) - { + let collection_config = Self::get_collection_config(&collection)?; + let deposit_amount = + match collection_config.is_setting_enabled(CollectionSetting::DepositRequired) { true => T::ItemDeposit::get(), false => Zero::zero(), }; - let deposit_account = match maybe_depositor { - None => collection_details.owner.clone(), - Some(depositor) => depositor, - }; - - let item_owner = mint_to.clone(); - Account::::insert((&item_owner, &collection, &item), ()); - - if let Ok(existing_config) = ItemConfigOf::::try_get(&collection, &item) { - ensure!(existing_config == item_config, Error::::InconsistentItemConfig); - } else { - ItemConfigOf::::insert(&collection, &item, item_config); - collection_details.item_configs.saturating_inc(); - } + let deposit_account = match maybe_depositor { + None => collection_details.owner.clone(), + Some(depositor) => depositor, + }; + + let item_owner = mint_to.clone(); + Account::::insert((&item_owner, &collection, &item), ()); + + if let Ok(existing_config) = ItemConfigOf::::try_get(collection, item) { + ensure!(existing_config == item_config, Error::::InconsistentItemConfig); + } else { + ItemConfigOf::::insert(collection, item, item_config); + collection_details.item_configs.saturating_inc(); + } - T::Currency::reserve(&deposit_account, deposit_amount)?; + T::Currency::reserve(&deposit_account, deposit_amount)?; - let deposit = ItemDeposit { account: deposit_account, amount: deposit_amount }; - let details = ItemDetails { - owner: item_owner, - approvals: ApprovalsOf::::default(), - deposit, - }; - Item::::insert(&collection, &item, details); - Ok(()) - }, - )?; + let deposit = ItemDeposit { account: deposit_account, amount: deposit_amount }; + let details = ItemDetails { + owner: item_owner, + approvals: ApprovalsOf::::default(), + deposit, + }; + Item::::insert(collection, item, details); + Ok(()) + })?; Self::deposit_event(Event::Issued { collection, item, owner: mint_to }); Ok(()) @@ -230,12 +226,12 @@ impl, I: 'static> Pallet { // then we keep the config record and don't remove it let remove_config = !item_config.has_disabled_settings(); let owner = Collection::::try_mutate( - &collection, + collection, |maybe_collection_details| -> Result { let collection_details = maybe_collection_details.as_mut().ok_or(Error::::UnknownCollection)?; - let details = Item::::get(&collection, &item) - .ok_or(Error::::UnknownCollection)?; + let details = + Item::::get(collection, item).ok_or(Error::::UnknownCollection)?; with_details(&details)?; // Return the deposit. @@ -248,7 +244,7 @@ impl, I: 'static> Pallet { // Clear the metadata if it's not locked. if item_config.is_setting_enabled(ItemSetting::UnlockedMetadata) { - if let Some(metadata) = ItemMetadataOf::::take(&collection, &item) { + if let Some(metadata) = ItemMetadataOf::::take(collection, item) { let depositor_account = metadata.deposit.account.unwrap_or(collection_details.owner.clone()); @@ -271,7 +267,7 @@ impl, I: 'static> Pallet { }, )?; - Item::::remove(&collection, &item); + Item::::remove(collection, item); Account::::remove((&owner, &collection, &item)); ItemPriceOf::::remove(&collection, &item); PendingSwapOf::::remove(&collection, &item); @@ -281,7 +277,7 @@ impl, I: 'static> Pallet { }); if remove_config { - ItemConfigOf::::remove(&collection, &item); + ItemConfigOf::::remove(collection, item); } Self::deposit_event(Event::Burned { collection, item, owner }); diff --git a/pallets/nfts/src/features/transfer.rs b/pallets/nfts/src/features/transfer.rs index 3f2dae3b..af34c471 100644 --- a/pallets/nfts/src/features/transfer.rs +++ b/pallets/nfts/src/features/transfer.rs @@ -81,8 +81,7 @@ impl, I: 'static> Pallet { ); // Retrieve the item details. - let mut details = - Item::::get(&collection, &item).ok_or(Error::::UnknownItem)?; + let mut details = Item::::get(collection, item).ok_or(Error::::UnknownItem)?; // Perform the transfer with custom details using the provided closure. with_details(&collection_details, &mut details)?; @@ -117,10 +116,10 @@ impl, I: 'static> Pallet { details.approvals.clear(); // Update item details. - Item::::insert(&collection, &item, &details); - Collection::::insert(&collection, &collection_details); - ItemPriceOf::::remove(&collection, &item); - PendingSwapOf::::remove(&collection, &item); + Item::::insert(collection, item, &details); + Collection::::insert(collection, &collection_details); + ItemPriceOf::::remove(collection, item); + PendingSwapOf::::remove(collection, item); // Emit `Transferred` event. Self::deposit_event(Event::Transferred { @@ -168,8 +167,8 @@ impl, I: 'static> Pallet { )?; // Update account ownership information. - CollectionAccount::::remove(&details.owner, &collection); - CollectionAccount::::insert(&new_owner, &collection, ()); + CollectionAccount::::remove(&details.owner, collection); + CollectionAccount::::insert(&new_owner, collection, ()); details.owner = new_owner.clone(); OwnershipAcceptance::::remove(&new_owner); @@ -243,8 +242,8 @@ impl, I: 'static> Pallet { )?; // Update collection accounts and set the new owner. - CollectionAccount::::remove(&details.owner, &collection); - CollectionAccount::::insert(&owner, &collection, ()); + CollectionAccount::::remove(&details.owner, collection); + CollectionAccount::::insert(&owner, collection, ()); details.owner = owner.clone(); // Emit `OwnerChanged` event. diff --git a/pallets/nfts/src/impl_nonfungibles.rs b/pallets/nfts/src/impl_nonfungibles.rs index 362cccd9..0c54f0b7 100644 --- a/pallets/nfts/src/impl_nonfungibles.rs +++ b/pallets/nfts/src/impl_nonfungibles.rs @@ -120,20 +120,15 @@ impl, I: 'static> Inspect<::AccountId> for Palle /// Default implementation is that all items are transferable. fn can_transfer(collection: &Self::CollectionId, item: &Self::ItemId) -> bool { use PalletAttributes::TransferDisabled; - match Self::has_system_attribute(&collection, &item, TransferDisabled) { + match Self::has_system_attribute(collection, item, TransferDisabled) { Ok(transfer_disabled) if transfer_disabled => return false, _ => (), } - match ( - CollectionConfigOf::::get(collection), - ItemConfigOf::::get(collection, item), - ) { - (Some(cc), Some(ic)) - if cc.is_setting_enabled(CollectionSetting::TransferableItems) && - ic.is_setting_enabled(ItemSetting::Transferable) => - true, - _ => false, - } + matches!( + (CollectionConfigOf::::get(collection), ItemConfigOf::::get(collection, item)), + (Some(cc), Some(ic)) if cc.is_setting_enabled(CollectionSetting::TransferableItems) + && ic.is_setting_enabled(ItemSetting::Transferable) + ) } } @@ -259,7 +254,7 @@ impl, I: 'static> Mutate<::AccountId, ItemConfig Self::do_burn(*collection, *item, |d| { if let Some(check_owner) = maybe_check_owner { if &d.owner != check_owner { - return Err(Error::::NoPermission.into()) + return Err(Error::::NoPermission.into()); } } Ok(()) @@ -421,10 +416,10 @@ impl, I: 'static> Transfer for Pallet { fn disable_transfer(collection: &Self::CollectionId, item: &Self::ItemId) -> DispatchResult { let transfer_disabled = - Self::has_system_attribute(&collection, &item, PalletAttributes::TransferDisabled)?; + Self::has_system_attribute(collection, item, PalletAttributes::TransferDisabled)?; // Can't lock the item twice if transfer_disabled { - return Err(Error::::ItemLocked.into()) + return Err(Error::::ItemLocked.into()); } >::set_attribute( diff --git a/pallets/nfts/src/lib.rs b/pallets/nfts/src/lib.rs index 37e8b29c..e86410c5 100644 --- a/pallets/nfts/src/lib.rs +++ b/pallets/nfts/src/lib.rs @@ -44,12 +44,14 @@ mod features; mod impl_nonfungibles; mod types; +#[allow(missing_docs)] pub mod macros; pub mod weights; extern crate alloc; use alloc::{boxed::Box, vec, vec::Vec}; +use core::cmp::Ordering; use codec::{Decode, Encode}; use frame_support::traits::{ @@ -66,7 +68,7 @@ pub use types::*; pub use weights::WeightInfo; /// The log target of this pallet. -pub const LOG_TARGET: &'static str = "runtime::nfts"; +pub const LOG_TARGET: &str = "runtime::nfts"; /// A type alias for the account ID type used in the dispatchable functions of this pallet. type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; @@ -86,6 +88,7 @@ pub mod pallet { pub struct Pallet(PhantomData<(T, I)>); #[cfg(feature = "runtime-benchmarks")] + #[allow(missing_docs)] pub trait BenchmarkHelper { fn collection(i: u16) -> CollectionId; fn item(i: u16) -> ItemId; @@ -443,6 +446,7 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] + #[allow(missing_docs)] pub enum Event, I: 'static = ()> { /// A `collection` was created. Created { collection: T::CollectionId, creator: T::AccountId, owner: T::AccountId }, @@ -1094,7 +1098,7 @@ pub mod pallet { let origin = ensure_signed(origin)?; let collection_details = - Collection::::get(&collection).ok_or(Error::::UnknownCollection)?; + Collection::::get(collection).ok_or(Error::::UnknownCollection)?; ensure!(collection_details.owner == origin, Error::::NoPermission); let config = Self::get_collection_config(&collection)?; @@ -1105,24 +1109,26 @@ pub mod pallet { let mut successful = Vec::with_capacity(items.len()); for item in items.into_iter() { - let mut details = match Item::::get(&collection, &item) { + let mut details = match Item::::get(collection, item) { Some(x) => x, None => continue, }; let old = details.deposit.amount; - if old > deposit { - T::Currency::unreserve(&details.deposit.account, old - deposit); - } else if deposit > old { - if T::Currency::reserve(&details.deposit.account, deposit - old).is_err() { - // NOTE: No alterations made to collection_details in this iteration so far, - // so this is OK to do. - continue; - } - } else { - continue; + match old.cmp(&deposit) { + Ordering::Greater => { + T::Currency::unreserve(&details.deposit.account, old - deposit); + }, + Ordering::Less => { + if T::Currency::reserve(&details.deposit.account, deposit - old).is_err() { + // NOTE: No alterations made to collection_details in this iteration so + // far, so this is OK to do. + continue; + } + }, + _ => continue, } details.deposit.amount = deposit; - Item::::insert(&collection, &item, &details); + Item::::insert(collection, item, &details); successful.push(item); }