Make ED of zero (kind of) work

frame/balances/Cargo.toml

@@ -39,5 +39,6 @@ std = [
 	"sp-runtime/std",
 	"sp-std/std",
 ]
+zero_ed = []
 runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks"]
 try-runtime = ["frame-support/try-runtime"]

frame/balances/src/lib.rs

@@ -151,6 +151,10 @@
 //! ## Assumptions
 //!
 //! * Total issued balanced of all accounts should be less than `Config::Balance::max_value()`.
+//! * Existential Deposit is set to a value greater than zero.
+//!
+//! Note, you may find the Balances pallet still functions with an ED of zero in some circumstances,
+//! however this is not a configuration which is generally supported, nor will it be.
 
 #![cfg_attr(not(feature = "std"), no_std)]
 mod benchmarking;
@@ -231,7 +235,14 @@ pub mod pallet {
 		/// Handler for the unbalanced reduction when removing a dust account.
 		type DustRemoval: OnUnbalanced<CreditOf<Self, I>>;
 
-		/// The minimum amount required to keep an account open.
+		/// The minimum amount required to keep an account open. MUST BE GREATER THAN ZERO!
+		///
+		/// If you *really* need it to be zero, you can enable the feature `zero_ed` for this
+		/// pallet. However, you do so at your own risk: this will open up a major DoS vector. In
+		/// case you have multiple sources of provider references, you may also get unexpected
+		/// behaviour if you set this to zero.
+		///
+		/// Bottom line: Do yourself a favour and make it at least one!
 		#[pallet::constant]
 		type ExistentialDeposit: Get<Self::Balance>;
 
@@ -444,7 +455,10 @@ pub mod pallet {
 	#[pallet::genesis_build]
 	impl<T: Config<I>, I: 'static> GenesisBuild<T, I> for GenesisConfig<T, I> {
 		fn build(&self) {
+			Pallet::<T, I>::integrity_test();
+
 			let total = self.balances.iter().fold(Zero::zero(), |acc: T::Balance, &(_, n)| acc + n);
+
 			<TotalIssuance<T, I>>::put(total);
 
 			for (_, balance) in &self.balances {
@@ -492,6 +506,17 @@ pub mod pallet {
 		}
 	}
 
+	#[pallet::hooks]
+	impl<T: Config<I>, I: 'static> Hooks<T::BlockNumber> for Pallet<T, I> {
+		#[cfg(not(feature = "zero_ed"))]
+		fn integrity_test() {
+			frame_support::defensive_assert!(
+				!<T as Config<I>>::ExistentialDeposit::get().is_zero(),
+				"The existential deposit must be greater than zero!"
+			);
+		}
+	}
+
 	#[pallet::call]
 	impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		/// Transfer some liquid free balance to another account.
@@ -533,7 +558,7 @@ pub mod pallet {
 		) -> DispatchResultWithPostInfo {
 			ensure_root(origin)?;
 			let who = T::Lookup::lookup(who)?;
-			let existential_deposit = T::ExistentialDeposit::get();
+			let existential_deposit = Self::ed();
 
 			let wipeout = new_free < existential_deposit;
 			let new_free = if wipeout { Zero::zero() } else { new_free };
@@ -716,7 +741,7 @@ pub mod pallet {
 		) -> DispatchResultWithPostInfo {
 			ensure_root(origin)?;
 			let who = T::Lookup::lookup(who)?;
-			let existential_deposit = T::ExistentialDeposit::get();
+			let existential_deposit = Self::ed();
 
 			let wipeout = new_free < existential_deposit;
 			let new_free = if wipeout { Zero::zero() } else { new_free };
@@ -742,6 +767,9 @@ pub mod pallet {
 	}
 
 	impl<T: Config<I>, I: 'static> Pallet<T, I> {
+		fn ed() -> T::Balance {
+			T::ExistentialDeposit::get()
+		}
 		/// Ensure the account `who` is using the new logic.
 		///
 		/// Returns `true` if the account did get upgraded, `false` if it didn't need upgrading.
@@ -756,7 +784,7 @@ pub mod pallet {
 					// Gah!! We have a non-zero reserve balance but no provider refs :(
 					// This shouldn't practically happen, but we need a failsafe anyway: let's give
 					// them enough for an ED.
-					a.free = a.free.min(T::ExistentialDeposit::get());
+					a.free = a.free.min(Self::ed());
 					system::Pallet::<T>::inc_providers(who);
 				}
 				let _ = system::Pallet::<T>::inc_consumers(who).defensive();
@@ -885,13 +913,14 @@ pub mod pallet {
 			let result = T::AccountStore::try_mutate_exists(who, |maybe_account| {
 				let is_new = maybe_account.is_none();
 				let mut account = maybe_account.take().unwrap_or_default();
-				let did_provide = account.free >= T::ExistentialDeposit::get();
+				let did_provide =
+					account.free >= Self::ed() && frame_system::Pallet::<T>::providers(who) > 0;
 				let did_consume =
 					!is_new && (!account.reserved.is_zero() || !account.frozen.is_zero());
 
 				let result = f(&mut account, is_new)?;
 
-				let does_provide = account.free >= T::ExistentialDeposit::get();
+				let does_provide = account.free >= Self::ed();
 				let does_consume = !account.reserved.is_zero() || !account.frozen.is_zero();
 
 				if !did_provide && does_provide {
@@ -930,7 +959,7 @@ pub mod pallet {
 				//
 				// We should never be dropping if reserved is non-zero. Reserved being non-zero
 				// should imply that we have a consumer ref, so this is economically safe.
-				let ed = T::ExistentialDeposit::get();
+				let ed = Self::ed();
 				let maybe_dust = if account.free < ed && account.reserved.is_zero() {
 					if account.free.is_zero() {
 						None

frame/balances/src/tests/currency_tests.rs

@@ -974,7 +974,7 @@ fn slash_reserved_on_non_existant_works() {
 fn operations_on_dead_account_should_not_change_state() {
 	// These functions all use `mutate_account` which may introduce a storage change when
 	// the account never existed to begin with, and shouldn't exist in the end.
-	ExtBuilder::default().existential_deposit(0).build_and_execute_with(|| {
+	ExtBuilder::default().existential_deposit(1).build_and_execute_with(|| {
 		assert!(!frame_system::Account::<Test>::contains_key(&1337));
 
 		// Unreserve
@@ -993,6 +993,14 @@ fn operations_on_dead_account_should_not_change_state() {
 	});
 }
 
+#[test]
+#[should_panic = "The existential deposit must be greater than zero!"]
+fn zero_ed_is_prohibited() {
+	// These functions all use `mutate_account` which may introduce a storage change when
+	// the account never existed to begin with, and shouldn't exist in the end.
+	ExtBuilder::default().existential_deposit(0).build_and_execute_with(|| {});
+}
+
 #[test]
 fn named_reserve_should_work() {
 	ExtBuilder::default().build_and_execute_with(|| {

frame/balances/src/tests/mod.rs

@@ -87,7 +87,7 @@ parameter_types! {
 		frame_system::limits::BlockWeights::simple_max(
 			frame_support::weights::Weight::from_parts(1024, u64::MAX),
 		);
-	pub static ExistentialDeposit: u64 = 0;
+	pub static ExistentialDeposit: u64 = 1;
 }
 impl frame_system::Config for Test {
 	type BaseCallFilter = frame_support::traits::Everything;

frame/vesting/src/mock.rs

@@ -89,7 +89,7 @@ parameter_types! {
 	pub const MinVestedTransfer: u64 = 256 * 2;
 	pub UnvestedFundsAllowedWithdrawReasons: WithdrawReasons =
 		WithdrawReasons::except(WithdrawReasons::TRANSFER | WithdrawReasons::RESERVE);
-	pub static ExistentialDeposit: u64 = 0;
+	pub static ExistentialDeposit: u64 = 1;
 }
 impl Config for Test {
 	type BlockNumberToBalance = Identity;