fix: account balance logic (#418)

pallets/nfts/src/features/atomic_swap.rs

@@ -210,14 +210,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 		// 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(),

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(&buyer, collection, item, buyer.clone(), |_, _| Ok(()))?;
+		Self::do_transfer(Some(&buyer), collection, item, buyer.clone(), |_, _| Ok(()))?;
 
 		Self::deposit_event(Event::ItemBought {
 			collection,

pallets/nfts/src/features/transfer.rs

@@ -46,7 +46,7 @@ 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(
-		caller: &T::AccountId,
+		caller: Option<&T::AccountId>,
 		collection: T::CollectionId,
 		item: T::ItemId,
 		dest: T::AccountId,
@@ -97,13 +97,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				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))?;
 

pallets/nfts/src/impl_nonfungibles.rs

@@ -411,7 +411,7 @@ impl<T: Config<I>, I: 'static> Transfer<T::AccountId> for Pallet<T, I> {
 		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 {

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)?;
 				}

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::<Test>::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::<Test>::contains_key(0, 69));
 		assert!(ItemConfigOf::<Test>::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::<Test>::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));