Skip to content

Commit

Permalink
chore: resolve feedback comments
Browse files Browse the repository at this point in the history
  • Loading branch information
chungquantin committed Jan 9, 2025
1 parent 986a5b6 commit 1aa79f3
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 11 deletions.
4 changes: 2 additions & 2 deletions pallets/nfts/src/features/atomic_swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,19 +210,19 @@ 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(),
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(()),
)?;

Expand Down
2 changes: 1 addition & 1 deletion pallets/nfts/src/features/buy_sell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(), Some(&buyer), |_, _| Ok(()))?;

Self::deposit_event(Event::ItemBought {
collection,
Expand Down
6 changes: 3 additions & 3 deletions pallets/nfts/src/features/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ 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.
/// - `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.
///
Expand All @@ -47,10 +47,10 @@ 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,
depositor: Option<&T::AccountId>,
with_details: impl FnOnce(
&CollectionDetailsFor<T, I>,
&mut ItemDetailsFor<T, I>,
Expand Down
8 changes: 4 additions & 4 deletions pallets/nfts/src/impl_nonfungibles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(), None, |_, _| Ok(()))
}

fn disable_transfer(collection: &Self::CollectionId, item: &Self::ItemId) -> DispatchResult {
Expand Down
2 changes: 1 addition & 1 deletion pallets/nfts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
}
Expand Down

0 comments on commit 1aa79f3

Please sign in to comment.