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

Implement transfer_all in Balances Pallet #9018

Merged
11 commits merged into from
Jun 11, 2021
24 changes: 21 additions & 3 deletions frame/balances/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ benchmarks_instance_pallet! {
let existential_deposit = T::ExistentialDeposit::get();
let caller = whitelisted_caller();

// Give some multiple of the existential deposit + creation fee + transfer fee
// Give some multiple of the existential deposit
let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&caller, balance);

Expand Down Expand Up @@ -130,7 +130,7 @@ benchmarks_instance_pallet! {
let source: T::AccountId = account("source", 0, SEED);
let source_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(source.clone());

// Give some multiple of the existential deposit + creation fee + transfer fee
// Give some multiple of the existential deposit
let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&source, balance);

Expand All @@ -154,7 +154,7 @@ benchmarks_instance_pallet! {
let existential_deposit = T::ExistentialDeposit::get();
let caller = whitelisted_caller();

// Give some multiple of the existential deposit + creation fee + transfer fee
// Give some multiple of the existential deposit
let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&caller, balance);

Expand All @@ -176,6 +176,24 @@ benchmarks_instance_pallet! {
assert_eq!(Balances::<T, I>::free_balance(&caller), Zero::zero());
assert_eq!(Balances::<T, I>::free_balance(&recipient), transfer_amount);
}

// Benchmark `transfer_all` with the worst possible condition:
// * The recipient account is created
// * The sender is killed
transfer_all {
let caller = whitelisted_caller();
let recipient: T::AccountId = account("recipient", 0, SEED);
let recipient_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(recipient.clone());

// Give some multiple of the existential deposit
let existential_deposit = T::ExistentialDeposit::get();
let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
let _ = <Balances<T, I> as Currency<_>>::make_free_balance_be(&caller, balance);
}: _(RawOrigin::Signed(caller.clone()), recipient_lookup, false)
verify {
assert!(Balances::<T, I>::free_balance(&caller).is_zero());
assert_eq!(Balances::<T, I>::free_balance(&recipient), balance);
}
}

impl_benchmark_test_suite!(
Expand Down
35 changes: 34 additions & 1 deletion frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,39 @@ pub mod pallet {
<Self as Currency<_>>::transfer(&transactor, &dest, value, KeepAlive)?;
Ok(().into())
}

/// Transfer the entire transferable balance from the caller account.
///
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
/// NOTE: This function only attempts to transfer _transferable_ balances. This means that
/// any locked, reserved, or existential deposits (when `keep_alive` is `true`), will not be
/// transferred by this function. To ensure that this function results in a killed account,
/// you might need to prepare the account by removing any reference counters, storage
/// deposits, etc...
///
/// The dispatch origin of this call must be Signed.
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
///
/// - `dest`: The recipient of the transfer.
/// - `keep_alive`: A boolean to determine if the `transfer_all` operation should send all
/// of the funds the account has, causing the sender account to be killed (false), or
/// transfer everything except at least the existential deposit, which will guarantee to
/// keep the sender account alive (true).
/// # <weight>
/// - O(1). Just like transfer, but reading the user's transferable balance first.
/// #</weight>
#[pallet::weight(T::WeightInfo::transfer_all())]
pub fn transfer_all(
origin: OriginFor<T>,
dest: <T::Lookup as StaticLookup>::Source,
keep_alive: bool,
) -> DispatchResultWithPostInfo {
use fungible::Inspect;
apopiak marked this conversation as resolved.
Show resolved Hide resolved
let transactor = ensure_signed(origin)?;
let reducible_balance = Self::reducible_balance(&transactor, keep_alive);
let dest = T::Lookup::lookup(dest)?;
let keep_alive = if keep_alive { KeepAlive } else { AllowDeath };
<Self as Currency<_>>::transfer(&transactor, &dest, reducible_balance, keep_alive.into())?;
Ok(().into())
}
}

#[pallet::event]
Expand Down Expand Up @@ -1696,7 +1729,7 @@ impl<T: Config<I>, I: 'static> NamedReservableCurrency<T::AccountId> for Pallet<
/// Is a no-op if value to be reserved is zero.
fn reserve_named(id: &Self::ReserveIdentifier, who: &T::AccountId, value: Self::Balance) -> DispatchResult {
if value.is_zero() { return Ok(()) }

Reserves::<T, I>::try_mutate(who, |reserves| -> DispatchResult {
match reserves.binary_search_by_key(id, |data| data.id) {
Ok(index) => {
Expand Down
40 changes: 40 additions & 0 deletions frame/balances/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,46 @@ macro_rules! decl_tests {
});
}

#[test]
fn transfer_all_works() {
<$ext_builder>::default()
.existential_deposit(100)
.build()
.execute_with(|| {
// setup
assert_ok!(Balances::set_balance(Origin::root(), 1, 200, 0));
assert_ok!(Balances::set_balance(Origin::root(), 2, 0, 0));
// transfer all and allow death
assert_ok!(Balances::transfer_all(Some(1).into(), 2, false));
assert_eq!(Balances::total_balance(&1), 0);
assert_eq!(Balances::total_balance(&2), 200);

// setup
assert_ok!(Balances::set_balance(Origin::root(), 1, 200, 0));
assert_ok!(Balances::set_balance(Origin::root(), 2, 0, 0));
// transfer all and keep alive
assert_ok!(Balances::transfer_all(Some(1).into(), 2, true));
assert_eq!(Balances::total_balance(&1), 100);
assert_eq!(Balances::total_balance(&2), 100);

// setup
assert_ok!(Balances::set_balance(Origin::root(), 1, 200, 10));
assert_ok!(Balances::set_balance(Origin::root(), 2, 0, 0));
// transfer all and allow death w/ reserved
assert_ok!(Balances::transfer_all(Some(1).into(), 2, false));
assert_eq!(Balances::total_balance(&1), 0);
assert_eq!(Balances::total_balance(&2), 200);

// setup
assert_ok!(Balances::set_balance(Origin::root(), 1, 200, 10));
assert_ok!(Balances::set_balance(Origin::root(), 2, 0, 0));
// transfer all and keep alive w/ reserved
assert_ok!(Balances::transfer_all(Some(1).into(), 2, true));
assert_eq!(Balances::total_balance(&1), 100);
assert_eq!(Balances::total_balance(&2), 110);
});
emostov marked this conversation as resolved.
Show resolved Hide resolved
}

#[test]
fn named_reserve_should_work() {
<$ext_builder>::default().build().execute_with(|| {
Expand Down
33 changes: 22 additions & 11 deletions frame/balances/src/weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
//! Autogenerated weights for pallet_balances
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0
//! DATE: 2021-04-08, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2021-06-04, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128

// Executed Command:
Expand Down Expand Up @@ -49,63 +49,74 @@ pub trait WeightInfo {
fn set_balance_creating() -> Weight;
fn set_balance_killing() -> Weight;
fn force_transfer() -> Weight;
fn transfer_all() -> Weight;
}

/// Weights for pallet_balances using the Substrate node and recommended hardware.
pub struct SubstrateWeight<T>(PhantomData<T>);
impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
fn transfer() -> Weight {
(81_909_000 as Weight)
(91_896_000 as Weight)
.saturating_add(T::DbWeight::get().reads(1 as Weight))
.saturating_add(T::DbWeight::get().writes(1 as Weight))
}
fn transfer_keep_alive() -> Weight {
(61_075_000 as Weight)
(67_779_000 as Weight)
.saturating_add(T::DbWeight::get().reads(1 as Weight))
.saturating_add(T::DbWeight::get().writes(1 as Weight))
}
fn set_balance_creating() -> Weight {
(32_255_000 as Weight)
(36_912_000 as Weight)
.saturating_add(T::DbWeight::get().reads(1 as Weight))
.saturating_add(T::DbWeight::get().writes(1 as Weight))
}
fn set_balance_killing() -> Weight {
(38_513_000 as Weight)
(44_416_000 as Weight)
.saturating_add(T::DbWeight::get().reads(1 as Weight))
.saturating_add(T::DbWeight::get().writes(1 as Weight))
}
fn force_transfer() -> Weight {
(80_448_000 as Weight)
(90_811_000 as Weight)
.saturating_add(T::DbWeight::get().reads(2 as Weight))
.saturating_add(T::DbWeight::get().writes(2 as Weight))
}
fn transfer_all() -> Weight {
(84_170_000 as Weight)
.saturating_add(T::DbWeight::get().reads(1 as Weight))
.saturating_add(T::DbWeight::get().writes(1 as Weight))
}
}

// For backwards compatibility and tests
impl WeightInfo for () {
fn transfer() -> Weight {
(81_909_000 as Weight)
(91_896_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(1 as Weight))
.saturating_add(RocksDbWeight::get().writes(1 as Weight))
}
fn transfer_keep_alive() -> Weight {
(61_075_000 as Weight)
(67_779_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(1 as Weight))
.saturating_add(RocksDbWeight::get().writes(1 as Weight))
}
fn set_balance_creating() -> Weight {
(32_255_000 as Weight)
(36_912_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(1 as Weight))
.saturating_add(RocksDbWeight::get().writes(1 as Weight))
}
fn set_balance_killing() -> Weight {
(38_513_000 as Weight)
(44_416_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(1 as Weight))
.saturating_add(RocksDbWeight::get().writes(1 as Weight))
}
fn force_transfer() -> Weight {
(80_448_000 as Weight)
(90_811_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(2 as Weight))
.saturating_add(RocksDbWeight::get().writes(2 as Weight))
}
fn transfer_all() -> Weight {
(84_170_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(1 as Weight))
.saturating_add(RocksDbWeight::get().writes(1 as Weight))
}
}