From 1aa79f342e483c37d34bf3b7f6ad0b3a8eeada0a Mon Sep 17 00:00:00 2001 From: chungquantin <56880684+chungquantin@users.noreply.github.com> Date: Thu, 9 Jan 2025 09:15:12 +0700 Subject: [PATCH] chore: resolve feedback comments --- pallets/nfts/src/features/atomic_swap.rs | 4 ++-- pallets/nfts/src/features/buy_sell.rs | 2 +- pallets/nfts/src/features/transfer.rs | 6 +++--- pallets/nfts/src/impl_nonfungibles.rs | 8 ++++---- pallets/nfts/src/lib.rs | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pallets/nfts/src/features/atomic_swap.rs b/pallets/nfts/src/features/atomic_swap.rs index 383fad05..1ffd6dce 100644 --- a/pallets/nfts/src/features/atomic_swap.rs +++ b/pallets/nfts/src/features/atomic_swap.rs @@ -210,19 +210,19 @@ impl, I: 'static> Pallet { // This also removes the swap. Self::do_transfer( - Some(&receive_item.owner), send_collection_id, send_item_id, receive_item.owner.clone(), + Some(&receive_item.owner), |_, _| Ok(()), )?; // 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(), + Some(&send_item.owner), |_, _| Ok(()), )?; diff --git a/pallets/nfts/src/features/buy_sell.rs b/pallets/nfts/src/features/buy_sell.rs index 25fd80c9..2b4d2967 100644 --- a/pallets/nfts/src/features/buy_sell.rs +++ b/pallets/nfts/src/features/buy_sell.rs @@ -158,7 +158,7 @@ impl, I: 'static> Pallet { let old_owner = details.owner.clone(); - Self::do_transfer(Some(&buyer), collection, item, buyer.clone(), |_, _| Ok(()))?; + Self::do_transfer(collection, item, buyer.clone(), Some(&buyer), |_, _| Ok(()))?; Self::deposit_event(Event::ItemBought { collection, diff --git a/pallets/nfts/src/features/transfer.rs b/pallets/nfts/src/features/transfer.rs index 3c0e704d..9f5dc77b 100644 --- a/pallets/nfts/src/features/transfer.rs +++ b/pallets/nfts/src/features/transfer.rs @@ -25,11 +25,11 @@ use crate::*; impl, I: 'static> Pallet { /// 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. + /// - `depositor`: The account reserving the `CollectionBalanceDeposit` from if `dest` holds no + /// items in the collection. /// - `with_details`: A closure that provides access to the collection and item details, /// allowing customization of the transfer process. /// @@ -47,10 +47,10 @@ impl, I: 'static> Pallet { /// - 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, + depositor: Option<&T::AccountId>, with_details: impl FnOnce( &CollectionDetailsFor, &mut ItemDetailsFor, diff --git a/pallets/nfts/src/impl_nonfungibles.rs b/pallets/nfts/src/impl_nonfungibles.rs index 22517397..e8fa4457 100644 --- a/pallets/nfts/src/impl_nonfungibles.rs +++ b/pallets/nfts/src/impl_nonfungibles.rs @@ -412,10 +412,10 @@ impl, I: 'static> Transfer for Pallet { 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(), None, |_, _| Ok(())) } fn disable_transfer(collection: &Self::CollectionId, item: &Self::ItemId) -> DispatchResult { diff --git a/pallets/nfts/src/lib.rs b/pallets/nfts/src/lib.rs index c9f6a503..ef1f5e5f 100644 --- a/pallets/nfts/src/lib.rs +++ b/pallets/nfts/src/lib.rs @@ -1096,7 +1096,7 @@ 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| { + Self::do_transfer(collection, item, dest, None, |_, details| { if details.owner != origin { Self::check_approval(&collection, &Some(item), &details.owner, &origin)?; }