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/Cargo.toml

@@ -33,6 +33,8 @@ runtime-benchmarks = [
     "frame-benchmarking/runtime-benchmarks",
     "frame-support/runtime-benchmarks",
     "frame-system/runtime-benchmarks",
+    "pallet-assets/runtime-benchmarks",
+    "pallet-balances/runtime-benchmarks",
     "sp-runtime/runtime-benchmarks",
 ]
 std = [

pallets/api/src/fungibles/benchmarking.rs

@@ -0,0 +1,60 @@
+//! Benchmarking setup for pallet-cards
+
+use super::{AccountIdOf, AssetIdOf, Assets, AssetsInstanceOf, BalanceOf, Call, Config, Pallet};
+use frame_benchmarking::{account, v2::*};
+use frame_support::{
+	assert_ok,
+	traits::{
+		fungibles::{
+			approvals::{Inspect as ApprovalInspect, Mutate},
+			Create, Inspect,
+		},
+		Currency,
+	},
+};
+use frame_system::RawOrigin;
+use sp_runtime::traits::Zero;
+
+const SEED: u32 = 1;
+
+#[benchmarks(
+	where
+	<pallet_assets::Pallet<T, AssetsInstanceOf<T>> as Inspect<<T as frame_system::Config>::AccountId>>::AssetId: Zero,
+)]
+mod benchmarks {
+	use super::*;
+
+	// The worst case scenario is when the allowance is set to a value which is lower than the
+	// current allowance.
+	#[benchmark]
+	fn approve() -> Result<(), BenchmarkError> {
+		let asset = AssetIdOf::<T>::zero();
+		let decreased_value = <BalanceOf<T>>::from(50u32);
+		let min_balance = <BalanceOf<T>>::from(1u32);
+		let owner: AccountIdOf<T> = account("Alice", 0, SEED);
+		let spender: AccountIdOf<T> = account("Bob", 0, SEED);
+		let value = <BalanceOf<T>>::from(100u32);
+		T::Currency::make_free_balance_be(&owner, 100u32.into());
+		assert_ok!(<Assets<T> as Create<AccountIdOf<T>>>::create(
+			asset.clone().into(),
+			owner.clone(),
+			true,
+			min_balance
+		));
+		assert_ok!(<Assets<T> as Mutate<AccountIdOf<T>>>::approve(
+			asset.clone(),
+			&owner,
+			&spender,
+			value
+		));
+
+		#[extrinsic_call]
+		_(RawOrigin::Signed(owner.clone()), asset.clone(), spender.clone(), decreased_value);
+
+		assert_eq!(Assets::<T>::allowance(asset, &owner, &spender), decreased_value);
+
+		Ok(())
+	}
+
+	impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test);
+}

pallets/api/src/fungibles/mod.rs

@@ -1,8 +1,10 @@
 /// The fungibles pallet serves as a wrapper around the pallet_assets, offering a streamlined
-/// interface for interacting with fungible assets. The goal is to provide a simplified, consistent
-/// API that adheres to standards in the smart contract space.
+/// interface for interacting with fungible assets. The goal is to provide a simplified,
+/// consistent API that adheres to standards in the smart contract space.
 pub use pallet::*;
 
+#[cfg(feature = "runtime-benchmarks")]
+mod benchmarking;
 #[cfg(test)]
 mod tests;
 
@@ -17,7 +19,10 @@ pub mod pallet {
 	};
 	use frame_system::pallet_prelude::*;
 	use pallet_assets::WeightInfo;
-	use sp_runtime::{traits::StaticLookup, Saturating};
+	use sp_runtime::{
+		traits::{StaticLookup, Zero},
+		Saturating,
+	};
 	use sp_std::vec::Vec;
 
 	pub(crate) type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
@@ -26,6 +31,7 @@ pub mod pallet {
 	>>::AssetId;
 	type AssetIdParameterOf<T> =
 		<T as pallet_assets::Config<AssetsInstanceOf<T>>>::AssetIdParameter;
+	type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;
 	type Assets<T> = pallet_assets::Pallet<T, AssetsInstanceOf<T>>;
 	type AssetsInstanceOf<T> = <T as Config>::AssetsInstance;
 	type AssetsWeightInfo<T> = <T as pallet_assets::Config<AssetsInstanceOf<T>>>::WeightInfo;
@@ -62,8 +68,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.
 		///
 		/// # Arguments
 		/// * `id` - The ID of the asset.
@@ -84,6 +90,33 @@ pub mod pallet {
 			Assets::<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.
 		///
 		/// # Arguments
@@ -94,14 +127,15 @@ pub mod pallet {
 		/// # Returns
 		/// Returns `Ok(())` if successful, or an error if the approval fails.
 		#[pallet::call_index(2)]
-		#[pallet::weight(T::DbWeight::get().reads(2) + AssetsWeightInfo::<T>::cancel_approval() + AssetsWeightInfo::<T>::approve_transfer())]
+		#[pallet::weight(T::DbWeight::get().reads(1) + AssetsWeightInfo::<T>::cancel_approval() + AssetsWeightInfo::<T>::approve_transfer())]
 		pub fn approve(
 			origin: OriginFor<T>,
 			id: AssetIdOf<T>,
 			spender: AccountIdOf<T>,
 			mut value: BalanceOf<T>,
 		) -> DispatchResultWithPostInfo {
 			let who = ensure_signed(origin.clone())
+				// To have the caller pay some fees.
 				.map_err(|e| e.with_weight(T::DbWeight::get().reads(1)))?;
 			let current_allowance = Assets::<T>::allowance(id.clone(), &who, &spender);
 			let spender = T::Lookup::unlookup(spender);
@@ -115,19 +149,13 @@ pub mod pallet {
 				value.saturating_reduce(current_allowance);
 				Assets::<T>::approve_transfer(origin, id, spender, value).map_err(|e| {
 					e.with_weight(
-						T::DbWeight::get().reads(2) + AssetsWeightInfo::<T>::approve_transfer(),
+						T::DbWeight::get().reads(1) + AssetsWeightInfo::<T>::approve_transfer(),
 					)
 				})?;
 			} else {
-				// If the new value is less than the current allowance, cancel the approval and set the new value
-				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(),
-						)
-					},
-				)?;
-				Assets::<T>::approve_transfer(origin, id, spender, value)?;
+				// If the new value is less than the current allowance, cancel the approval and set
+				// the new value
+				Self::do_set_allowance(origin, id, spender, value)?;
 			}
 
 			Ok(().into())
@@ -153,6 +181,34 @@ pub mod pallet {
 			let spender = T::Lookup::unlookup(spender);
 			Assets::<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> {
@@ -167,8 +223,8 @@ pub mod pallet {
 			Assets::<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.
 		///
 		/// # Arguments
 		/// * `id` - The ID of the asset.
@@ -226,9 +282,30 @@ pub mod pallet {
 		/// * `id` - The ID of the asset.
 		///
 		/// # Returns
-		///  The number of decimals of the token as a byte vector, or an error if the operation fails.
+		///  The number of decimals of the token as a byte vector, or an error if the operation
+		/// fails.
 		pub fn token_decimals(id: AssetIdOf<T>) -> u8 {
 			<Assets<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() {
@@ -53,6 +97,23 @@ fn increase_allowance_works() {
 	});
 }
 
+#[test]
+fn decrease_allowance_works() {
+	new_test_ext().execute_with(|| {
+		let amount: Balance = 100 * UNIT;
+		create_asset_and_mint_to(ALICE, ASSET, ALICE, amount);
+		assert_ok!(Fungibles::increase_allowance(signed(ALICE), ASSET, BOB, amount));
+		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount);
+
+		assert_ok!(Fungibles::decrease_allowance(signed(ALICE), ASSET, BOB, amount / 2));
+		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount / 2);
+
+		// Saturating if the allowance value is already zeros
+		assert_ok!(Fungibles::decrease_allowance(signed(ALICE), ASSET, BOB, amount / 2 + 1 * UNIT));
+		assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), 0);
+	});
+}
+
 #[test]
 fn total_supply_works() {
 	new_test_ext().execute_with(|| {