From 082530997b113228244f43f05df7d5d15a3ac011 Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Sat, 27 Jul 2024 08:21:54 +0700 Subject: [PATCH 01/21] feat: implement transfer_from and decrease_allowance method --- pallets/api/src/fungibles/benchmarking.rs | 24 ++++ pallets/api/src/fungibles/mod.rs | 76 +++++++++-- pallets/api/src/fungibles/tests.rs | 64 +++++++++- pallets/api/src/mock.rs | 3 +- .../integration-tests/src/local_fungibles.rs | 119 +++++++++++++++++- pop-api/src/lib.rs | 2 +- runtime/devnet/src/config/api.rs | 2 + 7 files changed, 275 insertions(+), 15 deletions(-) diff --git a/pallets/api/src/fungibles/benchmarking.rs b/pallets/api/src/fungibles/benchmarking.rs index d3d65b97..5bfa1a4d 100644 --- a/pallets/api/src/fungibles/benchmarking.rs +++ b/pallets/api/src/fungibles/benchmarking.rs @@ -102,5 +102,29 @@ mod benchmarks { Ok(()) } + #[benchmark] + fn decrease_allowance() -> Result<(), BenchmarkError> { + assert_has_event::( + pallet_assets::Event::ApprovalCancelled { + asset_id: asset_id.clone(), + owner: owner.clone(), + delegate: spender.clone(), + } + .into(), + ); + + assert_has_event::( + pallet_assets::Event::ApprovedTransfer { + asset_id, + source: owner, + delegate: spender, + amount, + } + .into(), + ); + + Ok(()) + } + impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test); } diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index cd34664a..31e6f363 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -110,6 +110,28 @@ pub mod pallet { AssetsOf::::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::::transfer_approved())] + pub fn transfer_from( + origin: OriginFor, + id: AssetIdOf, + owner: AccountIdOf, + target: AccountIdOf, + amount: BalanceOf, + ) -> DispatchResult { + let owner = T::Lookup::unlookup(owner); + let target = T::Lookup::unlookup(target); + AssetsOf::::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, value: BalanceOf, ) -> DispatchResultWithPostInfo { - let weight = |approve: u32, cancel: u32| -> Weight { - ::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::::allowance(id.clone(), &who, &spender); let spender = T::Lookup::unlookup(spender); let id: AssetIdParameterOf = 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::::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::::approve_transfer(origin, id, spender, value)?; - weight(1, 1) + Self::weight_approve(1, 1) }; Ok(Some(return_weight).into()) } @@ -177,6 +197,38 @@ pub mod pallet { let spender = T::Lookup::unlookup(spender); AssetsOf::::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(::WeightInfo::approve(1, 1))] + pub fn decrease_allowance( + origin: OriginFor, + id: AssetIdOf, + spender: AccountIdOf, + value: BalanceOf, + ) -> DispatchResultWithPostInfo { + let who = ensure_signed(origin.clone()) + .map_err(|e| e.with_weight(T::DbWeight::get().reads(1)))?; + let mut current_allowance = AssetsOf::::allowance(id.clone(), &who, &spender); + let spender = T::Lookup::unlookup(spender); + let id: AssetIdParameterOf = id.into(); + + current_allowance.saturating_reduce(value); + + // Cancel the aproval and set the new value if `current_allowance` is more than zero + AssetsOf::::cancel_approval(origin.clone(), id.clone(), spender.clone()) + .map_err(|e| e.with_weight(Self::weight_approve(0, 1)))?; + if value.is_zero() { + return Ok(Some(Self::weight_approve(0, 1)).into()); + } + AssetsOf::::approve_transfer(origin, id, spender, current_allowance)?; + Ok(Some(Self::weight_approve(1, 1)).into()) + } } impl Pallet { @@ -236,5 +288,9 @@ pub mod pallet { pub fn token_decimals(id: AssetIdOf) -> u8 { as MetadataInspect>>::decimals(id) } + + pub fn weight_approve(approve: u32, cancel: u32) -> Weight { + ::WeightInfo::approve(cancel, approve) + } } } diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index dbfa0b34..402e720c 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/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,59 @@ 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); + }); +} + +#[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); + + // 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 what the + 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() { diff --git a/pallets/api/src/mock.rs b/pallets/api/src/mock.rs index f5d155ef..b20e2635 100644 --- a/pallets/api/src/mock.rs +++ b/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:: { - 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"); diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 6de57ca4..76f14665 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/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 = 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,53 @@ 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; + + // Asset does not exist. + assert_eq!( + decoded::(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::(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::(transfer_from(addr.clone(), asset, ALICE, BOB, amount)), + Ok(Module { index: 52, error: 16 }), + ); + thaw_asset(ALICE, asset); + // Not enough balance. + assert_eq!( + decoded::(transfer_from(addr.clone(), asset, ALICE, BOB, amount + 1 * UNIT,)), + Ok(Module { index: 52, error: 0 }), + ); + // Successful transfer. + 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 bob_balance_after_transfer = Assets::balance(asset, &BOB); + assert_eq!(bob_balance_after_transfer, bob_balance_before_transfer + amount / 2); + }); +} + #[test] fn approve_works() { new_test_ext().execute_with(|| { @@ -458,6 +534,45 @@ fn increase_allowance_works() { }); } +#[test] +fn decrease_allowance_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; + // Asset does not exist. + assert_eq!( + decoded::(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::(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::(decrease_allowance(addr.clone(), asset, BOB, amount)), + Ok(Module { index: 52, error: 16 }), + ); + }); +} + /// 2. PSP-22 Metadata Interface: /// - token_name /// - token_symbol diff --git a/pop-api/src/lib.rs b/pop-api/src/lib.rs index 9bf9d50b..efa7ecb2 100644 --- a/pop-api/src/lib.rs +++ b/pop-api/src/lib.rs @@ -1,6 +1,6 @@ #![cfg_attr(not(feature = "std"), no_std, no_main)] -use ink::env::chain_extension::{ChainExtensionMethod, FromStatusCode}; +use ink::env::chain_extension::ChainExtensionMethod; use constants::DECODING_FAILED; use ink::env::chain_extension::FromStatusCode; diff --git a/runtime/devnet/src/config/api.rs b/runtime/devnet/src/config/api.rs index ae179e4a..ff915f23 100644 --- a/runtime/devnet/src/config/api.rs +++ b/runtime/devnet/src/config/api.rs @@ -26,8 +26,10 @@ impl Contains for AllowedApiCalls { c, RuntimeCall::Fungibles( FungiblesCall::transfer { .. } + | FungiblesCall::transfer_from { .. } | FungiblesCall::approve { .. } | FungiblesCall::increase_allowance { .. } + | FungiblesCall::decrease_allowance { .. } ) ) } From c15e1032b297c4ad91b7de780ca5c03fe4755beb Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Sat, 27 Jul 2024 08:54:55 +0700 Subject: [PATCH 02/21] remove unused benchmark method --- pallets/api/src/fungibles/benchmarking.rs | 24 ----------------------- 1 file changed, 24 deletions(-) diff --git a/pallets/api/src/fungibles/benchmarking.rs b/pallets/api/src/fungibles/benchmarking.rs index 5bfa1a4d..d3d65b97 100644 --- a/pallets/api/src/fungibles/benchmarking.rs +++ b/pallets/api/src/fungibles/benchmarking.rs @@ -102,29 +102,5 @@ mod benchmarks { Ok(()) } - #[benchmark] - fn decrease_allowance() -> Result<(), BenchmarkError> { - assert_has_event::( - pallet_assets::Event::ApprovalCancelled { - asset_id: asset_id.clone(), - owner: owner.clone(), - delegate: spender.clone(), - } - .into(), - ); - - assert_has_event::( - pallet_assets::Event::ApprovedTransfer { - asset_id, - source: owner, - delegate: spender, - amount, - } - .into(), - ); - - Ok(()) - } - impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test); } From ea649f89bc758bf2803e15bd336ec578ac2c33a9 Mon Sep 17 00:00:00 2001 From: Tin Chung <56880684+chungquantin@users.noreply.github.com> Date: Sun, 28 Jul 2024 13:43:08 +0700 Subject: [PATCH 03/21] Update pop-api/integration-tests/src/local_fungibles.rs Co-authored-by: Daan van der Plas <93204684+Daanvdplas@users.noreply.github.com> --- pop-api/integration-tests/src/local_fungibles.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 76f14665..c1945174 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -557,7 +557,7 @@ fn decrease_allowance_works() { ); thaw_asset(addr.clone(), asset); - // Successfully decrease allowance + // 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!"); From db387e8d9e6eb34feecf5d1cba33d3767c69c412 Mon Sep 17 00:00:00 2001 From: Tin Chung <56880684+chungquantin@users.noreply.github.com> Date: Sun, 28 Jul 2024 13:43:14 +0700 Subject: [PATCH 04/21] Update pop-api/integration-tests/src/local_fungibles.rs Co-authored-by: Daan van der Plas <93204684+Daanvdplas@users.noreply.github.com> --- pop-api/integration-tests/src/local_fungibles.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index c1945174..497bf803 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -546,7 +546,7 @@ fn decrease_allowance_works() { Ok(Module { index: 52, error: 3 }), ); - // Create asset and mint to the address contract, delegate Bob to spend the `amount` + // 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. From a76efa0bff04cf5258f55b0df6a339907d198753 Mon Sep 17 00:00:00 2001 From: Tin Chung <56880684+chungquantin@users.noreply.github.com> Date: Sun, 28 Jul 2024 13:43:27 +0700 Subject: [PATCH 05/21] Update pallets/api/src/fungibles/tests.rs Co-authored-by: Daan van der Plas <93204684+Daanvdplas@users.noreply.github.com> --- pallets/api/src/fungibles/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 402e720c..26db7004 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -70,7 +70,7 @@ fn decrease_allowance_works() { assert_eq!(Assets::allowance(ASSET, &ALICE, &BOB), amount); - // Decrease allowance successfully + // 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); From 73f6ef8c35069a963153247e94e283e905ec1549 Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Sun, 28 Jul 2024 15:33:02 +0800 Subject: [PATCH 06/21] remove wrong empty lines & dispatch error tests --- pallets/api/src/fungibles/tests.rs | 38 ++++--------------- .../integration-tests/src/local_fungibles.rs | 6 +-- 2 files changed, 9 insertions(+), 35 deletions(-) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index 26db7004..f2f4a4ba 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -1,20 +1,12 @@ use crate::mock::*; use frame_support::{ - assert_noop, assert_ok, + 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(|| { @@ -33,24 +25,8 @@ fn transfer_from_works() { 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") - ); - + // Successfully call transfer from 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)); @@ -67,14 +43,14 @@ 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); - - // Decrease allowance successfully. + // 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 what the + // 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); }); diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 497bf803..9eafdf13 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -546,7 +546,7 @@ fn decrease_allowance_works() { Ok(Module { index: 52, error: 3 }), ); - // Create asset and mint to the address contract, delegate Bob to spend the `amount`. + // 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. @@ -556,14 +556,12 @@ fn decrease_allowance_works() { Ok(Module { index: 52, error: 16 }), ); thaw_asset(addr.clone(), asset); - - // Successfully decrease allowance. + // 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!( From 11a7689fed944a3a291a521e7f3cc31d253469e0 Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Sun, 28 Jul 2024 15:34:07 +0800 Subject: [PATCH 07/21] remove empty line --- pop-api/integration-tests/src/local_fungibles.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 9eafdf13..83c88771 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -545,7 +545,6 @@ fn decrease_allowance_works() { decoded::(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); From 8a69845960ba07ab038c50dee1fedc22fa531953 Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Sun, 28 Jul 2024 15:34:59 +0800 Subject: [PATCH 08/21] remove empty line --- pallets/api/src/fungibles/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 31e6f363..214ccd69 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -219,7 +219,6 @@ pub mod pallet { let id: AssetIdParameterOf = id.into(); current_allowance.saturating_reduce(value); - // Cancel the aproval and set the new value if `current_allowance` is more than zero AssetsOf::::cancel_approval(origin.clone(), id.clone(), spender.clone()) .map_err(|e| e.with_weight(Self::weight_approve(0, 1)))?; From 79ce20d453bd65177edb096a7aeac4ee0083c628 Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Sun, 28 Jul 2024 23:22:15 +0800 Subject: [PATCH 09/21] fix decrease zero value from allowance --- pallets/api/src/fungibles/mod.rs | 13 ++++++++++--- pallets/api/src/fungibles/tests.rs | 1 - 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 214ccd69..2804e7af 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -214,18 +214,25 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin.clone()) .map_err(|e| e.with_weight(T::DbWeight::get().reads(1)))?; - let mut current_allowance = AssetsOf::::allowance(id.clone(), &who, &spender); + let current_allowance = AssetsOf::::allowance(id.clone(), &who, &spender); let spender = T::Lookup::unlookup(spender); let id: AssetIdParameterOf = id.into(); - current_allowance.saturating_reduce(value); + if value == Zero::zero() { + return Ok(Some(Self::weight_approve(0, 0)).into()); + } // Cancel the aproval and set the new value if `current_allowance` is more than zero AssetsOf::::cancel_approval(origin.clone(), id.clone(), spender.clone()) .map_err(|e| e.with_weight(Self::weight_approve(0, 1)))?; if value.is_zero() { return Ok(Some(Self::weight_approve(0, 1)).into()); } - AssetsOf::::approve_transfer(origin, id, spender, current_allowance)?; + AssetsOf::::approve_transfer( + origin, + id, + spender, + current_allowance.saturating_sub(value), + )?; Ok(Some(Self::weight_approve(1, 1)).into()) } } diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index f2f4a4ba..5a4f2bc7 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -3,7 +3,6 @@ use frame_support::{ assert_ok, traits::fungibles::{approvals::Inspect, metadata::Inspect as MetadataInspect}, }; -use sp_runtime::{DispatchError, ModuleError}; const ASSET: u32 = 42; From 3f237a349a23c2fa97f29c8942d6c2cbd0f4cd1b Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Sun, 28 Jul 2024 23:26:24 +0800 Subject: [PATCH 10/21] remove empty line --- pop-api/integration-tests/src/local_fungibles.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 83c88771..2bfcd5a8 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -420,7 +420,6 @@ fn transfer_from_works() { addr.clone().into(), amount + 1 * UNIT, )); - // Asset is not live, i.e. frozen or being destroyed. freeze_asset(ALICE, asset); assert_eq!( From 1a7bc690d5927d4528ea54658b92cee63c85dfe0 Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Sun, 28 Jul 2024 23:27:56 +0800 Subject: [PATCH 11/21] reformat style --- runtime/devnet/src/config/api.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/runtime/devnet/src/config/api.rs b/runtime/devnet/src/config/api.rs index 493529bf..b3932a79 100644 --- a/runtime/devnet/src/config/api.rs +++ b/runtime/devnet/src/config/api.rs @@ -21,10 +21,11 @@ impl Contains for AllowedApiCalls { matches!( c, RuntimeCall::Fungibles( - transfer { .. } - | transfer_from { .. } | approve { .. } - | increase_allowance { .. } - | decrease_allowance { .. }) + transfer { .. } + | transfer_from { .. } + | approve { .. } | increase_allowance { .. } + | decrease_allowance { .. } + ) ) } } From 42877566b96561a9ea2c9cf96a409840de7efc69 Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Mon, 29 Jul 2024 08:15:27 +0800 Subject: [PATCH 12/21] refactor code in decrease_allowance --- pallets/api/src/fungibles/mod.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 22a8e3d6..7b7a316f 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -214,25 +214,23 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin.clone()) .map_err(|e| e.with_weight(T::DbWeight::get().reads(1)))?; - let current_allowance = AssetsOf::::allowance(id.clone(), &who, &spender); + let mut current_allowance = AssetsOf::::allowance(id.clone(), &who, &spender); let spender = T::Lookup::unlookup(spender); let id: AssetIdParameterOf = id.into(); - if value == Zero::zero() { + 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::::cancel_approval(origin.clone(), id.clone(), spender.clone()) .map_err(|e| e.with_weight(Self::weight_approve(0, 1)))?; - if value.is_zero() { + + if current_allowance.is_zero() { return Ok(Some(Self::weight_approve(0, 1)).into()); } - AssetsOf::::approve_transfer( - origin, - id, - spender, - current_allowance.saturating_sub(value), - )?; + AssetsOf::::approve_transfer(origin, id, spender, current_allowance)?; Ok(Some(Self::weight_approve(1, 1)).into()) } } From 6ddb08bdc58db100b4610a2f825633e57df6dbce Mon Sep 17 00:00:00 2001 From: Tin Chung <56880684+chungquantin@users.noreply.github.com> Date: Tue, 30 Jul 2024 14:46:20 +0700 Subject: [PATCH 13/21] Update pallets/api/src/fungibles/mod.rs Co-authored-by: Daan van der Plas <93204684+Daanvdplas@users.noreply.github.com> --- pallets/api/src/fungibles/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 7b7a316f..add62375 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -213,7 +213,7 @@ pub mod pallet { value: BalanceOf, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin.clone()) - .map_err(|e| e.with_weight(T::DbWeight::get().reads(1)))?; + .map_err(|e| e.with_weight(Self::weight_approve))?; let mut current_allowance = AssetsOf::::allowance(id.clone(), &who, &spender); let spender = T::Lookup::unlookup(spender); let id: AssetIdParameterOf = id.into(); From a58b2d061f8d468772fa6e730444d471289437ad Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Tue, 30 Jul 2024 15:56:50 +0800 Subject: [PATCH 14/21] fix: return weight approve --- pallets/api/src/fungibles/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index add62375..1247573f 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -213,7 +213,7 @@ pub mod pallet { value: BalanceOf, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin.clone()) - .map_err(|e| e.with_weight(Self::weight_approve))?; + .map_err(|e| e.with_weight(Self::weight_approve(0, 0)))?; let mut current_allowance = AssetsOf::::allowance(id.clone(), &who, &spender); let spender = T::Lookup::unlookup(spender); let id: AssetIdParameterOf = id.into(); From 9e90b6ab3c0e7e3526728c968e70c3222931fb37 Mon Sep 17 00:00:00 2001 From: Tin Chung <56880684+chungquantin@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:01:30 +0800 Subject: [PATCH 15/21] Update pallets/api/src/fungibles/mod.rs Co-authored-by: Daan van der Plas <93204684+Daanvdplas@users.noreply.github.com> --- pallets/api/src/fungibles/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 1247573f..50bcbddc 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -223,7 +223,7 @@ pub mod pallet { } current_allowance.saturating_reduce(value); - // Cancel the aproval and set the new value if `current_allowance` is more than zero + // Cancel the aproval and set the new value if `current_allowance` is more than zero. AssetsOf::::cancel_approval(origin.clone(), id.clone(), spender.clone()) .map_err(|e| e.with_weight(Self::weight_approve(0, 1)))?; From e39551b4296321e9e5a65f9de7302925d851e964 Mon Sep 17 00:00:00 2001 From: Tin Chung <56880684+chungquantin@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:01:50 +0800 Subject: [PATCH 16/21] Update pallets/api/src/fungibles/mod.rs Co-authored-by: Daan van der Plas <93204684+Daanvdplas@users.noreply.github.com> --- pallets/api/src/fungibles/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 50bcbddc..9fb444f8 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -231,7 +231,7 @@ pub mod pallet { return Ok(Some(Self::weight_approve(0, 1)).into()); } AssetsOf::::approve_transfer(origin, id, spender, current_allowance)?; - Ok(Some(Self::weight_approve(1, 1)).into()) + Ok(().into()) } } From 553b0f6ea95dbba312cf919d97c6788b488a335f Mon Sep 17 00:00:00 2001 From: Tin Chung <56880684+chungquantin@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:02:01 +0800 Subject: [PATCH 17/21] Update pop-api/integration-tests/src/local_fungibles.rs Co-authored-by: Daan van der Plas <93204684+Daanvdplas@users.noreply.github.com> --- pop-api/integration-tests/src/local_fungibles.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 2bfcd5a8..dd51db89 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -399,7 +399,7 @@ fn transfer_works() { 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 addr = instantiate(CONTRACT, INIT_VALUE, vec![]); let amount: Balance = 100 * UNIT; // Asset does not exist. From ea10e2dc1f15712bcaa49de3c32119a4d1ee9eef Mon Sep 17 00:00:00 2001 From: Tin Chung <56880684+chungquantin@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:02:07 +0800 Subject: [PATCH 18/21] Update pop-api/integration-tests/src/local_fungibles.rs Co-authored-by: Daan van der Plas <93204684+Daanvdplas@users.noreply.github.com> --- pop-api/integration-tests/src/local_fungibles.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index dd51db89..14b6a501 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -537,7 +537,7 @@ fn increase_allowance_works() { fn decrease_allowance_works() { new_test_ext().execute_with(|| { let _ = env_logger::try_init(); - let addr = instantiate("contracts/fungibles/target/ink/fungibles.wasm", INIT_VALUE, vec![]); + let addr = instantiate(CONTRACT, INIT_VALUE, vec![]); let amount: Balance = 100 * UNIT; // Asset does not exist. assert_eq!( From 12fa8e2a5ab57cdd5fa99697e0c49cc1e3cb457b Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:05:31 +0800 Subject: [PATCH 19/21] fix: comment & unnecessary code --- pallets/api/src/fungibles/mod.rs | 4 ---- pallets/api/src/fungibles/tests.rs | 12 ++++++------ pop-api/integration-tests/src/local_fungibles.rs | 6 +++--- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 9fb444f8..4b23e9af 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -218,10 +218,6 @@ pub mod pallet { let spender = T::Lookup::unlookup(spender); let id: AssetIdParameterOf = 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::::cancel_approval(origin.clone(), id.clone(), spender.clone()) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index c2fdee91..baa9fbeb 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -23,16 +23,16 @@ fn transfer_works() { fn transfer_from_works() { new_test_ext().execute_with(|| { let amount: Balance = 100 * UNIT; - // Approve CHARLIE to transfer up to `amount` to BOB + // 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 + // Successfully call transfer from. 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 + // 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); }); @@ -44,13 +44,13 @@ fn decrease_allowance_works() { 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 + // 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 + // 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 + // 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); }); diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 14b6a501..5649613c 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -409,7 +409,7 @@ fn transfer_from_works() { ); // 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 + // Unapproved transfer. assert_eq!( decoded::(transfer_from(addr.clone(), asset, ALICE, BOB, amount / 2)), Ok(Module { index: 52, error: 10 }) @@ -544,7 +544,7 @@ fn decrease_allowance_works() { decoded::(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` + // 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. @@ -554,7 +554,7 @@ fn decrease_allowance_works() { Ok(Module { index: 52, error: 16 }), ); thaw_asset(addr.clone(), asset); - // Successfully decrease allowance + // 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!"); From 686d920af5312dfd33ab035a4811e7471bf8051c Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Tue, 30 Jul 2024 20:18:14 +0800 Subject: [PATCH 20/21] fix: remove test variable prefix --- pop-api/integration-tests/src/local_fungibles.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pop-api/integration-tests/src/local_fungibles.rs b/pop-api/integration-tests/src/local_fungibles.rs index 5649613c..15a644ad 100644 --- a/pop-api/integration-tests/src/local_fungibles.rs +++ b/pop-api/integration-tests/src/local_fungibles.rs @@ -433,11 +433,11 @@ fn transfer_from_works() { Ok(Module { index: 52, error: 0 }), ); // Successful transfer. - let bob_balance_before_transfer = Assets::balance(asset, &BOB); + 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 bob_balance_after_transfer = Assets::balance(asset, &BOB); - assert_eq!(bob_balance_after_transfer, bob_balance_before_transfer + amount / 2); + let balance_after_transfer = Assets::balance(asset, &BOB); + assert_eq!(balance_after_transfer, balance_before_transfer + amount / 2); }); } From 90fbe6a9ba2871b107a676880d2671ec95d319c7 Mon Sep 17 00:00:00 2001 From: tin-snowflake <56880684+chungquantin@users.noreply.github.com> Date: Wed, 31 Jul 2024 09:26:54 +0800 Subject: [PATCH 21/21] refactor: remove prefix in test variable --- pallets/api/src/fungibles/mod.rs | 4 ++++ pallets/api/src/fungibles/tests.rs | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pallets/api/src/fungibles/mod.rs b/pallets/api/src/fungibles/mod.rs index 4b23e9af..9fb444f8 100644 --- a/pallets/api/src/fungibles/mod.rs +++ b/pallets/api/src/fungibles/mod.rs @@ -218,6 +218,10 @@ pub mod pallet { let spender = T::Lookup::unlookup(spender); let id: AssetIdParameterOf = 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::::cancel_approval(origin.clone(), id.clone(), spender.clone()) diff --git a/pallets/api/src/fungibles/tests.rs b/pallets/api/src/fungibles/tests.rs index baa9fbeb..4377fc83 100644 --- a/pallets/api/src/fungibles/tests.rs +++ b/pallets/api/src/fungibles/tests.rs @@ -28,12 +28,12 @@ fn transfer_from_works() { let transferred = amount / 2; // Successfully call transfer from. let alice_balance_before_transfer = Assets::balance(ASSET, &ALICE); - let bob_balance_before_transfer = Assets::balance(ASSET, &BOB); + 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 bob_balance_after_transfer = Assets::balance(ASSET, &BOB); + let 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!(balance_after_transfer, balance_before_transfer + transferred); assert_eq!(alice_balance_after_transfer, alice_balance_before_transfer - transferred); }); }