Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: transfer_from and decrease_allowance #134

Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0825309
feat: implement transfer_from and decrease_allowance method
chungquantin Jul 27, 2024
c15e103
remove unused benchmark method
chungquantin Jul 27, 2024
ea649f8
Update pop-api/integration-tests/src/local_fungibles.rs
chungquantin Jul 28, 2024
db387e8
Update pop-api/integration-tests/src/local_fungibles.rs
chungquantin Jul 28, 2024
a76efa0
Update pallets/api/src/fungibles/tests.rs
chungquantin Jul 28, 2024
73f6ef8
remove wrong empty lines & dispatch error tests
chungquantin Jul 28, 2024
11a7689
remove empty line
chungquantin Jul 28, 2024
8a69845
remove empty line
chungquantin Jul 28, 2024
79ce20d
fix decrease zero value from allowance
chungquantin Jul 28, 2024
0331474
Merge branch 'daan/api' into chungquantin/feat-transfer_from_decrease…
chungquantin Jul 28, 2024
3f237a3
remove empty line
chungquantin Jul 28, 2024
051f27c
Merge remote-tracking branch 'refs/remotes/personal/chungquantin/feat…
chungquantin Jul 28, 2024
1a7bc69
reformat style
chungquantin Jul 28, 2024
4287756
refactor code in decrease_allowance
chungquantin Jul 29, 2024
6ddb08b
Update pallets/api/src/fungibles/mod.rs
chungquantin Jul 30, 2024
a58b2d0
fix: return weight approve
chungquantin Jul 30, 2024
9e90b6a
Update pallets/api/src/fungibles/mod.rs
chungquantin Jul 30, 2024
e39551b
Update pallets/api/src/fungibles/mod.rs
chungquantin Jul 30, 2024
553b0f6
Update pop-api/integration-tests/src/local_fungibles.rs
chungquantin Jul 30, 2024
ea10e2d
Update pop-api/integration-tests/src/local_fungibles.rs
chungquantin Jul 30, 2024
12fa8e2
fix: comment & unnecessary code
chungquantin Jul 30, 2024
686d920
fix: remove test variable prefix
chungquantin Jul 30, 2024
90fbe6a
refactor: remove prefix in test variable
chungquantin Jul 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 66 additions & 10 deletions pallets/api/src/fungibles/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`).
Expand All @@ -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())
}
Expand All @@ -177,6 +197,38 @@ 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))]
Daanvdplas marked this conversation as resolved.
Show resolved Hide resolved
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();

current_allowance.saturating_reduce(value);
chungquantin marked this conversation as resolved.
Show resolved Hide resolved
// 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> {
Expand Down Expand Up @@ -208,5 +260,9 @@ pub mod pallet {
},
}
}

pub fn weight_approve(approve: u32, cancel: u32) -> Weight {
Daanvdplas marked this conversation as resolved.
Show resolved Hide resolved
<T as Config>::WeightInfo::approve(cancel, approve)
}
}
}
37 changes: 37 additions & 0 deletions pallets/api/src/fungibles/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,43 @@ fn transfer_works() {
});
}

#[test]
fn transfer_from_works() {
chungquantin marked this conversation as resolved.
Show resolved Hide resolved
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);
Daanvdplas marked this conversation as resolved.
Show resolved Hide resolved
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);
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() {
chungquantin marked this conversation as resolved.
Show resolved Hide resolved
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() {
Expand Down
3 changes: 2 additions & 1 deletion pallets/api/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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");
Expand Down
115 changes: 113 additions & 2 deletions pop-api/integration-tests/src/local_fungibles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(|| {
Expand Down Expand Up @@ -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);
chungquantin marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
7 changes: 6 additions & 1 deletion runtime/devnet/src/config/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 { .. }
)
)
}
}
Expand Down
Loading