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

Add ExistenceRequirement to Currency trait #4000

Merged
merged 12 commits into from
Nov 7, 2019
37 changes: 34 additions & 3 deletions srml/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,8 @@ decl_module! {
/// `T::OnNewAccount::on_new_account` to be called.
/// - Removing enough funds from an account will trigger
/// `T::DustRemoval::on_unbalanced` and `T::OnFreeBalanceZero::on_free_balance_zero`.
/// - `transfer_keep_alive` works the same way as `transfer`, but has an additional
/// check that the transfer will not kill the origin account.
///
/// # </weight>
#[weight = SimpleDispatchInfo::FixedNormal(1_000_000)]
Expand All @@ -407,7 +409,7 @@ decl_module! {
) {
let transactor = ensure_signed(origin)?;
let dest = T::Lookup::lookup(dest)?;
<Self as Currency<_>>::transfer(&transactor, &dest, value)?;
<Self as Currency<_>>::transfer(&transactor, &dest, value, ExistenceRequirement::AllowDeath)?;
}

/// Set the balances of a given account.
Expand Down Expand Up @@ -462,8 +464,26 @@ decl_module! {
ensure_root(origin)?;
let source = T::Lookup::lookup(source)?;
let dest = T::Lookup::lookup(dest)?;
<Self as Currency<_>>::transfer(&source, &dest, value)?;
<Self as Currency<_>>::transfer(&source, &dest, value, ExistenceRequirement::AllowDeath)?;
}

/// Same as the [`transfer`] call, but with a check that the transfer will not kill the
/// origin account.
///
/// 99% of the time you want [`transfer`] instead.
///
/// [`transfer`]: struct.Module.html#method.transfer
#[weight = SimpleDispatchInfo::FixedNormal(1_000_000)]
pub fn transfer_keep_alive(
origin,
dest: <T::Lookup as StaticLookup>::Source,
#[compact] value: T::Balance
) {
let transactor = ensure_signed(origin)?;
let dest = T::Lookup::lookup(dest)?;
<Self as Currency<_>>::transfer(&transactor, &dest, value, ExistenceRequirement::KeepAlive)?;
}

}
}

Expand Down Expand Up @@ -851,7 +871,12 @@ where
}
}

fn transfer(transactor: &T::AccountId, dest: &T::AccountId, value: Self::Balance) -> Result {
fn transfer(
transactor: &T::AccountId,
dest: &T::AccountId,
value: Self::Balance,
existence_requirement: ExistenceRequirement,
expenses marked this conversation as resolved.
Show resolved Hide resolved
) -> Result {
let from_balance = Self::free_balance(transactor);
let to_balance = Self::free_balance(dest);
let would_create = to_balance.is_zero();
Expand All @@ -878,6 +903,12 @@ where
};

if transactor != dest {
if existence_requirement == ExistenceRequirement::KeepAlive {
if new_from_balance < Self::minimum_balance() {
return Err("transfer would kill account");
}
}

Self::set_free_balance(transactor, new_from_balance);
if !<FreeBalance<T, I>>::exists(dest) {
Self::new_account(dest, new_to_balance);
Expand Down
56 changes: 35 additions & 21 deletions srml/balances/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use sr_primitives::traits::SignedExtension;
use support::{
assert_noop, assert_ok, assert_err,
traits::{LockableCurrency, LockIdentifier, WithdrawReason, WithdrawReasons,
Currency, ReservableCurrency}
Currency, ReservableCurrency, ExistenceRequirement::AllowDeath}
};
use transaction_payment::ChargeTransactionPayment;
use system::RawOrigin;
Expand All @@ -39,7 +39,7 @@ fn basic_locking_should_work() {
assert_eq!(Balances::free_balance(&1), 10);
Balances::set_lock(ID_1, &1, 9, u64::max_value(), WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 5),
<Balances as Currency<_>>::transfer(&1, &2, 5, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
});
Expand All @@ -49,7 +49,7 @@ fn basic_locking_should_work() {
fn partial_locking_should_work() {
ExtBuilder::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, 5, u64::max_value(), WithdrawReasons::all());
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1));
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
});
}

Expand All @@ -58,7 +58,7 @@ fn lock_removal_should_work() {
ExtBuilder::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, u64::max_value(), u64::max_value(), WithdrawReasons::all());
Balances::remove_lock(ID_1, &1);
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1));
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
});
}

Expand All @@ -67,7 +67,7 @@ fn lock_replacement_should_work() {
ExtBuilder::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, u64::max_value(), u64::max_value(), WithdrawReasons::all());
Balances::set_lock(ID_1, &1, 5, u64::max_value(), WithdrawReasons::all());
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1));
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
});
}

Expand All @@ -76,7 +76,7 @@ fn double_locking_should_work() {
ExtBuilder::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, 5, u64::max_value(), WithdrawReasons::all());
Balances::set_lock(ID_2, &1, 5, u64::max_value(), WithdrawReasons::all());
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1));
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
});
}

Expand All @@ -86,7 +86,7 @@ fn combination_locking_should_work() {
Balances::set_lock(ID_1, &1, u64::max_value(), 0, WithdrawReasons::none());
Balances::set_lock(ID_2, &1, 0, u64::max_value(), WithdrawReasons::none());
Balances::set_lock(ID_3, &1, 0, 0, WithdrawReasons::all());
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1));
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
});
}

Expand All @@ -95,17 +95,17 @@ fn lock_value_extension_should_work() {
ExtBuilder::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, 5, u64::max_value(), WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6),
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
Balances::extend_lock(ID_1, &1, 2, u64::max_value(), WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6),
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
Balances::extend_lock(ID_1, &1, 8, u64::max_value(), WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 3),
<Balances as Currency<_>>::transfer(&1, &2, 3, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
});
Expand All @@ -120,7 +120,7 @@ fn lock_reasons_should_work() {
.execute_with(|| {
Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::Transfer.into());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 1),
<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
assert_ok!(<Balances as ReservableCurrency<_>>::reserve(&1, 1));
Expand All @@ -134,7 +134,7 @@ fn lock_reasons_should_work() {
).is_ok());

Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::Reserve.into());
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1));
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
assert_noop!(
<Balances as ReservableCurrency<_>>::reserve(&1, 1),
"account liquidity restrictions prevent withdrawal"
Expand All @@ -148,7 +148,7 @@ fn lock_reasons_should_work() {
).is_ok());

Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::TransactionPayment.into());
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1));
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
assert_ok!(<Balances as ReservableCurrency<_>>::reserve(&1, 1));
assert!(<ChargeTransactionPayment<Runtime> as SignedExtension>::pre_dispatch(
ChargeTransactionPayment::from(1),
Expand All @@ -165,12 +165,12 @@ fn lock_block_number_should_work() {
ExtBuilder::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, 10, 2, WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 1),
<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);

System::set_block_number(2);
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1));
assert_ok!(<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath));
});
}

Expand All @@ -179,18 +179,18 @@ fn lock_block_number_extension_should_work() {
ExtBuilder::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, 10, 2, WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6),
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
Balances::extend_lock(ID_1, &1, 10, 1, WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6),
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
System::set_block_number(2);
Balances::extend_lock(ID_1, &1, 10, 8, WithdrawReasons::all());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 3),
<Balances as Currency<_>>::transfer(&1, &2, 3, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
});
Expand All @@ -201,17 +201,17 @@ fn lock_reasons_extension_should_work() {
ExtBuilder::default().existential_deposit(1).monied(true).build().execute_with(|| {
Balances::set_lock(ID_1, &1, 10, 10, WithdrawReason::Transfer.into());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6),
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
Balances::extend_lock(ID_1, &1, 10, 10, WithdrawReasons::none());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6),
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
Balances::extend_lock(ID_1, &1, 10, 10, WithdrawReason::Reserve.into());
assert_noop!(
<Balances as Currency<_>>::transfer(&1, &2, 6),
<Balances as Currency<_>>::transfer(&1, &2, 6, AllowDeath),
"account liquidity restrictions prevent withdrawal"
);
});
Expand Down Expand Up @@ -746,3 +746,17 @@ fn burn_must_work() {
assert_eq!(Balances::total_issuance(), init_total_issuance);
});
}

#[test]
fn transfer_keep_alive_works() {
ExtBuilder::default().existential_deposit(1).build().execute_with(|| {
let _ = Balances::deposit_creating(&1, 100);
assert_err!(
Balances::transfer_keep_alive(Some(1).into(), 2, 100),
"transfer would kill account"
);
assert_eq!(Balances::is_dead_account(&1), false);
assert_eq!(Balances::total_balance(&1), 100);
assert_eq!(Balances::total_balance(&2), 0);
});
}
7 changes: 6 additions & 1 deletion srml/generic-asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,12 @@ where
Zero::zero()
}

fn transfer(transactor: &T::AccountId, dest: &T::AccountId, value: Self::Balance) -> Result {
fn transfer(
transactor: &T::AccountId,
dest: &T::AccountId,
value: Self::Balance,
_: ExistenceRequirement, // no existential deposit policy for generic asset
) -> Result {
<Module<T>>::make_transfer(&U::asset_id(), transactor, dest, value)
}

Expand Down
1 change: 1 addition & 0 deletions srml/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ pub trait Currency<AccountId> {
source: &AccountId,
dest: &AccountId,
value: Self::Balance,
existence_requirement: ExistenceRequirement,
) -> result::Result<(), &'static str>;

/// Deducts up to `value` from the combined balance of `who`, preferring to deduct from the
Expand Down