From 58be496f61f98b8e04fbf11a46e56bb9c2b3d9c7 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Thu, 27 Apr 2023 13:27:12 +0200 Subject: [PATCH] improve staking interface methods (#14023) * improve staking interface methods * fix * fix * fix build * restart * fix --------- Co-authored-by: parity-processbot <> --- Cargo.lock | 1 + .../nomination-pools/benchmarking/src/lib.rs | 4 +- frame/nomination-pools/src/mock.rs | 12 +++++- frame/staking/src/lib.rs | 13 +------ frame/staking/src/pallet/impls.rs | 30 ++++++++++++++- frame/staking/src/tests.rs | 23 +++++++++++ primitives/staking/Cargo.toml | 2 + primitives/staking/src/lib.rs | 38 ++++++++++++++++--- 8 files changed, 99 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8d8d2d59a3c88..c06bf41c24b4d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10862,6 +10862,7 @@ version = "4.0.0-dev" dependencies = [ "parity-scale-codec", "scale-info", + "serde", "sp-core", "sp-runtime", "sp-std", diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index d58bbaf3d117c..137b9e9af63e3 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -684,12 +684,12 @@ frame_benchmarking::benchmarks! { .collect(); assert_ok!(T::Staking::nominate(&pool_account, validators)); - assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_some()); + assert!(T::Staking::nominations(&Pools::::create_bonded_account(1)).is_some()); whitelist_account!(depositor); }:_(RuntimeOrigin::Signed(depositor.clone()), 1) verify { - assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_none()); + assert!(T::Staking::nominations(&Pools::::create_bonded_account(1)).is_none()); } set_commission { diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 3565ff14e6c82..3ab9be516fdb9 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -67,6 +67,14 @@ impl sp_staking::StakingInterface for StakingMock { BondingDuration::get() } + fn status( + _: &Self::AccountId, + ) -> Result, DispatchError> { + Nominations::get() + .map(|noms| sp_staking::StakerStatus::Nominator(noms)) + .ok_or(DispatchError::Other("NotStash")) + } + fn bond_extra(who: &Self::AccountId, extra: Self::Balance) -> DispatchResult { let mut x = BondedBalanceMap::get(); x.get_mut(who).map(|v| *v += extra); @@ -108,7 +116,7 @@ impl sp_staking::StakingInterface for StakingMock { } #[cfg(feature = "runtime-benchmarks")] - fn nominations(_: Self::AccountId) -> Option> { + fn nominations(_: &Self::AccountId) -> Option> { Nominations::get() } @@ -116,7 +124,7 @@ impl sp_staking::StakingInterface for StakingMock { unimplemented!("method currently not used in testing") } - fn stake(who: &Self::AccountId) -> Result, DispatchError> { + fn stake(who: &Self::AccountId) -> Result, DispatchError> { match ( UnbondingBalanceMap::get().get(who).copied(), BondedBalanceMap::get().get(who).copied(), diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index d6345c2161f73..28d970b9121e7 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -311,6 +311,7 @@ use sp_runtime::{ traits::{AtLeast32BitUnsigned, Convert, Saturating, StaticLookup, Zero}, Perbill, Perquintill, Rounding, RuntimeDebug, }; +pub use sp_staking::StakerStatus; use sp_staking::{ offence::{Offence, OffenceError, ReportOffence}, EraIndex, SessionIndex, @@ -381,18 +382,6 @@ impl Default for EraRewardPoints { } } -/// Indicates the initial status of the staker. -#[derive(RuntimeDebug, TypeInfo)] -#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize, Clone))] -pub enum StakerStatus { - /// Chilling. - Idle, - /// Declared desire in validating or already participating in it. - Validator, - /// Nominating for a group of other stakers. - Nominator(Vec), -} - /// A destination account for payment. #[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub enum RewardDestination { diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 3058d3edc6fd6..984871f7f9813 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -22,6 +22,7 @@ use frame_election_provider_support::{ SortedListProvider, VoteWeight, VoterOf, }; use frame_support::{ + defensive, dispatch::WithPostDispatchInfo, pallet_prelude::*, traits::{ @@ -1608,7 +1609,7 @@ impl StakingInterface for Pallet { Self::current_era().unwrap_or(Zero::zero()) } - fn stake(who: &Self::AccountId) -> Result, DispatchError> { + fn stake(who: &Self::AccountId) -> Result>, DispatchError> { Self::bonded(who) .and_then(|c| Self::ledger(c)) .map(|l| Stake { total: l.total, active: l.active }) @@ -1662,8 +1663,33 @@ impl StakingInterface for Pallet { Self::nominate(RawOrigin::Signed(ctrl).into(), targets) } + fn status( + who: &Self::AccountId, + ) -> Result, DispatchError> { + let is_bonded = Self::bonded(who).is_some(); + if !is_bonded { + return Err(Error::::NotStash.into()) + } + + let is_validator = Validators::::contains_key(&who); + let is_nominator = Nominators::::get(&who); + + use sp_staking::StakerStatus; + match (is_validator, is_nominator.is_some()) { + (false, false) => Ok(StakerStatus::Idle), + (true, false) => Ok(StakerStatus::Validator), + (false, true) => Ok(StakerStatus::Nominator( + is_nominator.expect("is checked above; qed").targets.into_inner(), + )), + (true, true) => { + defensive!("cannot be both validators and nominator"); + Err(Error::::BadState.into()) + }, + } + } + sp_staking::runtime_benchmarks_enabled! { - fn nominations(who: Self::AccountId) -> Option> { + fn nominations(who: &Self::AccountId) -> Option> { Nominators::::get(who).map(|n| n.targets.into_inner()) } diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index d97eb3ef89cab..affee60026500 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -5824,4 +5824,27 @@ mod staking_interface { )); }); } + + #[test] + fn status() { + ExtBuilder::default().build_and_execute(|| { + // stash of a validator is identified as a validator + assert_eq!(Staking::status(&11).unwrap(), StakerStatus::Validator); + // .. but not the controller. + assert!(Staking::status(&10).is_err()); + + // stash of nominator is identified as a nominator + assert_eq!(Staking::status(&101).unwrap(), StakerStatus::Nominator(vec![11, 21])); + // .. but not the controller. + assert!(Staking::status(&100).is_err()); + + // stash of chilled is identified as a chilled + assert_eq!(Staking::status(&41).unwrap(), StakerStatus::Idle); + // .. but not the controller. + assert!(Staking::status(&40).is_err()); + + // random other account. + assert!(Staking::status(&42).is_err()); + }) + } } diff --git a/primitives/staking/Cargo.toml b/primitives/staking/Cargo.toml index f92bca2788c41..f383a5e88759f 100644 --- a/primitives/staking/Cargo.toml +++ b/primitives/staking/Cargo.toml @@ -13,6 +13,7 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] +serde = { version = "1.0.136", optional = true } codec = { package = "parity-scale-codec", version = "3.2.2", default-features = false, features = ["derive"] } scale-info = { version = "2.5.0", default-features = false, features = ["derive"] } sp-core = { version = "7.0.0", default-features = false, path = "../core" } @@ -22,6 +23,7 @@ sp-std = { version = "5.0.0", default-features = false, path = "../std" } [features] default = ["std"] std = [ + "serde", "codec/std", "scale-info/std", "sp-core/std", diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index f328696551eed..57128bd327d9e 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -20,6 +20,8 @@ //! A crate which contains primitives that are useful for implementation that uses staking //! approaches in general. Definitions related to sessions, slashing, etc go here. +use scale_info::TypeInfo; +use sp_core::RuntimeDebug; use sp_runtime::{DispatchError, DispatchResult}; use sp_std::{collections::btree_map::BTreeMap, vec::Vec}; @@ -31,6 +33,18 @@ pub type SessionIndex = u32; /// Counter for the number of eras that have passed. pub type EraIndex = u32; +/// Representation of the status of a staker. +#[derive(RuntimeDebug, TypeInfo)] +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize, PartialEq, Eq, Clone))] +pub enum StakerStatus { + /// Chilling. + Idle, + /// Declaring desire in validate, i.e author blocks. + Validator, + /// Declaring desire to nominate, delegate, or generally approve of the given set of others. + Nominator(Vec), +} + /// Trait describing something that implements a hook for any operations to perform when a staker is /// slashed. pub trait OnStakerSlash { @@ -57,7 +71,7 @@ impl OnStakerSlash for () { /// A struct that reflects stake that an account has in the staking system. Provides a set of /// methods to operate on it's properties. Aimed at making `StakingInterface` more concise. -pub struct Stake { +pub struct Stake { /// The total stake that `stash` has in the staking system. This includes the /// `active` stake, and any funds currently in the process of unbonding via /// [`StakingInterface::unbond`]. @@ -67,10 +81,10 @@ pub struct Stake { /// 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 amount is going to be larger in /// reality. - pub total: T::Balance, + pub total: Balance, /// The total amount of the stash's balance that will be at stake in any forthcoming /// rounds. - pub active: T::Balance, + pub active: Balance, } /// A generic representation of a staking implementation. @@ -109,21 +123,25 @@ pub trait StakingInterface { /// This should be the latest planned era that the staking system knows about. fn current_era() -> EraIndex; - /// Returns the stake of `who`. - fn stake(who: &Self::AccountId) -> Result, DispatchError>; + /// Returns the [`Stake`] of `who`. + fn stake(who: &Self::AccountId) -> Result, DispatchError>; + /// Total stake of a staker, `Err` if not a staker. fn total_stake(who: &Self::AccountId) -> Result { Self::stake(who).map(|s| s.total) } + /// Total active portion of a staker's [`Stake`], `Err` if not a staker. fn active_stake(who: &Self::AccountId) -> Result { Self::stake(who).map(|s| s.active) } + /// Returns whether a staker is unbonding, `Err` if not a staker at all. fn is_unbonding(who: &Self::AccountId) -> Result { Self::stake(who).map(|s| s.active != s.total) } + /// Returns whether a staker is FULLY unbonding, `Err` if not a staker at all. fn fully_unbond(who: &Self::AccountId) -> DispatchResult { Self::unbond(who, Self::stake(who)?.active) } @@ -174,9 +192,17 @@ pub trait StakingInterface { /// Checks whether an account `staker` has been exposed in an era. fn is_exposed_in_era(who: &Self::AccountId, era: &EraIndex) -> bool; + /// Return the status of the given staker, `None` if not staked at all. + fn status(who: &Self::AccountId) -> Result, DispatchError>; + /// Get the nominations of a stash, if they are a nominator, `None` otherwise. #[cfg(feature = "runtime-benchmarks")] - fn nominations(who: Self::AccountId) -> Option>; + fn nominations(who: &Self::AccountId) -> Option> { + match Self::status(who) { + Ok(StakerStatus::Nominator(t)) => Some(t), + _ => None, + } + } #[cfg(feature = "runtime-benchmarks")] fn add_era_stakers(