chore: resolve feedback comments

pallets/nfts/src/features/atomic_swap.rs

@@ -210,20 +210,20 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		// This also removes the swap.
 		Self::do_transfer(
-			Some(&receive_item.owner),
 			send_collection_id,
 			send_item_id,
 			receive_item.owner.clone(),
 			|_, _| Ok(()),
+			Some(&receive_item.owner),
 		)?;
 		// Owner of `send_item` is responsible for the deposit if the collection balance
 		// went to zero due to the previous transfer.
 		Self::do_transfer(
-			Some(&send_item.owner),
 			receive_collection_id,
 			receive_item_id,
 			send_item.owner.clone(),
 			|_, _| Ok(()),
+			Some(&send_item.owner),
 		)?;
 
 		Self::deposit_event(Event::SwapClaimed {

pallets/nfts/src/features/buy_sell.rs

@@ -158,7 +158,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		let old_owner = details.owner.clone();
 
-		Self::do_transfer(Some(&buyer), collection, item, buyer.clone(), |_, _| Ok(()))?;
+		Self::do_transfer(collection, item, buyer.clone(), |_, _| Ok(()), Some(&buyer))?;
 
 		Self::deposit_event(Event::ItemBought {
 			collection,

pallets/nfts/src/features/transfer.rs

@@ -25,13 +25,13 @@ use crate::*;
 impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	/// Transfer an NFT to the specified destination account.
 	///
-	/// - `depositor`: The account reserving the `CollectionBalanceDeposit` from if `dest` holds no
-	///   items in the collection.
 	/// - `collection`: The ID of the collection to which the NFT belongs.
 	/// - `item`: The ID of the NFT to transfer.
 	/// - `dest`: The destination account to which the NFT will be transferred.
 	/// - `with_details`: A closure that provides access to the collection and item details,
 	///   allowing customization of the transfer process.
+	/// - `depositor`: The account reserving the `CollectionBalanceDeposit` from if `dest` holds no
+	///   items in the collection.
 	///
 	/// This function performs the actual transfer of an NFT to the destination account.
 	/// It checks various conditions like item lock status and transferability settings
@@ -47,14 +47,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	/// - If the collection or item is non-transferable
 	///   ([`ItemsNonTransferable`](crate::Error::ItemsNonTransferable)).
 	pub fn do_transfer(
-		depositor: Option<&T::AccountId>,
 		collection: T::CollectionId,
 		item: T::ItemId,
 		dest: T::AccountId,
 		with_details: impl FnOnce(
 			&CollectionDetailsFor<T, I>,
 			&mut ItemDetailsFor<T, I>,
 		) -> DispatchResult,
+		depositor: Option<&T::AccountId>,
 	) -> DispatchResult {
 		// Retrieve collection details.
 		let collection_details =

pallets/nfts/src/impl_nonfungibles.rs

@@ -412,10 +412,10 @@ impl<T: Config<I>, I: 'static> Transfer<T::AccountId> for Pallet<T, I> {
 		destination: &T::AccountId,
 	) -> DispatchResult {
 		// The item's owner pays for the deposit of `AccountBalance` if the `dest` holds no items
-		// in `collection`. A malicious actor could have a deposit reserved from `dest` without
-		// them knowing about the transfer. The deposit amount can be accounted for in the off chain
-		// price of the NFT.
-		Self::do_transfer(None, *collection, *item, destination.clone(), |_, _| Ok(()))
+		// in `collection`. If `dest` paid the deposit, a malicious actor could transfer NFTs to
+		// reserve involuntary deposits for `dest` . The deposit amount can be accounted for in the
+		// off chain price of the NFT.
+		Self::do_transfer(*collection, *item, destination.clone(), |_, _| Ok(()), None)
 	}
 
 	fn disable_transfer(collection: &Self::CollectionId, item: &Self::ItemId) -> DispatchResult {

pallets/nfts/src/lib.rs

@@ -1096,12 +1096,18 @@ pub mod pallet {
 			// items in `collection`. A malicious actor could have a deposit reserved from `dest`
 			// without them knowing about the transfer. The deposit amount can be accounted for
 			// in the off chain price of the NFT.
-			Self::do_transfer(None, collection, item, dest, |_, details| {
-				if details.owner != origin {
-					Self::check_approval(&collection, &Some(item), &details.owner, &origin)?;
-				}
-				Ok(())
-			})
+			Self::do_transfer(
+				collection,
+				item,
+				dest,
+				|_, details| {
+					if details.owner != origin {
+						Self::check_approval(&collection, &Some(item), &details.owner, &origin)?;
+					}
+					Ok(())
+				},
+				None,
+			)
 		}
 
 		/// Re-evaluate the deposits on some items.