diff --git a/pallets/nfts/src/features/atomic_swap.rs b/pallets/nfts/src/features/atomic_swap.rs index 35e857525..2b70e112a 100644 --- a/pallets/nfts/src/features/atomic_swap.rs +++ b/pallets/nfts/src/features/atomic_swap.rs @@ -210,14 +210,14 @@ impl, I: 'static> Pallet { // This also removes the swap. Self::do_transfer( - &caller, + Some(&caller), send_collection_id, send_item_id, receive_item.owner.clone(), |_, _| Ok(()), )?; Self::do_transfer( - &caller, + Some(&caller), receive_collection_id, receive_item_id, send_item.owner.clone(), diff --git a/pallets/nfts/src/features/buy_sell.rs b/pallets/nfts/src/features/buy_sell.rs index d28478cb7..25fd80c95 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(&buyer, collection, item, buyer.clone(), |_, _| Ok(()))?; + Self::do_transfer(Some(&buyer), collection, item, buyer.clone(), |_, _| Ok(()))?; Self::deposit_event(Event::ItemBought { collection, diff --git a/pallets/nfts/src/features/transfer.rs b/pallets/nfts/src/features/transfer.rs index 28b30e271..68d6c39ff 100644 --- a/pallets/nfts/src/features/transfer.rs +++ b/pallets/nfts/src/features/transfer.rs @@ -46,7 +46,7 @@ impl, I: 'static> Pallet { /// - If the collection or item is non-transferable /// ([`ItemsNonTransferable`](crate::Error::ItemsNonTransferable)). pub fn do_transfer( - caller: &T::AccountId, + caller: Option<&T::AccountId>, collection: T::CollectionId, item: T::ItemId, dest: T::AccountId, @@ -97,13 +97,9 @@ impl, I: 'static> Pallet { true => T::CollectionBalanceDeposit::get(), false => Zero::zero(), }; - // The destination account covers the `CollectionBalanceDeposit` if it has sufficient - // balance. Otherwise, the caller is accountable for it. - let deposit_account = - match T::Currency::can_reserve(&dest, T::CollectionBalanceDeposit::get()) { - true => &dest, - false => caller, - }; + // Reserve `CollectionBalanceDeposit` from the caller if provided. Otherwise, reserve from + // the collection item's owner. + let deposit_account = caller.unwrap_or(&details.owner); Self::increment_account_balance(collection, &dest, (deposit_account, deposit_amount))?; diff --git a/pallets/nfts/src/impl_nonfungibles.rs b/pallets/nfts/src/impl_nonfungibles.rs index 60e89a747..5186d86da 100644 --- a/pallets/nfts/src/impl_nonfungibles.rs +++ b/pallets/nfts/src/impl_nonfungibles.rs @@ -411,7 +411,7 @@ impl, I: 'static> Transfer for Pallet { item: &Self::ItemId, destination: &T::AccountId, ) -> DispatchResult { - Self::do_transfer(destination, *collection, *item, destination.clone(), |_, _| Ok(())) + Self::do_transfer(None, *collection, *item, destination.clone(), |_, _| 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 1b1174c55..0cc0d93f1 100644 --- a/pallets/nfts/src/lib.rs +++ b/pallets/nfts/src/lib.rs @@ -1092,7 +1092,7 @@ pub mod pallet { let origin = ensure_signed(origin)?; let dest = T::Lookup::lookup(dest)?; - Self::do_transfer(&origin, collection, item, dest, |_, details| { + Self::do_transfer(None, collection, item, dest, |_, details| { if details.owner != origin { Self::check_approval(&collection, &Some(item), &details.owner, &origin)?; } diff --git a/pallets/nfts/src/tests.rs b/pallets/nfts/src/tests.rs index 2b80f61ae..6a457c9cc 100644 --- a/pallets/nfts/src/tests.rs +++ b/pallets/nfts/src/tests.rs @@ -293,14 +293,14 @@ fn lifecycle_should_work() { assert_eq!(Balances::reserved_balance(&account(1)), 8 + 3 * balance_deposit); assert_ok!(Nfts::transfer(RuntimeOrigin::signed(account(1)), 0, 70, account(2))); - assert_eq!(Balances::reserved_balance(&account(1)), 8 + 2 * balance_deposit); - assert_eq!(Balances::reserved_balance(&account(2)), balance_deposit); + assert_eq!(Balances::reserved_balance(&account(1)), 8 + 3 * balance_deposit); + assert_eq!(Balances::reserved_balance(&account(2)), 0); assert_ok!(Nfts::set_metadata(RuntimeOrigin::signed(account(1)), 0, 42, bvec![42, 42])); - assert_eq!(Balances::reserved_balance(&account(1)), 11 + 2 * balance_deposit); + assert_eq!(Balances::reserved_balance(&account(1)), 11 + 3 * balance_deposit); assert!(ItemMetadataOf::::contains_key(0, 42)); assert_ok!(Nfts::set_metadata(RuntimeOrigin::signed(account(1)), 0, 69, bvec![69, 69])); - assert_eq!(Balances::reserved_balance(&account(1)), 14 + 2 * balance_deposit); + assert_eq!(Balances::reserved_balance(&account(1)), 14 + 3 * balance_deposit); assert!(ItemMetadataOf::::contains_key(0, 69)); assert!(ItemConfigOf::::contains_key(0, 69)); let w = Nfts::get_destroy_witness(&0).unwrap(); @@ -859,10 +859,10 @@ fn transfer_requires_deposit_works() { )); assert_eq!( AccountBalance::get(collection_id, &dest), - Some((1, (dest.clone(), balance_deposit))) + Some((1, (item_owner.clone(), balance_deposit))) ); - assert_eq!(Balances::reserved_balance(&item_owner), 2 * balance_deposit); - assert_eq!(Balances::reserved_balance(&dest), balance_deposit); + assert_eq!(Balances::reserved_balance(&item_owner), 3 * balance_deposit); + assert_eq!(Balances::reserved_balance(&dest), 0); assert!(items().contains(&(dest, collection_id, item_id))); } }); @@ -1011,10 +1011,10 @@ fn transfer_owner_should_work() { assert_eq!(Balances::reserved_balance(&account(3)), 44); assert_ok!(Nfts::transfer(RuntimeOrigin::signed(account(1)), 0, 42, account(2))); - // The reserved balance of account 1 should remain the same. For account 2 the - // `balance_deposit` is reserved because it is the new item owner. - assert_eq!(Balances::reserved_balance(&account(1)), 1); - assert_eq!(Balances::reserved_balance(&account(2)), balance_deposit); + // The reserved balance of account 1 is incremented to create a new storage record for the + // account 2. + assert_eq!(Balances::reserved_balance(&account(1)), 1 + balance_deposit); + assert_eq!(Balances::reserved_balance(&account(2)), 0); // 2's acceptance from before is reset when it became an owner, so it cannot be transferred // without a fresh acceptance. @@ -1585,8 +1585,8 @@ fn set_item_owner_attributes_should_work() { Attribute::::get((0, Some(0), AttributeNamespace::ItemOwner, &key)).unwrap(); assert_eq!(deposit.account, Some(account(3))); assert_eq!(deposit.amount, 13); - assert_eq!(Balances::reserved_balance(account(2)), 3); - assert_eq!(Balances::reserved_balance(account(3)), 13 + balance_deposit()); + assert_eq!(Balances::reserved_balance(account(2)), 3 + balance_deposit()); + assert_eq!(Balances::reserved_balance(account(3)), 13); // validate attributes on item deletion assert_ok!(Nfts::burn(RuntimeOrigin::signed(account(3)), 0, 0));