Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Use can_hold instead of can_reserve in fn hold #12004

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
38 changes: 23 additions & 15 deletions frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1173,32 +1173,40 @@ impl<T: Config<I>, I: 'static> fungible::InspectHold<T::AccountId> for Pallet<T,
fn balance_on_hold(who: &T::AccountId) -> T::Balance {
Self::account(who).reserved
}
fn can_hold(who: &T::AccountId, amount: T::Balance) -> bool {
fn can_hold(who: &T::AccountId, amount: T::Balance, keep_alive: bool) -> bool {
let a = Self::account(who);
let min_balance = T::ExistentialDeposit::get().max(a.frozen(Reasons::All));
if a.reserved.checked_add(&amount).is_none() {
return false
}
// We require it to be min_balance + amount to ensure that the full reserved funds may be
// slashed without compromising locked funds or destroying the account.
let required_free = match min_balance.checked_add(&amount) {
Some(x) => x,
None => return false,
};
a.free >= required_free

if keep_alive {
let min_balance = T::ExistentialDeposit::get().max(a.frozen(Reasons::All));
// We require it to be min_balance + amount to ensure that the full reserved funds may
// be slashed without compromising locked funds or destroying the account.
let required_free = match min_balance.checked_add(&amount) {
Some(x) => x,
None => return false,
};
a.free >= required_free
} else {
a.free >= amount
}
}
}
impl<T: Config<I>, I: 'static> fungible::MutateHold<T::AccountId> for Pallet<T, I> {
fn hold(who: &T::AccountId, amount: Self::Balance) -> DispatchResult {
if amount.is_zero() {
return Ok(())
}
ensure!(Self::can_reserve(who, amount), Error::<T, I>::InsufficientBalance);
Self::mutate_account(who, |a| {
a.free -= amount;
a.reserved += amount;
})?;
Ok(())
ensure!(
<Self as fungible::InspectHold<T::AccountId>>::can_hold(who, amount, false),
Error::<T, I>::InsufficientBalance,
);
Self::try_mutate_account(who, |a, _| -> DispatchResult {
a.free = a.free.checked_sub(&amount).ok_or(Error::<T, I>::InsufficientBalance)?;
a.reserved = a.reserved.checked_add(&amount).ok_or(ArithmeticError::Overflow)?;
Ok(())
})
}
fn release(
who: &T::AccountId,
Expand Down
142 changes: 142 additions & 0 deletions frame/balances/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ macro_rules! decl_tests {
use frame_support::{
assert_noop, assert_storage_noop, assert_ok, assert_err,
traits::{
fungible::{self, BalancedHold, Inspect, InspectHold, MutateHold},
LockableCurrency, LockIdentifier, WithdrawReasons,
Currency, ReservableCurrency, ExistenceRequirement::AllowDeath
}
Expand Down Expand Up @@ -393,6 +394,23 @@ macro_rules! decl_tests {
});
}

#[test]
fn holding_fungible_should_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);

assert_eq!(Balances::balance(&1), 111);
assert_eq!(Balances::reducible_balance(&1, false), 111);
assert_eq!(Balances::balance_on_hold(&1), 0);

assert_ok!(Balances::hold(&1, 69));

assert_eq!(Balances::balance(&1), 111);
assert_eq!(Balances::reducible_balance(&1, false), 42);
assert_eq!(Balances::balance_on_hold(&1), 69);
});
}

#[test]
fn balance_transfer_when_reserved_should_not_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand All @@ -405,6 +423,18 @@ macro_rules! decl_tests {
});
}

#[test]
fn balance_transfer_when_held_should_not_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
assert_ok!(Balances::hold(&1, 69));
assert_noop!(
<Balances as fungible::Transfer<_>>::transfer(&1, &2, 69, false),
Error::<$test, _>::InsufficientBalance,
);
});
}

#[test]
fn deducting_balance_should_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand Down Expand Up @@ -437,6 +467,18 @@ macro_rules! decl_tests {
});
}

#[test]
fn slashing_fungible_should_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
assert_ok!(Balances::hold(&1, 69));
assert_eq!(<Balances as fungible::Mutate<_>>::slash(&1, 69), Ok(42));
assert_eq!(Balances::reducible_balance(&1, false), 0);
assert_eq!(Balances::balance_on_hold(&1), 69);
assert_eq!(Balances::total_issuance(), 69);
});
}

#[test]
fn withdrawing_balance_should_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand All @@ -462,6 +504,18 @@ macro_rules! decl_tests {
});
}

#[test]
fn slashing_incomplete_fungible_should_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 42);
assert_ok!(Balances::hold(&1, 21));
assert_eq!(<Balances as fungible::Mutate<_>>::slash(&1, 69), Ok(21));
assert_eq!(Balances::reducible_balance(&1, false), 0);
assert_eq!(Balances::balance_on_hold(&1), 21);
assert_eq!(Balances::total_issuance(), 21);
});
}

#[test]
fn unreserving_balance_should_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand All @@ -473,6 +527,17 @@ macro_rules! decl_tests {
});
}

#[test]
fn releasing_fungible_should_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
assert_ok!(Balances::hold(&1, 111));
assert_eq!(Balances::release(&1, 42, false), Ok(42));
assert_eq!(Balances::balance_on_hold(&1), 69);
assert_eq!(Balances::reducible_balance(&1, false), 42);
});
}

#[test]
fn slashing_reserved_balance_should_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand All @@ -485,6 +550,18 @@ macro_rules! decl_tests {
});
}

#[test]
fn slashing_held_fungible_should_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
assert_ok!(Balances::hold(&1, 111));
assert_eq!(Balances::slash_held(&1, 42).1, 0);
assert_eq!(Balances::balance_on_hold(&1), 69);
assert_eq!(Balances::reducible_balance(&1, false), 0);
assert_eq!(Balances::total_issuance(), 69);
});
}

#[test]
fn slashing_incomplete_reserved_balance_should_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand All @@ -497,6 +574,18 @@ macro_rules! decl_tests {
});
}

#[test]
fn slashing_incomplete_held_fungible_should_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
assert_ok!(Balances::hold(&1, 42));
assert_eq!(Balances::slash_held(&1, 69).1, 0);
assert_eq!(Balances::reducible_balance(&1, false), 69);
assert_eq!(Balances::balance_on_hold(&1), 0);
assert_eq!(Balances::total_issuance(), 69);
});
}

#[test]
fn repatriating_reserved_balance_should_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand Down Expand Up @@ -528,6 +617,20 @@ macro_rules! decl_tests {
});
}

#[test]
fn transferring_held_fungible_should_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 110);
let _ = <Balances as fungible::Mutate<_>>::mint_into(&2, 1);
assert_ok!(Balances::hold(&1, 110));
assert_ok!(Balances::transfer_held(&1, &2, 41, false, true), 41);
assert_eq!(Balances::balance_on_hold(&1), 69);
assert_eq!(Balances::reducible_balance(&1, false), 0);
assert_eq!(Balances::balance_on_hold(&2), 41);
assert_eq!(Balances::reducible_balance(&2, false), 1);
});
}

#[test]
fn transferring_reserved_balance_to_yourself_should_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand All @@ -544,6 +647,22 @@ macro_rules! decl_tests {
});
}

#[test]
fn transferring_held_fungible_to_yourself_should_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 110);
assert_ok!(Balances::hold(&1, 50));
assert_ok!(Balances::transfer_held(&1, &1, 50, true, false), 50);
assert_eq!(Balances::reducible_balance(&1, false), 110);
assert_eq!(Balances::balance_on_hold(&1), 0);

assert_ok!(Balances::hold(&1, 50));
assert_ok!(Balances::transfer_held(&1, &1, 60, true, false), 50);
assert_eq!(Balances::reducible_balance(&1, false), 110);
assert_eq!(Balances::balance_on_hold(&1), 0);
});
}

#[test]
fn transferring_reserved_balance_to_nonexistent_should_fail() {
<$ext_builder>::default().build().execute_with(|| {
Expand All @@ -553,6 +672,15 @@ macro_rules! decl_tests {
});
}

#[test]
fn transferring_held_fungible_to_nonexistent_should_fail() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 111);
assert_ok!(Balances::hold(&1, 111));
assert_noop!(Balances::transfer_held(&1, &2, 42, false, false), Error::<$test, _>::DeadAccount);
});
}

#[test]
fn transferring_incomplete_reserved_balance_should_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand All @@ -567,6 +695,20 @@ macro_rules! decl_tests {
});
}

#[test]
fn transferring_incomplete_held_fungible_should_work() {
<$ext_builder>::default().build().execute_with(|| {
let _ = <Balances as fungible::Mutate<_>>::mint_into(&1, 110);
let _ = <Balances as fungible::Mutate<_>>::mint_into(&2, 1);
assert_ok!(Balances::hold(&1, 41));
assert_ok!(Balances::transfer_held(&1, &2, 69, true, false), 41);
assert_eq!(Balances::balance_on_hold(&1), 0);
assert_eq!(Balances::reducible_balance(&1, false), 69);
assert_eq!(Balances::balance_on_hold(&2), 0);
assert_eq!(Balances::reducible_balance(&2, false), 42);
});
}

#[test]
fn transferring_too_high_value_should_not_panic() {
<$ext_builder>::default().build().execute_with(|| {
Expand Down
6 changes: 3 additions & 3 deletions frame/support/src/traits/tokens/fungible.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub trait InspectHold<AccountId>: Inspect<AccountId> {
fn balance_on_hold(who: &AccountId) -> Self::Balance;

/// Check to see if some `amount` of funds of `who` may be placed on hold.
fn can_hold(who: &AccountId, amount: Self::Balance) -> bool;
fn can_hold(who: &AccountId, amount: Self::Balance, keep_alive: bool) -> bool;
}

/// Trait for mutating a fungible asset which can be reserved.
Expand Down Expand Up @@ -269,8 +269,8 @@ impl<
fn balance_on_hold(who: &AccountId) -> Self::Balance {
<F as fungibles::InspectHold<AccountId>>::balance_on_hold(A::get(), who)
}
fn can_hold(who: &AccountId, amount: Self::Balance) -> bool {
<F as fungibles::InspectHold<AccountId>>::can_hold(A::get(), who, amount)
fn can_hold(who: &AccountId, amount: Self::Balance, keep_alive: bool) -> bool {
<F as fungibles::InspectHold<AccountId>>::can_hold(A::get(), who, amount, keep_alive)
}
}

Expand Down
10 changes: 9 additions & 1 deletion frame/support/src/traits/tokens/fungibles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,15 @@ pub trait InspectHold<AccountId>: Inspect<AccountId> {
fn balance_on_hold(asset: Self::AssetId, who: &AccountId) -> Self::Balance;

/// Check to see if some `amount` of `asset` may be held on the account of `who`.
fn can_hold(asset: Self::AssetId, who: &AccountId, amount: Self::Balance) -> bool;
///
/// If `keep_alive` is set to `true`, then it also checks so that the free balance of `asset`
/// does not go below its existential deposit after holding `amount`.
fn can_hold(
asset: Self::AssetId,
who: &AccountId,
amount: Self::Balance,
keep_alive: bool,
) -> bool;
}

/// Trait for mutating a set of named fungible assets which can be placed on hold.
Expand Down