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

@@ -1,6 +1,6 @@
 /// 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(test)]
@@ -62,8 +62,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 +84,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
@@ -119,7 +146,8 @@ pub mod pallet {
 					)
 				})?;
 			} else {
-				// If the new value is less than the current allowance, cancel the approval and set the new value
+				// 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(
@@ -167,8 +195,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,7 +254,8 @@ 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)
 		}

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() {

pallets/api/src/mock.rs

@@ -102,6 +102,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;
 
@@ -111,7 +113,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/local_fungibles.rs

@@ -79,6 +79,19 @@ 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 params = [function, asset_id.encode(), from.encode(), to.encode(), value.encode()].concat();
+	let result = bare_call(addr, params, 0).expect("should work");
+	result
+}
+
 fn approve(
 	addr: AccountId32,
 	asset_id: AssetId,
@@ -367,6 +380,64 @@ fn transfer_works() {
 	});
 }
 
+#[test]
+fn transfer_from_works() {
+	new_test_ext().execute_with(|| {
+		let _ = env_logger::try_init();
+		let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]);
+		let amount: Balance = 100 * UNIT;
+		// TODO: Add approval process and finalize the integration test
+		// Asset does not exist.
+		assert_eq!(
+			decoded::<Error>(transfer_from(addr.clone(), 1, ALICE, BOB, amount,)),
+			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, addr.clone(), amount);
+		// 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,)),
+			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)),
+			Module { index: 52, error: 0 },
+		);
+		// Not enough balance due to ED.
+		assert_eq!(
+			decoded::<Error>(transfer_from(addr.clone(), asset, ALICE, BOB, amount)),
+			Module { index: 52, error: 0 },
+		);
+		// Successful transfer.
+		let alice_balance_before_transfer = Assets::balance(asset, &ALICE);
+		let bob_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 alice_balance_after_transfer = Assets::balance(asset, &BOB);
+		let bob_balance_after_transfer = Assets::balance(asset, &BOB);
+
+		assert_eq!(bob_balance_after_transfer, bob_balance_before_transfer + amount / 2);
+		assert_eq!(alice_balance_after_transfer, alice_balance_before_transfer - amount / 2);
+
+		// Transfer asset to account that does not exist.
+		assert_eq!(
+			decoded::<Error>(transfer_from(addr.clone(), asset, ALICE, FERDIE, amount / 4)),
+			Token(CannotCreate)
+		);
+		// Asset is not live, i.e. frozen or being destroyed.
+		start_destroy_asset(ALICE, asset);
+		assert_eq!(
+			decoded::<Error>(transfer_from(addr.clone(), asset, ALICE, BOB, amount / 4)),
+			Module { index: 52, error: 16 },
+		);
+	});
+}
+
 #[test]
 fn approve_works() {
 	new_test_ext().execute_with(|| {

primitives/src/lib.rs

@@ -20,12 +20,13 @@ pub mod v0 {
 		#[allow(clippy::unnecessary_cast)]
 		pub enum Error {
 			/// An unknown error occurred. This variant captures any unexpected errors that the
-			/// contract cannot specifically handle. It is useful for cases where there are breaking
-			/// changes in the runtime or when an error falls outside the predefined categories. The
-			/// variant includes:
+			/// contract cannot specifically handle. It is useful for cases where there are
+			/// breaking changes in the runtime or when an error falls outside the predefined
+			/// categories. The variant includes:
 			///
 			/// - `dispatch_error_index`: The index within the `DispatchError`.
-			/// - `error_index`: The index within the `DispatchError` variant (e.g. a `TokenError`).
+			/// - `error_index`: The index within the `DispatchError` variant (e.g. a
+			///   `TokenError`).
 			/// - `error`: The specific error code or sub-index, providing additional context (e.g.
 			///   `error` in `ModuleError`).
 			Other { dispatch_error_index: u8, error_index: u8, error: u8 } = 0,
@@ -48,14 +49,16 @@ pub mod v0 {
 			Token(TokenError) = 7,
 			/// An arithmetic error.
 			Arithmetic(ArithmeticError) = 8,
-			/// The number of transactional layers has been reached, or we are not in a transactional
-			/// layer.
+			/// The number of transactional layers has been reached, or we are not in a
+			/// transactional layer.
 			Transactional(TransactionalError) = 9,
-			/// Resources exhausted, e.g. attempt to read/write data which is too large to manipulate.
+			/// Resources exhausted, e.g. attempt to read/write data which is too large to
+			/// manipulate.
 			Exhausted = 10,
 			/// The state is corrupt; this is generally not going to fix itself.
 			Corruption = 11,
-			/// Some resource (e.g. a preimage) is unavailable right now. This might fix itself later.
+			/// Some resource (e.g. a preimage) is unavailable right now. This might fix itself
+			/// later.
 			Unavailable = 12,
 			/// Root origin is not allowed.
 			RootNotAllowed = 13,
@@ -83,8 +86,8 @@ pub mod v0 {
 		pub enum TokenError {
 			/// Funds are unavailable.
 			FundsUnavailable,
-			/// Some part of the balance gives the only provider reference to the account and thus cannot
-			/// be (re)moved.
+			/// Some part of the balance gives the only provider reference to the account and thus
+			/// cannot be (re)moved.
 			OnlyProvider,
 			/// Account cannot exist with the funds that would be given.
 			BelowMinimum,