Add ExistenceRequirement to Currency trait (#4000)

* Added a public transfer_some function and a private transfer_inner fn

* Move transfer_some to the end of the module impl to fix failing contracts test

* Change whitespace

* Remove needless change to transfer logic

* Fix error

* Update srml/balances/src/lib.rs

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Improve documentation and add test

* Update srml/balances/src/lib.rs

Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Switch to changing Currency trait instead

substrate/srml/balances/src/lib.rs

@@ -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)]
@@ -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.
@@ -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)?;
+		}
+
 	}
 }
 
@@ -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,
+	) -> Result {
 		let from_balance = Self::free_balance(transactor);
 		let to_balance = Self::free_balance(dest);
 		let would_create = to_balance.is_zero();
@@ -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);

substrate/srml/balances/src/tests.rs

@@ -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;
@@ -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"
 		);
 	});
@@ -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));
 	});
 }
 
@@ -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));
 	});
 }
 
@@ -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));
 	});
 }
 
@@ -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));
 	});
 }
 
@@ -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));
 	});
 }
 
@@ -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"
 		);
 	});
@@ -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));
@@ -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"
@@ -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),
@@ -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));
 	});
 }
 
@@ -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"
 		);
 	});
@@ -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"
 		);
 	});
@@ -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);
+	});
+}

substrate/srml/generic-asset/src/lib.rs

@@ -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)
 	}
 

substrate/srml/support/src/traits.rs

@@ -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