From 4ca88c93543711abe6068c99c1c0277f3e203492 Mon Sep 17 00:00:00 2001
From: chungquantin <56880684+chungquantin@users.noreply.github.com>
Date: Fri, 10 Jan 2025 18:05:24 +0700
Subject: [PATCH] feat: not require deposit for
 force_approve_collection_transfer

---
 pallets/nfts/src/features/approvals.rs |  7 ++++---
 pallets/nfts/src/lib.rs                | 18 ++++++++++++++++--
 pallets/nfts/src/tests.rs              | 22 +++++++++-------------
 3 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/pallets/nfts/src/features/approvals.rs b/pallets/nfts/src/features/approvals.rs
index 7c654d51..f7db22a7 100644
--- a/pallets/nfts/src/features/approvals.rs
+++ b/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(())
 			},
 		)?;
diff --git a/pallets/nfts/src/lib.rs b/pallets/nfts/src/lib.rs
index 90ac9561..5fba2f23 100644
--- a/pallets/nfts/src/lib.rs
+++ b/pallets/nfts/src/lib.rs
@@ -2033,7 +2033,13 @@ 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
@@ -2042,6 +2048,8 @@ pub mod pallet {
 		///
 		/// 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 +2072,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.
diff --git a/pallets/nfts/src/tests.rs b/pallets/nfts/src/tests.rs
index 053fa873..d0c6bff9 100644
--- a/pallets/nfts/src/tests.rs
+++ b/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))
 			);
 		}