From 6100fb14e64c4b0e0789b63cb06f5f301942ff25 Mon Sep 17 00:00:00 2001 From: Marcio Diaz <marcio@parity.io> Date: Thu, 9 Jan 2020 11:33:27 +0100 Subject: [PATCH] Introduce rebond (#4374) * Implement rebond: allowing to re-bond stake unbonded. --- substrate/bin/node/runtime/src/lib.rs | 4 +- substrate/frame/staking/src/lib.rs | 48 ++++++++- substrate/frame/staking/src/tests.rs | 140 ++++++++++++++++++++++++++ 3 files changed, 189 insertions(+), 3 deletions(-) diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 22d0e4cca31..39908cef12f 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -78,8 +78,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: 199, - impl_version: 199, + spec_version: 200, + impl_version: 200, apis: RUNTIME_API_VERSIONS, }; diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index d8ce3fa78dd..deb451f35c8 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -395,7 +395,7 @@ pub struct StakingLedger<AccountId, Balance: HasCompact> { impl< AccountId, - Balance: HasCompact + Copy + Saturating, + Balance: HasCompact + Copy + Saturating + SimpleArithmetic, > StakingLedger<AccountId, Balance> { /// Remove entries from `unlocking` that are sufficiently old and reduce the /// total by the sum of their balances. @@ -412,6 +412,30 @@ impl< Self { total, active: self.active, stash: self.stash, unlocking } } + /// Re-bond funds that were scheduled for unlocking. + fn rebond(mut self, value: Balance) -> Self { + let mut unlocking_balance: Balance = Zero::zero(); + + while let Some(last) = self.unlocking.last_mut() { + if unlocking_balance + last.value <= value { + unlocking_balance += last.value; + self.active += last.value; + self.unlocking.pop(); + } else { + let diff = value - unlocking_balance; + + unlocking_balance += diff; + self.active += diff; + last.value -= diff; + } + + if unlocking_balance >= value { + break + } + } + + self + } } impl<AccountId, Balance> StakingLedger<AccountId, Balance> where @@ -804,6 +828,8 @@ decl_error! { InsufficientValue, /// Can not schedule more unlock chunks. NoMoreChunks, + /// Can not rebond without unlocking chunks. + NoUnlockChunk, } } @@ -959,6 +985,26 @@ decl_module! { } } + /// Rebond a portion of the stash scheduled to be unlocked. + /// + /// # <weight> + /// - Time complexity: O(1). Bounded by `MAX_UNLOCKING_CHUNKS`. + /// - Storage changes: Can't increase storage, only decrease it. + /// # </weight> + #[weight = SimpleDispatchInfo::FixedNormal(500_000)] + fn rebond(origin, #[compact] value: BalanceOf<T>) { + let controller = ensure_signed(origin)?; + let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?; + ensure!( + ledger.unlocking.len() > 0, + Error::<T>::NoUnlockChunk, + ); + + let ledger = ledger.rebond(value); + + Self::update_ledger(&controller, &ledger); + } + /// Remove any unlocked chunks from the `unlocking` queue from our management. /// /// This essentially frees up that balance to be used by the stash account to do diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 1ab43910c7f..572a6667ef7 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -1196,6 +1196,146 @@ fn too_many_unbond_calls_should_not_work() { }) } +#[test] +fn rebond_works() { + // * Should test + // * Given an account being bonded [and chosen as a validator](not mandatory) + // * it can unbond a portion of its funds from the stash account. + // * it can re-bond a portion of the funds scheduled to unlock. + ExtBuilder::default() + .nominate(false) + .build() + .execute_with(|| { + // Set payee to controller. avoids confusion + assert_ok!(Staking::set_payee( + Origin::signed(10), + RewardDestination::Controller + )); + + // Give account 11 some large free balance greater than total + let _ = Balances::make_free_balance_be(&11, 1000000); + + // confirm that 10 is a normal validator and gets paid at the end of the era. + start_era(1); + + // Initial state of 10 + assert_eq!( + Staking::ledger(&10), + Some(StakingLedger { + stash: 11, + total: 1000, + active: 1000, + unlocking: vec![], + }) + ); + + start_era(2); + assert_eq!(Staking::current_era(), 2); + + // Try to rebond some funds. We get an error since no fund is unbonded. + assert_noop!( + Staking::rebond(Origin::signed(10), 500), + Error::<Test>::NoUnlockChunk, + ); + + // Unbond almost all of the funds in stash. + Staking::unbond(Origin::signed(10), 900).unwrap(); + assert_eq!( + Staking::ledger(&10), + Some(StakingLedger { + stash: 11, + total: 1000, + active: 100, + unlocking: vec![UnlockChunk { + value: 900, + era: 2 + 3 + },] + }) + ); + + // Re-bond all the funds unbonded. + Staking::rebond(Origin::signed(10), 900).unwrap(); + assert_eq!( + Staking::ledger(&10), + Some(StakingLedger { + stash: 11, + total: 1000, + active: 1000, + unlocking: vec![], + }) + ); + + // Unbond almost all of the funds in stash. + Staking::unbond(Origin::signed(10), 900).unwrap(); + assert_eq!( + Staking::ledger(&10), + Some(StakingLedger { + stash: 11, + total: 1000, + active: 100, + unlocking: vec![UnlockChunk { value: 900, era: 5 }], + }) + ); + + // Re-bond part of the funds unbonded. + Staking::rebond(Origin::signed(10), 500).unwrap(); + assert_eq!( + Staking::ledger(&10), + Some(StakingLedger { + stash: 11, + total: 1000, + active: 600, + unlocking: vec![UnlockChunk { value: 400, era: 5 }], + }) + ); + + // Re-bond the remainder of the funds unbonded. + Staking::rebond(Origin::signed(10), 500).unwrap(); + assert_eq!( + Staking::ledger(&10), + Some(StakingLedger { + stash: 11, + total: 1000, + active: 1000, + unlocking: vec![] + }) + ); + + // Unbond parts of the funds in stash. + Staking::unbond(Origin::signed(10), 300).unwrap(); + Staking::unbond(Origin::signed(10), 300).unwrap(); + Staking::unbond(Origin::signed(10), 300).unwrap(); + assert_eq!( + Staking::ledger(&10), + Some(StakingLedger { + stash: 11, + total: 1000, + active: 100, + unlocking: vec![ + UnlockChunk { value: 300, era: 5 }, + UnlockChunk { value: 300, era: 5 }, + UnlockChunk { value: 300, era: 5 }, + ] + }) + ); + + // Re-bond part of the funds unbonded. + Staking::rebond(Origin::signed(10), 500).unwrap(); + assert_eq!( + Staking::ledger(&10), + Some(StakingLedger { + stash: 11, + total: 1000, + active: 600, + unlocking: vec![ + UnlockChunk { value: 300, era: 5 }, + UnlockChunk { value: 100, era: 5 }, + ] + }) + ); + }) +} + #[test] fn slot_stake_is_least_staked_validator_and_exposure_defines_maximum_punishment() { // Test that slot_stake is determined by the least staked validator -- GitLab