From 007b320377ee26c2cdb44e998ecc06b296a9f4bc Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Sat, 29 Oct 2022 11:22:58 +0200 Subject: [PATCH] =?UTF-8?q?[Enhancement]=20Convert=20fast-unstake=20to=20u?= =?UTF-8?q?se=20StakingInterface,=20decouplin=E2=80=A6=20(#12424)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Enhancement] Convert fast-unstake to use StakingInterface, decoupling it from Staking * Update primitives/staking/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update primitives/staking/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * fix validator comment * remove todo * ideas from Kian (#12474) Co-authored-by: kianenigma * Rename StakingInterface -> Staking for nomination-pools * Staking fixes * StakingInterface changes * fix fast-unstake * fix nomination-pools * Fix fast-unstake tests * Fix benches for fast-unstake * fix is_unbonding * fix nomination pools * fix node code * add mock comments * fix imports * remove todo * more fixes * more fixes * bench fixes * more fixes * more fixes * import fix * more fixes * more bench fix * refix * refix * Update primitives/staking/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * is_unbonding returns a result * fix * review fixes * more review fixes * more fixes * more fixes * Update frame/fast-unstake/src/benchmarking.rs Co-authored-by: Squirrel * remove redundant CurrencyBalance from nom-pools * remove CB * rephrase * Apply suggestions from code review * Update frame/nomination-pools/src/tests.rs * finish damn renamed * clippy fix * fix Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: kianenigma Co-authored-by: parity-processbot <> Co-authored-by: Squirrel --- bin/node/runtime/src/lib.rs | 6 +- frame/fast-unstake/Cargo.toml | 13 +- frame/fast-unstake/src/benchmarking.rs | 40 ++--- frame/fast-unstake/src/lib.rs | 85 +++++------ frame/fast-unstake/src/mock.rs | 3 +- frame/fast-unstake/src/tests.rs | 14 +- frame/fast-unstake/src/types.rs | 7 +- .../nomination-pools/benchmarking/src/lib.rs | 54 ++++--- .../nomination-pools/benchmarking/src/mock.rs | 3 +- frame/nomination-pools/src/lib.rs | 79 ++++------ frame/nomination-pools/src/mock.rs | 93 ++++++++---- frame/nomination-pools/src/tests.rs | 29 ++-- .../nomination-pools/test-staking/src/mock.rs | 3 +- frame/staking/src/pallet/impls.rs | 107 +++++++++---- frame/staking/src/pallet/mod.rs | 2 +- primitives/staking/src/lib.rs | 141 ++++++++++++------ 16 files changed, 373 insertions(+), 306 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index a23fe64c90628..d49312bd6fe3f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -585,7 +585,8 @@ impl pallet_fast_unstake::Config for Runtime { type RuntimeEvent = RuntimeEvent; type ControlOrigin = frame_system::EnsureRoot; type Deposit = ConstU128<{ DOLLARS }>; - type DepositCurrency = Balances; + type Currency = Balances; + type Staking = Staking; type WeightInfo = (); } @@ -773,11 +774,10 @@ impl pallet_nomination_pools::Config for Runtime { type WeightInfo = (); type RuntimeEvent = RuntimeEvent; type Currency = Balances; - type CurrencyBalance = Balance; type RewardCounter = FixedU128; type BalanceToU256 = BalanceToU256; type U256ToBalance = U256ToBalance; - type StakingInterface = pallet_staking::Pallet; + type Staking = Staking; type PostUnbondingPoolsWindow = PostUnbondPoolsWindow; type MaxMetadataLen = ConstU32<256>; type MaxUnbonding = ConstU32<8>; diff --git a/frame/fast-unstake/Cargo.toml b/frame/fast-unstake/Cargo.toml index 69aeaff35993c..ea1abeb0b48c5 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" } @@ -37,6 +33,10 @@ pallet-staking-reward-curve = { version = "4.0.0-dev", path = "../staking/reward sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" } substrate-test-utils = { version = "4.0.0-dev", path = "../../test-utils" } sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } +pallet-staking = { path = "../staking" } +pallet-balances = { path = "../balances" } +pallet-timestamp = { path = "../timestamp" } + [features] default = ["std"] @@ -53,9 +53,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 +60,6 @@ std = [ runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", "frame-system/runtime-benchmarks", - "pallet-staking/runtime-benchmarks", + "sp-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..762a59c96bcfa 100644 --- a/frame/fast-unstake/src/benchmarking.rs +++ b/frame/fast-unstake/src/benchmarking.rs @@ -26,22 +26,15 @@ 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_runtime::traits::Zero; +use sp_staking::{EraIndex, StakingInterface}; use sp_std::prelude::*; const USER_SEED: u32 = 0; const DEFAULT_BACKER_PER_VALIDATOR: u32 = 128; const MAX_VALIDATORS: u32 = 128; -type CurrencyOf = ::Currency; - -fn l( - who: T::AccountId, -) -> <::Lookup as StaticLookup>::Source { - T::Lookup::unlookup(who) -} +type CurrencyOf = ::Currency; fn create_unexposed_nominator() -> T::AccountId { let account = frame_benchmarking::account::("nominator_42", 0, USER_SEED); @@ -53,18 +46,9 @@ fn fund_and_bond_account(account: &T::AccountId) { let stake = CurrencyOf::::minimum_balance() * 100u32.into(); CurrencyOf::::make_free_balance_be(&account, stake * 10u32.into()); - let account_lookup = l::(account.clone()); // bond and nominate ourselves, this will guarantee that we are not backing anyone. - assert_ok!(Staking::::bond( - RawOrigin::Signed(account.clone()).into(), - account_lookup.clone(), - stake, - pallet_staking::RewardDestination::Controller, - )); - assert_ok!(Staking::::nominate( - RawOrigin::Signed(account.clone()).into(), - vec![account_lookup] - )); + assert_ok!(T::Staking::bond(account, stake, account)); + assert_ok!(T::Staking::nominate(account, vec![account.clone()])); } pub(crate) fn fast_unstake_events() -> Vec> { @@ -91,13 +75,11 @@ fn setup_staking(v: u32, until: EraIndex) { .map(|s| { let who = frame_benchmarking::account::("nominator", era, s); let value = ed; - pallet_staking::IndividualExposure { who, value } + (who, value) }) .collect::>(); - let exposure = - pallet_staking::Exposure { total: Default::default(), own: Default::default(), others }; validators.iter().for_each(|v| { - Staking::::add_era_stakers(era, v.clone(), exposure.clone()); + T::Staking::add_era_stakers(&era, &v, others.clone()); }); } } @@ -137,13 +119,13 @@ benchmarks! { // on_idle, when we check some number of eras, on_idle_check { // number of eras multiplied by validators in that era. - let x in (::BondingDuration::get() * 1) .. (::BondingDuration::get() * MAX_VALIDATORS); + let x in (T::Staking::bonding_duration() * 1) .. (T::Staking::bonding_duration() * MAX_VALIDATORS); - let v = x / ::BondingDuration::get(); - let u = ::BondingDuration::get(); + let u = T::Staking::bonding_duration(); + let v = x / u; ErasToCheckPerBlock::::put(u); - pallet_staking::CurrentEra::::put(u); + T::Staking::set_current_era(u); // setup staking with v validators and u eras of data (0..=u) setup_staking::(v, u); diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 8fdb7a79dd537..0477bb251aa00 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -80,18 +80,16 @@ macro_rules! log { pub mod pallet { use super::*; use crate::types::*; - use frame_election_provider_support::ElectionProviderBase; use frame_support::{ pallet_prelude::*, traits::{Defensive, ReservableCurrency}, }; - use frame_system::{pallet_prelude::*, RawOrigin}; - use pallet_staking::Pallet as Staking; + use frame_system::pallet_prelude::*; 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 +99,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::Staking::bonding_duration() + 1 } } @@ -109,14 +107,14 @@ pub mod pallet { pub struct Pallet(_); #[pallet::config] - pub trait Config: frame_system::Config + pallet_staking::Config { + pub trait Config: frame_system::Config { /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent> + TryInto>; /// The currency used for deposits. - type DepositCurrency: ReservableCurrency>; + type Currency: ReservableCurrency; /// Deposit to take for unstaking, to make sure we're able to slash the it in order to cover /// the costs of resources on unsuccessful unstake. @@ -125,6 +123,9 @@ pub mod pallet { /// The origin that can control this pallet. type ControlOrigin: frame_support::traits::EnsureOrigin; + /// The access to staking functionality. + type Staking: StakingInterface, AccountId = Self::AccountId>; + /// The weight information of this pallet. type WeightInfo: WeightInfo; } @@ -221,28 +222,25 @@ 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_account = + T::Staking::stash_by_ctrl(&ctrl).map_err(|_| Error::::NotController)?; + ensure!(!Queue::::contains_key(&stash_account), Error::::AlreadyQueued); ensure!( - Head::::get().map_or(true, |UnstakeRequest { stash, .. }| stash != ledger.stash), + Head::::get() + .map_or(true, |UnstakeRequest { stash, .. }| stash_account != stash), Error::::AlreadyHead ); - // second part of the && is defensive. - ensure!( - ledger.active == ledger.total && ledger.unlocking.is_empty(), - Error::::NotFullyBonded - ); + + ensure!(!T::Staking::is_unbonding(&stash_account)?, Error::::NotFullyBonded); // chill and fully unstake. - Staking::::chill(RawOrigin::Signed(ctrl.clone()).into())?; - Staking::::unbond(RawOrigin::Signed(ctrl).into(), ledger.total)?; + T::Staking::chill(&stash_account)?; + T::Staking::fully_unbond(&stash_account)?; - T::DepositCurrency::reserve(&ledger.stash, T::Deposit::get())?; + T::Currency::reserve(&stash_account, T::Deposit::get())?; // enqueue them. - Queue::::insert(ledger.stash, T::Deposit::get()); + Queue::::insert(stash_account, T::Deposit::get()); Ok(()) } @@ -259,18 +257,18 @@ pub mod pallet { ensure!(ErasToCheckPerBlock::::get() != 0, >::CallNotAllowed); - let stash = pallet_staking::Ledger::::get(&ctrl) - .map(|l| l.stash) - .ok_or(Error::::NotController)?; - ensure!(Queue::::contains_key(&stash), Error::::NotQueued); + let stash_account = + T::Staking::stash_by_ctrl(&ctrl).map_err(|_| Error::::NotController)?; + ensure!(Queue::::contains_key(&stash_account), Error::::NotQueued); ensure!( - Head::::get().map_or(true, |UnstakeRequest { stash, .. }| stash != stash), + Head::::get() + .map_or(true, |UnstakeRequest { stash, .. }| stash_account != stash), Error::::AlreadyHead ); - let deposit = Queue::::take(stash.clone()); + let deposit = Queue::::take(stash_account.clone()); if let Some(deposit) = deposit.defensive() { - let remaining = T::DepositCurrency::unreserve(&stash, deposit); + let remaining = T::Currency::unreserve(&stash_account, deposit); if !remaining.is_zero() { frame_support::defensive!("`not enough balance to unreserve`"); ErasToCheckPerBlock::::put(0); @@ -314,7 +312,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::Staking::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. @@ -330,8 +328,7 @@ pub mod pallet { } } - if <::ElectionProvider as ElectionProviderBase>::ongoing() - { + if T::Staking::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 @@ -366,8 +363,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::Staking::current_era(); + let bonding_duration = T::Staking::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)); @@ -401,16 +398,9 @@ pub mod pallet { ); if unchecked_eras_to_check.is_empty() { - // `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 = T::Staking::force_unstake(stash.clone()); - let result = pallet_staking::Pallet::::force_unstake( - RawOrigin::Root.into(), - stash.clone(), - num_slashing_spans, - ); - - let remaining = T::DepositCurrency::unreserve(&stash, deposit); + let remaining = T::Currency::unreserve(&stash, deposit); if !remaining.is_zero() { frame_support::defensive!("`not enough balance to unreserve`"); ErasToCheckPerBlock::::put(0); @@ -426,7 +416,7 @@ pub mod pallet { let mut eras_checked = 0u32; let is_exposed = unchecked_eras_to_check.iter().any(|e| { eras_checked.saturating_inc(); - Self::is_exposed_in_era(&stash, e) + T::Staking::is_exposed_in_era(&stash, e) }); log!( @@ -442,7 +432,7 @@ pub mod pallet { // the last 28 eras, have registered yourself to be unstaked, midway being checked, // you are exposed. if is_exposed { - T::DepositCurrency::slash_reserved(&stash, deposit); + T::Currency::slash_reserved(&stash, deposit); log!(info, "slashed {:?} by {:?}", stash, deposit); Self::deposit_event(Event::::Slashed { stash, amount: deposit }); } else { @@ -472,12 +462,5 @@ pub mod pallet { ::WeightInfo::on_idle_check(validator_count * eras_checked) } } - - /// Checks whether an account `staker` has been exposed in an era. - fn is_exposed_in_era(staker: &T::AccountId, era: &EraIndex) -> bool { - pallet_staking::ErasStakers::::iter_prefix(era).any(|(validator, exposures)| { - validator == *staker || exposures.others.iter().any(|i| i.who == *staker) - }) - } } } diff --git a/frame/fast-unstake/src/mock.rs b/frame/fast-unstake/src/mock.rs index 71fc2d4ba905a..87e89d90d6304 100644 --- a/frame/fast-unstake/src/mock.rs +++ b/frame/fast-unstake/src/mock.rs @@ -174,7 +174,8 @@ parameter_types! { impl fast_unstake::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Deposit = DepositAmount; - type DepositCurrency = Balances; + type Currency = Balances; + type Staking = Staking; type ControlOrigin = frame_system::EnsureRoot; type WeightInfo = (); } diff --git a/frame/fast-unstake/src/tests.rs b/frame/fast-unstake/src/tests.rs index 6e617fd992028..18a9d59e9da66 100644 --- a/frame/fast-unstake/src/tests.rs +++ b/frame/fast-unstake/src/tests.rs @@ -48,7 +48,7 @@ fn register_insufficient_funds_fails() { use pallet_balances::Error as BalancesError; ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); - ::DepositCurrency::make_free_balance_be(&1, 3); + ::Currency::make_free_balance_be(&1, 3); // Controller account registers for fast unstake. assert_noop!( @@ -138,15 +138,15 @@ fn deregister_works() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); - assert_eq!(::DepositCurrency::reserved_balance(&1), 0); + assert_eq!(::Currency::reserved_balance(&1), 0); // Controller account registers for fast unstake. assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); - assert_eq!(::DepositCurrency::reserved_balance(&1), DepositAmount::get()); + assert_eq!(::Currency::reserved_balance(&1), DepositAmount::get()); // Controller then changes mind and deregisters. assert_ok!(FastUnstake::deregister(RuntimeOrigin::signed(2))); - assert_eq!(::DepositCurrency::reserved_balance(&1), 0); + assert_eq!(::Currency::reserved_balance(&1), 0); // Ensure stash no longer exists in the queue. assert_eq!(Queue::::get(1), None); @@ -363,7 +363,7 @@ mod on_idle { CurrentEra::::put(BondingDuration::get()); // given - assert_eq!(::DepositCurrency::reserved_balance(&1), 0); + assert_eq!(::Currency::reserved_balance(&1), 0); assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(4))); @@ -371,7 +371,7 @@ mod on_idle { assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(8))); assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(10))); - assert_eq!(::DepositCurrency::reserved_balance(&1), DepositAmount::get()); + assert_eq!(::Currency::reserved_balance(&1), DepositAmount::get()); assert_eq!(Queue::::count(), 5); assert_eq!(Head::::get(), None); @@ -411,7 +411,7 @@ mod on_idle { ); assert_eq!(Queue::::count(), 3); - assert_eq!(::DepositCurrency::reserved_balance(&1), 0); + assert_eq!(::Currency::reserved_balance(&1), 0); assert_eq!( fast_unstake_events_since_last_call(), diff --git a/frame/fast-unstake/src/types.rs b/frame/fast-unstake/src/types.rs index 08b9ab4326eb2..460c9fa4354e5 100644 --- a/frame/fast-unstake/src/types.rs +++ b/frame/fast-unstake/src/types.rs @@ -17,6 +17,7 @@ //! Types used in the Fast Unstake pallet. +use crate::Config; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ traits::{Currency, Get}, @@ -26,10 +27,8 @@ use scale_info::TypeInfo; use sp_staking::EraIndex; use sp_std::{fmt::Debug, prelude::*}; -pub type BalanceOf = <::Currency as Currency< - ::AccountId, ->>::Balance; - +pub type BalanceOf = + <::Currency as Currency<::AccountId>>::Balance; /// An unstake request. #[derive( Encode, Decode, EqNoBound, PartialEqNoBound, Clone, TypeInfo, RuntimeDebugNoBound, MaxEncodedLen, diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index c31bcb1546ecd..9b063539152b7 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -133,15 +133,15 @@ impl ListScenario { // Create accounts with the origin weight let (pool_creator1, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); - T::StakingInterface::nominate( - pool_origin1.clone(), + T::Staking::nominate( + &pool_origin1, // NOTE: these don't really need to be validators. vec![account("random_validator", 0, USER_SEED)], )?; let (_, pool_origin2) = create_pool_account::(USER_SEED + 2, origin_weight); - T::StakingInterface::nominate( - pool_origin2.clone(), + T::Staking::nominate( + &pool_origin2, vec![account("random_validator", 0, USER_SEED)].clone(), )?; @@ -156,10 +156,7 @@ impl ListScenario { // Create an account with the worst case destination weight let (_, pool_dest1) = create_pool_account::(USER_SEED + 3, dest_weight); - T::StakingInterface::nominate( - pool_dest1.clone(), - vec![account("random_validator", 0, USER_SEED)], - )?; + T::Staking::nominate(&pool_dest1, vec![account("random_validator", 0, USER_SEED)])?; let weight_of = pallet_staking::Pallet::::weight_of_fn(); assert_eq!(vote_to_balance::(weight_of(&pool_origin1)).unwrap(), origin_weight); @@ -185,12 +182,11 @@ impl ListScenario { self.origin1_member = Some(joiner.clone()); CurrencyOf::::make_free_balance_be(&joiner, amount * 2u32.into()); - let original_bonded = T::StakingInterface::active_stake(&self.origin1).unwrap(); + let original_bonded = T::Staking::active_stake(&self.origin1).unwrap(); // Unbond `amount` from the underlying pool account so when the member joins // we will maintain `current_bonded`. - T::StakingInterface::unbond(self.origin1.clone(), amount) - .expect("the pool was created in `Self::new`."); + T::Staking::unbond(&self.origin1, amount).expect("the pool was created in `Self::new`."); // Account pool points for the unbonded balance. BondedPools::::mutate(&1, |maybe_pool| { @@ -219,7 +215,7 @@ frame_benchmarking::benchmarks! { // setup the worst case list scenario. let scenario = ListScenario::::new(origin_weight, true)?; assert_eq!( - T::StakingInterface::active_stake(&scenario.origin1).unwrap(), + T::Staking::active_stake(&scenario.origin1).unwrap(), origin_weight ); @@ -234,7 +230,7 @@ frame_benchmarking::benchmarks! { verify { assert_eq!(CurrencyOf::::free_balance(&joiner), joiner_free - max_additional); assert_eq!( - T::StakingInterface::active_stake(&scenario.origin1).unwrap(), + T::Staking::active_stake(&scenario.origin1).unwrap(), scenario.dest_weight ); } @@ -249,7 +245,7 @@ frame_benchmarking::benchmarks! { }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::FreeBalance(extra)) verify { assert!( - T::StakingInterface::active_stake(&scenario.origin1).unwrap() >= + T::Staking::active_stake(&scenario.origin1).unwrap() >= scenario.dest_weight ); } @@ -267,7 +263,7 @@ frame_benchmarking::benchmarks! { }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::Rewards) verify { assert!( - T::StakingInterface::active_stake(&scenario.origin1).unwrap() >= + T::Staking::active_stake(&scenario.origin1).unwrap() >= scenario.dest_weight ); } @@ -314,7 +310,7 @@ frame_benchmarking::benchmarks! { whitelist_account!(member_id); }: _(RuntimeOrigin::Signed(member_id.clone()), member_id_lookup, all_points) verify { - let bonded_after = T::StakingInterface::active_stake(&scenario.origin1).unwrap(); + let bonded_after = T::Staking::active_stake(&scenario.origin1).unwrap(); // We at least went down to the destination bag assert!(bonded_after <= scenario.dest_weight); let member = PoolMembers::::get( @@ -323,7 +319,7 @@ frame_benchmarking::benchmarks! { .unwrap(); assert_eq!( member.unbonding_eras.keys().cloned().collect::>(), - vec![0 + T::StakingInterface::bonding_duration()] + vec![0 + T::Staking::bonding_duration()] ); assert_eq!( member.unbonding_eras.values().cloned().collect::>(), @@ -345,7 +341,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::StakingInterface::active_stake(&pool_account).unwrap(), + T::Staking::active_stake(&pool_account).unwrap(), min_create_bond + min_join_bond ); assert_eq!(CurrencyOf::::free_balance(&joiner), min_join_bond); @@ -355,7 +351,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::StakingInterface::active_stake(&pool_account).unwrap(), + T::Staking::active_stake(&pool_account).unwrap(), min_create_bond ); assert_eq!(pallet_staking::Ledger::::get(&pool_account).unwrap().unlocking.len(), 1); @@ -388,7 +384,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::StakingInterface::active_stake(&pool_account).unwrap(), + T::Staking::active_stake(&pool_account).unwrap(), min_create_bond + min_join_bond ); assert_eq!(CurrencyOf::::free_balance(&joiner), min_join_bond); @@ -399,7 +395,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::StakingInterface::active_stake(&pool_account).unwrap(), + T::Staking::active_stake(&pool_account).unwrap(), min_create_bond ); assert_eq!(pallet_staking::Ledger::::get(&pool_account).unwrap().unlocking.len(), 1); @@ -446,7 +442,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::StakingInterface::active_stake(&pool_account).unwrap(), + T::Staking::active_stake(&pool_account).unwrap(), Zero::zero() ); assert_eq!( @@ -525,8 +521,8 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::StakingInterface::active_stake(&Pools::::create_bonded_account(1)), - Some(min_create_bond) + T::Staking::active_stake(&Pools::::create_bonded_account(1)), + Ok(min_create_bond) ); } @@ -564,8 +560,8 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::StakingInterface::active_stake(&Pools::::create_bonded_account(1)), - Some(min_create_bond) + T::Staking::active_stake(&Pools::::create_bonded_account(1)), + Ok(min_create_bond) ); } @@ -647,13 +643,13 @@ frame_benchmarking::benchmarks! { .map(|i| account("stash", USER_SEED, i)) .collect(); - assert_ok!(Pools::::nominate(RuntimeOrigin::Signed(depositor.clone()).into(), 1, validators)); - assert!(T::StakingInterface::nominations(Pools::::create_bonded_account(1)).is_some()); + assert_ok!(T::Staking::nominate(&pool_account, validators)); + assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_some()); whitelist_account!(depositor); }:_(RuntimeOrigin::Signed(depositor.clone()), 1) verify { - assert!(T::StakingInterface::nominations(Pools::::create_bonded_account(1)).is_none()); + assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_none()); } impl_benchmark_test_suite!( diff --git a/frame/nomination-pools/benchmarking/src/mock.rs b/frame/nomination-pools/benchmarking/src/mock.rs index 0f617f0bf6f4b..db01989f2b563 100644 --- a/frame/nomination-pools/benchmarking/src/mock.rs +++ b/frame/nomination-pools/benchmarking/src/mock.rs @@ -157,11 +157,10 @@ impl pallet_nomination_pools::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = (); type Currency = Balances; - type CurrencyBalance = Balance; type RewardCounter = FixedU128; type BalanceToU256 = BalanceToU256; type U256ToBalance = U256ToBalance; - type StakingInterface = Staking; + type Staking = Staking; type PostUnbondingPoolsWindow = PostUnbondingPoolsWindow; type MaxMetadataLen = ConstU32<256>; type MaxUnbonding = ConstU32<8>; diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 9e77adaeee677..5173b2e7e7d21 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -286,7 +286,7 @@ use sp_runtime::{ AccountIdConversion, Bounded, CheckedAdd, CheckedSub, Convert, Saturating, StaticLookup, Zero, }, - FixedPointNumber, FixedPointOperand, + FixedPointNumber, }; use sp_staking::{EraIndex, OnStakerSlash, StakingInterface}; use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, ops::Div, vec::Vec}; @@ -417,7 +417,7 @@ impl PoolMember { /// /// This is derived from the ratio of points in the pool to which the member belongs to. /// Might return different values based on the pool state for the same member and points. - fn active_balance(&self) -> BalanceOf { + fn active_stake(&self) -> BalanceOf { if let Some(pool) = BondedPool::::get(self.pool_id).defensive() { pool.points_to_balance(self.points) } else { @@ -623,7 +623,7 @@ impl BondedPool { /// This is often used for bonding and issuing new funds into the pool. fn balance_to_point(&self, new_funds: BalanceOf) -> BalanceOf { let bonded_balance = - T::StakingInterface::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); + T::Staking::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); Pallet::::balance_to_point(bonded_balance, self.points, new_funds) } @@ -632,7 +632,7 @@ impl BondedPool { /// This is often used for unbonding. fn points_to_balance(&self, points: BalanceOf) -> BalanceOf { let bonded_balance = - T::StakingInterface::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); + T::Staking::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); Pallet::::point_to_balance(bonded_balance, self.points, points) } @@ -683,7 +683,7 @@ impl BondedPool { fn transferrable_balance(&self) -> BalanceOf { let account = self.bonded_account(); T::Currency::free_balance(&account) - .saturating_sub(T::StakingInterface::active_stake(&account).unwrap_or_default()) + .saturating_sub(T::Staking::active_stake(&account).unwrap_or_default()) } fn is_root(&self, who: &T::AccountId) -> bool { @@ -738,7 +738,7 @@ impl BondedPool { ensure!(!self.is_destroying(), Error::::CanNotChangeState); let bonded_balance = - T::StakingInterface::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); + T::Staking::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); ensure!(!bonded_balance.is_zero(), Error::::OverflowRisk); let points_to_balance_ratio_floor = self @@ -791,7 +791,7 @@ impl BondedPool { target_member.active_points().saturating_sub(unbonding_points); let mut target_member_after_unbond = (*target_member).clone(); target_member_after_unbond.points = new_depositor_points; - target_member_after_unbond.active_balance() + target_member_after_unbond.active_stake() }; // any partial unbonding is only ever allowed if this unbond is permissioned. @@ -862,8 +862,8 @@ impl BondedPool { /// Bond exactly `amount` from `who`'s funds into this pool. /// - /// If the bond type is `Create`, `StakingInterface::bond` is called, and `who` - /// is allowed to be killed. Otherwise, `StakingInterface::bond_extra` is called and `who` + /// If the bond type is `Create`, `Staking::bond` is called, and `who` + /// is allowed to be killed. Otherwise, `Staking::bond_extra` is called and `who` /// cannot be killed. /// /// Returns `Ok(points_issues)`, `Err` otherwise. @@ -889,16 +889,11 @@ impl BondedPool { let points_issued = self.issue(amount); match ty { - BondType::Create => T::StakingInterface::bond( - bonded_account.clone(), - bonded_account, - amount, - self.reward_account(), - )?, + BondType::Create => T::Staking::bond(&bonded_account, amount, &self.reward_account())?, // The pool should always be created in such a way its in a state to bond extra, but if // the active balance is slashed below the minimum bonded or the account cannot be // found, we exit early. - BondType::Later => T::StakingInterface::bond_extra(bonded_account, amount)?, + BondType::Later => T::Staking::bond_extra(&bonded_account, amount)?, } Ok(points_issued) @@ -1129,7 +1124,7 @@ impl Get for TotalUnbondingPools { // NOTE: this may be dangerous in the scenario bonding_duration gets decreased because // we would no longer be able to decode `UnbondingPoolsWithEra`, which uses // `TotalUnbondingPools` as the bound - T::StakingInterface::bonding_duration() + T::PostUnbondingPoolsWindow::get() + T::Staking::bonding_duration() + T::PostUnbondingPoolsWindow::get() } } @@ -1138,7 +1133,6 @@ pub mod pallet { use super::*; use frame_support::traits::StorageVersion; use frame_system::{ensure_signed, pallet_prelude::*}; - use sp_runtime::traits::CheckedAdd; /// The current storage version. const STORAGE_VERSION: StorageVersion = StorageVersion::new(3); @@ -1157,20 +1151,7 @@ pub mod pallet { type WeightInfo: weights::WeightInfo; /// The nominating balance. - type Currency: Currency; - - /// Sadly needed to bound it to `FixedPointOperand`. - // The only alternative is to sprinkle a `where BalanceOf: FixedPointOperand` in roughly - // a million places, so we prefer doing this. - type CurrencyBalance: sp_runtime::traits::AtLeast32BitUnsigned - + codec::FullCodec - + MaybeSerializeDeserialize - + sp_std::fmt::Debug - + Default - + FixedPointOperand - + CheckedAdd - + TypeInfo - + MaxEncodedLen; + type Currency: Currency; /// The type that is used for reward counter. /// @@ -1212,10 +1193,7 @@ pub mod pallet { type U256ToBalance: Convert>; /// The interface for nominating. - type StakingInterface: StakingInterface< - Balance = BalanceOf, - AccountId = Self::AccountId, - >; + type Staking: StakingInterface, AccountId = Self::AccountId>; /// The amount of eras a `SubPools::with_era` pool can exist before it gets merged into the /// `SubPools::no_era` pool. In other words, this is the amount of eras a member will be @@ -1650,12 +1628,12 @@ pub mod pallet { let _ = reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; let _ = Self::do_reward_payout(&who, &mut member, &mut bonded_pool, &mut reward_pool)?; - let current_era = T::StakingInterface::current_era(); - let unbond_era = T::StakingInterface::bonding_duration().saturating_add(current_era); + let current_era = T::Staking::current_era(); + let unbond_era = T::Staking::bonding_duration().saturating_add(current_era); // Unbond in the actual underlying nominator. let unbonding_balance = bonded_pool.dissolve(unbonding_points); - T::StakingInterface::unbond(bonded_pool.bonded_account(), unbonding_balance)?; + T::Staking::unbond(&bonded_pool.bonded_account(), unbonding_balance)?; // Note that we lazily create the unbonding pools here if they don't already exist let mut sub_pools = SubPoolsStorage::::get(member.pool_id) @@ -1718,7 +1696,7 @@ pub mod pallet { // For now we only allow a pool to withdraw unbonded if its not destroying. If the pool // is destroying then `withdraw_unbonded` can be used. ensure!(pool.state != PoolState::Destroying, Error::::NotDestroying); - T::StakingInterface::withdraw_unbonded(pool.bonded_account(), num_slashing_spans)?; + T::Staking::withdraw_unbonded(pool.bonded_account(), num_slashing_spans)?; Ok(()) } @@ -1754,7 +1732,7 @@ pub mod pallet { let member_account = T::Lookup::lookup(member_account)?; let mut member = PoolMembers::::get(&member_account).ok_or(Error::::PoolMemberNotFound)?; - let current_era = T::StakingInterface::current_era(); + let current_era = T::Staking::current_era(); let bonded_pool = BondedPool::::get(member.pool_id) .defensive_ok_or::>(DefensiveError::PoolNotFound.into())?; @@ -1769,10 +1747,8 @@ pub mod pallet { // Before calculate the `balance_to_unbond`, with call withdraw unbonded to ensure the // `transferrable_balance` is correct. - let stash_killed = T::StakingInterface::withdraw_unbonded( - bonded_pool.bonded_account(), - num_slashing_spans, - )?; + let stash_killed = + T::Staking::withdraw_unbonded(bonded_pool.bonded_account(), num_slashing_spans)?; // defensive-only: the depositor puts enough funds into the stash so that it will only // be destroyed when they are leaving. @@ -1960,7 +1936,7 @@ pub mod pallet { let who = ensure_signed(origin)?; let bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; ensure!(bonded_pool.can_nominate(&who), Error::::NotNominator); - T::StakingInterface::nominate(bonded_pool.bonded_account(), validators) + T::Staking::nominate(&bonded_pool.bonded_account(), validators) } /// Set a new state for the pool. @@ -2132,7 +2108,7 @@ pub mod pallet { let who = ensure_signed(origin)?; let bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; ensure!(bonded_pool.can_nominate(&who), Error::::NotNominator); - T::StakingInterface::chill(bonded_pool.bonded_account()) + T::Staking::chill(&bonded_pool.bonded_account()) } } @@ -2149,7 +2125,7 @@ pub mod pallet { "Minimum points to balance ratio must be greater than 0" ); assert!( - T::StakingInterface::bonding_duration() < TotalUnbondingPools::::get(), + T::Staking::bonding_duration() < TotalUnbondingPools::::get(), "There must be more unbonding pools then the bonding duration / so a slash can be applied to relevant unboding pools. (We assume / the bonding duration > slash deffer duration.", @@ -2185,7 +2161,7 @@ impl Pallet { /// It is essentially `max { MinNominatorBond, MinCreateBond, MinJoinBond }`, where the former /// is coming from the staking pallet and the latter two are configured in this pallet. pub fn depositor_min_bond() -> BalanceOf { - T::StakingInterface::minimum_bond() + T::Staking::minimum_nominator_bond() .max(MinCreateBond::::get()) .max(MinJoinBond::::get()) .max(T::Currency::minimum_balance()) @@ -2212,7 +2188,7 @@ impl Pallet { debug_assert_eq!(frame_system::Pallet::::consumers(&reward_account), 0); debug_assert_eq!(frame_system::Pallet::::consumers(&bonded_account), 0); debug_assert_eq!( - T::StakingInterface::total_stake(&bonded_account).unwrap_or_default(), + T::Staking::total_stake(&bonded_account).unwrap_or_default(), Zero::zero() ); @@ -2487,8 +2463,7 @@ impl Pallet { let subs = SubPoolsStorage::::get(pool_id).unwrap_or_default(); let sum_unbonding_balance = subs.sum_unbonding_balance(); - let bonded_balance = - T::StakingInterface::active_stake(&pool_account).unwrap_or_default(); + let bonded_balance = T::Staking::active_stake(&pool_account).unwrap_or_default(); let total_balance = T::Currency::total_balance(&pool_account); assert!( diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 1b3372dae56ee..e1af456e31fd4 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -3,6 +3,7 @@ use crate::{self as pools}; use frame_support::{assert_ok, parameter_types, PalletId}; use frame_system::RawOrigin; use sp_runtime::FixedU128; +use sp_staking::Stake; pub type BlockNumber = u64; pub type AccountId = u128; @@ -47,10 +48,17 @@ impl sp_staking::StakingInterface for StakingMock { type Balance = Balance; type AccountId = AccountId; - fn minimum_bond() -> Self::Balance { + fn minimum_nominator_bond() -> Self::Balance { + StakingMinBond::get() + } + fn minimum_validator_bond() -> Self::Balance { StakingMinBond::get() } + fn desired_validator_count() -> u32 { + unimplemented!("method currently not used in testing") + } + fn current_era() -> EraIndex { CurrentEra::get() } @@ -59,39 +67,24 @@ impl sp_staking::StakingInterface for StakingMock { BondingDuration::get() } - fn active_stake(who: &Self::AccountId) -> Option { - BondedBalanceMap::get().get(who).map(|v| *v) - } - - fn total_stake(who: &Self::AccountId) -> Option { - match ( - UnbondingBalanceMap::get().get(who).map(|v| *v), - BondedBalanceMap::get().get(who).map(|v| *v), - ) { - (None, None) => None, - (Some(v), None) | (None, Some(v)) => Some(v), - (Some(a), Some(b)) => Some(a + b), - } - } - - fn bond_extra(who: Self::AccountId, extra: Self::Balance) -> DispatchResult { + fn bond_extra(who: &Self::AccountId, extra: Self::Balance) -> DispatchResult { let mut x = BondedBalanceMap::get(); - x.get_mut(&who).map(|v| *v += extra); + x.get_mut(who).map(|v| *v += extra); BondedBalanceMap::set(&x); Ok(()) } - fn unbond(who: Self::AccountId, amount: Self::Balance) -> DispatchResult { + fn unbond(who: &Self::AccountId, amount: Self::Balance) -> DispatchResult { let mut x = BondedBalanceMap::get(); - *x.get_mut(&who).unwrap() = x.get_mut(&who).unwrap().saturating_sub(amount); + *x.get_mut(who).unwrap() = x.get_mut(who).unwrap().saturating_sub(amount); BondedBalanceMap::set(&x); let mut y = UnbondingBalanceMap::get(); - *y.entry(who).or_insert(Self::Balance::zero()) += amount; + *y.entry(*who).or_insert(Self::Balance::zero()) += amount; UnbondingBalanceMap::set(&y); Ok(()) } - fn chill(_: Self::AccountId) -> sp_runtime::DispatchResult { + fn chill(_: &Self::AccountId) -> sp_runtime::DispatchResult { Ok(()) } @@ -104,17 +97,12 @@ impl sp_staking::StakingInterface for StakingMock { Ok(UnbondingBalanceMap::get().is_empty() && BondedBalanceMap::get().is_empty()) } - fn bond( - stash: Self::AccountId, - _: Self::AccountId, - value: Self::Balance, - _: Self::AccountId, - ) -> DispatchResult { - StakingMock::set_bonded_balance(stash, value); + fn bond(stash: &Self::AccountId, value: Self::Balance, _: &Self::AccountId) -> DispatchResult { + StakingMock::set_bonded_balance(*stash, value); Ok(()) } - fn nominate(_: Self::AccountId, nominations: Vec) -> DispatchResult { + fn nominate(_: &Self::AccountId, nominations: Vec) -> DispatchResult { Nominations::set(&Some(nominations)); Ok(()) } @@ -123,6 +111,48 @@ impl sp_staking::StakingInterface for StakingMock { fn nominations(_: Self::AccountId) -> Option> { Nominations::get() } + + fn stash_by_ctrl(_controller: &Self::AccountId) -> Result { + unimplemented!("method currently not used in testing") + } + + fn stake(who: &Self::AccountId) -> Result, DispatchError> { + match ( + UnbondingBalanceMap::get().get(who).map(|v| *v), + BondedBalanceMap::get().get(who).map(|v| *v), + ) { + (None, None) => Err(DispatchError::Other("balance not found")), + (Some(v), None) => Ok(Stake { total: v, active: 0, stash: *who }), + (None, Some(v)) => Ok(Stake { total: v, active: v, stash: *who }), + (Some(a), Some(b)) => Ok(Stake { total: a + b, active: b, stash: *who }), + } + } + + fn election_ongoing() -> bool { + unimplemented!("method currently not used in testing") + } + + fn force_unstake(_who: Self::AccountId) -> sp_runtime::DispatchResult { + unimplemented!("method currently not used in testing") + } + + fn is_exposed_in_era(_who: &Self::AccountId, _era: &EraIndex) -> bool { + unimplemented!("method currently not used in testing") + } + + #[cfg(feature = "runtime-benchmarks")] + fn add_era_stakers( + _current_era: &EraIndex, + _stash: &Self::AccountId, + _exposures: Vec<(Self::AccountId, Self::Balance)>, + ) { + unimplemented!("method currently not used in testing") + } + + #[cfg(feature = "runtime-benchmarks")] + fn set_current_era(_era: EraIndex) { + unimplemented!("method currently not used in testing") + } } impl frame_system::Config for Runtime { @@ -192,11 +222,10 @@ impl pools::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = (); type Currency = Balances; - type CurrencyBalance = Balance; type RewardCounter = RewardCounter; type BalanceToU256 = BalanceToU256; type U256ToBalance = U256ToBalance; - type StakingInterface = StakingMock; + type Staking = StakingMock; type PostUnbondingPoolsWindow = PostUnbondingPoolsWindow; type PalletId = PoolsPalletId; type MaxMetadataLen = MaxMetadataLen; diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 5074a7ffa695a..0c909b68152bb 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -3083,18 +3083,18 @@ mod pool_withdraw_unbonded { fn pool_withdraw_unbonded_works() { ExtBuilder::default().build_and_execute(|| { // Given 10 unbond'ed directly against the pool account - assert_ok!(StakingMock::unbond(default_bonded_account(), 5)); + assert_ok!(StakingMock::unbond(&default_bonded_account(), 5)); // and the pool account only has 10 balance - assert_eq!(StakingMock::active_stake(&default_bonded_account()), Some(5)); - assert_eq!(StakingMock::total_stake(&default_bonded_account()), Some(10)); + assert_eq!(StakingMock::active_stake(&default_bonded_account()), Ok(5)); + assert_eq!(StakingMock::total_stake(&default_bonded_account()), Ok(10)); assert_eq!(Balances::free_balance(&default_bonded_account()), 10); // When assert_ok!(Pools::pool_withdraw_unbonded(RuntimeOrigin::signed(10), 1, 0)); // Then there unbonding balance is no longer locked - assert_eq!(StakingMock::active_stake(&default_bonded_account()), Some(5)); - assert_eq!(StakingMock::total_stake(&default_bonded_account()), Some(5)); + assert_eq!(StakingMock::active_stake(&default_bonded_account()), Ok(5)); + assert_eq!(StakingMock::total_stake(&default_bonded_account()), Ok(5)); assert_eq!(Balances::free_balance(&default_bonded_account()), 10); }); } @@ -3270,7 +3270,7 @@ mod withdraw_unbonded { // current bond is 600, we slash it all to 300. StakingMock::set_bonded_balance(default_bonded_account(), 300); Balances::make_free_balance_be(&default_bonded_account(), 300); - assert_eq!(StakingMock::total_stake(&default_bonded_account()), Some(300)); + assert_eq!(StakingMock::total_stake(&default_bonded_account()), Ok(300)); assert_ok!(fully_unbond_permissioned(40)); assert_ok!(fully_unbond_permissioned(550)); @@ -4074,12 +4074,15 @@ mod create { assert!(!BondedPools::::contains_key(2)); assert!(!RewardPools::::contains_key(2)); assert!(!PoolMembers::::contains_key(11)); - assert_eq!(StakingMock::active_stake(&next_pool_stash), None); + assert_err!( + StakingMock::active_stake(&next_pool_stash), + DispatchError::Other("balance not found") + ); - Balances::make_free_balance_be(&11, StakingMock::minimum_bond() + ed); + Balances::make_free_balance_be(&11, StakingMock::minimum_nominator_bond() + ed); assert_ok!(Pools::create( RuntimeOrigin::signed(11), - StakingMock::minimum_bond(), + StakingMock::minimum_nominator_bond(), 123, 456, 789 @@ -4090,7 +4093,7 @@ mod create { PoolMembers::::get(11).unwrap(), PoolMember { pool_id: 2, - points: StakingMock::minimum_bond(), + points: StakingMock::minimum_nominator_bond(), ..Default::default() } ); @@ -4099,7 +4102,7 @@ mod create { BondedPool { id: 2, inner: BondedPoolInner { - points: StakingMock::minimum_bond(), + points: StakingMock::minimum_nominator_bond(), member_counter: 1, state: PoolState::Open, roles: PoolRoles { @@ -4113,7 +4116,7 @@ mod create { ); assert_eq!( StakingMock::active_stake(&next_pool_stash).unwrap(), - StakingMock::minimum_bond() + StakingMock::minimum_nominator_bond() ); assert_eq!( RewardPools::::get(2).unwrap(), @@ -4142,7 +4145,7 @@ mod create { // Given assert_eq!(MinCreateBond::::get(), 2); - assert_eq!(StakingMock::minimum_bond(), 10); + assert_eq!(StakingMock::minimum_nominator_bond(), 10); // Then assert_noop!( diff --git a/frame/nomination-pools/test-staking/src/mock.rs b/frame/nomination-pools/test-staking/src/mock.rs index 8c04e40d71df4..5758b884e348d 100644 --- a/frame/nomination-pools/test-staking/src/mock.rs +++ b/frame/nomination-pools/test-staking/src/mock.rs @@ -171,11 +171,10 @@ impl pallet_nomination_pools::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = (); type Currency = Balances; - type CurrencyBalance = Balance; type RewardCounter = FixedU128; type BalanceToU256 = BalanceToU256; type U256ToBalance = U256ToBalance; - type StakingInterface = Staking; + type Staking = Staking; type PostUnbondingPoolsWindow = PostUnbondingPoolsWindow; type MaxMetadataLen = ConstU32<256>; type MaxUnbonding = ConstU32<8>; diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 4ef064229693c..a9e9899b9761a 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, @@ -38,7 +38,7 @@ use sp_runtime::{ }; use sp_staking::{ offence::{DisableStrategy, OffenceDetails, OnOffenceHandler}, - EraIndex, SessionIndex, StakingInterface, + EraIndex, SessionIndex, Stake, StakingInterface, }; use sp_std::{collections::btree_map::BTreeMap, prelude::*}; @@ -1482,14 +1482,44 @@ 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 { + let num_slashing_spans = Self::slashing_spans(&who).iter().count() as u32; + Self::force_unstake(RawOrigin::Root.into(), who.clone(), num_slashing_spans) + } + + fn stash_by_ctrl(controller: &Self::AccountId) -> Result { + Self::ledger(controller) + .map(|l| l.stash) + .ok_or(Error::::NotController.into()) + } + + fn is_exposed_in_era(who: &Self::AccountId, era: &EraIndex) -> bool { + ErasStakers::::iter_prefix(era).any(|(validator, exposures)| { + validator == *who || exposures.others.iter().any(|i| i.who == *who) + }) + } + fn bonding_duration() -> EraIndex { T::BondingDuration::get() } @@ -1498,58 +1528,81 @@ 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 total_stake(controller: &Self::AccountId) -> Option { - Self::ledger(controller).map(|l| l.total) + fn stake(who: &Self::AccountId) -> Result, DispatchError> { + Self::bonded(who) + .and_then(|c| Self::ledger(c)) + .map(|l| Stake { stash: l.stash, total: l.total, active: l.active }) + .ok_or(Error::::NotStash.into()) } - 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")] fn nominations(who: Self::AccountId) -> Option> { Nominators::::get(who).map(|n| n.targets.into_inner()) } + + #[cfg(feature = "runtime-benchmarks")] + fn add_era_stakers( + current_era: &EraIndex, + stash: &T::AccountId, + exposures: Vec<(Self::AccountId, Self::Balance)>, + ) { + let others = exposures + .iter() + .map(|(who, value)| IndividualExposure { who: who.clone(), value: value.clone() }) + .collect::>(); + let exposure = Exposure { total: Default::default(), own: Default::default(), others }; + Self::add_era_stakers(current_era.clone(), stash.clone(), exposure) + } + + #[cfg(feature = "runtime-benchmarks")] + fn set_current_era(era: EraIndex) { + CurrentEra::::put(era); + } } #[cfg(any(test, feature = "try-runtime"))] diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 560c3b6ed830c..df90873ab2e59 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -262,7 +262,7 @@ pub mod pallet { type WeightInfo: WeightInfo; } - /// The ideal number of staking participants. + /// The ideal number of active validators. #[pallet::storage] #[pallet::getter(fn validator_count)] pub type ValidatorCount = StorageValue<_, u32, ValueQuery>; diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 5a3e97b4d5274..703f0abe80458 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -19,8 +19,9 @@ //! 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 sp_runtime::{DispatchError, DispatchResult}; -use sp_std::collections::btree_map::BTreeMap; +use sp_std::{collections::btree_map::BTreeMap, vec::Vec}; pub mod offence; @@ -54,25 +55,55 @@ impl OnStakerSlash for () { } } -/// Trait for communication with the staking pallet. +/// 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 { + /// The stash account whose balance is actually locked and at stake. + pub stash: T::AccountId, + /// 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`]. + /// + /// # Note + /// + /// 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, + /// The total amount of the stash's balance that will be at stake in any forthcoming + /// rounds. + pub active: T::Balance, +} + +/// 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; + type Balance: PartialEq; /// 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 a stash account that is controlled by a `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 stash_by_ctrl(controller: &Self::AccountId) -> Result; + + /// Number of eras that staked funds must remain bonded for. fn bonding_duration() -> EraIndex; /// The current era index. @@ -80,41 +111,39 @@ 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; + /// Returns the stake of `who`. + fn stake(who: &Self::AccountId) -> Result, DispatchError>; - /// The total stake that `stash` has in the staking system. This includes the - /// [`Self::active_stake`], and any funds currently in the process of unbonding via - /// [`Self::unbond`]. - /// - /// # Note - /// - /// 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) -> Result { + Self::stake(who).map(|s| s.total) + } - /// 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; - - /// Have `controller` nominate `validators`. - fn nominate( - controller: Self::AccountId, - validators: sp_std::vec::Vec, - ) -> DispatchResult; - - /// Chill `stash`. - fn chill(controller: 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 + fn active_stake(who: &Self::AccountId) -> Result { + Self::stake(who).map(|s| s.active) + } + + fn is_unbonding(who: &Self::AccountId) -> Result { + Self::stake(who).map(|s| s.active != s.total) + } + + fn fully_unbond(who: &Self::AccountId) -> DispatchResult { + Self::unbond(who, Self::stake(who)?.active) + } + + /// 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 `who` nominate `validators`. + fn nominate(who: &Self::AccountId, validators: Vec) -> DispatchResult; + + /// Chill `who`. + fn chill(who: &Self::AccountId) -> DispatchResult; + + /// 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 +154,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,7 +164,29 @@ pub trait StakingInterface { num_slashing_spans: u32, ) -> Result; + /// The ideal number of active validators. + fn desired_validator_count() -> u32; + + /// Whether or not there is an ongoing election. + fn election_ongoing() -> bool; + + /// Force a current staker to become completely unstaked, immediately. + fn force_unstake(who: Self::AccountId) -> DispatchResult; + + /// Checks whether an account `staker` has been exposed in an era. + 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")] - fn nominations(who: Self::AccountId) -> Option>; + fn nominations(who: Self::AccountId) -> Option>; + + #[cfg(feature = "runtime-benchmarks")] + fn add_era_stakers( + current_era: &EraIndex, + stash: &Self::AccountId, + exposures: Vec<(Self::AccountId, Self::Balance)>, + ); + + #[cfg(feature = "runtime-benchmarks")] + fn set_current_era(era: EraIndex); }