feat: transfer_from and decrease_allowance (#134)

Co-authored-by: Daan van der Plas <[email protected]>

pallets/api/src/fungibles/mod.rs

@@ -110,6 +110,28 @@ pub mod pallet {
 			AssetsOf::<T>::transfer_keep_alive(origin, id.into(), target, amount)
 		}
 
+		/// Transfers `value` amount of tokens from the delegated account approved by the `owner` to
+		/// account `to`, with additional `data` in unspecified format.
+		///
+		/// # Parameters
+		/// * `id` - The ID of the asset.
+		/// * `owner` - The account from which the asset balance will be withdrawn.
+		/// * `to` - The recipient account.
+		/// * `value` - The number of tokens to transfer.
+		#[pallet::call_index(4)]
+		#[pallet::weight(AssetsWeightInfoOf::<T>::transfer_approved())]
+		pub fn transfer_from(
+			origin: OriginFor<T>,
+			id: AssetIdOf<T>,
+			owner: AccountIdOf<T>,
+			target: AccountIdOf<T>,
+			amount: BalanceOf<T>,
+		) -> DispatchResult {
+			let owner = T::Lookup::unlookup(owner);
+			let target = T::Lookup::unlookup(target);
+			AssetsOf::<T>::transfer_approved(origin, id.into(), owner, target, amount)
+		}
+
 		/// Approves an account to spend a specified number of tokens on behalf of the caller.
 		///
 		/// # Parameters
@@ -124,17 +146,15 @@ pub mod pallet {
 			spender: AccountIdOf<T>,
 			value: BalanceOf<T>,
 		) -> DispatchResultWithPostInfo {
-			let weight = |approve: u32, cancel: u32| -> Weight {
-				<T as Config>::WeightInfo::approve(cancel, approve)
-			};
-			let who = ensure_signed(origin.clone()).map_err(|e| e.with_weight(weight(0, 0)))?;
+			let who = ensure_signed(origin.clone())
+				.map_err(|e| e.with_weight(Self::weight_approve(0, 0)))?;
 			let current_allowance = AssetsOf::<T>::allowance(id.clone(), &who, &spender);
 			let spender = T::Lookup::unlookup(spender);
 			let id: AssetIdParameterOf<T> = id.into();
 
 			// If the new value is equal to the current allowance, do nothing.
 			let return_weight = if value == current_allowance {
-				weight(0, 0)
+				Self::weight_approve(0, 0)
 			}
 			// If the new value is greater than the current allowance, approve the difference
 			// because `approve_transfer` works additively (see `pallet-assets`).
@@ -145,17 +165,17 @@ pub mod pallet {
 					spender,
 					value.saturating_sub(current_allowance),
 				)
-				.map_err(|e| e.with_weight(weight(1, 0)))?;
-				weight(1, 0)
+				.map_err(|e| e.with_weight(Self::weight_approve(1, 0)))?;
+				Self::weight_approve(1, 0)
 			} else {
 				// If the new value is less than the current allowance, cancel the approval and set the new value
 				AssetsOf::<T>::cancel_approval(origin.clone(), id.clone(), spender.clone())
-					.map_err(|e| e.with_weight(weight(0, 1)))?;
+					.map_err(|e| e.with_weight(Self::weight_approve(0, 1)))?;
 				if value.is_zero() {
-					return Ok(Some(weight(0, 1)).into());
+					return Ok(Some(Self::weight_approve(0, 1)).into());
 				}
 				AssetsOf::<T>::approve_transfer(origin, id, spender, value)?;
-				weight(1, 1)
+				Self::weight_approve(1, 1)
 			};
 			Ok(Some(return_weight).into())
 		}
@@ -177,6 +197,42 @@ pub mod pallet {
 			let spender = T::Lookup::unlookup(spender);
 			AssetsOf::<T>::approve_transfer(origin, id.into(), spender, value)
 		}
+
+		/// Decreases the allowance of a spender.
+		///
+		/// # Parameters
+		/// * `id` - The ID of the asset.
+		/// * `spender` - The account that is allowed to spend the tokens.
+		/// * `value` - The number of tokens to decrease the allowance by.
+		#[pallet::call_index(7)]
+		#[pallet::weight(<T as Config>::WeightInfo::approve(1, 1))]
+		pub fn decrease_allowance(
+			origin: OriginFor<T>,
+			id: AssetIdOf<T>,
+			spender: AccountIdOf<T>,
+			value: BalanceOf<T>,
+		) -> DispatchResultWithPostInfo {
+			let who = ensure_signed(origin.clone())
+				.map_err(|e| e.with_weight(Self::weight_approve(0, 0)))?;
+			let mut current_allowance = AssetsOf::<T>::allowance(id.clone(), &who, &spender);
+			let spender = T::Lookup::unlookup(spender);
+			let id: AssetIdParameterOf<T> = id.into();
+
+			if value.is_zero() {
+				return Ok(Some(Self::weight_approve(0, 0)).into());
+			}
+
+			current_allowance.saturating_reduce(value);
+			// Cancel the aproval and set the new value if `current_allowance` is more than zero.
+			AssetsOf::<T>::cancel_approval(origin.clone(), id.clone(), spender.clone())
+				.map_err(|e| e.with_weight(Self::weight_approve(0, 1)))?;
+
+			if current_allowance.is_zero() {
+				return Ok(Some(Self::weight_approve(0, 1)).into());
+			}
+			AssetsOf::<T>::approve_transfer(origin, id, spender, current_allowance)?;
+			Ok(().into())
+		}
 	}
 
 	impl<T: Config> Pallet<T> {
@@ -208,5 +264,9 @@ pub mod pallet {
 				},
 			}
 		}
+
+		pub fn weight_approve(approve: u32, cancel: u32) -> Weight {
+			<T as Config>::WeightInfo::approve(cancel, approve)
+		}
 	}
 }

pallets/api/src/fungibles/tests.rs

@@ -19,6 +19,43 @@ fn transfer_works() {
 	});
 }
 
+#[test]
+fn transfer_from_works() {
+	new_test_ext().execute_with(|| {
+		let amount: Balance = 100 * UNIT;
+		// Approve CHARLIE to transfer up to `amount` to BOB.
+		create_asset_mint_and_approve(ALICE, ASSET, ALICE, amount * 2, CHARLIE, amount / 2);
+		let transferred = amount / 2;
+		// Successfully call transfer from.
+		let alice_balance_before_transfer = Assets::balance(ASSET, &ALICE);
+		let balance_before_transfer = Assets::balance(ASSET, &BOB);
+		assert_ok!(Fungibles::transfer_from(signed(CHARLIE), ASSET, ALICE, BOB, transferred));
+		let alice_balance_after_transfer = Assets::balance(ASSET, &ALICE);
+		let balance_after_transfer = Assets::balance(ASSET, &BOB);
+		// Check that BOB receives the `amount` and ALICE `amount` is spent successfully by CHARLIE.
+		assert_eq!(balance_after_transfer, balance_before_transfer + transferred);
+		assert_eq!(alice_balance_after_transfer, alice_balance_before_transfer - transferred);
+	});
+}
+
+#[test]
+fn decrease_allowance_works() {
+	new_test_ext().execute_with(|| {
+		let amount: Balance = 100 * UNIT;
+		create_asset_mint_and_approve(ALICE, ASSET, ALICE, amount, BOB, amount);
+		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount);
+		// Owner balance is not changed if decreased by zero.
+		assert_ok!(Fungibles::decrease_allowance(signed(ALICE), ASSET, BOB, 0));
+		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount);
+		// Decrease allowance successfully.
+		assert_ok!(Fungibles::decrease_allowance(signed(ALICE), ASSET, BOB, amount / 2 - 1 * UNIT));
+		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount / 2 + 1 * UNIT);
+		// Saturating if current allowance is decreased more than the owner balance.
+		assert_ok!(Fungibles::decrease_allowance(signed(ALICE), ASSET, BOB, amount));
+		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), 0);
+	});
+}
+
 // Non-additive, sets new value.
 #[test]
 fn approve_works() {

pallets/api/src/mock.rs

@@ -103,6 +103,7 @@ impl crate::fungibles::Config for Test {
 
 pub(crate) const ALICE: AccountId = 1;
 pub(crate) const BOB: AccountId = 2;
+pub(crate) const CHARLIE: AccountId = 3;
 pub(crate) const INIT_AMOUNT: Balance = 100_000_000 * UNIT;
 pub(crate) const UNIT: Balance = 10_000_000_000;
 
@@ -112,7 +113,7 @@ pub(crate) fn new_test_ext() -> sp_io::TestExternalities {
 		.expect("Frame system builds valid default genesis config");
 
 	pallet_balances::GenesisConfig::<Test> {
-		balances: vec![(ALICE, INIT_AMOUNT), (BOB, INIT_AMOUNT)],
+		balances: vec![(ALICE, INIT_AMOUNT), (BOB, INIT_AMOUNT), (CHARLIE, INIT_AMOUNT)],
 	}
 	.assimilate_storage(&mut t)
 	.expect("Pallet balances storage can be assimilated");

pop-api/integration-tests/src/local_fungibles.rs

@@ -78,6 +78,22 @@ fn transfer(
 	result
 }
 
+fn transfer_from(
+	addr: AccountId32,
+	asset_id: AssetId,
+	from: AccountId32,
+	to: AccountId32,
+	value: Balance,
+) -> ExecReturnValue {
+	let function = function_selector("transfer_from");
+	let data: Vec<u8> = vec![];
+	let params =
+		[function, asset_id.encode(), from.encode(), to.encode(), value.encode(), data.encode()]
+			.concat();
+	let result = bare_call(addr, params, 0).expect("should work");
+	result
+}
+
 fn approve(
 	addr: AccountId32,
 	asset_id: AssetId,
@@ -102,6 +118,18 @@ fn increase_allowance(
 	result
 }
 
+fn decrease_allowance(
+	addr: AccountId32,
+	asset_id: AssetId,
+	spender: AccountId32,
+	value: Balance,
+) -> ExecReturnValue {
+	let function = function_selector("decrease_allowance");
+	let params = [function, asset_id.encode(), spender.encode(), value.encode()].concat();
+	let result = bare_call(addr, params, 0).expect("should work");
+	result
+}
+
 // fn asset_exists(addr: AccountId32, asset_id: AssetId) -> bool {
 // 	let function = function_selector("asset_exists");
 // 	let params = [function, asset_id.encode()].concat();
@@ -171,14 +199,15 @@ fn create_asset_mint_and_approve(
 	mint: Balance,
 	spender: AccountId32,
 	approve: Balance,
-) {
-	create_asset_and_mint_to(owner.clone(), asset_id, to.clone(), mint);
+) -> AssetId {
+	let asset = create_asset_and_mint_to(owner.clone(), asset_id, to.clone(), mint);
 	assert_ok!(Assets::approve_transfer(
 		RuntimeOrigin::signed(to.into()),
 		asset_id.into(),
 		spender.into(),
 		approve,
 	));
+	asset
 }
 
 // Freeze an asset.
@@ -366,6 +395,52 @@ fn transfer_works() {
 	});
 }
 
+#[test]
+fn transfer_from_works() {
+	new_test_ext().execute_with(|| {
+		let _ = env_logger::try_init();
+		let addr = instantiate(CONTRACT, INIT_VALUE, vec![]);
+		let amount: Balance = 100 * UNIT;
+
+		// Asset does not exist.
+		assert_eq!(
+			decoded::<Error>(transfer_from(addr.clone(), 1, ALICE, BOB, amount / 2)),
+			Ok(Module { index: 52, error: 3 }),
+		);
+		// Create asset with Alice as owner and mint `amount` to contract address.
+		let asset = create_asset_and_mint_to(ALICE, 1, ALICE, amount);
+		// Unapproved transfer.
+		assert_eq!(
+			decoded::<Error>(transfer_from(addr.clone(), asset, ALICE, BOB, amount / 2)),
+			Ok(Module { index: 52, error: 10 })
+		);
+		assert_ok!(Assets::approve_transfer(
+			RuntimeOrigin::signed(ALICE.into()),
+			asset.into(),
+			addr.clone().into(),
+			amount + 1 * UNIT,
+		));
+		// Asset is not live, i.e. frozen or being destroyed.
+		freeze_asset(ALICE, asset);
+		assert_eq!(
+			decoded::<Error>(transfer_from(addr.clone(), asset, ALICE, BOB, amount)),
+			Ok(Module { index: 52, error: 16 }),
+		);
+		thaw_asset(ALICE, asset);
+		// Not enough balance.
+		assert_eq!(
+			decoded::<Error>(transfer_from(addr.clone(), asset, ALICE, BOB, amount + 1 * UNIT,)),
+			Ok(Module { index: 52, error: 0 }),
+		);
+		// Successful transfer.
+		let balance_before_transfer = Assets::balance(asset, &BOB);
+		let result = transfer_from(addr.clone(), asset, ALICE, BOB, amount / 2);
+		assert!(!result.did_revert(), "Contract reverted!");
+		let balance_after_transfer = Assets::balance(asset, &BOB);
+		assert_eq!(balance_after_transfer, balance_before_transfer + amount / 2);
+	});
+}
+
 #[test]
 fn approve_works() {
 	new_test_ext().execute_with(|| {
@@ -458,6 +533,42 @@ fn increase_allowance_works() {
 	});
 }
 
+#[test]
+fn decrease_allowance_works() {
+	new_test_ext().execute_with(|| {
+		let _ = env_logger::try_init();
+		let addr = instantiate(CONTRACT, INIT_VALUE, vec![]);
+		let amount: Balance = 100 * UNIT;
+		// Asset does not exist.
+		assert_eq!(
+			decoded::<Error>(decrease_allowance(addr.clone(), 0, BOB, amount)),
+			Ok(Module { index: 52, error: 3 }),
+		);
+		// Create asset and mint to the address contract, delegate Bob to spend the `amount`.
+		let asset =
+			create_asset_mint_and_approve(addr.clone(), 0, addr.clone(), amount, BOB, amount);
+		// Asset is not live, i.e. frozen or being destroyed.
+		freeze_asset(addr.clone(), asset);
+		assert_eq!(
+			decoded::<Error>(decrease_allowance(addr.clone(), asset, BOB, amount)),
+			Ok(Module { index: 52, error: 16 }),
+		);
+		thaw_asset(addr.clone(), asset);
+		// Successfully decrease allowance.
+		let bob_allowance_before = Assets::allowance(asset, &addr, &BOB);
+		let result = decrease_allowance(addr.clone(), 0, BOB, amount / 2 - 1 * UNIT);
+		assert!(!result.did_revert(), "Contract reverted!");
+		let bob_allowance_after = Assets::allowance(asset, &addr, &BOB);
+		assert_eq!(bob_allowance_before - bob_allowance_after, amount / 2 - 1 * UNIT);
+		// Asset is not live, i.e. frozen or being destroyed.
+		start_destroy_asset(addr.clone(), asset);
+		assert_eq!(
+			decoded::<Error>(decrease_allowance(addr.clone(), asset, BOB, amount)),
+			Ok(Module { index: 52, error: 16 }),
+		);
+	});
+}
+
 /// 2. PSP-22 Metadata Interface:
 /// - token_name
 /// - token_symbol

runtime/devnet/src/config/api.rs

@@ -20,7 +20,12 @@ impl Contains<RuntimeCall> for AllowedApiCalls {
 		use fungibles::Call::*;
 		matches!(
 			c,
-			RuntimeCall::Fungibles(transfer { .. } | approve { .. } | increase_allowance { .. })
+			RuntimeCall::Fungibles(
+				transfer { .. }
+					| transfer_from { .. }
+					| approve { .. } | increase_allowance { .. }
+					| decrease_allowance { .. }
+			)
 		)
 	}
 }