feat: implement transfer_from and decrease_allowance

integration-tests/src/lib.rs

@@ -17,8 +17,9 @@ use emulated_integration_tests_common::{
 	},
 };
 use frame_support::{
-	dispatch::RawOrigin, pallet_prelude::Weight, sp_runtime::traits::Dispatchable,
-	sp_runtime::DispatchResult,
+	dispatch::RawOrigin,
+	pallet_prelude::Weight,
+	sp_runtime::{traits::Dispatchable, DispatchResult},
 };
 use polkadot_runtime_parachains::assigner_on_demand;
 use pop_runtime_common::Balance;
@@ -424,7 +425,8 @@ fn reserve_transfer_native_asset_from_system_para_to_para() {
 fn reserve_transfer_native_asset_from_para_to_system_para() {
 	init_tracing();
 
-	// Setup: reserve transfer from AH to Pop, so that sovereign account accurate for return transfer
+	// Setup: reserve transfer from AH to Pop, so that sovereign account accurate for return
+	// transfer
 	let amount_to_send: Balance = ASSET_HUB_ROCOCO_ED * 1000;
 	fund_pop_from_system_para(
 		AssetHubRococoParaSender::get(),
@@ -488,7 +490,8 @@ fn place_coretime_spot_order_from_para_to_relay() {
 
 	let beneficiary: sp_runtime::AccountId32 = [1u8; 32].into();
 
-	// Setup: reserve transfer from relay to Pop, so that sovereign account accurate for return transfer
+	// Setup: reserve transfer from relay to Pop, so that sovereign account accurate for return
+	// transfer
 	let amount_to_send: Balance = pop_runtime::UNIT * 1000;
 	fund_pop_from_relay(RococoRelaySender::get(), amount_to_send, beneficiary.clone());
 

pallets/api/src/fungibles/mod.rs

@@ -91,8 +91,8 @@ pub mod pallet {
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
-		/// Transfers `value` amount of tokens from the caller's account to account `to`, with additional
-		/// `data` in unspecified format.
+		/// Transfers `value` amount of tokens from the caller's account to account `to`, with
+		/// additional `data` in unspecified format.
 		///
 		/// # Parameters
 		/// * `id` - The ID of the asset.
@@ -110,6 +110,33 @@ 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.
+		///
+		/// # Arguments
+		/// * `id` - The ID of the asset.
+		/// * `owner` - The account which previously approved for a transfer of at least `amount`
+		///   and
+		/// from which the asset balance will be withdrawn.
+		/// * `to` - The recipient account.
+		/// * `value` - The number of tokens to transfer.
+		///
+		/// # Returns
+		/// Returns `Ok(())` if successful, or an error if the transfer fails.
+		#[pallet::call_index(1)]
+		#[pallet::weight(AssetsWeightInfo::<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);
+			Assets::<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
@@ -177,6 +204,34 @@ pub mod pallet {
 			let spender = T::Lookup::unlookup(spender);
 			AssetsOf::<T>::approve_transfer(origin, id.into(), spender, value)
 		}
+
+		/// Decreases the allowance of a spender.
+		///
+		/// # Arguments
+		/// * `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.
+		///
+		/// # Returns
+		/// Returns `Ok(())` if successful, or an error if the operation fails.
+		#[pallet::call_index(4)]
+		#[pallet::weight(T::DbWeight::get().reads(2) + AssetsWeightInfo::<T>::cancel_approval() + AssetsWeightInfo::<T>::approve_transfer())]
+		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(T::DbWeight::get().reads(1)))?;
+			let mut current_allowance = Assets::<T>::allowance(id.clone(), &who, &spender);
+			let spender = T::Lookup::unlookup(spender);
+			let id: AssetIdParameterOf<T> = id.into();
+
+			current_allowance.saturating_reduce(value);
+			Self::do_set_allowance(origin, id, spender, current_allowance)?;
+			Ok(().into())
+		}
 	}
 
 	impl<T: Config> Pallet<T> {
@@ -188,8 +243,8 @@ pub mod pallet {
 			AssetsOf::<T>::total_supply(id)
 		}
 
-		/// Returns the account balance for the specified `owner` for a given asset ID. Returns `0` if
-		/// the account is non-existent.
+		/// Returns the account balance for the specified `owner` for a given asset ID. Returns `0`
+		/// if the account is non-existent.
 		///
 		/// # Parameters
 		/// * `id` - The ID of the asset.
@@ -236,5 +291,25 @@ pub mod pallet {
 		pub fn token_decimals(id: AssetIdOf<T>) -> u8 {
 			<AssetsOf<T> as MetadataInspect<AccountIdOf<T>>>::decimals(id)
 		}
+
+		/// Set the allowance `value` of the `spender` delegated by `origin`
+		pub(crate) fn do_set_allowance(
+			origin: OriginFor<T>,
+			id: AssetIdParameterOf<T>,
+			spender: AccountIdLookupOf<T>,
+			value: BalanceOf<T>,
+		) -> DispatchResultWithPostInfo {
+			Assets::<T>::cancel_approval(origin.clone(), id.clone(), spender.clone()).map_err(
+				|e| {
+					e.with_weight(
+						T::DbWeight::get().reads(2) + AssetsWeightInfo::<T>::cancel_approval(),
+					)
+				},
+			)?;
+			if value > Zero::zero() {
+				Assets::<T>::approve_transfer(origin, id, spender, value)?;
+			}
+			Ok(().into())
+		}
 	}
 }

pallets/api/src/fungibles/tests.rs

@@ -1,11 +1,20 @@
 use crate::mock::*;
 use frame_support::{
-	assert_ok,
+	assert_noop, assert_ok,
 	traits::fungibles::{approvals::Inspect, metadata::Inspect as MetadataInspect},
 };
+use sp_runtime::{DispatchError, ModuleError};
 
 const ASSET: u32 = 42;
 
+fn get_dispatch_error(index: u8, error_index: u8, error_message: &'static str) -> DispatchError {
+	DispatchError::Module(ModuleError {
+		index,
+		error: [error_index, 0, 0, 0],
+		message: Some(error_message),
+	})
+}
+
 #[test]
 fn transfer_works() {
 	new_test_ext().execute_with(|| {
@@ -18,6 +27,41 @@ 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;
+
+		assert_eq!(transferred, Assets::allowance(ASSET, &ALICE, &CHARLIE));
+		assert_eq!(0, Assets::allowance(ASSET, &ALICE, &BOB));
+
+		// Transfer `amount` from an unapproved spender
+		assert_noop!(
+			Fungibles::transfer_from(signed(BOB), ASSET, ALICE, BOB, transferred),
+			get_dispatch_error(1, 10, "Unapproved")
+		);
+
+		// Transfer `amount` more than the allowed allowance
+		assert_noop!(
+			Fungibles::transfer_from(signed(CHARLIE), ASSET, ALICE, BOB, amount),
+			get_dispatch_error(1, 10, "Unapproved")
+		);
+
+		let alice_balance_before_transfer = Assets::balance(ASSET, &ALICE);
+		let bob_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 bob_balance_after_transfer = Assets::balance(ASSET, &BOB);
+		// Check that BOB receives the `amount` and ALICE `amount` is spent successfully by CHARLIE
+		assert_eq!(bob_balance_after_transfer, bob_balance_before_transfer + transferred);
+		assert_eq!(alice_balance_after_transfer, alice_balance_before_transfer - transferred);
+	});
+}
+
 // Non-additive, sets new value.
 #[test]
 fn approve_works() {
@@ -56,6 +100,21 @@ fn increase_allowance_works() {
 	});
 }
 
+#[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);
+		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 the allowance value is already zero.
+		assert_ok!(Fungibles::decrease_allowance(signed(ALICE), ASSET, BOB, amount));
+		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), 0);
+	});
+}
+
 #[test]
 fn total_supply_works() {
 	new_test_ext().execute_with(|| {

pallets/api/src/mock.rs

@@ -103,6 +103,8 @@ 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 DAVE: AccountId = 4;
 pub(crate) const INIT_AMOUNT: Balance = 100_000_000 * UNIT;
 pub(crate) const UNIT: Balance = 10_000_000_000;
 
@@ -112,7 +114,12 @@ 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),
+			(DAVE, INIT_AMOUNT),
+		],
 	}
 	.assimilate_storage(&mut t)
 	.expect("Pallet balances storage can be assimilated");

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

@@ -23,9 +23,10 @@ type Balance = u128;
 
 const ALICE: AccountId32 = AccountId32::new([1_u8; 32]);
 const BOB: AccountId32 = AccountId32::new([2_u8; 32]);
+const CHARLIE: AccountId32 = AccountId32::new([3_u8; 32]);
 const DEBUG_OUTPUT: pallet_contracts::DebugInfo = pallet_contracts::DebugInfo::UnsafeDebug;
 // FERDIE has no initial balance.
-const FERDIE: AccountId32 = AccountId32::new([3_u8; 32]);
+const FERDIE: AccountId32 = AccountId32::new([99_u8; 32]);
 const GAS_LIMIT: Weight = Weight::from_parts(100_000_000_000, 3 * 1024 * 1024);
 const INIT_AMOUNT: Balance = 100_000_000 * UNIT;
 const INIT_VALUE: Balance = 100 * UNIT;
@@ -36,7 +37,7 @@ fn new_test_ext() -> sp_io::TestExternalities {
 		.expect("Frame system builds valid default genesis config");
 
 	pallet_balances::GenesisConfig::<Runtime> {
-		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");