From 87c8c2d22676844ab9304db56174ec8ebebd4e87 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 10 Oct 2022 12:41:20 +0200 Subject: [PATCH] ideas from Kian --- Cargo.lock | 3 - frame/fast-unstake/Cargo.toml | 8 --- frame/fast-unstake/src/benchmarking.rs | 1 - frame/fast-unstake/src/lib.rs | 45 ++++++-------- frame/staking/src/pallet/impls.rs | 78 ++++++++++++++++------- primitives/staking/src/lib.rs | 86 +++++++++++++++----------- 6 files changed, 122 insertions(+), 99 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f0a2df0f101b..58daa8d6553f5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5707,10 +5707,7 @@ dependencies = [ "frame-support", "frame-system", "log", - "pallet-balances", - "pallet-staking", "pallet-staking-reward-curve", - "pallet-timestamp", "parity-scale-codec", "scale-info", "sp-core", diff --git a/frame/fast-unstake/Cargo.toml b/frame/fast-unstake/Cargo.toml index 69aeaff35993c..19b79190ed4c1 100644 --- a/frame/fast-unstake/Cargo.toml +++ b/frame/fast-unstake/Cargo.toml @@ -24,10 +24,6 @@ sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/ sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } sp-staking = { default-features = false, path = "../../primitives/staking" } - -pallet-balances = { default-features = false, path = "../balances" } -pallet-timestamp = { default-features = false, path = "../timestamp" } -pallet-staking = { default-features = false, path = "../staking" } frame-election-provider-support = { default-features = false, path = "../election-provider-support" } frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" } @@ -53,9 +49,6 @@ std = [ "sp-runtime/std", "sp-std/std", - "pallet-staking/std", - "pallet-balances/std", - "pallet-timestamp/std", "frame-election-provider-support/std", "frame-benchmarking/std", @@ -63,6 +56,5 @@ std = [ runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", "frame-system/runtime-benchmarks", - "pallet-staking/runtime-benchmarks", ] try-runtime = ["frame-support/try-runtime"] diff --git a/frame/fast-unstake/src/benchmarking.rs b/frame/fast-unstake/src/benchmarking.rs index 8770cc6b64c0d..b9f43fc87ab45 100644 --- a/frame/fast-unstake/src/benchmarking.rs +++ b/frame/fast-unstake/src/benchmarking.rs @@ -26,7 +26,6 @@ use frame_support::{ traits::{Currency, EnsureOrigin, Get, Hooks}, }; use frame_system::RawOrigin; -use pallet_staking::Pallet as Staking; use sp_runtime::traits::{StaticLookup, Zero}; use sp_staking::EraIndex; use sp_std::prelude::*; diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 873008cb2fef5..c3df336d8e1f6 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -86,12 +86,11 @@ pub mod pallet { traits::{Defensive, ReservableCurrency}, }; use frame_system::{pallet_prelude::*, RawOrigin}; - use pallet_staking::Pallet as Staking; use sp_runtime::{ traits::{Saturating, Zero}, DispatchResult, }; - use sp_staking::EraIndex; + use sp_staking::{EraIndex, StakingInterface}; use sp_std::{prelude::*, vec::Vec}; pub use weights::WeightInfo; @@ -101,7 +100,7 @@ pub mod pallet { pub struct MaxChecking(sp_std::marker::PhantomData); impl frame_support::traits::Get for MaxChecking { fn get() -> u32 { - ::BondingDuration::get() + 1 + T::StakingInterface::bonding_duration() + 1 } } @@ -126,7 +125,10 @@ pub mod pallet { type ControlOrigin: frame_support::traits::EnsureOrigin; /// The access to staking functionality. - type Staking: StakingInterface, AccountId = Self::AccountId>; + type StakingInterface: StakingInterface< + Balance = BalanceOf, + AccountId = Self::AccountId, + >; /// The weight information of this pallet. type WeightInfo: WeightInfo; @@ -224,23 +226,19 @@ pub mod pallet { let ctrl = ensure_signed(origin)?; ensure!(ErasToCheckPerBlock::::get() != 0, >::CallNotAllowed); - - let ledger = - pallet_staking::Ledger::::get(&ctrl).ok_or(Error::::NotController)?; - ensure!(!Queue::::contains_key(&ledger.stash), Error::::AlreadyQueued); + let stash = T::StakingInterface::can_control(&ctrl)?; + ensure!(!Queue::::contains_key(&stash), Error::::AlreadyQueued); ensure!( Head::::get().map_or(true, |UnstakeRequest { stash, .. }| stash != ledger.stash), Error::::AlreadyHead ); + // second part of the && is defensive. - ensure!( - ledger.active == ledger.total && ledger.unlocking.is_empty(), - Error::::NotFullyBonded - ); + ensure!(!T::StakingInterface::is_unbonding(&who), Error::::NotFullyBonded); // chill and fully unstake. - Staking::::chill(RawOrigin::Signed(ctrl.clone()).into())?; - Staking::::unbond(RawOrigin::Signed(ctrl).into(), ledger.total)?; + T::StakingInterface::chill(&stash)?; + T::StakingInterface::fully_unbond(&stash)?; T::DepositCurrency::reserve(&ledger.stash, T::Deposit::get())?; @@ -262,9 +260,7 @@ pub mod pallet { ensure!(ErasToCheckPerBlock::::get() != 0, >::CallNotAllowed); - let stash = pallet_staking::Ledger::::get(&ctrl) - .map(|l| l.stash) - .ok_or(Error::::NotController)?; + let stash = T::StakingInterface::can_control(&ctrl)?; ensure!(Queue::::contains_key(&stash), Error::::NotQueued); ensure!( Head::::get().map_or(true, |UnstakeRequest { stash, .. }| stash != stash), @@ -317,7 +313,7 @@ pub mod pallet { // NOTE: here we're assuming that the number of validators has only ever increased, // meaning that the number of exposures to check is either this per era, or less. - let validator_count = pallet_staking::ValidatorCount::::get(); + let validator_count = T::StakingInterface::desired_validator_count(); // determine the number of eras to check. This is based on both `ErasToCheckPerBlock` // and `remaining_weight` passed on to us from the runtime executive. @@ -333,8 +329,7 @@ pub mod pallet { } } - if <::ElectionProvider as ElectionProviderBase>::ongoing() - { + if T::StakingInterface::election_ongoing() { // NOTE: we assume `ongoing` does not consume any weight. // there is an ongoing election -- we better not do anything. Imagine someone is not // exposed anywhere in the last era, and the snapshot for the election is already @@ -369,8 +364,8 @@ pub mod pallet { ); // the range that we're allowed to check in this round. - let current_era = pallet_staking::CurrentEra::::get().unwrap_or_default(); - let bonding_duration = ::BondingDuration::get(); + let current_era = T::StakingInterface::current_era(); + let bonding_duration = T::StakingInterface::bonding_duration(); // prune all the old eras that we don't care about. This will help us keep the bound // of `checked`. checked.retain(|e| *e >= current_era.saturating_sub(bonding_duration)); @@ -407,11 +402,7 @@ pub mod pallet { // `stash` is not exposed in any era now -- we can let go of them now. let num_slashing_spans = Staking::::slashing_spans(&stash).iter().count() as u32; - let result = pallet_staking::Pallet::::force_unstake( - RawOrigin::Root.into(), - stash.clone(), - num_slashing_spans, - ); + let result = T::StakingInterface::force_unstake(stash.clone()); let remaining = T::DepositCurrency::unreserve(&stash, deposit); if !remaining.is_zero() { diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 6da27da362b53..47961ee93171d 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -18,8 +18,8 @@ //! Implementations for the Staking FRAME Pallet. use frame_election_provider_support::{ - data_provider, ElectionDataProvider, ElectionProvider, ScoreProvider, SortedListProvider, - Supports, VoteWeight, VoterOf, + data_provider, ElectionDataProvider, ElectionProvider, ElectionProviderBase, ScoreProvider, + SortedListProvider, Supports, VoteWeight, VoterOf, }; use frame_support::{ dispatch::WithPostDispatchInfo, @@ -1482,14 +1482,39 @@ impl SortedListProvider for UseNominatorsAndValidatorsM } } +// NOTE: in this entire impl block, the assumption is that `who` is a stash account. impl StakingInterface for Pallet { type AccountId = T::AccountId; type Balance = BalanceOf; - fn minimum_bond() -> Self::Balance { + fn minimum_nominator_bond() -> Self::Balance { MinNominatorBond::::get() } + fn minimum_validator_bond() -> Self::Balance { + MinValidatorBond::::get() + } + + fn desired_validator_count() -> u32 { + ValidatorCount::::get() + } + + fn election_ongoing() -> bool { + ::ongoing() + } + + fn force_unstake(who: Self::AccountId) -> sp_runtime::DispatchResult { + todo!(); + } + + fn can_control(controller: &Self::AccountId) -> Result { + Self::ledger(controller).map(|l| l.stash).ok_or(Error::::NotController) + } + + fn is_exposed_in_era(who: &Self::AccountId, era: &EraIndex) -> bool { + todo!() + } + fn bonding_duration() -> EraIndex { T::BondingDuration::get() } @@ -1498,52 +1523,57 @@ impl StakingInterface for Pallet { Self::current_era().unwrap_or(Zero::zero()) } - fn active_stake(controller: &Self::AccountId) -> Option { - Self::ledger(controller).map(|l| l.active) + fn active_stake(who: &Self::AccountId) -> Option { + Self::bonded(who).and_then(|c| Self::ledger(c)).map(|l| l.active) } - fn total_stake(controller: &Self::AccountId) -> Option { - Self::ledger(controller).map(|l| l.total) + fn total_stake(who: &Self::AccountId) -> Option { + Self::bonded(who).and_then(|c| Self::ledger(c)).map(|l| l.active) } - fn bond_extra(stash: Self::AccountId, extra: Self::Balance) -> DispatchResult { - Self::bond_extra(RawOrigin::Signed(stash).into(), extra) + fn bond_extra(who: &Self::AccountId, extra: Self::Balance) -> DispatchResult { + Self::bond_extra(RawOrigin::Signed(who.clone()).into(), extra) } - fn unbond(controller: Self::AccountId, value: Self::Balance) -> DispatchResult { - Self::unbond(RawOrigin::Signed(controller).into(), value) + fn unbond(who: Self::AccountId, value: Self::Balance) -> DispatchResult { + let ctrl = Self::bonded(who).ok_or(Error::::NotStash)?; + Self::unbond(RawOrigin::Signed(ctrl).into(), value) } - fn chill(controller: Self::AccountId) -> DispatchResult { - Self::chill(RawOrigin::Signed(controller).into()) + fn chill(who: &Self::AccountId) -> DispatchResult { + // defensive-only: any account bonded via this interface has the stash set as the + // controller, but we have to be sure. Same comment anywhere else that we read this. + let ctrl = Self::bonded(who).ok_or(Error::::NotStash)?; + Self::chill(RawOrigin::Signed(ctrl).into()) } fn withdraw_unbonded( - controller: Self::AccountId, + who: Self::AccountId, num_slashing_spans: u32, ) -> Result { - Self::withdraw_unbonded(RawOrigin::Signed(controller.clone()).into(), num_slashing_spans) - .map(|_| !Ledger::::contains_key(&controller)) + let ctrl = Self::bonded(who).ok_or(Error::::NotStash)?; + Self::withdraw_unbonded(RawOrigin::Signed(ctrl.clone()).into(), num_slashing_spans) + .map(|_| !Ledger::::contains_key(&ctrl)) .map_err(|with_post| with_post.error) } fn bond( - stash: Self::AccountId, - controller: Self::AccountId, + who: &Self::AccountId, value: Self::Balance, - payee: Self::AccountId, + payee: &Self::AccountId, ) -> DispatchResult { Self::bond( - RawOrigin::Signed(stash).into(), - T::Lookup::unlookup(controller), + RawOrigin::Signed(who.clone()).into(), + T::Lookup::unlookup(who.clone()), value, - RewardDestination::Account(payee), + RewardDestination::Account(payee.clone()), ) } - fn nominate(controller: Self::AccountId, targets: Vec) -> DispatchResult { + fn nominate(who: &Self::AccountId, targets: Vec) -> DispatchResult { + let ctrl = Self::bonded(who).ok_or(Error::::NotStash)?; let targets = targets.into_iter().map(T::Lookup::unlookup).collect::>(); - Self::nominate(RawOrigin::Signed(controller).into(), targets) + Self::nominate(RawOrigin::Signed(ctrl).into(), targets) } #[cfg(feature = "runtime-benchmarks")] diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index e406f62d4d81a..4eb0cc31b34dc 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -54,7 +54,10 @@ impl OnStakerSlash for () { } } -/// Trait for communication with the staking pallet. +/// A generic representation of a staking implementation. +/// +/// This interface uses the terminology of NPoS, but it is aims to be generic enough to cover other +/// implementations as well. pub trait StakingInterface { /// Balance type used by the staking system. type Balance; @@ -62,17 +65,24 @@ pub trait StakingInterface { /// AccountId type used by the staking system type AccountId; - /// The minimum amount required to bond in order to be a nominator. This does not necessarily - /// mean the nomination will be counted in an election, but instead just enough to be stored as - /// a nominator. In other words, this is the minimum amount to register the intention to - /// nominate. - fn minimum_bond() -> Self::Balance; + /// The minimum amount required to bond in order to set nomination intentions. This does not + /// necessarily mean the nomination will be counted in an election, but instead just enough to + /// be stored as a nominator. In other words, this is the minimum amount to register the + /// intention to nominate. + fn minimum_nominator_bond() -> Self::Balance; - /// Number of eras that staked funds must remain bonded for. + /// The minimum amount required to bond in order to set validation intentions. + fn minimum_validator_bond() -> Self::Balance; + + /// Return an account that can be potentially controlled by `controller`. /// - /// # Note + /// ## Note /// - /// This must be strictly greater than the staking systems slash deffer duration. + /// The controller abstraction is not permanent and might go away. Avoid using this as much as + /// possible. + fn can_control(controller: &Self::AccountId) -> Result; + + /// Number of eras that staked funds must remain bonded for. fn bonding_duration() -> EraIndex; /// The current era index. @@ -80,10 +90,10 @@ pub trait StakingInterface { /// This should be the latest planned era that the staking system knows about. fn current_era() -> EraIndex; - /// The amount of active stake that `stash` has in the staking system. - fn active_stake(stash: &Self::AccountId) -> Option; + /// The amount of active stake `who` has in the staking system. + fn active_stake(who: &Self::AccountId) -> Option; - /// The total stake that `stash` has in the staking system. This includes the + /// The total stake that `who` has in the staking system. This includes the /// [`Self::active_stake`], and any funds currently in the process of unbonding via /// [`Self::unbond`]. /// @@ -91,30 +101,37 @@ pub trait StakingInterface { /// /// This is only guaranteed to reflect the amount locked by the staking system. If there are /// non-staking locks on the bonded pair's balance this may not be accurate. - fn total_stake(stash: &Self::AccountId) -> Option; + fn total_stake(who: &Self::AccountId) -> Option; - /// Bond (lock) `value` of `stash`'s balance. `controller` will be set as the account - /// controlling `stash`. This creates what is referred to as "bonded pair". - fn bond( - stash: Self::AccountId, - controller: Self::AccountId, - value: Self::Balance, - payee: Self::AccountId, - ) -> DispatchResult; + fn is_unbonding(who: &Self::AccountId) -> bool { + match (Self::active_stake(who), Self::total_stake(who)) { + (Some(x), Some(y)) if x == y => true, + _ => false + } + } + + fn fully_unbond(who: &Self::AccountId) -> DispatchResult { + // TODO: active_stake and others should also return `Result`. + Self::unbond(who, Self::active_stake(who).unwrap()) + } + + /// Bond (lock) `value` of `who`'s balance, while forwarding any rewards to `payee`. + fn bond(who: &Self::AccountId, value: Self::Balance, payee: &Self::AccountId) + -> DispatchResult; - /// Have `controller` nominate `validators`. + /// Have `who` nominate `validators`. fn nominate( - controller: Self::AccountId, + who: &Self::AccountId, validators: sp_std::vec::Vec, ) -> DispatchResult; - /// Chill `stash`. - fn chill(controller: Self::AccountId) -> DispatchResult; + /// Chill `who`. + fn chill(who: &Self::AccountId) -> DispatchResult; - /// Bond some extra amount in the _Stash_'s free balance against the active bonded balance of - /// the account. The amount extra actually bonded will never be more than the _Stash_'s free + /// Bond some extra amount in `who`'s free balance against the active bonded balance of + /// the account. The amount extra actually bonded will never be more than `who`'s free /// balance. - fn bond_extra(stash: Self::AccountId, extra: Self::Balance) -> DispatchResult; + fn bond_extra(who: &Self::AccountId, extra: Self::Balance) -> DispatchResult; /// Schedule a portion of the active bonded balance to be unlocked at era /// [Self::current_era] + [`Self::bonding_duration`]. @@ -125,7 +142,7 @@ pub trait StakingInterface { /// The amount of times this can be successfully called is limited based on how many distinct /// eras funds are schedule to unlock in. Calling [`Self::withdraw_unbonded`] after some unlock /// schedules have reached their unlocking era should allow more calls to this function. - fn unbond(stash: Self::AccountId, value: Self::Balance) -> DispatchResult; + fn unbond(stash: &Self::AccountId, value: Self::Balance) -> DispatchResult; /// Unlock any funds schedule to unlock before or at the current era. /// @@ -135,20 +152,17 @@ pub trait StakingInterface { num_slashing_spans: u32, ) -> Result; - /// Get `StakingLedger` by controller account. - fn get_ledger(ctrl: &Self::AccountId) -> StakingLedger; - /// The ideal number of active validators. - fn validator_count() -> u32; + fn desired_validator_count() -> u32; /// Whether or not there is an ongoing election. - fn ongoing() -> bool; + fn election_ongoing() -> bool; /// Force a current staker to become completely unstaked, immediately. - fn force_unstake(stash: T::AccountId) -> DispatchResult; + fn force_unstake(who: Self::AccountId) -> DispatchResult; /// Checks whether an account `staker` has been exposed in an era. - fn is_exposed_in_era(staker: &T::AccountId, era: &EraIndex) -> bool; + fn is_exposed_in_era(who: &Self::AccountId, era: &EraIndex) -> bool; /// Get the nominations of a stash, if they are a nominator, `None` otherwise. #[cfg(feature = "runtime-benchmarks")]