Prevent dust in staking by disallowing cheap bond_extra (#7718)

* prevent bond_extra to cause staking actve lower than ed

* prevent bond_extra to cause staking actve lower than ed

* Check in post conditions.

* check rebond as well.

* also change withdraw_unbonded.

* Fix build

* change check format.

* Apply suggestions from code review

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

substrate/frame/staking/src/lib.rs

@@ -1474,11 +1474,13 @@ decl_module! {
 			let mut ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
 
 			let stash_balance = T::Currency::free_balance(&stash);
-
 			if let Some(extra) = stash_balance.checked_sub(&ledger.total) {
 				let extra = extra.min(max_additional);
 				ledger.total += extra;
 				ledger.active += extra;
+				// last check: the new active amount of ledger must be more than ED.
+				ensure!(ledger.active >= T::Currency::minimum_balance(), Error::<T>::InsufficientValue);
+
 				Self::deposit_event(RawEvent::Bonded(stash, extra));
 				Self::update_ledger(&controller, &ledger);
 			}
@@ -1586,7 +1588,7 @@ decl_module! {
 				ledger = ledger.consolidate_unlocked(current_era)
 			}
 
-			let post_info_weight = if ledger.unlocking.is_empty() && ledger.active.is_zero() {
+			let post_info_weight = if ledger.unlocking.is_empty() && ledger.active <= T::Currency::minimum_balance() {
 				// This account must have called `unbond()` with some value that caused the active
 				// portion to fall below existential deposit + will have no more unlocking chunks
 				// left. We can now safely remove all staking-related information.
@@ -1973,6 +1975,9 @@ decl_module! {
 			ensure!(!ledger.unlocking.is_empty(), Error::<T>::NoUnlockChunk);
 
 			let ledger = ledger.rebond(value);
+			// last check: the new active amount of ledger must be more than ED.
+			ensure!(ledger.active >= T::Currency::minimum_balance(), Error::<T>::InsufficientValue);
+
 			Self::update_ledger(&controller, &ledger);
 			Ok(Some(
 				35 * WEIGHT_PER_MICROS

substrate/frame/staking/src/mock.rs

@@ -579,8 +579,18 @@ fn assert_is_stash(acc: AccountId) {
 fn assert_ledger_consistent(ctrl: AccountId) {
 	// ensures ledger.total == ledger.active + sum(ledger.unlocking).
 	let ledger = Staking::ledger(ctrl).expect("Not a controller.");
-	let real_total: Balance = ledger.unlocking.iter().fold(ledger.active, |a, c| a + c.value);
+	let real_total: Balance = ledger
+		.unlocking
+		.iter()
+		.fold(ledger.active, |a, c| a + c.value);
 	assert_eq!(real_total, ledger.total);
+	assert!(
+		ledger.active >= Balances::minimum_balance() || ledger.active == 0,
+		"{}: active ledger amount ({}) must be greater than ED {}",
+		ctrl,
+		ledger.active,
+		Balances::minimum_balance()
+	);
 }
 
 pub(crate) fn bond_validator(stash: AccountId, ctrl: AccountId, val: Balance) {

substrate/frame/staking/src/tests.rs

@@ -4696,3 +4696,85 @@ fn payout_to_any_account_works() {
 		assert!(Balances::free_balance(42) > 0);
 	})
 }
+
+#[test]
+fn cannot_bond_extra_to_lower_than_ed() {
+	ExtBuilder::default()
+		.existential_deposit(10)
+		.build_and_execute(|| {
+			// stash must have more balance than bonded for this to work.
+			assert_eq!(Balances::free_balance(&21), 512_000);
+
+			// initial stuff.
+			assert_eq!(
+				Staking::ledger(&20).unwrap(),
+				StakingLedger {
+					stash: 21,
+					total: 1000,
+					active: 1000,
+					unlocking: vec![],
+					claimed_rewards: vec![]
+				}
+			);
+
+			// unbond all of it.
+			assert_ok!(Staking::unbond(Origin::signed(20), 1000));
+			assert_eq!(
+				Staking::ledger(&20).unwrap(),
+				StakingLedger {
+					stash: 21,
+					total: 1000,
+					active: 0,
+					unlocking: vec![UnlockChunk { value: 1000, era: 3 }],
+					claimed_rewards: vec![]
+				}
+			);
+
+			// now bond a wee bit more
+			assert_noop!(
+				Staking::bond_extra(Origin::signed(21), 5),
+				Error::<Test>::InsufficientValue,
+			);
+		})
+}
+
+#[test]
+fn cannot_rebond_to_lower_than_ed() {
+	ExtBuilder::default()
+		.existential_deposit(10)
+		.build_and_execute(|| {
+			// stash must have more balance than bonded for this to work.
+			assert_eq!(Balances::free_balance(&21), 512_000);
+
+			// initial stuff.
+			assert_eq!(
+				Staking::ledger(&20).unwrap(),
+				StakingLedger {
+					stash: 21,
+					total: 1000,
+					active: 1000,
+					unlocking: vec![],
+					claimed_rewards: vec![]
+				}
+			);
+
+			// unbond all of it.
+			assert_ok!(Staking::unbond(Origin::signed(20), 1000));
+			assert_eq!(
+				Staking::ledger(&20).unwrap(),
+				StakingLedger {
+					stash: 21,
+					total: 1000,
+					active: 0,
+					unlocking: vec![UnlockChunk { value: 1000, era: 3 }],
+					claimed_rewards: vec![]
+				}
+			);
+
+			// now bond a wee bit more
+			assert_noop!(
+				Staking::rebond(Origin::signed(20), 5),
+				Error::<Test>::InsufficientValue,
+			);
+		})
+}