From d40d7f1666a35ecc2d2ed0afb4228956c94cce92 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Fri, 6 Oct 2023 08:13:45 +0200 Subject: [PATCH 1/8] Preparations --- .../frame/nfts/src/features/atomic_swap.rs | 9 +-- .../frame/nfts/src/features/attributes.rs | 50 ++++++++++++++--- substrate/frame/nfts/src/features/buy_sell.rs | 14 +---- .../src/features/create_delete_collection.rs | 23 ++++++-- .../nfts/src/features/create_delete_item.rs | 29 ++++++---- substrate/frame/nfts/src/features/metadata.rs | 49 +++++++++++++--- substrate/frame/nfts/src/features/transfer.rs | 8 +-- substrate/frame/nfts/src/lib.rs | 56 +++++++++++++++---- substrate/frame/nfts/src/mock.rs | 11 ++-- substrate/frame/nfts/src/types.rs | 6 +- 10 files changed, 184 insertions(+), 71 deletions(-) diff --git a/substrate/frame/nfts/src/features/atomic_swap.rs b/substrate/frame/nfts/src/features/atomic_swap.rs index 830283b73c2a..ff5e39acef4f 100644 --- a/substrate/frame/nfts/src/features/atomic_swap.rs +++ b/substrate/frame/nfts/src/features/atomic_swap.rs @@ -21,10 +21,7 @@ //! to have the functionality defined in this module. use crate::*; -use frame_support::{ - pallet_prelude::*, - traits::{Currency, ExistenceRequirement::KeepAlive}, -}; +use frame_support::pallet_prelude::*; impl, I: 'static> Pallet { /// Creates a new swap offer for the specified item. @@ -196,13 +193,13 @@ impl, I: 'static> Pallet { &receive_item.owner, &send_item.owner, price.amount, - KeepAlive, + Preserve, )?, PriceDirection::Receive => T::Currency::transfer( &send_item.owner, &receive_item.owner, price.amount, - KeepAlive, + Preserve, )?, }; } diff --git a/substrate/frame/nfts/src/features/attributes.rs b/substrate/frame/nfts/src/features/attributes.rs index 28f7bd2c58ce..0d67c87920b2 100644 --- a/substrate/frame/nfts/src/features/attributes.rs +++ b/substrate/frame/nfts/src/features/attributes.rs @@ -126,13 +126,27 @@ impl, I: 'static> Pallet { // and return the deposit to the previous owner. if depositor_has_changed { if let Some(old_depositor) = old_depositor { - T::Currency::unreserve(&old_depositor, old_deposit.amount); + T::Currency::release( + &HoldReason::AttributeDeposit.into(), + &old_depositor, + old_deposit.amount, + BestEffort, + )?; } - T::Currency::reserve(&depositor, deposit)?; + T::Currency::hold(&HoldReason::AttributeDeposit.into(), &depositor, deposit)?; } else if deposit > old_deposit.amount { - T::Currency::reserve(&depositor, deposit - old_deposit.amount)?; + T::Currency::hold( + &HoldReason::AttributeDeposit.into(), + &depositor, + deposit - old_deposit.amount, + )?; } else if deposit < old_deposit.amount { - T::Currency::unreserve(&depositor, old_deposit.amount - deposit); + T::Currency::release( + &HoldReason::AttributeDeposit.into(), + &depositor, + old_deposit.amount - deposit, + BestEffort, + )?; } if is_depositor_collection_owner { @@ -188,7 +202,12 @@ impl, I: 'static> Pallet { if let Some((_, deposit)) = attribute { if deposit.account != set_as && deposit.amount != Zero::zero() { if let Some(deposit_account) = deposit.account { - T::Currency::unreserve(&deposit_account, deposit.amount); + T::Currency::release( + &HoldReason::AttributeDeposit.into(), + &deposit_account, + deposit.amount, + BestEffort, + )?; } } } else { @@ -343,11 +362,21 @@ impl, I: 'static> Pallet { match deposit.account { Some(deposit_account) => { - T::Currency::unreserve(&deposit_account, deposit.amount); + T::Currency::release( + &HoldReason::AttributeDeposit.into(), + &deposit_account, + deposit.amount, + BestEffort, + )?; }, None if namespace == AttributeNamespace::CollectionOwner => { collection_details.owner_deposit.saturating_reduce(deposit.amount); - T::Currency::unreserve(&collection_details.owner, deposit.amount); + T::Currency::release( + &HoldReason::AttributeDeposit.into(), + &collection_details.owner, + deposit.amount, + BestEffort, + )?; }, _ => (), } @@ -440,7 +469,12 @@ impl, I: 'static> Pallet { ensure!(attributes <= witness.account_attributes, Error::::BadWitness); if !deposited.is_zero() { - T::Currency::unreserve(&delegate, deposited); + T::Currency::release( + &HoldReason::AttributeDeposit.into(), + &delegate, + deposited, + BestEffort, + )?; } Self::deposit_event(Event::ItemAttributesApprovalRemoved { diff --git a/substrate/frame/nfts/src/features/buy_sell.rs b/substrate/frame/nfts/src/features/buy_sell.rs index d6ec6f50d272..7355edd48cff 100644 --- a/substrate/frame/nfts/src/features/buy_sell.rs +++ b/substrate/frame/nfts/src/features/buy_sell.rs @@ -21,10 +21,7 @@ //! to have the functionality defined in this module. use crate::*; -use frame_support::{ - pallet_prelude::*, - traits::{Currency, ExistenceRequirement, ExistenceRequirement::KeepAlive}, -}; +use frame_support::pallet_prelude::*; impl, I: 'static> Pallet { /// Pays the specified tips to the corresponding receivers. @@ -43,7 +40,7 @@ impl, I: 'static> Pallet { ) -> DispatchResult { for tip in tips { let ItemTip { collection, item, receiver, amount } = tip; - T::Currency::transfer(&sender, &receiver, amount, KeepAlive)?; + T::Currency::transfer(&sender, &receiver, amount, Preserve)?; Self::deposit_event(Event::TipSent { collection, item, @@ -148,12 +145,7 @@ impl, I: 'static> Pallet { ensure!(only_buyer == buyer, Error::::NoPermission); } - T::Currency::transfer( - &buyer, - &details.owner, - price_info.0, - ExistenceRequirement::KeepAlive, - )?; + T::Currency::transfer(&buyer, &details.owner, price_info.0, Preserve)?; let old_owner = details.owner.clone(); diff --git a/substrate/frame/nfts/src/features/create_delete_collection.rs b/substrate/frame/nfts/src/features/create_delete_collection.rs index e343ad18e504..bb4e734c2f47 100644 --- a/substrate/frame/nfts/src/features/create_delete_collection.rs +++ b/substrate/frame/nfts/src/features/create_delete_collection.rs @@ -43,7 +43,7 @@ impl, I: 'static> Pallet { ) -> DispatchResult { ensure!(!Collection::::contains_key(collection), Error::::CollectionIdInUse); - T::Currency::reserve(&owner, deposit)?; + T::Currency::hold(&HoldReason::CollectionOwnerDeposit.into(), &owner, deposit)?; Collection::::insert( collection, @@ -115,7 +115,12 @@ impl, I: 'static> Pallet { for (_, metadata) in ItemMetadataOf::::drain_prefix(&collection) { if let Some(depositor) = metadata.deposit.account { - T::Currency::unreserve(&depositor, metadata.deposit.amount); + T::Currency::release( + &HoldReason::MetadataDeposit.into(), + &depositor, + metadata.deposit.amount, + BestEffort, + )?; } } @@ -125,13 +130,23 @@ impl, I: 'static> Pallet { for (_, (_, deposit)) in Attribute::::drain_prefix((&collection,)) { if !deposit.amount.is_zero() { if let Some(account) = deposit.account { - T::Currency::unreserve(&account, deposit.amount); + T::Currency::release( + &HoldReason::AttributeDeposit.into(), + &account, + deposit.amount, + BestEffort, + )?; } } } CollectionAccount::::remove(&collection_details.owner, &collection); - T::Currency::unreserve(&collection_details.owner, collection_details.owner_deposit); + T::Currency::release( + &HoldReason::CollectionOwnerDeposit.into(), + &collection_details.owner, + collection_details.owner_deposit, + BestEffort, + )?; CollectionConfigOf::::remove(&collection); let _ = ItemConfigOf::::clear_prefix(&collection, witness.item_configs, None); diff --git a/substrate/frame/nfts/src/features/create_delete_item.rs b/substrate/frame/nfts/src/features/create_delete_item.rs index 37f64ae1b1b9..798ec749566d 100644 --- a/substrate/frame/nfts/src/features/create_delete_item.rs +++ b/substrate/frame/nfts/src/features/create_delete_item.rs @@ -19,7 +19,7 @@ //! items for the NFTs pallet. use crate::*; -use frame_support::{pallet_prelude::*, traits::ExistenceRequirement}; +use frame_support::pallet_prelude::*; impl, I: 'static> Pallet { /// Mint a new unique item with the given `collection`, `item`, and other minting configuration @@ -91,7 +91,11 @@ impl, I: 'static> Pallet { collection_details.item_configs.saturating_inc(); } - T::Currency::reserve(&deposit_account, deposit_amount)?; + T::Currency::hold( + &HoldReason::ItemDeposit.into(), + &deposit_account, + deposit_amount, + )?; let deposit = ItemDeposit { account: deposit_account, amount: deposit_amount }; let details = ItemDetails { @@ -162,12 +166,7 @@ impl, I: 'static> Pallet { item_config, |collection_details, _| { if let Some(price) = mint_price { - T::Currency::transfer( - &mint_to, - &collection_details.owner, - price, - ExistenceRequirement::KeepAlive, - )?; + T::Currency::transfer(&mint_to, &collection_details.owner, price, Preserve)?; } Ok(()) }, @@ -229,7 +228,12 @@ impl, I: 'static> Pallet { with_details(&details)?; // Return the deposit. - T::Currency::unreserve(&details.deposit.account, details.deposit.amount); + T::Currency::release( + &HoldReason::ItemDeposit.into(), + &details.deposit.account, + details.deposit.amount, + BestEffort, + )?; collection_details.items.saturating_dec(); if remove_config { @@ -242,7 +246,12 @@ impl, I: 'static> Pallet { let depositor_account = metadata.deposit.account.unwrap_or(collection_details.owner.clone()); - T::Currency::unreserve(&depositor_account, metadata.deposit.amount); + T::Currency::release( + &HoldReason::MetadataDeposit.into(), + &depositor_account, + metadata.deposit.amount, + BestEffort, + )?; collection_details.item_metadatas.saturating_dec(); if depositor_account == collection_details.owner { diff --git a/substrate/frame/nfts/src/features/metadata.rs b/substrate/frame/nfts/src/features/metadata.rs index e177f39bb8b8..48280aab34c0 100644 --- a/substrate/frame/nfts/src/features/metadata.rs +++ b/substrate/frame/nfts/src/features/metadata.rs @@ -85,12 +85,26 @@ impl, I: 'static> Pallet { let old_depositor = old_deposit.account.unwrap_or(collection_details.owner.clone()); if depositor != old_depositor { - T::Currency::unreserve(&old_depositor, old_deposit.amount); - T::Currency::reserve(&depositor, deposit)?; + T::Currency::release( + &HoldReason::MetadataDeposit.into(), + &old_depositor, + old_deposit.amount, + BestEffort, + )?; + T::Currency::hold(&HoldReason::MetadataDeposit.into(), &depositor, deposit)?; } else if deposit > old_deposit.amount { - T::Currency::reserve(&depositor, deposit - old_deposit.amount)?; + T::Currency::hold( + &HoldReason::MetadataDeposit.into(), + &depositor, + deposit - old_deposit.amount, + )?; } else if deposit < old_deposit.amount { - T::Currency::unreserve(&depositor, old_deposit.amount - deposit); + T::Currency::release( + &HoldReason::MetadataDeposit.into(), + &depositor, + old_deposit.amount - deposit, + BestEffort, + )?; } if maybe_depositor.is_none() { @@ -150,7 +164,12 @@ impl, I: 'static> Pallet { ensure!(is_root || !is_locked, Error::::LockedItemMetadata); collection_details.item_metadatas.saturating_dec(); - T::Currency::unreserve(&depositor_account, metadata.deposit.amount); + T::Currency::release( + &HoldReason::MetadataDeposit.into(), + &depositor_account, + metadata.deposit.amount, + BestEffort, + )?; if depositor_account == collection_details.owner { collection_details.owner_deposit.saturating_reduce(metadata.deposit.amount); @@ -208,9 +227,18 @@ impl, I: 'static> Pallet { .saturating_add(T::MetadataDepositBase::get()); } if deposit > old_deposit { - T::Currency::reserve(&details.owner, deposit - old_deposit)?; + T::Currency::hold( + &HoldReason::MetadataDeposit.into(), + &details.owner, + deposit - old_deposit, + )?; } else if deposit < old_deposit { - T::Currency::unreserve(&details.owner, old_deposit - deposit); + T::Currency::release( + &HoldReason::MetadataDeposit.into(), + &details.owner, + old_deposit - deposit, + BestEffort, + )?; } details.owner_deposit.saturating_accrue(deposit); @@ -259,7 +287,12 @@ impl, I: 'static> Pallet { CollectionMetadataOf::::try_mutate_exists(collection, |metadata| { let deposit = metadata.take().ok_or(Error::::UnknownCollection)?.deposit; - T::Currency::unreserve(&details.owner, deposit); + T::Currency::release( + &HoldReason::MetadataDeposit.into(), + &details.owner, + deposit, + BestEffort, + )?; Self::deposit_event(Event::CollectionMetadataCleared { collection }); Ok(()) }) diff --git a/substrate/frame/nfts/src/features/transfer.rs b/substrate/frame/nfts/src/features/transfer.rs index 0471bd67b291..1d57484a9740 100644 --- a/substrate/frame/nfts/src/features/transfer.rs +++ b/substrate/frame/nfts/src/features/transfer.rs @@ -140,12 +140,12 @@ impl, I: 'static> Pallet { } // Move the deposit to the new owner. - T::Currency::repatriate_reserved( + /*T::Currency::repatriate_reserved( &details.owner, &owner, details.owner_deposit, Reserved, - )?; + )?;*/ // Update account ownership information. CollectionAccount::::remove(&details.owner, &collection); @@ -213,12 +213,12 @@ impl, I: 'static> Pallet { } // Move the deposit to the new owner. - T::Currency::repatriate_reserved( + /*T::Currency::repatriate_reserved( &details.owner, &owner, details.owner_deposit, Reserved, - )?; + )?;*/ // Update collection accounts and set the new owner. CollectionAccount::::remove(&details.owner, &collection); diff --git a/substrate/frame/nfts/src/lib.rs b/substrate/frame/nfts/src/lib.rs index 92b27432ab21..f8bec62801ed 100644 --- a/substrate/frame/nfts/src/lib.rs +++ b/substrate/frame/nfts/src/lib.rs @@ -50,8 +50,12 @@ pub mod weights; use codec::{Decode, Encode}; use frame_support::traits::{ - tokens::Locker, BalanceStatus::Reserved, Currency, EnsureOriginWithArg, Incrementable, - ReservableCurrency, + fungible::{ + hold::Mutate as HoldMutateFungible, Inspect as InspectFungible, Mutate as MutateFungible, + }, + tokens::{Locker, Precision::BestEffort, Preservation::Preserve}, + BalanceStatus::Reserved, + EnsureOriginWithArg, Incrementable, }; use frame_system::Config as SystemConfig; use sp_runtime::{ @@ -73,7 +77,7 @@ type AccountIdLookupOf = <::Lookup as StaticLookup>::Sourc #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::{pallet_prelude::*, traits::ExistenceRequirement}; + use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; /// The current storage version. @@ -120,7 +124,12 @@ pub mod pallet { type ItemId: Member + Parameter + MaxEncodedLen + Copy; /// The currency mechanism, used for paying for reserves. - type Currency: ReservableCurrency; + type Currency: InspectFungible + + MutateFungible + + HoldMutateFungible; + + /// Overarching hold reason. + type RuntimeHoldReason: From; /// The origin which may forcibly create or destroy an item or otherwise alter privileged /// attributes. @@ -654,6 +663,23 @@ pub mod pallet { WitnessRequired, } + /// A reason for the pallet placing a hold on funds. + #[pallet::composite_enum] + pub enum HoldReason { + /// Reserved for a collection's storage record. + #[codec(index = 0)] + CollectionOwnerDeposit, + /// Reserved for an item's storage record. + #[codec(index = 1)] + ItemDeposit, + /// Reserved for metadata storage. + #[codec(index = 2)] + MetadataDeposit, + /// Reserved for attribute storage. + #[codec(index = 3)] + AttributeDeposit, + } + #[pallet::call] impl, I: 'static> Pallet { /// Issue a new collection of non-fungible items from a public origin. @@ -888,12 +914,7 @@ pub mod pallet { witness_data.clone().ok_or(Error::::WitnessRequired)?; let mint_price = mint_price.ok_or(Error::::BadWitness)?; ensure!(mint_price >= price, Error::::BadWitness); - T::Currency::transfer( - &caller, - &collection_details.owner, - price, - ExistenceRequirement::KeepAlive, - )?; + T::Currency::transfer(&caller, &collection_details.owner, price, Preserve)?; } Ok(()) @@ -1049,9 +1070,20 @@ pub mod pallet { }; let old = details.deposit.amount; if old > deposit { - T::Currency::unreserve(&details.deposit.account, old - deposit); + T::Currency::release( + &HoldReason::CollectionOwnerDeposit.into(), + &details.deposit.account, + old - deposit, + BestEffort, + )?; } else if deposit > old { - if T::Currency::reserve(&details.deposit.account, deposit - old).is_err() { + if T::Currency::hold( + &HoldReason::CollectionOwnerDeposit.into(), + &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 diff --git a/substrate/frame/nfts/src/mock.rs b/substrate/frame/nfts/src/mock.rs index f091a53f8d7c..1c86f9cd41e9 100644 --- a/substrate/frame/nfts/src/mock.rs +++ b/substrate/frame/nfts/src/mock.rs @@ -36,9 +36,9 @@ type Block = frame_system::mocking::MockBlock; construct_runtime!( pub enum Test { - System: frame_system::{Pallet, Call, Config, Storage, Event}, - Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - Nfts: pallet_nfts::{Pallet, Call, Storage, Event}, + System: frame_system, + Balances: pallet_balances, + Nfts: pallet_nfts, } ); @@ -84,8 +84,8 @@ impl pallet_balances::Config for Test { type ReserveIdentifier = [u8; 8]; type FreezeIdentifier = (); type MaxFreezes = (); - type RuntimeHoldReason = (); - type MaxHolds = (); + type RuntimeHoldReason = RuntimeHoldReason; + type MaxHolds = ConstU32<1>; } parameter_types! { @@ -97,6 +97,7 @@ impl Config for Test { type CollectionId = u32; type ItemId = u32; type Currency = Balances; + type RuntimeHoldReason = RuntimeHoldReason; type CreateOrigin = AsEnsureOriginWithArg>; type ForceOrigin = frame_system::EnsureRoot; type Locker = (); diff --git a/substrate/frame/nfts/src/types.rs b/substrate/frame/nfts/src/types.rs index 5a9f6ae2f0e2..7ebf0f4adbac 100644 --- a/substrate/frame/nfts/src/types.rs +++ b/substrate/frame/nfts/src/types.rs @@ -23,7 +23,7 @@ use codec::EncodeLike; use enumflags2::{bitflags, BitFlags}; use frame_support::{ pallet_prelude::{BoundedVec, MaxEncodedLen}, - traits::Get, + traits::{fungible::Inspect, Get}, BoundedBTreeMap, BoundedBTreeSet, }; use frame_system::pallet_prelude::BlockNumberFor; @@ -31,7 +31,7 @@ use scale_info::{build::Fields, meta_type, Path, Type, TypeInfo, TypeParameter}; /// A type alias for handling balance deposits. pub(super) type DepositBalanceOf = - <>::Currency as Currency<::AccountId>>::Balance; + <>::Currency as Inspect<::AccountId>>::Balance; /// A type alias representing the details of a collection. pub(super) type CollectionDetailsFor = CollectionDetails<::AccountId, DepositBalanceOf>; @@ -58,7 +58,7 @@ pub(super) type ItemDetailsFor = ItemDetails<::AccountId, ItemDepositOf, ApprovalsOf>; /// A type alias for an accounts balance. pub(super) type BalanceOf = - <>::Currency as Currency<::AccountId>>::Balance; + <>::Currency as Inspect<::AccountId>>::Balance; /// A type alias to represent the price of an item. pub(super) type ItemPrice = BalanceOf; /// A type alias for the tips held by a single item. From ba29f8541b2b6a0fac4a7fadb275e15d9aed8014 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Sat, 7 Oct 2023 18:24:14 +0200 Subject: [PATCH 2/8] Change the way we store aggregated deposit + cancel_item_attributes_approval() returns now the DispatchResultWithPostInfo --- .../frame/nfts/src/features/attributes.rs | 40 +++++++++++++------ .../src/features/create_delete_collection.rs | 4 +- .../nfts/src/features/create_delete_item.rs | 7 +++- substrate/frame/nfts/src/features/metadata.rs | 37 +++++++++++------ substrate/frame/nfts/src/lib.rs | 17 +++++--- 5 files changed, 72 insertions(+), 33 deletions(-) diff --git a/substrate/frame/nfts/src/features/attributes.rs b/substrate/frame/nfts/src/features/attributes.rs index 0d67c87920b2..5281a5c44f4c 100644 --- a/substrate/frame/nfts/src/features/attributes.rs +++ b/substrate/frame/nfts/src/features/attributes.rs @@ -110,6 +110,11 @@ impl, I: 'static> Pallet { let is_collection_owner_namespace = namespace == AttributeNamespace::CollectionOwner; let is_depositor_collection_owner = is_collection_owner_namespace && collection_details.owner == depositor; + let reason = if is_depositor_collection_owner { + HoldReason::CollectionOwnerAggregatedDeposit + } else { + HoldReason::AttributeDeposit + }; // NOTE: in the CollectionOwner namespace if the depositor is `None` that means the deposit // was paid by the collection's owner. @@ -126,23 +131,24 @@ impl, I: 'static> Pallet { // and return the deposit to the previous owner. if depositor_has_changed { if let Some(old_depositor) = old_depositor { + let release_reason = if old_depositor == collection_details.owner { + HoldReason::CollectionOwnerAggregatedDeposit + } else { + HoldReason::AttributeDeposit + }; T::Currency::release( - &HoldReason::AttributeDeposit.into(), + &release_reason.into(), &old_depositor, old_deposit.amount, BestEffort, )?; } - T::Currency::hold(&HoldReason::AttributeDeposit.into(), &depositor, deposit)?; + T::Currency::hold(&reason.into(), &depositor, deposit)?; } else if deposit > old_deposit.amount { - T::Currency::hold( - &HoldReason::AttributeDeposit.into(), - &depositor, - deposit - old_deposit.amount, - )?; + T::Currency::hold(&reason.into(), &depositor, deposit - old_deposit.amount)?; } else if deposit < old_deposit.amount { T::Currency::release( - &HoldReason::AttributeDeposit.into(), + &reason.into(), &depositor, old_deposit.amount - deposit, BestEffort, @@ -372,7 +378,7 @@ impl, I: 'static> Pallet { None if namespace == AttributeNamespace::CollectionOwner => { collection_details.owner_deposit.saturating_reduce(deposit.amount); T::Currency::release( - &HoldReason::AttributeDeposit.into(), + &HoldReason::CollectionOwnerAggregatedDeposit.into(), &collection_details.owner, deposit.amount, BestEffort, @@ -444,7 +450,7 @@ impl, I: 'static> Pallet { item: T::ItemId, delegate: T::AccountId, witness: CancelAttributesApprovalWitness, - ) -> DispatchResult { + ) -> Result { ensure!( Self::is_pallet_feature_enabled(PalletFeature::Attributes), Error::::MethodDisabled @@ -464,7 +470,16 @@ impl, I: 'static> Pallet { AttributeNamespace::Account(delegate.clone()), )) { attributes.saturating_inc(); - deposited = deposited.saturating_add(deposit.amount); + if deposit.account == Some(delegate.clone()) { + deposited = deposited.saturating_add(deposit.amount); + } else if let Some(depositor) = deposit.account { + T::Currency::release( + &HoldReason::AttributeDeposit.into(), + &depositor, + deposited, + BestEffort, + )?; + } } ensure!(attributes <= witness.account_attributes, Error::::BadWitness); @@ -482,7 +497,8 @@ impl, I: 'static> Pallet { item, delegate, }); - Ok(()) + + Ok(CancelAttributesApprovalWitness { account_attributes: attributes }) }) } diff --git a/substrate/frame/nfts/src/features/create_delete_collection.rs b/substrate/frame/nfts/src/features/create_delete_collection.rs index bb4e734c2f47..ae93a2b01e20 100644 --- a/substrate/frame/nfts/src/features/create_delete_collection.rs +++ b/substrate/frame/nfts/src/features/create_delete_collection.rs @@ -43,7 +43,7 @@ impl, I: 'static> Pallet { ) -> DispatchResult { ensure!(!Collection::::contains_key(collection), Error::::CollectionIdInUse); - T::Currency::hold(&HoldReason::CollectionOwnerDeposit.into(), &owner, deposit)?; + T::Currency::hold(&HoldReason::CollectionOwnerAggregatedDeposit.into(), &owner, deposit)?; Collection::::insert( collection, @@ -142,7 +142,7 @@ impl, I: 'static> Pallet { CollectionAccount::::remove(&collection_details.owner, &collection); T::Currency::release( - &HoldReason::CollectionOwnerDeposit.into(), + &HoldReason::CollectionOwnerAggregatedDeposit.into(), &collection_details.owner, collection_details.owner_deposit, BestEffort, diff --git a/substrate/frame/nfts/src/features/create_delete_item.rs b/substrate/frame/nfts/src/features/create_delete_item.rs index 798ec749566d..d17d98466ab4 100644 --- a/substrate/frame/nfts/src/features/create_delete_item.rs +++ b/substrate/frame/nfts/src/features/create_delete_item.rs @@ -246,8 +246,13 @@ impl, I: 'static> Pallet { let depositor_account = metadata.deposit.account.unwrap_or(collection_details.owner.clone()); + let reason = if depositor_account == collection_details.owner { + HoldReason::CollectionOwnerAggregatedDeposit + } else { + HoldReason::MetadataDeposit + }; T::Currency::release( - &HoldReason::MetadataDeposit.into(), + &reason.into(), &depositor_account, metadata.deposit.amount, BestEffort, diff --git a/substrate/frame/nfts/src/features/metadata.rs b/substrate/frame/nfts/src/features/metadata.rs index 48280aab34c0..bdade0fc6ac2 100644 --- a/substrate/frame/nfts/src/features/metadata.rs +++ b/substrate/frame/nfts/src/features/metadata.rs @@ -84,23 +84,30 @@ impl, I: 'static> Pallet { let depositor = maybe_depositor.clone().unwrap_or(collection_details.owner.clone()); let old_depositor = old_deposit.account.unwrap_or(collection_details.owner.clone()); + let release_reason = if old_depositor == collection_details.owner { + HoldReason::CollectionOwnerAggregatedDeposit + } else { + HoldReason::MetadataDeposit + }; + let hold_reason = if depositor == collection_details.owner { + HoldReason::CollectionOwnerAggregatedDeposit + } else { + HoldReason::MetadataDeposit + }; + if depositor != old_depositor { T::Currency::release( - &HoldReason::MetadataDeposit.into(), + &release_reason.into(), &old_depositor, old_deposit.amount, BestEffort, )?; - T::Currency::hold(&HoldReason::MetadataDeposit.into(), &depositor, deposit)?; + T::Currency::hold(&hold_reason.into(), &depositor, deposit)?; } else if deposit > old_deposit.amount { - T::Currency::hold( - &HoldReason::MetadataDeposit.into(), - &depositor, - deposit - old_deposit.amount, - )?; + T::Currency::hold(&hold_reason.into(), &depositor, deposit - old_deposit.amount)?; } else if deposit < old_deposit.amount { T::Currency::release( - &HoldReason::MetadataDeposit.into(), + &release_reason.into(), &depositor, old_deposit.amount - deposit, BestEffort, @@ -164,8 +171,14 @@ impl, I: 'static> Pallet { ensure!(is_root || !is_locked, Error::::LockedItemMetadata); collection_details.item_metadatas.saturating_dec(); + + let reason = if depositor_account == collection_details.owner { + HoldReason::CollectionOwnerAggregatedDeposit + } else { + HoldReason::MetadataDeposit + }; T::Currency::release( - &HoldReason::MetadataDeposit.into(), + &reason.into(), &depositor_account, metadata.deposit.amount, BestEffort, @@ -228,13 +241,13 @@ impl, I: 'static> Pallet { } if deposit > old_deposit { T::Currency::hold( - &HoldReason::MetadataDeposit.into(), + &HoldReason::CollectionOwnerAggregatedDeposit.into(), &details.owner, deposit - old_deposit, )?; } else if deposit < old_deposit { T::Currency::release( - &HoldReason::MetadataDeposit.into(), + &HoldReason::CollectionOwnerAggregatedDeposit.into(), &details.owner, old_deposit - deposit, BestEffort, @@ -288,7 +301,7 @@ impl, I: 'static> Pallet { CollectionMetadataOf::::try_mutate_exists(collection, |metadata| { let deposit = metadata.take().ok_or(Error::::UnknownCollection)?.deposit; T::Currency::release( - &HoldReason::MetadataDeposit.into(), + &HoldReason::CollectionOwnerAggregatedDeposit.into(), &details.owner, deposit, BestEffort, diff --git a/substrate/frame/nfts/src/lib.rs b/substrate/frame/nfts/src/lib.rs index f8bec62801ed..33d721f95c73 100644 --- a/substrate/frame/nfts/src/lib.rs +++ b/substrate/frame/nfts/src/lib.rs @@ -666,9 +666,9 @@ pub mod pallet { /// A reason for the pallet placing a hold on funds. #[pallet::composite_enum] pub enum HoldReason { - /// Reserved for a collection's storage record. + /// An aggregated reserve for all the deposits made by collection's owner. #[codec(index = 0)] - CollectionOwnerDeposit, + CollectionOwnerAggregatedDeposit, /// Reserved for an item's storage record. #[codec(index = 1)] ItemDeposit, @@ -1071,14 +1071,14 @@ pub mod pallet { let old = details.deposit.amount; if old > deposit { T::Currency::release( - &HoldReason::CollectionOwnerDeposit.into(), + &HoldReason::ItemDeposit.into(), &details.deposit.account, old - deposit, BestEffort, )?; } else if deposit > old { if T::Currency::hold( - &HoldReason::CollectionOwnerDeposit.into(), + &HoldReason::ItemDeposit.into(), &details.deposit.account, deposit - old, ) @@ -1543,10 +1543,15 @@ pub mod pallet { item: T::ItemId, delegate: AccountIdLookupOf, witness: CancelAttributesApprovalWitness, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let origin = ensure_signed(origin)?; let delegate = T::Lookup::lookup(delegate)?; - Self::do_cancel_item_attributes_approval(origin, collection, item, delegate, witness) + let details = Self::do_cancel_item_attributes_approval( + origin, collection, item, delegate, witness, + )?; + + Ok(Some(T::WeightInfo::cancel_item_attributes_approval(details.account_attributes)) + .into()) } /// Set the metadata for an item. From a7f38938093d65f7225eae933b161b8abca99cae Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Sat, 7 Oct 2023 18:30:46 +0200 Subject: [PATCH 3/8] Change the way we repatriate reserved deposits --- substrate/frame/nfts/src/features/transfer.rs | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/substrate/frame/nfts/src/features/transfer.rs b/substrate/frame/nfts/src/features/transfer.rs index 1d57484a9740..35fb486aebd2 100644 --- a/substrate/frame/nfts/src/features/transfer.rs +++ b/substrate/frame/nfts/src/features/transfer.rs @@ -140,12 +140,17 @@ impl, I: 'static> Pallet { } // Move the deposit to the new owner. - /*T::Currency::repatriate_reserved( + T::Currency::release( + &HoldReason::CollectionOwnerAggregatedDeposit.into(), &details.owner, + details.owner_deposit, + BestEffort, + )?; + T::Currency::hold( + &HoldReason::CollectionOwnerAggregatedDeposit.into(), &owner, details.owner_deposit, - Reserved, - )?;*/ + )?; // Update account ownership information. CollectionAccount::::remove(&details.owner, &collection); @@ -213,12 +218,17 @@ impl, I: 'static> Pallet { } // Move the deposit to the new owner. - /*T::Currency::repatriate_reserved( + T::Currency::release( + &HoldReason::CollectionOwnerAggregatedDeposit.into(), &details.owner, + details.owner_deposit, + BestEffort, + )?; + T::Currency::hold( + &HoldReason::CollectionOwnerAggregatedDeposit.into(), &owner, details.owner_deposit, - Reserved, - )?;*/ + )?; // Update collection accounts and set the new owner. CollectionAccount::::remove(&details.owner, &collection); From f5187402d5cb9da159736d429a6348ca92aa82f8 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Sat, 7 Oct 2023 18:46:38 +0200 Subject: [PATCH 4/8] Use transfer_on_hold instead --- substrate/frame/nfts/src/features/transfer.rs | 54 +++++++++---------- substrate/frame/nfts/src/lib.rs | 1 - 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/substrate/frame/nfts/src/features/transfer.rs b/substrate/frame/nfts/src/features/transfer.rs index 35fb486aebd2..2655606726c5 100644 --- a/substrate/frame/nfts/src/features/transfer.rs +++ b/substrate/frame/nfts/src/features/transfer.rs @@ -19,7 +19,10 @@ //! of the NFTs pallet. use crate::*; -use frame_support::pallet_prelude::*; +use frame_support::{ + pallet_prelude::*, + traits::tokens::{Fortitude, Restriction}, +}; impl, I: 'static> Pallet { /// Transfer an NFT to the specified destination account. @@ -116,7 +119,7 @@ impl, I: 'static> Pallet { /// /// - `origin`: The account requesting the transfer. /// - `collection`: The ID of the collection to transfer ownership. - /// - `owner`: The new account that will become the owner of the collection. + /// - `new_owner`: The account that will become the new owner of the collection. /// /// This function transfers the ownership of a collection to the specified account. /// It performs checks to ensure that the `origin` is the current owner and that the @@ -124,10 +127,10 @@ impl, I: 'static> Pallet { pub(crate) fn do_transfer_ownership( origin: T::AccountId, collection: T::CollectionId, - owner: T::AccountId, + new_owner: T::AccountId, ) -> DispatchResult { // Check if the new owner is acceptable based on the collection's acceptance settings. - let acceptable_collection = OwnershipAcceptance::::get(&owner); + let acceptable_collection = OwnershipAcceptance::::get(&new_owner); ensure!(acceptable_collection.as_ref() == Some(&collection), Error::::Unaccepted); // Try to retrieve and mutate the collection details. @@ -135,35 +138,34 @@ impl, I: 'static> Pallet { let details = maybe_details.as_mut().ok_or(Error::::UnknownCollection)?; // Check if the `origin` is the current owner of the collection. ensure!(origin == details.owner, Error::::NoPermission); - if details.owner == owner { + if details.owner == new_owner { return Ok(()) } // Move the deposit to the new owner. - T::Currency::release( + T::Currency::transfer_on_hold( &HoldReason::CollectionOwnerAggregatedDeposit.into(), &details.owner, + &new_owner, details.owner_deposit, BestEffort, - )?; - T::Currency::hold( - &HoldReason::CollectionOwnerAggregatedDeposit.into(), - &owner, - details.owner_deposit, + Restriction::Free, + Fortitude::Force, )?; // Update account ownership information. CollectionAccount::::remove(&details.owner, &collection); - CollectionAccount::::insert(&owner, &collection, ()); + CollectionAccount::::insert(&new_owner, &collection, ()); - details.owner = owner.clone(); - OwnershipAcceptance::::remove(&owner); + details.owner = new_owner.clone(); + OwnershipAcceptance::::remove(&new_owner); // Emit `OwnerChanged` event. - Self::deposit_event(Event::OwnerChanged { collection, new_owner: owner }); + Self::deposit_event(Event::OwnerChanged { collection, new_owner }); Ok(()) }) } + /// Set or unset the ownership acceptance for an account regarding a specific collection. /// /// - `who`: The account for which to set or unset the ownership acceptance. @@ -201,42 +203,40 @@ impl, I: 'static> Pallet { /// Forcefully change the owner of a collection. /// /// - `collection`: The ID of the collection to change ownership. - /// - `owner`: The new account that will become the owner of the collection. + /// - `new_owner`: The account that will become the new owner of the collection. /// /// This function allows for changing the ownership of a collection without any checks. /// It moves the deposit to the new owner, updates the collection's owner, and emits /// an `OwnerChanged` event. pub(crate) fn do_force_collection_owner( collection: T::CollectionId, - owner: T::AccountId, + new_owner: T::AccountId, ) -> DispatchResult { // Try to retrieve and mutate the collection details. Collection::::try_mutate(collection, |maybe_details| { let details = maybe_details.as_mut().ok_or(Error::::UnknownCollection)?; - if details.owner == owner { + if details.owner == new_owner { return Ok(()) } // Move the deposit to the new owner. - T::Currency::release( + T::Currency::transfer_on_hold( &HoldReason::CollectionOwnerAggregatedDeposit.into(), &details.owner, + &new_owner, details.owner_deposit, BestEffort, - )?; - T::Currency::hold( - &HoldReason::CollectionOwnerAggregatedDeposit.into(), - &owner, - details.owner_deposit, + Restriction::Free, + Fortitude::Force, )?; // Update collection accounts and set the new owner. CollectionAccount::::remove(&details.owner, &collection); - CollectionAccount::::insert(&owner, &collection, ()); - details.owner = owner.clone(); + CollectionAccount::::insert(&new_owner, &collection, ()); + details.owner = new_owner.clone(); // Emit `OwnerChanged` event. - Self::deposit_event(Event::OwnerChanged { collection, new_owner: owner }); + Self::deposit_event(Event::OwnerChanged { collection, new_owner }); Ok(()) }) } diff --git a/substrate/frame/nfts/src/lib.rs b/substrate/frame/nfts/src/lib.rs index 33d721f95c73..ecb29eed1d50 100644 --- a/substrate/frame/nfts/src/lib.rs +++ b/substrate/frame/nfts/src/lib.rs @@ -54,7 +54,6 @@ use frame_support::traits::{ hold::Mutate as HoldMutateFungible, Inspect as InspectFungible, Mutate as MutateFungible, }, tokens::{Locker, Precision::BestEffort, Preservation::Preserve}, - BalanceStatus::Reserved, EnsureOriginWithArg, Incrementable, }; use frame_system::Config as SystemConfig; From 8b72a7da8b397d60b485e4d6ea87dafb395f8776 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Sun, 8 Oct 2023 07:33:25 +0200 Subject: [PATCH 5/8] Fix tests --- substrate/frame/nfts/src/features/transfer.rs | 8 +- substrate/frame/nfts/src/mock.rs | 25 ++- substrate/frame/nfts/src/tests.rs | 169 ++++++++++-------- 3 files changed, 123 insertions(+), 79 deletions(-) diff --git a/substrate/frame/nfts/src/features/transfer.rs b/substrate/frame/nfts/src/features/transfer.rs index 2655606726c5..3830b2f05f6c 100644 --- a/substrate/frame/nfts/src/features/transfer.rs +++ b/substrate/frame/nfts/src/features/transfer.rs @@ -149,8 +149,8 @@ impl, I: 'static> Pallet { &new_owner, details.owner_deposit, BestEffort, - Restriction::Free, - Fortitude::Force, + Restriction::OnHold, + Fortitude::Polite, )?; // Update account ownership information. @@ -226,8 +226,8 @@ impl, I: 'static> Pallet { &new_owner, details.owner_deposit, BestEffort, - Restriction::Free, - Fortitude::Force, + Restriction::OnHold, + Fortitude::Polite, )?; // Update collection accounts and set the new owner. diff --git a/substrate/frame/nfts/src/mock.rs b/substrate/frame/nfts/src/mock.rs index 1c86f9cd41e9..ddf07c144339 100644 --- a/substrate/frame/nfts/src/mock.rs +++ b/substrate/frame/nfts/src/mock.rs @@ -33,6 +33,12 @@ use sp_runtime::{ type Block = frame_system::mocking::MockBlock; +pub type AccountIdOf = ::AccountId; + +pub fn account(id: u8) -> AccountIdOf { + [id; 32].into() +} + construct_runtime!( pub enum Test { @@ -85,7 +91,7 @@ impl pallet_balances::Config for Test { type FreezeIdentifier = (); type MaxFreezes = (); type RuntimeHoldReason = RuntimeHoldReason; - type MaxHolds = ConstU32<1>; + type MaxHolds = ConstU32<4>; } parameter_types! { @@ -126,7 +132,22 @@ impl Config for Test { } pub(crate) fn new_test_ext() -> sp_io::TestExternalities { - let t = frame_system::GenesisConfig::::default().build_storage().unwrap(); + let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); + + pallet_balances::GenesisConfig:: { + balances: vec![ + (account(1), 10000), + (account(2), 20000), + (account(3), 30000), + (account(4), 40000), + (account(5), 50000), + (account(6), 60000), + (account(7), 70000), + (account(8), 80000), + ], + } + .assimilate_storage(&mut t) + .unwrap(); let mut ext = sp_io::TestExternalities::new(t); ext.register_extension(KeystoreExt::new(MemoryKeystore::new())); diff --git a/substrate/frame/nfts/src/tests.rs b/substrate/frame/nfts/src/tests.rs index 6e264048f11a..26cb8193a4f1 100644 --- a/substrate/frame/nfts/src/tests.rs +++ b/substrate/frame/nfts/src/tests.rs @@ -23,23 +23,17 @@ use frame_support::{ assert_noop, assert_ok, traits::{ tokens::nonfungibles_v2::{Create, Destroy, Mutate}, - Currency, Get, + Get, }, }; -use pallet_balances::Error as BalancesError; use sp_core::{bounded::BoundedVec, Pair}; use sp_runtime::{ traits::{Dispatchable, IdentifyAccount}, MultiSignature, MultiSigner, + TokenError::FundsUnavailable, }; use sp_std::prelude::*; -type AccountIdOf = ::AccountId; - -fn account(id: u8) -> AccountIdOf { - [id; 32].into() -} - fn items() -> Vec<(AccountIdOf, u32, u32)> { let mut r: Vec<_> = Account::::iter().map(|x| x.0).collect(); r.sort(); @@ -180,8 +174,8 @@ fn basic_minting_should_work() { #[test] fn lifecycle_should_work() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); - Balances::make_free_balance_be(&account(2), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(2), 100)); assert_ok!(Nfts::create( RuntimeOrigin::signed(account(1)), account(1), @@ -201,7 +195,7 @@ fn lifecycle_should_work() { RuntimeOrigin::signed(account(1)), 0, 42, - account(10), + account(3), default_item_config() )); assert_eq!(Balances::reserved_balance(&account(1)), 6); @@ -209,12 +203,12 @@ fn lifecycle_should_work() { RuntimeOrigin::signed(account(1)), 0, 69, - account(20), + account(4), default_item_config() )); assert_eq!(Balances::reserved_balance(&account(1)), 7); assert_ok!(Nfts::mint(RuntimeOrigin::signed(account(1)), 0, 70, account(1), None)); - assert_eq!(items(), vec![(account(1), 0, 70), (account(10), 0, 42), (account(20), 0, 69)]); + assert_eq!(items(), vec![(account(1), 0, 70), (account(3), 0, 42), (account(4), 0, 69)]); assert_eq!(Collection::::get(0).unwrap().items, 3); assert_eq!(Collection::::get(0).unwrap().item_metadatas, 0); assert_eq!(Collection::::get(0).unwrap().item_configs, 3); @@ -247,8 +241,8 @@ fn lifecycle_should_work() { bvec![0], bvec![0], )); - assert_ok!(Nfts::burn(RuntimeOrigin::signed(account(10)), 0, 42)); - assert_ok!(Nfts::burn(RuntimeOrigin::signed(account(20)), 0, 69)); + assert_ok!(Nfts::burn(RuntimeOrigin::signed(account(3)), 0, 42)); + assert_ok!(Nfts::burn(RuntimeOrigin::signed(account(4)), 0, 69)); assert_ok!(Nfts::burn(RuntimeOrigin::root(), 0, 70)); let w = Nfts::get_destroy_witness(&0).unwrap(); @@ -275,7 +269,7 @@ fn lifecycle_should_work() { #[test] fn destroy_with_bad_witness_should_not_work() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); assert_ok!(Nfts::create( RuntimeOrigin::signed(account(1)), account(1), @@ -297,7 +291,7 @@ fn destroy_with_bad_witness_should_not_work() { #[test] fn destroy_should_work() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); assert_ok!(Nfts::create( RuntimeOrigin::signed(account(1)), account(1), @@ -370,7 +364,7 @@ fn mint_should_work() { 0, MintSettings { mint_type: MintType::Public, price: Some(1), ..Default::default() } )); - Balances::make_free_balance_be(&account(2), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(2), 100)); assert_noop!( Nfts::mint(RuntimeOrigin::signed(account(2)), 0, 43, account(2), None,), Error::::WitnessRequired @@ -402,7 +396,7 @@ fn mint_should_work() { account(2), Some(MintWitness { mint_price: Some(1), ..Default::default() }) )); - assert_eq!(Balances::total_balance(&account(2)), 99); + assert_eq!(::Currency::total_balance(&account(2)), 99); // validate types assert_ok!(Nfts::force_create( @@ -557,7 +551,7 @@ fn origin_guards_should_work() { )); assert_ok!(Nfts::mint(RuntimeOrigin::signed(account(1)), 0, 42, account(1), None)); - Balances::make_free_balance_be(&account(2), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(2), 100)); assert_ok!(Nfts::set_accept_ownership(RuntimeOrigin::signed(account(2)), Some(0))); assert_noop!( Nfts::transfer_ownership(RuntimeOrigin::signed(account(2)), 0, account(2)), @@ -601,9 +595,9 @@ fn origin_guards_should_work() { #[test] fn transfer_owner_should_work() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); - Balances::make_free_balance_be(&account(2), 100); - Balances::make_free_balance_be(&account(3), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(2), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(3), 100)); assert_ok!(Nfts::create( RuntimeOrigin::signed(account(1)), account(1), @@ -618,8 +612,8 @@ fn transfer_owner_should_work() { assert_ok!(Nfts::transfer_ownership(RuntimeOrigin::signed(account(1)), 0, account(2))); assert_eq!(collections(), vec![(account(2), 0)]); - assert_eq!(Balances::total_balance(&account(1)), 98); - assert_eq!(Balances::total_balance(&account(2)), 102); + assert_eq!(::Currency::total_balance(&account(1)), 98); + assert_eq!(::Currency::total_balance(&account(2)), 102); assert_eq!(Balances::reserved_balance(&account(1)), 0); assert_eq!(Balances::reserved_balance(&account(2)), 2); @@ -641,8 +635,8 @@ fn transfer_owner_should_work() { assert_ok!(Nfts::set_accept_ownership(RuntimeOrigin::signed(account(3)), Some(0))); assert_ok!(Nfts::transfer_ownership(RuntimeOrigin::signed(account(2)), 0, account(3))); assert_eq!(collections(), vec![(account(3), 0)]); - assert_eq!(Balances::total_balance(&account(2)), 58); - assert_eq!(Balances::total_balance(&account(3)), 144); + assert_eq!(::Currency::total_balance(&account(2)), 58); + assert_eq!(::Currency::total_balance(&account(3)), 144); assert_eq!(Balances::reserved_balance(&account(2)), 0); assert_eq!(Balances::reserved_balance(&account(3)), 44); @@ -750,7 +744,7 @@ fn set_collection_metadata_should_work() { ); // Successfully add metadata and take deposit - Balances::make_free_balance_be(&account(1), 30); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 30)); assert_ok!(Nfts::set_collection_metadata( RuntimeOrigin::signed(account(1)), 0, @@ -779,7 +773,7 @@ fn set_collection_metadata_should_work() { // Cannot over-reserve assert_noop!( Nfts::set_collection_metadata(RuntimeOrigin::signed(account(1)), 0, bvec![0u8; 40]), - BalancesError::::InsufficientBalance, + FundsUnavailable, ); // Can't set or clear metadata once frozen @@ -824,7 +818,7 @@ fn set_collection_metadata_should_work() { #[test] fn set_item_metadata_should_work() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 30); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 30)); // Cannot add metadata to unknown item assert_ok!(Nfts::force_create( @@ -856,7 +850,7 @@ fn set_item_metadata_should_work() { // Cannot over-reserve assert_noop!( Nfts::set_metadata(RuntimeOrigin::signed(account(1)), 0, 42, bvec![0u8; 40]), - BalancesError::::InsufficientBalance, + FundsUnavailable, ); // Can't set or clear metadata once frozen @@ -895,7 +889,7 @@ fn set_item_metadata_should_work() { #[test] fn set_collection_owner_attributes_should_work() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); assert_ok!(Nfts::force_create( RuntimeOrigin::root(), @@ -985,9 +979,9 @@ fn set_collection_owner_attributes_should_work() { #[test] fn set_item_owner_attributes_should_work() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); - Balances::make_free_balance_be(&account(2), 100); - Balances::make_free_balance_be(&account(3), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(2), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(3), 100)); assert_ok!(Nfts::force_create( RuntimeOrigin::root(), @@ -1170,8 +1164,8 @@ fn set_item_owner_attributes_should_work() { #[test] fn set_external_account_attributes_should_work() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); - Balances::make_free_balance_be(&account(2), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(2), 100)); assert_ok!(Nfts::force_create( RuntimeOrigin::root(), @@ -1255,9 +1249,9 @@ fn set_external_account_attributes_should_work() { #[test] fn validate_deposit_required_setting() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); - Balances::make_free_balance_be(&account(2), 100); - Balances::make_free_balance_be(&account(3), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(2), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(3), 100)); // with the disabled DepositRequired setting, only the collection's owner can set the // attributes for free. @@ -1344,7 +1338,7 @@ fn validate_deposit_required_setting() { #[test] fn set_attribute_should_respect_lock() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); assert_ok!(Nfts::force_create( RuntimeOrigin::root(), @@ -1449,7 +1443,7 @@ fn set_attribute_should_respect_lock() { #[test] fn preserve_config_for_frozen_items() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); assert_ok!(Nfts::force_create( RuntimeOrigin::root(), @@ -1505,7 +1499,7 @@ fn preserve_config_for_frozen_items() { #[test] fn force_update_collection_should_work() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); assert_ok!(Nfts::force_create( RuntimeOrigin::root(), @@ -1545,8 +1539,7 @@ fn force_update_collection_should_work() { )); assert_ok!(Nfts::set_metadata(RuntimeOrigin::signed(account(1)), 0, 142, bvec![0; 20])); assert_ok!(Nfts::set_metadata(RuntimeOrigin::signed(account(1)), 0, 169, bvec![0; 20])); - - Balances::make_free_balance_be(&account(5), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(5), 100)); assert_ok!(Nfts::force_collection_owner(RuntimeOrigin::root(), 0, account(5))); assert_ok!(Nfts::set_team( RuntimeOrigin::root(), @@ -1622,7 +1615,7 @@ fn force_update_collection_should_work() { #[test] fn burn_works() { new_test_ext().execute_with(|| { - Balances::make_free_balance_be(&account(1), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); assert_ok!(Nfts::force_create( RuntimeOrigin::root(), account(1), @@ -2346,9 +2339,21 @@ fn buy_item_should_work() { let price_2 = 30; let initial_balance = 100; - Balances::make_free_balance_be(&user_1, initial_balance); - Balances::make_free_balance_be(&user_2, initial_balance); - Balances::make_free_balance_be(&user_3, initial_balance); + assert_ok!(Balances::force_set_balance( + RuntimeOrigin::root(), + user_1.clone(), + initial_balance, + )); + assert_ok!(Balances::force_set_balance( + RuntimeOrigin::root(), + user_2.clone(), + initial_balance, + )); + assert_ok!(Balances::force_set_balance( + RuntimeOrigin::root(), + user_3.clone(), + initial_balance, + )); assert_ok!(Nfts::force_create( RuntimeOrigin::root(), @@ -2411,8 +2416,14 @@ fn buy_item_should_work() { // validate the new owner & balances let item = Item::::get(collection_id, item_1).unwrap(); assert_eq!(item.owner, user_2.clone()); - assert_eq!(Balances::total_balance(&user_1.clone()), initial_balance + price_1); - assert_eq!(Balances::total_balance(&user_2.clone()), initial_balance - price_1); + assert_eq!( + ::Currency::total_balance(&user_1.clone()), + initial_balance + price_1 + ); + assert_eq!( + ::Currency::total_balance(&user_2.clone()), + initial_balance - price_1 + ); // can't buy from yourself assert_noop!( @@ -2516,9 +2527,21 @@ fn pay_tips_should_work() { let tip = 2; let initial_balance = 100; - Balances::make_free_balance_be(&user_1, initial_balance); - Balances::make_free_balance_be(&user_2, initial_balance); - Balances::make_free_balance_be(&user_3, initial_balance); + assert_ok!(Balances::force_set_balance( + RuntimeOrigin::root(), + user_1.clone(), + initial_balance, + )); + assert_ok!(Balances::force_set_balance( + RuntimeOrigin::root(), + user_2.clone(), + initial_balance, + )); + assert_ok!(Balances::force_set_balance( + RuntimeOrigin::root(), + user_3.clone(), + initial_balance, + )); assert_ok!(Nfts::pay_tips( RuntimeOrigin::signed(user_1.clone()), @@ -2538,9 +2561,9 @@ fn pay_tips_should_work() { ] )); - assert_eq!(Balances::total_balance(&user_1), initial_balance - tip * 2); - assert_eq!(Balances::total_balance(&user_2), initial_balance + tip); - assert_eq!(Balances::total_balance(&user_3), initial_balance + tip); + assert_eq!(::Currency::total_balance(&user_1), initial_balance - tip * 2); + assert_eq!(::Currency::total_balance(&user_2), initial_balance + tip); + assert_eq!(::Currency::total_balance(&user_3), initial_balance + tip); let events = events(); assert!(events.contains(&Event::::TipSent { @@ -2729,13 +2752,13 @@ fn claim_swap_should_work() { let initial_balance = 1000; let deadline = 1 + duration; - Balances::make_free_balance_be(&user_1, initial_balance); - Balances::make_free_balance_be(&user_2, initial_balance); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user_1.clone(), initial_balance)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user_2.clone(), initial_balance)); assert_ok!(Nfts::force_create( RuntimeOrigin::root(), user_1.clone(), - default_collection_config() + default_collection_config(), )); assert_ok!(Nfts::mint( @@ -2872,8 +2895,8 @@ fn claim_swap_should_work() { assert_eq!(item.owner, user_1.clone()); // validate the balances - assert_eq!(Balances::total_balance(&user_1), initial_balance + price); - assert_eq!(Balances::total_balance(&user_2), initial_balance - price); + assert_eq!(::Currency::total_balance(&user_1), initial_balance + price); + assert_eq!(::Currency::total_balance(&user_2), initial_balance - price); // ensure we reset the swap assert!(!PendingSwapOf::::contains_key(collection_id, item_1)); @@ -2893,8 +2916,8 @@ fn claim_swap_should_work() { // validate the optional desired_item param and another price direction let price_direction = PriceDirection::Send; let price_with_direction = PriceWithDirection { amount: price, direction: price_direction }; - Balances::make_free_balance_be(&user_1, initial_balance); - Balances::make_free_balance_be(&user_2, initial_balance); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user_1.clone(), initial_balance)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user_2.clone(), initial_balance)); assert_ok!(Nfts::create_swap( RuntimeOrigin::signed(user_1.clone()), @@ -2918,8 +2941,8 @@ fn claim_swap_should_work() { let item = Item::::get(collection_id, item_4).unwrap(); assert_eq!(item.owner, user_2); - assert_eq!(Balances::total_balance(&user_1), initial_balance - price); - assert_eq!(Balances::total_balance(&user_2), initial_balance + price); + assert_eq!(::Currency::total_balance(&user_1), initial_balance - price); + assert_eq!(::Currency::total_balance(&user_2), initial_balance + price); }); } @@ -3222,8 +3245,8 @@ fn pre_signed_mints_should_work() { let user_2 = account(2); let user_3 = account(3); - Balances::make_free_balance_be(&user_0, 100); - Balances::make_free_balance_be(&user_2, 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user_0.clone(), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user_2.clone(), 100)); assert_ok!(Nfts::create( RuntimeOrigin::signed(user_0.clone()), user_1.clone(), @@ -3386,9 +3409,9 @@ fn pre_signed_attributes_should_work() { let collection_id = 0; let item_id = 0; - Balances::make_free_balance_be(&user_1, 100); - Balances::make_free_balance_be(&user_2, 100); - Balances::make_free_balance_be(&user_3, 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user_1.clone(), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user_2.clone(), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user_3.clone(), 100)); assert_ok!(Nfts::create( RuntimeOrigin::signed(user_1.clone()), user_1.clone(), @@ -3694,8 +3717,8 @@ fn basic_create_collection_with_id_should_work() { Error::::WrongSetting ); - Balances::make_free_balance_be(&account(1), 100); - Balances::make_free_balance_be(&account(2), 100); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(1), 100)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), account(2), 100)); assert_ok!(Nfts::create_collection_with_id( 0u32, From 2198e5f19bfc00842fed9ee81c35e1f42996521a Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Sun, 8 Oct 2023 09:59:54 +0200 Subject: [PATCH 6/8] Migrate benchmarks --- substrate/frame/nfts/src/benchmarking.rs | 40 ++++++++++++++---------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/substrate/frame/nfts/src/benchmarking.rs b/substrate/frame/nfts/src/benchmarking.rs index 8792af675fc1..bbacba7260b9 100644 --- a/substrate/frame/nfts/src/benchmarking.rs +++ b/substrate/frame/nfts/src/benchmarking.rs @@ -26,7 +26,7 @@ use frame_benchmarking::v1::{ }; use frame_support::{ assert_ok, - traits::{EnsureOrigin, Get, UnfilteredDispatchable}, + traits::{fungible::Inspect, EnsureOrigin, Get, UnfilteredDispatchable}, BoundedVec, }; use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin as SystemOrigin}; @@ -35,18 +35,22 @@ use sp_runtime::{ traits::{Bounded, IdentifyAccount, One}, AccountId32, MultiSignature, MultiSigner, }; -use sp_std::prelude::*; +use sp_std::{ops::Div, prelude::*}; use crate::Pallet as Nfts; const SEED: u32 = 0; +fn set_default_balance, I: 'static>(who: &T::AccountId) { + T::Currency::set_balance(&who, BalanceOf::::max_value().div(1000u32.into())); +} + fn create_collection, I: 'static>( ) -> (T::CollectionId, T::AccountId, AccountIdLookupOf) { let caller: T::AccountId = whitelisted_caller(); let caller_lookup = T::Lookup::unlookup(caller.clone()); let collection = T::Helper::collection(0); - T::Currency::make_free_balance_be(&caller, DepositBalanceOf::::max_value()); + set_default_balance::(&caller); assert_ok!(Nfts::::force_create( SystemOrigin::Root.into(), caller_lookup.clone(), @@ -242,7 +246,7 @@ benchmarks_instance_pallet! { let caller = T::CreateOrigin::ensure_origin(origin.clone(), &collection).unwrap(); whitelist_account!(caller); let admin = T::Lookup::unlookup(caller.clone()); - T::Currency::make_free_balance_be(&caller, DepositBalanceOf::::max_value()); + set_default_balance::(&caller); let call = Call::::create { admin, config: default_collection_config::() }; }: { call.dispatch_bypass_filter(origin)? } verify { @@ -250,9 +254,10 @@ benchmarks_instance_pallet! { } force_create { - let caller: T::AccountId = whitelisted_caller(); - let caller_lookup = T::Lookup::unlookup(caller.clone()); - }: _(SystemOrigin::Root, caller_lookup, default_collection_config::()) + let collection_owner: T::AccountId = whitelisted_caller(); + let collection_owner_lookup = T::Lookup::unlookup(collection_owner.clone()); + set_default_balance::(&collection_owner); + }: _(SystemOrigin::Root, collection_owner_lookup, default_collection_config::()) verify { assert_last_event::(Event::NextCollectionIdIncremented { next_id: Some(T::Helper::collection(1)) }.into()); } @@ -314,7 +319,7 @@ benchmarks_instance_pallet! { let target: T::AccountId = account("target", 0, SEED); let target_lookup = T::Lookup::unlookup(target.clone()); - T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance()); + T::Currency::set_balance(&target, T::Currency::minimum_balance()); }: _(SystemOrigin::Signed(caller.clone()), collection, item, target_lookup) verify { assert_last_event::(Event::Transferred { collection, item, from: caller, to: target }.into()); @@ -372,7 +377,7 @@ benchmarks_instance_pallet! { let (collection, caller, _) = create_collection::(); let target: T::AccountId = account("target", 0, SEED); let target_lookup = T::Lookup::unlookup(target.clone()); - T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance()); + T::Currency::set_balance(&target, T::Currency::minimum_balance()); let origin = SystemOrigin::Signed(target.clone()).into(); Nfts::::set_accept_ownership(origin, Some(collection))?; }: _(SystemOrigin::Signed(caller), collection, target_lookup) @@ -401,7 +406,7 @@ benchmarks_instance_pallet! { T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let target: T::AccountId = account("target", 0, SEED); let target_lookup = T::Lookup::unlookup(target.clone()); - T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance()); + T::Currency::set_balance(&target, T::Currency::minimum_balance()); let call = Call::::force_collection_owner { collection, owner: target_lookup, @@ -521,7 +526,7 @@ benchmarks_instance_pallet! { item, target_lookup.clone(), )?; - T::Currency::make_free_balance_be(&target, DepositBalanceOf::::max_value()); + set_default_balance::(&target); let value: BoundedVec<_, _> = vec![0u8; T::ValueLimit::get() as usize].try_into().unwrap(); for i in 0..n { let key = make_filled_vec(i as u16, T::KeyLimit::get() as usize); @@ -622,7 +627,7 @@ benchmarks_instance_pallet! { set_accept_ownership { let caller: T::AccountId = whitelisted_caller(); - T::Currency::make_free_balance_be(&caller, DepositBalanceOf::::max_value()); + set_default_balance::(&caller); let collection = T::Helper::collection(0); }: _(SystemOrigin::Signed(caller.clone()), Some(collection)) verify { @@ -680,7 +685,7 @@ benchmarks_instance_pallet! { let price = ItemPrice::::from(0u32); let origin = SystemOrigin::Signed(seller.clone()).into(); Nfts::::set_price(origin, collection, item, Some(price), Some(buyer_lookup))?; - T::Currency::make_free_balance_be(&buyer, DepositBalanceOf::::max_value()); + set_default_balance::(&buyer); }: _(SystemOrigin::Signed(buyer.clone()), collection, item, price) verify { assert_last_event::(Event::ItemBought { @@ -696,6 +701,7 @@ benchmarks_instance_pallet! { let n in 0 .. T::MaxTips::get() as u32; let amount = BalanceOf::::from(100u32); let caller: T::AccountId = whitelisted_caller(); + set_default_balance::(&caller); let collection = T::Helper::collection(0); let item = T::Helper::item(0); let tips: BoundedVec<_, _> = vec![ @@ -770,7 +776,7 @@ benchmarks_instance_pallet! { let duration = T::MaxDeadlineDuration::get(); let target: T::AccountId = account("target", 0, SEED); let target_lookup = T::Lookup::unlookup(target.clone()); - T::Currency::make_free_balance_be(&target, T::Currency::minimum_balance()); + T::Currency::set_balance(&target, T::Currency::minimum_balance()); let origin = SystemOrigin::Signed(caller.clone()); frame_system::Pallet::::set_block_number(One::one()); Nfts::::transfer(origin.clone().into(), collection, item2, target_lookup)?; @@ -802,7 +808,7 @@ benchmarks_instance_pallet! { let n in 0 .. T::MaxAttributesPerCall::get() as u32; let caller_public = sr25519_generate(0.into(), None); let caller = MultiSigner::Sr25519(caller_public).into_account().into(); - T::Currency::make_free_balance_be(&caller, DepositBalanceOf::::max_value()); + set_default_balance::(&caller); let caller_lookup = T::Lookup::unlookup(caller.clone()); let collection = T::Helper::collection(0); @@ -833,7 +839,7 @@ benchmarks_instance_pallet! { let signature = MultiSignature::Sr25519(sr25519_sign(0.into(), &caller_public, &message).unwrap()); let target: T::AccountId = account("target", 0, SEED); - T::Currency::make_free_balance_be(&target, DepositBalanceOf::::max_value()); + set_default_balance::(&target); frame_system::Pallet::::set_block_number(One::one()); }: _(SystemOrigin::Signed(target.clone()), Box::new(mint_data), signature.into(), caller) verify { @@ -851,7 +857,7 @@ benchmarks_instance_pallet! { let signer_public = sr25519_generate(0.into(), None); let signer: T::AccountId = MultiSigner::Sr25519(signer_public).into_account().into(); - T::Currency::make_free_balance_be(&item_owner, DepositBalanceOf::::max_value()); + set_default_balance::(&item_owner); let item = T::Helper::item(0); assert_ok!(Nfts::::force_mint( From 28fd7f99e131274ee14e3e8a3ed2505eedd144c8 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Sun, 8 Oct 2023 10:05:10 +0200 Subject: [PATCH 7/8] Update depenedant code --- substrate/bin/node/runtime/src/lib.rs | 3 ++- substrate/frame/nft-fractionalization/src/mock.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index f018639b732e..ad2537eaa9dc 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -524,7 +524,7 @@ impl pallet_balances::Config for Runtime { type FreezeIdentifier = RuntimeFreezeReason; type MaxFreezes = ConstU32<1>; type RuntimeHoldReason = RuntimeHoldReason; - type MaxHolds = ConstU32<2>; + type MaxHolds = ConstU32<6>; } parameter_types! { @@ -1793,6 +1793,7 @@ impl pallet_nfts::Config for Runtime { type CollectionId = u32; type ItemId = u32; type Currency = Balances; + type RuntimeHoldReason = RuntimeHoldReason; type ForceOrigin = frame_system::EnsureRoot; type CollectionDeposit = CollectionDeposit; type ItemDeposit = ItemDeposit; diff --git a/substrate/frame/nft-fractionalization/src/mock.rs b/substrate/frame/nft-fractionalization/src/mock.rs index c690f0e580ed..c46a8b9ce20e 100644 --- a/substrate/frame/nft-fractionalization/src/mock.rs +++ b/substrate/frame/nft-fractionalization/src/mock.rs @@ -86,7 +86,7 @@ impl pallet_balances::Config for Test { type MaxReserves = ConstU32<50>; type ReserveIdentifier = [u8; 8]; type RuntimeHoldReason = RuntimeHoldReason; - type MaxHolds = ConstU32<1>; + type MaxHolds = ConstU32<5>; type FreezeIdentifier = (); type MaxFreezes = (); } @@ -124,6 +124,7 @@ impl pallet_nfts::Config for Test { type CollectionId = u32; type ItemId = u32; type Currency = Balances; + type RuntimeHoldReason = RuntimeHoldReason; type CreateOrigin = AsEnsureOriginWithArg>; type ForceOrigin = frame_system::EnsureRoot; type Locker = (); From 7e1ea2404154ce03f93a99a2a0902eb0e0d14f82 Mon Sep 17 00:00:00 2001 From: Jegor Sidorenko Date: Thu, 12 Oct 2023 21:49:10 +0200 Subject: [PATCH 8/8] Improvements --- substrate/frame/nfts/src/benchmarking.rs | 2 +- substrate/frame/nfts/src/features/transfer.rs | 6 ++-- substrate/frame/nfts/src/tests.rs | 34 ++++++++----------- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/substrate/frame/nfts/src/benchmarking.rs b/substrate/frame/nfts/src/benchmarking.rs index bbacba7260b9..285e44e747bc 100644 --- a/substrate/frame/nfts/src/benchmarking.rs +++ b/substrate/frame/nfts/src/benchmarking.rs @@ -857,7 +857,7 @@ benchmarks_instance_pallet! { let signer_public = sr25519_generate(0.into(), None); let signer: T::AccountId = MultiSigner::Sr25519(signer_public).into_account().into(); - set_default_balance::(&item_owner); + set_default_balance::(&item_owner); let item = T::Helper::item(0); assert_ok!(Nfts::::force_mint( diff --git a/substrate/frame/nfts/src/features/transfer.rs b/substrate/frame/nfts/src/features/transfer.rs index 3830b2f05f6c..e279a3d50e5f 100644 --- a/substrate/frame/nfts/src/features/transfer.rs +++ b/substrate/frame/nfts/src/features/transfer.rs @@ -21,7 +21,7 @@ use crate::*; use frame_support::{ pallet_prelude::*, - traits::tokens::{Fortitude, Restriction}, + traits::tokens::{Fortitude, Precision::Exact, Restriction}, }; impl, I: 'static> Pallet { @@ -148,7 +148,7 @@ impl, I: 'static> Pallet { &details.owner, &new_owner, details.owner_deposit, - BestEffort, + Exact, Restriction::OnHold, Fortitude::Polite, )?; @@ -225,7 +225,7 @@ impl, I: 'static> Pallet { &details.owner, &new_owner, details.owner_deposit, - BestEffort, + Exact, Restriction::OnHold, Fortitude::Polite, )?; diff --git a/substrate/frame/nfts/src/tests.rs b/substrate/frame/nfts/src/tests.rs index 26cb8193a4f1..9ce0f7f34cd3 100644 --- a/substrate/frame/nfts/src/tests.rs +++ b/substrate/frame/nfts/src/tests.rs @@ -396,7 +396,7 @@ fn mint_should_work() { account(2), Some(MintWitness { mint_price: Some(1), ..Default::default() }) )); - assert_eq!(::Currency::total_balance(&account(2)), 99); + assert_eq!(Balances::total_balance(&account(2)), 99); // validate types assert_ok!(Nfts::force_create( @@ -612,8 +612,8 @@ fn transfer_owner_should_work() { assert_ok!(Nfts::transfer_ownership(RuntimeOrigin::signed(account(1)), 0, account(2))); assert_eq!(collections(), vec![(account(2), 0)]); - assert_eq!(::Currency::total_balance(&account(1)), 98); - assert_eq!(::Currency::total_balance(&account(2)), 102); + assert_eq!(Balances::total_balance(&account(1)), 98); + assert_eq!(Balances::total_balance(&account(2)), 102); assert_eq!(Balances::reserved_balance(&account(1)), 0); assert_eq!(Balances::reserved_balance(&account(2)), 2); @@ -635,8 +635,8 @@ fn transfer_owner_should_work() { assert_ok!(Nfts::set_accept_ownership(RuntimeOrigin::signed(account(3)), Some(0))); assert_ok!(Nfts::transfer_ownership(RuntimeOrigin::signed(account(2)), 0, account(3))); assert_eq!(collections(), vec![(account(3), 0)]); - assert_eq!(::Currency::total_balance(&account(2)), 58); - assert_eq!(::Currency::total_balance(&account(3)), 144); + assert_eq!(Balances::total_balance(&account(2)), 58); + assert_eq!(Balances::total_balance(&account(3)), 144); assert_eq!(Balances::reserved_balance(&account(2)), 0); assert_eq!(Balances::reserved_balance(&account(3)), 44); @@ -2416,14 +2416,8 @@ fn buy_item_should_work() { // validate the new owner & balances let item = Item::::get(collection_id, item_1).unwrap(); assert_eq!(item.owner, user_2.clone()); - assert_eq!( - ::Currency::total_balance(&user_1.clone()), - initial_balance + price_1 - ); - assert_eq!( - ::Currency::total_balance(&user_2.clone()), - initial_balance - price_1 - ); + assert_eq!(Balances::total_balance(&user_1.clone()), initial_balance + price_1); + assert_eq!(Balances::total_balance(&user_2.clone()), initial_balance - price_1); // can't buy from yourself assert_noop!( @@ -2561,9 +2555,9 @@ fn pay_tips_should_work() { ] )); - assert_eq!(::Currency::total_balance(&user_1), initial_balance - tip * 2); - assert_eq!(::Currency::total_balance(&user_2), initial_balance + tip); - assert_eq!(::Currency::total_balance(&user_3), initial_balance + tip); + assert_eq!(Balances::total_balance(&user_1), initial_balance - tip * 2); + assert_eq!(Balances::total_balance(&user_2), initial_balance + tip); + assert_eq!(Balances::total_balance(&user_3), initial_balance + tip); let events = events(); assert!(events.contains(&Event::::TipSent { @@ -2895,8 +2889,8 @@ fn claim_swap_should_work() { assert_eq!(item.owner, user_1.clone()); // validate the balances - assert_eq!(::Currency::total_balance(&user_1), initial_balance + price); - assert_eq!(::Currency::total_balance(&user_2), initial_balance - price); + assert_eq!(Balances::total_balance(&user_1), initial_balance + price); + assert_eq!(Balances::total_balance(&user_2), initial_balance - price); // ensure we reset the swap assert!(!PendingSwapOf::::contains_key(collection_id, item_1)); @@ -2941,8 +2935,8 @@ fn claim_swap_should_work() { let item = Item::::get(collection_id, item_4).unwrap(); assert_eq!(item.owner, user_2); - assert_eq!(::Currency::total_balance(&user_1), initial_balance - price); - assert_eq!(::Currency::total_balance(&user_2), initial_balance + price); + assert_eq!(Balances::total_balance(&user_1), initial_balance - price); + assert_eq!(Balances::total_balance(&user_2), initial_balance + price); }); }