diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index 2378a60bef..456fb898dd 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/substrate/node/runtime/src/lib.rs @@ -69,8 +69,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to equal spec_version. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 108, - impl_version: 108, + spec_version: 109, + impl_version: 109, apis: RUNTIME_API_VERSIONS, }; diff --git a/substrate/srml/staking/src/lib.rs b/substrate/srml/staking/src/lib.rs index 519087aa20..b87155fda2 100644 --- a/substrate/srml/staking/src/lib.rs +++ b/substrate/srml/staking/src/lib.rs @@ -653,6 +653,8 @@ decl_module! { /// Take the origin account as a stash and lock up `value` of its balance. `controller` will /// be the account that controls it. /// + /// `value` must be more than the `existential_deposit` defined in the Balances module. + /// /// The dispatch origin for this call must be _Signed_ by the stash account. /// /// # @@ -662,10 +664,6 @@ decl_module! { /// /// NOTE: Two of the storage writes (`Self::bonded`, `Self::payee`) are _never_ cleaned unless /// the `origin` falls below _existential deposit_ and gets removed as dust. - /// - /// NOTE: At the moment, there are no financial restrictions to bond - /// (which creates a bunch of storage items for an account). In essence, nothing prevents many accounts from - /// spamming `Staking` storage by bonding 1 UNIT. See test case: `bond_with_no_staked_value`. /// # fn bond(origin, controller: ::Source, @@ -684,6 +682,11 @@ decl_module! { return Err("controller already paired") } + // reject a bond which is considered to be _dust_. + if value < T::Currency::minimum_balance() { + return Err("can not bond with value less than minimum balance") + } + // You're auto-bonded forever, here. We might improve this by only bonding when // you actually validate/nominate and remove once you unbond __everything__. >::insert(&stash, controller.clone()); @@ -699,6 +702,8 @@ decl_module! { /// for staking. /// /// Use this if there are additional funds in your stash account that you wish to bond. + /// Unlike [`bond`] or [`unbond`] this function does not impose any limitation on the amount + /// that can be added. /// /// The dispatch origin for this call must be _Signed_ by the stash, not the controller. /// @@ -781,9 +786,9 @@ decl_module! { /// See also [`Call::unbond`]. /// /// # - /// - Could be dependent on the `origin` argument and how much `unlocking` chunks exist. It implies - /// `consolidate_unlocked` which loops over `Ledger.unlocking`, which is indirectly - /// user-controlled. See [`unbond`] for more detail. + /// - Could be dependent on the `origin` argument and how much `unlocking` chunks exist. + /// It implies `consolidate_unlocked` which loops over `Ledger.unlocking`, which is + /// indirectly user-controlled. See [`unbond`] for more detail. /// - Contains a limited number of reads, yet the size of which could be large based on `ledger`. /// - Writes are limited to the `origin` account key. /// # @@ -791,7 +796,20 @@ decl_module! { let controller = ensure_signed(origin)?; let ledger = Self::ledger(&controller).ok_or("not a controller")?; let ledger = ledger.consolidate_unlocked(Self::current_era()); - Self::update_ledger(&controller, &ledger); + + if ledger.unlocking.is_empty() && ledger.active.is_zero() { + // 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 this. + let stash = ledger.stash; + // remove the lock. + T::Currency::remove_lock(STAKING_ID, &stash); + // remove all staking-related information. + Self::kill_stash(&stash); + } else { + // This was the consequence of a partial unbond. just update the ledger and move on. + Self::update_ledger(&controller, &ledger); + } } /// Declare the desire to validate for the origin controller. @@ -1241,6 +1259,21 @@ impl Module { ForceNewEra::put(true); } + /// Remove all associated data of a stash account from the staking system. + /// + /// This is called : + /// - Immediately when an account's balance falls below existential deposit. + /// - after a `withdraw_unbond()` call that frees all of a stash's bonded balance. + fn kill_stash(stash: &T::AccountId) { + if let Some(controller) = >::take(stash) { + >::remove(&controller); + } + >::remove(stash); + >::remove(stash); + >::remove(stash); + >::remove(stash); + } + /// Call when a validator is determined to be offline. `count` is the /// number of offenses the validator has committed. /// @@ -1315,13 +1348,7 @@ impl OnSessionEnding impl OnFreeBalanceZero for Module { fn on_free_balance_zero(stash: &T::AccountId) { - if let Some(controller) = >::take(stash) { - >::remove(&controller); - } - >::remove(stash); - >::remove(stash); - >::remove(stash); - >::remove(stash); + Self::kill_stash(stash); } } diff --git a/substrate/srml/staking/src/tests.rs b/substrate/srml/staking/src/tests.rs index 774a801687..5987d9800d 100644 --- a/substrate/srml/staking/src/tests.rs +++ b/substrate/srml/staking/src/tests.rs @@ -1714,59 +1714,46 @@ fn bond_with_no_staked_value() { // Particularly when she votes and the candidate is elected. with_externalities(&mut ExtBuilder::default() .validator_count(3) + .existential_deposit(5) .nominate(false) .minimum_validator_count(1) .build(), || { - // setup - assert_ok!(Staking::set_payee(Origin::signed(10), RewardDestination::Controller)); - let _ = Balances::deposit_creating(&3, 1000); - let initial_balance_2 = Balances::free_balance(&2); - let initial_balance_4 = Balances::free_balance(&4); + // Can't bond with 1 + assert_noop!( + Staking::bond(Origin::signed(1), 2, 1, RewardDestination::Controller), + "can not bond with value less than minimum balance" + ); + // bonded with absolute minimum value possible. + assert_ok!(Staking::bond(Origin::signed(1), 2, 5, RewardDestination::Controller)); + assert_eq!(Balances::locks(&1)[0].amount, 5); - // Stingy validator. - assert_ok!(Staking::bond(Origin::signed(1), 2, 1, RewardDestination::Controller)); - assert_ok!(Staking::validate(Origin::signed(2), ValidatorPrefs::default())); + // unbonding even 1 will cause all to be unbonded. + assert_ok!(Staking::unbond(Origin::signed(2), 1)); + assert_eq!( + Staking::ledger(2), + Some(StakingLedger { + stash: 1, + active: 0, + total: 5, + unlocking: vec![UnlockChunk {value: 5, era: 3}] + }) + ); start_era(1); - - assert_eq_uvec!(validator_controllers(), vec![30, 20, 10]); - - // min of 10, 20 and 30 (30 got a payout into staking so it raised it from 1 to 31). - assert_eq!(Staking::slot_stake(), 31); - - // make the stingy one elected. - assert_ok!(Staking::bond(Origin::signed(3), 4, 500, RewardDestination::Controller)); - assert_ok!(Staking::nominate(Origin::signed(4), vec![1])); - - // no rewards paid to 2 and 4 yet - assert_eq!(Balances::free_balance(&2), initial_balance_2); - assert_eq!(Balances::free_balance(&4), initial_balance_4); - start_era(2); - // Stingy one is selected - assert_eq_uvec!(validator_controllers(), vec![20, 10, 2]); - assert_eq!(Staking::stakers(1), Exposure { - own: 1, - total: 501, - others: vec![IndividualExposure { who: 3, value: 500}], - }); - // New slot stake. - assert_eq!(Staking::slot_stake(), 501); - - // no rewards paid to 2 and 4 yet - assert_eq!(Balances::free_balance(&2), initial_balance_2); - assert_eq!(Balances::free_balance(&4), initial_balance_4); + // not yet removed. + assert_ok!(Staking::withdraw_unbonded(Origin::signed(2))); + assert!(Staking::ledger(2).is_some()); + assert_eq!(Balances::locks(&1)[0].amount, 5); start_era(3); - // Approximation resulting from Perbill conversion - let approximation = 1; - let reward = Staking::current_session_reward() * 3; - // 2 will not get a reward of only 1 - // 4 will get the rest - assert_eq!(Balances::free_balance(&2), initial_balance_2 + 3 - approximation); - assert_eq!(Balances::free_balance(&4), initial_balance_4 + reward - 3 - approximation); + // poof. Account 1 is removed from the staking system. + assert_ok!(Staking::withdraw_unbonded(Origin::signed(2))); + assert!(Staking::ledger(2).is_none()); + assert_eq!(Balances::locks(&1).len(), 0); + }); }