feat: not require deposit for force_approve_collection_transfer

Author: chungquantin

pallets/nfts/src/features/approvals.rs

@@ -204,12 +204,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	/// - `owner`: The owner of the collection items.
 	/// - `collection`: The identifier of the collection.
 	/// - `delegate`: The account that will be approved to take control of the collection items.
+	/// - `deposit`: The reserved amount for granting a collection approval.
 	/// - `maybe_deadline`: The optional deadline (in block numbers) specifying the time limit for
 	///   the approval.
 	pub(crate) fn do_approve_collection_transfer(
 		owner: T::AccountId,
 		collection: T::CollectionId,
 		delegate: T::AccountId,
+		deposit: DepositBalanceOf<T, I>,
 		maybe_deadline: Option<BlockNumberFor<T>>,
 	) -> DispatchResult {
 		ensure!(
@@ -234,11 +236,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		CollectionApprovals::<T, I>::try_mutate_exists(
 			(&collection, &owner, &delegate),
 			|maybe_approval| -> DispatchResult {
-				let deposit_required = T::CollectionApprovalDeposit::get();
 				let current_deposit =
 					maybe_approval.map(|(_, deposit)| deposit).unwrap_or_default();
-				T::Currency::reserve(&owner, deposit_required.saturating_sub(current_deposit))?;
-				*maybe_approval = Some((deadline, deposit_required));
+				T::Currency::reserve(&owner, deposit.saturating_sub(current_deposit))?;
+				*maybe_approval = Some((deadline, deposit));
 				Ok(())
 			},
 		)?;

pallets/nfts/src/lib.rs

@@ -2033,15 +2033,22 @@ pub mod pallet {
 		) -> DispatchResult {
 			let origin = ensure_signed(origin)?;
 			let delegate = T::Lookup::lookup(delegate)?;
-			Self::do_approve_collection_transfer(origin, collection, delegate, maybe_deadline)
+			Self::do_approve_collection_transfer(
+				origin,
+				collection,
+				delegate,
+				T::CollectionApprovalDeposit::get(),
+				maybe_deadline,
+			)
 		}
 
 		/// Force-approve collection items owned by the `owner` to be transferred by a delegated
-		/// third-party account. This function reserves the required deposit
-		/// `CollectionApprovalDeposit` from the `owner` account.
+		/// third-party account.
 		///
 		/// Origin must be the `ForceOrigin`.
 		///
+		/// Any deposit is left alone.
+		///
 		/// - `owner`: The owner of the collection items to be force-approved by the `origin`.
 		/// - `collection`: The collection of the items to be approved for delegated transfer.
 		/// - `delegate`: The account to delegate permission to transfer collection items owned by
@@ -2064,7 +2071,13 @@ pub mod pallet {
 			T::ForceOrigin::ensure_origin(origin)?;
 			let delegate = T::Lookup::lookup(delegate)?;
 			let owner = T::Lookup::lookup(owner)?;
-			Self::do_approve_collection_transfer(owner, collection, delegate, maybe_deadline)
+			Self::do_approve_collection_transfer(
+				owner,
+				collection,
+				delegate,
+				Zero::zero(),
+				maybe_deadline,
+			)
 		}
 
 		/// Cancel a collection approval.

pallets/nfts/src/tests.rs

@@ -4815,7 +4815,6 @@ fn force_approve_collection_transfer_works() {
 		let collection_owner = account(1);
 		let deadline: BlockNumberFor<Test> = 20;
 		let delegate = account(3);
-		let deposit = CollectionApprovalDeposit::get();
 		let item_id = 42;
 		let item_owner = account(2);
 
@@ -4866,16 +4865,13 @@ fn force_approve_collection_transfer_works() {
 			default_item_config()
 		));
 		// Approve collection without balance.
-		assert_noop!(
-			Nfts::force_approve_collection_transfer(
-				RuntimeOrigin::root(),
-				item_owner.clone(),
-				collection_id,
-				delegate.clone(),
-				None
-			),
-			BalancesError::<Test, _>::InsufficientBalance,
-		);
+		assert_ok!(Nfts::force_approve_collection_transfer(
+			RuntimeOrigin::root(),
+			item_owner.clone(),
+			collection_id,
+			delegate.clone(),
+			None
+		));
 
 		Balances::make_free_balance_be(&item_owner, 100);
 		// Approving a collection to a delegate with:
@@ -4909,10 +4905,10 @@ fn force_approve_collection_transfer_works() {
 				}
 				.into(),
 			);
-			assert_eq!(Balances::reserved_balance(&item_owner), deposit);
+			assert_eq!(Balances::reserved_balance(&item_owner), 0);
 			assert_eq!(
 				CollectionApprovals::get((collection_id, &item_owner, &delegate)),
-				Some((deadline, deposit))
+				Some((deadline, 0))
 			);
 		}