Implement transfer_all in Balances Pallet (paritytech#9018)

* transfer_all * benchmark * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs * update * add note * typo Co-authored-by: Parity Bot <admin@parity.io> Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
Cerebellum-Network · Sep 9, 2022 · 0dc0438 · 0dc0438
1 parent 33fecad
commit 0dc0438
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 15 deletions.
diff --git a/frame/balances/src/benchmarking.rs b/frame/balances/src/benchmarking.rs
@@ -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);
 
@@ -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);
 
@@ -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);
 
@@ -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!(

diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs
@@ -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.
+		///
+		/// 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.
+		///
+		/// - `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;
+			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]
@@ -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) => {

diff --git a/frame/balances/src/tests.rs b/frame/balances/src/tests.rs
@@ -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);
+				});
+		}
+
 		#[test]
 		fn named_reserve_should_work() {
 			<$ext_builder>::default().build().execute_with(|| {

diff --git a/frame/balances/src/weights.rs b/frame/balances/src/weights.rs
@@ -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:
@@ -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))
+	}
 }