Refactors pallet-nfts to use fungible traits

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<AccountId>;
 	type CollectionDeposit = CollectionDeposit;
 	type ItemDeposit = ItemDeposit;

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<frame_system::EnsureSigned<Self::AccountId>>;
 	type ForceOrigin = frame_system::EnsureRoot<Self::AccountId>;
 	type Locker = ();

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<T: Config<I>, I: 'static>(who: &T::AccountId) {
+	T::Currency::set_balance(&who, BalanceOf::<T, I>::max_value().div(1000u32.into()));
+}
+
 fn create_collection<T: Config<I>, I: 'static>(
 ) -> (T::CollectionId, T::AccountId, AccountIdLookupOf<T>) {
 	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::<T, I>::max_value());
+	set_default_balance::<T, I>(&caller);
 	assert_ok!(Nfts::<T, I>::force_create(
 		SystemOrigin::Root.into(),
 		caller_lookup.clone(),
@@ -242,17 +246,18 @@ 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::<T, I>::max_value());
+		set_default_balance::<T, I>(&caller);
 		let call = Call::<T, I>::create { admin, config: default_collection_config::<T, I>() };
 	}: { call.dispatch_bypass_filter(origin)? }
 	verify {
 		assert_last_event::<T, I>(Event::NextCollectionIdIncremented { next_id: Some(T::Helper::collection(1)) }.into());
 	}
 
 	force_create {
-		let caller: T::AccountId = whitelisted_caller();
-		let caller_lookup = T::Lookup::unlookup(caller.clone());
-	}: _(SystemOrigin::Root, caller_lookup, default_collection_config::<T, I>())
+		let collection_owner: T::AccountId = whitelisted_caller();
+		let collection_owner_lookup = T::Lookup::unlookup(collection_owner.clone());
+		set_default_balance::<T, I>(&collection_owner);
+	}: _(SystemOrigin::Root, collection_owner_lookup, default_collection_config::<T, I>())
 	verify {
 		assert_last_event::<T, I>(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::<T, I>(Event::Transferred { collection, item, from: caller, to: target }.into());
@@ -372,7 +377,7 @@ benchmarks_instance_pallet! {
 		let (collection, caller, _) = create_collection::<T, I>();
 		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::<T, I>::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::<T, I>::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::<T, I>::max_value());
+		set_default_balance::<T, I>(&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::<T, I>::max_value());
+		set_default_balance::<T, I>(&caller);
 		let collection = T::Helper::collection(0);
 	}: _(SystemOrigin::Signed(caller.clone()), Some(collection))
 	verify {
@@ -680,7 +685,7 @@ benchmarks_instance_pallet! {
 		let price = ItemPrice::<T, I>::from(0u32);
 		let origin = SystemOrigin::Signed(seller.clone()).into();
 		Nfts::<T, I>::set_price(origin, collection, item, Some(price), Some(buyer_lookup))?;
-		T::Currency::make_free_balance_be(&buyer, DepositBalanceOf::<T, I>::max_value());
+		set_default_balance::<T, I>(&buyer);
 	}: _(SystemOrigin::Signed(buyer.clone()), collection, item, price)
 	verify {
 		assert_last_event::<T, I>(Event::ItemBought {
@@ -696,6 +701,7 @@ benchmarks_instance_pallet! {
 		let n in 0 .. T::MaxTips::get() as u32;
 		let amount = BalanceOf::<T, I>::from(100u32);
 		let caller: T::AccountId = whitelisted_caller();
+		set_default_balance::<T, I>(&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::<T>::set_block_number(One::one());
 		Nfts::<T, I>::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::<T, I>::max_value());
+		set_default_balance::<T, I>(&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::<T, I>::max_value());
+		set_default_balance::<T, I>(&target);
 		frame_system::Pallet::<T>::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::<T, I>::max_value());
+		set_default_balance::<T, I>(&item_owner);
 
 		let item = T::Helper::item(0);
 		assert_ok!(Nfts::<T, I>::force_mint(

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<T: Config<I>, I: 'static> Pallet<T, I> {
 	/// Creates a new swap offer for the specified item.
@@ -196,13 +193,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 					&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,
 				)?,
 			};
 		}

substrate/frame/nfts/src/features/attributes.rs

@@ -110,6 +110,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		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,13 +131,28 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		// 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);
+				let release_reason = if old_depositor == collection_details.owner {
+					HoldReason::CollectionOwnerAggregatedDeposit
+				} else {
+					HoldReason::AttributeDeposit
+				};
+				T::Currency::release(
+					&release_reason.into(),
+					&old_depositor,
+					old_deposit.amount,
+					BestEffort,
+				)?;
 			}
-			T::Currency::reserve(&depositor, deposit)?;
+			T::Currency::hold(&reason.into(), &depositor, deposit)?;
 		} else if deposit > old_deposit.amount {
-			T::Currency::reserve(&depositor, deposit - old_deposit.amount)?;
+			T::Currency::hold(&reason.into(), &depositor, deposit - old_deposit.amount)?;
 		} else if deposit < old_deposit.amount {
-			T::Currency::unreserve(&depositor, old_deposit.amount - deposit);
+			T::Currency::release(
+				&reason.into(),
+				&depositor,
+				old_deposit.amount - deposit,
+				BestEffort,
+			)?;
 		}
 
 		if is_depositor_collection_owner {
@@ -188,7 +208,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		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 +368,21 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		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::CollectionOwnerAggregatedDeposit.into(),
+					&collection_details.owner,
+					deposit.amount,
+					BestEffort,
+				)?;
 			},
 			_ => (),
 		}
@@ -415,7 +450,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		item: T::ItemId,
 		delegate: T::AccountId,
 		witness: CancelAttributesApprovalWitness,
-	) -> DispatchResult {
+	) -> Result<CancelAttributesApprovalWitness, DispatchError> {
 		ensure!(
 			Self::is_pallet_feature_enabled(PalletFeature::Attributes),
 			Error::<T, I>::MethodDisabled
@@ -435,20 +470,35 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				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::<T, I>::BadWitness);
 
 			if !deposited.is_zero() {
-				T::Currency::unreserve(&delegate, deposited);
+				T::Currency::release(
+					&HoldReason::AttributeDeposit.into(),
+					&delegate,
+					deposited,
+					BestEffort,
+				)?;
 			}
 
 			Self::deposit_event(Event::ItemAttributesApprovalRemoved {
 				collection,
 				item,
 				delegate,
 			});
-			Ok(())
+
+			Ok(CancelAttributesApprovalWitness { account_attributes: attributes })
 		})
 	}
 

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<T: Config<I>, I: 'static> Pallet<T, I> {
 	/// Pays the specified tips to the corresponding receivers.
@@ -43,7 +40,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	) -> 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<T: Config<I>, I: 'static> Pallet<T, I> {
 			ensure!(only_buyer == buyer, Error::<T, I>::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();