From 3a11e899c7c269a37e24831286b8c9b8e517973b Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 5 Oct 2022 15:42:31 +0200 Subject: [PATCH 01/45] [Enhancement] Convert fast-unstake to use StakingInterface, decoupling it from Staking --- frame/fast-unstake/src/lib.rs | 5 ++++- primitives/staking/src/lib.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 8fdb7a79dd537..873008cb2fef5 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -109,7 +109,7 @@ 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> @@ -125,6 +125,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; } diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 5a3e97b4d5274..62378484d834e 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -135,6 +135,32 @@ pub trait StakingInterface { num_slashing_spans: u32, ) -> Result; + /// Get StakingLedger by controller account. + fn get_ledger(ctrl: &Self::AccountId) -> StakingLedger; + + /// The ideal number of staking participants. + fn validator_count() -> u32; + + /// Whether or not there is an ongoing election. + fn ongoing() -> bool; + + /// Force a current staker to become completely unstaked, immediately. + /// + /// The dispatch origin must be Root. + /// TODO: Do we need `num_slashing_spans` in the original method or should + /// we do Staking::::slashing_spans(&stash).iter().count() within it? + /// Or should we introduce another method force_unstake_with_spans that will do it? + fn force_unstake( + origin: OriginFor, + stash: T::AccountId, + num_slashing_spans: u32, + ) -> DispatchResult; + + /// Checks whether an account `staker` has been exposed in an era. + /// TODO: shall we have this helper here or just expose ErasStakers + /// storage instead for the caller to do as they please? + fn is_exposed_in_era(staker: &T::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>; From 4f39589503fe6e23dcb92075728977221c0e8061 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 6 Oct 2022 10:41:17 +0200 Subject: [PATCH 02/45] Update primitives/staking/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- primitives/staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 62378484d834e..aa5f697d39a7a 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -135,7 +135,7 @@ pub trait StakingInterface { num_slashing_spans: u32, ) -> Result; - /// Get StakingLedger by controller account. + /// Get `StakingLedger` by controller account. fn get_ledger(ctrl: &Self::AccountId) -> StakingLedger; /// The ideal number of staking participants. From 9b8e1239c45a8f00eeddbb56b21df054a1bc3ed8 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 6 Oct 2022 10:42:24 +0200 Subject: [PATCH 03/45] Update primitives/staking/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- primitives/staking/src/lib.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index aa5f697d39a7a..c238856f3412d 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -145,16 +145,7 @@ pub trait StakingInterface { fn ongoing() -> bool; /// Force a current staker to become completely unstaked, immediately. - /// - /// The dispatch origin must be Root. - /// TODO: Do we need `num_slashing_spans` in the original method or should - /// we do Staking::::slashing_spans(&stash).iter().count() within it? - /// Or should we introduce another method force_unstake_with_spans that will do it? - fn force_unstake( - origin: OriginFor, - stash: T::AccountId, - num_slashing_spans: u32, - ) -> DispatchResult; + fn force_unstake(stash: T::AccountId) -> DispatchResult; /// Checks whether an account `staker` has been exposed in an era. /// TODO: shall we have this helper here or just expose ErasStakers From 25520d60a5f5a56e40ab809a078d4be5dc87b668 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 6 Oct 2022 11:10:48 +0200 Subject: [PATCH 04/45] fix validator comment --- frame/staking/src/pallet/mod.rs | 2 +- primitives/staking/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 c238856f3412d..90d24ad5935ad 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -138,7 +138,7 @@ pub trait StakingInterface { /// Get `StakingLedger` by controller account. fn get_ledger(ctrl: &Self::AccountId) -> StakingLedger; - /// The ideal number of staking participants. + /// The ideal number of active validators. fn validator_count() -> u32; /// Whether or not there is an ongoing election. From ad909038f44634cdaa82e911aa5ef59a3199a0fc Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Mon, 10 Oct 2022 11:51:37 +0200 Subject: [PATCH 05/45] remove todo --- primitives/staking/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 90d24ad5935ad..e406f62d4d81a 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -148,8 +148,6 @@ pub trait StakingInterface { fn force_unstake(stash: T::AccountId) -> DispatchResult; /// Checks whether an account `staker` has been exposed in an era. - /// TODO: shall we have this helper here or just expose ErasStakers - /// storage instead for the caller to do as they please? fn is_exposed_in_era(staker: &T::AccountId, era: &EraIndex) -> bool; /// Get the nominations of a stash, if they are a nominator, `None` otherwise. From 89b32b56eecb6d5639734ba7ed970b8002d50b03 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 11 Oct 2022 17:58:45 +0200 Subject: [PATCH 06/45] ideas from Kian (#12474) Co-authored-by: kianenigma --- 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")] From 41d04ade20b3560428b5e718b6f765653807e23a Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 12 Oct 2022 14:41:37 +0200 Subject: [PATCH 07/45] Rename StakingInterface -> Staking for nomination-pools --- .../nomination-pools/benchmarking/src/lib.rs | 43 +++++++-------- .../nomination-pools/benchmarking/src/mock.rs | 2 +- frame/nomination-pools/src/lib.rs | 52 ++++++++----------- frame/nomination-pools/src/mock.rs | 2 +- .../nomination-pools/test-staking/src/mock.rs | 2 +- 5 files changed, 46 insertions(+), 55 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index c31bcb1546ecd..90f64d7abf7b4 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -133,14 +133,14 @@ impl ListScenario { // Create accounts with the origin weight let (pool_creator1, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); - T::StakingInterface::nominate( + T::Staking::nominate( pool_origin1.clone(), // 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( + T::Staking::nominate( pool_origin2.clone(), 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.clone(), 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,11 +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) + T::Staking::unbond(self.origin1.clone(), amount) .expect("the pool was created in `Self::new`."); // Account pool points for the unbonded balance. @@ -219,7 +216,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 +231,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 +246,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 +264,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 +311,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 +320,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 +342,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 +352,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 +385,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 +396,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 +443,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,7 +522,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::StakingInterface::active_stake(&Pools::::create_bonded_account(1)), + T::Staking::active_stake(&Pools::::create_bonded_account(1)), Some(min_create_bond) ); } @@ -564,7 +561,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::StakingInterface::active_stake(&Pools::::create_bonded_account(1)), + T::Staking::active_stake(&Pools::::create_bonded_account(1)), Some(min_create_bond) ); } @@ -648,12 +645,12 @@ frame_benchmarking::benchmarks! { .collect(); assert_ok!(Pools::::nominate(RuntimeOrigin::Signed(depositor.clone()).into(), 1, validators)); - assert!(T::StakingInterface::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::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..d82d071086e1e 100644 --- a/frame/nomination-pools/benchmarking/src/mock.rs +++ b/frame/nomination-pools/benchmarking/src/mock.rs @@ -161,7 +161,7 @@ impl pallet_nomination_pools::Config for Runtime { 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..dc94d6d005da0 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -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 @@ -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,7 +889,7 @@ impl BondedPool { let points_issued = self.issue(amount); match ty { - BondType::Create => T::StakingInterface::bond( + BondType::Create => T::Staking::bond( bonded_account.clone(), bonded_account, amount, @@ -898,7 +898,7 @@ impl BondedPool { // 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 +1129,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() } } @@ -1212,10 +1212,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 +1647,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 +1715,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 +1751,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 +1766,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 +1955,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 +2127,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 +2144,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 +2180,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_bond() .max(MinCreateBond::::get()) .max(MinJoinBond::::get()) .max(T::Currency::minimum_balance()) @@ -2212,7 +2207,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 +2482,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..2673fc814c3a2 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -196,7 +196,7 @@ impl pools::Config for Runtime { 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/test-staking/src/mock.rs b/frame/nomination-pools/test-staking/src/mock.rs index 8c04e40d71df4..ece1798ade0e2 100644 --- a/frame/nomination-pools/test-staking/src/mock.rs +++ b/frame/nomination-pools/test-staking/src/mock.rs @@ -175,7 +175,7 @@ impl pallet_nomination_pools::Config for Runtime { 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>; From 7fc52f86459e869a994fd2ae58dc6f38bfe796fe Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 12 Oct 2022 14:42:33 +0200 Subject: [PATCH 08/45] Staking fixes --- frame/staking/src/pallet/impls.rs | 35 +++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 47961ee93171d..41c4eb0a024b7 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -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::*}; @@ -1504,15 +1504,20 @@ impl StakingInterface for Pallet { } fn force_unstake(who: Self::AccountId) -> sp_runtime::DispatchResult { - todo!(); + let num_slashing_spans = Self::slashing_spans(&who).iter().count() as u32; + Self::force_unstake(RawOrigin::Root.into(), who.clone(), num_slashing_spans) } - fn can_control(controller: &Self::AccountId) -> Result { - Self::ledger(controller).map(|l| l.stash).ok_or(Error::::NotController) + fn stash(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 { - todo!() + ErasStakers::::iter_prefix(era).any(|(validator, exposures)| { + validator == *who || exposures.others.iter().any(|i| i.who == *who) + }) } fn bonding_duration() -> EraIndex { @@ -1523,19 +1528,27 @@ impl StakingInterface for Pallet { Self::current_era().unwrap_or(Zero::zero()) } - fn active_stake(who: &Self::AccountId) -> Option { - Self::bonded(who).and_then(|c| Self::ledger(c)).map(|l| l.active) + 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 total_stake(who: &Self::AccountId) -> Option { - Self::bonded(who).and_then(|c| Self::ledger(c)).map(|l| l.active) - } + /// TODO: Find usages and depreacte those in favour of stake + // fn active_stake(who: &Self::AccountId) -> Option { + // Self::bonded(who).and_then(|c| Self::stake(c)).map(|l| l.active) + // } + // + // fn total_stake(who: &Self::AccountId) -> Option { + // Self::bonded(who).and_then(|c| Self::ledger(c)).map(|l| l.active) + // } fn bond_extra(who: &Self::AccountId, extra: Self::Balance) -> DispatchResult { Self::bond_extra(RawOrigin::Signed(who.clone()).into(), extra) } - fn unbond(who: Self::AccountId, value: Self::Balance) -> DispatchResult { + 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) } From d3958ba93b7f805976a6fd33b1eb07770b91263d Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 12 Oct 2022 14:43:53 +0200 Subject: [PATCH 09/45] StakingInterface changes --- primitives/staking/src/lib.rs | 49 +++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 4eb0cc31b34dc..4641012584a45 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -54,13 +54,32 @@ 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 { + /// 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 may not be accurate. + 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; @@ -74,13 +93,13 @@ pub trait StakingInterface { /// 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`. + /// Return a stash account that is controlled by a `controller`. /// /// ## Note /// /// The controller abstraction is not permanent and might go away. Avoid using this as much as /// possible. - fn can_control(controller: &Self::AccountId) -> Result; + fn stash(controller: &Self::AccountId) -> Result; /// Number of eras that staked funds must remain bonded for. fn bonding_duration() -> EraIndex; @@ -90,29 +109,19 @@ 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 `who` has in the staking system. - fn active_stake(who: &Self::AccountId) -> Option; - - /// 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`]. - /// - /// # 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(who: &Self::AccountId) -> Option; + /// Returns the stake of `who`. + fn stake(who: &Self::AccountId) -> Result, DispatchError>; + /// TODO: Possibly return a result here too for consistency. 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 + match Self::stake(who) { + Ok(stake) if stake.active == stake.total => 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()) + Self::unbond(who, Self::stake(who)?.active) } /// Bond (lock) `value` of `who`'s balance, while forwarding any rewards to `payee`. From 2ff689c4977591fca85421b20a449137ce6bc4e4 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 12 Oct 2022 18:11:42 +0200 Subject: [PATCH 10/45] fix fast-unstake --- frame/fast-unstake/src/lib.rs | 68 ++++++++++++++++----------------- frame/fast-unstake/src/types.rs | 7 ++-- 2 files changed, 36 insertions(+), 39 deletions(-) diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index c3df336d8e1f6..8cad7acef4788 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -80,12 +80,11 @@ 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 frame_system::pallet_prelude::*; use sp_runtime::{ traits::{Saturating, Zero}, DispatchResult, @@ -100,7 +99,7 @@ pub mod pallet { pub struct MaxChecking(sp_std::marker::PhantomData); impl frame_support::traits::Get for MaxChecking { fn get() -> u32 { - T::StakingInterface::bonding_duration() + 1 + T::Staking::bonding_duration() + 1 } } @@ -115,7 +114,7 @@ pub mod pallet { + 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. @@ -124,11 +123,20 @@ pub mod pallet { /// The origin that can control this pallet. type ControlOrigin: frame_support::traits::EnsureOrigin; + /// Just the `Currency::Balance` type; we have this item to allow us to constrain it to + /// `From`. + type CurrencyBalance: sp_runtime::traits::AtLeast32BitUnsigned + + codec::FullCodec + + Copy + + MaybeSerializeDeserialize + + sp_std::fmt::Debug + + Default + + From + + TypeInfo + + MaxEncodedLen; + /// The access to staking functionality. - type StakingInterface: StakingInterface< - Balance = BalanceOf, - AccountId = Self::AccountId, - >; + type Staking: StakingInterface, AccountId = Self::AccountId>; /// The weight information of this pallet. type WeightInfo: WeightInfo; @@ -226,24 +234,24 @@ pub mod pallet { let ctrl = ensure_signed(origin)?; ensure!(ErasToCheckPerBlock::::get() != 0, >::CallNotAllowed); - let stash = T::StakingInterface::can_control(&ctrl)?; + let stash = T::Staking::stash(&ctrl)?; ensure!(!Queue::::contains_key(&stash), Error::::AlreadyQueued); ensure!( - Head::::get().map_or(true, |UnstakeRequest { stash, .. }| stash != ledger.stash), + Head::::get().map_or(true, |UnstakeRequest { stash, .. }| stash != stash), Error::::AlreadyHead ); // second part of the && is defensive. - ensure!(!T::StakingInterface::is_unbonding(&who), Error::::NotFullyBonded); + ensure!(!T::Staking::is_unbonding(&stash), Error::::NotFullyBonded); // chill and fully unstake. - T::StakingInterface::chill(&stash)?; - T::StakingInterface::fully_unbond(&stash)?; + T::Staking::chill(&stash)?; + T::Staking::fully_unbond(&stash)?; - T::DepositCurrency::reserve(&ledger.stash, T::Deposit::get())?; + T::Currency::reserve(&stash, T::Deposit::get())?; // enqueue them. - Queue::::insert(ledger.stash, T::Deposit::get()); + Queue::::insert(stash, T::Deposit::get()); Ok(()) } @@ -260,7 +268,7 @@ pub mod pallet { ensure!(ErasToCheckPerBlock::::get() != 0, >::CallNotAllowed); - let stash = T::StakingInterface::can_control(&ctrl)?; + let stash = T::Staking::stash(&ctrl)?; ensure!(Queue::::contains_key(&stash), Error::::NotQueued); ensure!( Head::::get().map_or(true, |UnstakeRequest { stash, .. }| stash != stash), @@ -269,7 +277,7 @@ pub mod pallet { let deposit = Queue::::take(stash.clone()); if let Some(deposit) = deposit.defensive() { - 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); @@ -313,7 +321,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 = T::StakingInterface::desired_validator_count(); + 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. @@ -329,7 +337,7 @@ pub mod pallet { } } - if T::StakingInterface::election_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 @@ -364,8 +372,8 @@ pub mod pallet { ); // the range that we're allowed to check in this round. - let current_era = T::StakingInterface::current_era(); - let bonding_duration = T::StakingInterface::bonding_duration(); + 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)); @@ -399,12 +407,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::StakingInterface::force_unstake(stash.clone()); + let result = T::Staking::force_unstake(stash.clone()); - 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); @@ -420,7 +425,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!( @@ -436,7 +441,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 { @@ -466,12 +471,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/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, From 1fc651018f5dbbec972e41484cdb01d68973bc68 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 12 Oct 2022 18:35:54 +0200 Subject: [PATCH 11/45] fix nomination-pools --- frame/nomination-pools/src/lib.rs | 39 +++++++++++++++---------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index dc94d6d005da0..149e0cef594e7 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -622,8 +622,9 @@ 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::Staking::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); + let bonded_balance = T::Staking::stake(&self.bonded_account()) + .map(|s| s.active) + .unwrap_or(Zero::zero()); Pallet::::balance_to_point(bonded_balance, self.points, new_funds) } @@ -631,8 +632,9 @@ impl BondedPool { /// /// This is often used for unbonding. fn points_to_balance(&self, points: BalanceOf) -> BalanceOf { - let bonded_balance = - T::Staking::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); + let bonded_balance = T::Staking::stake(&self.bonded_account()) + .map(|s| s.active) + .unwrap_or(Zero::zero()); Pallet::::point_to_balance(bonded_balance, self.points, points) } @@ -683,7 +685,7 @@ impl BondedPool { fn transferrable_balance(&self) -> BalanceOf { let account = self.bonded_account(); T::Currency::free_balance(&account) - .saturating_sub(T::Staking::active_stake(&account).unwrap_or_default()) + .saturating_sub(T::Staking::stake(&account).map(|s| s.active).unwrap_or_default()) } fn is_root(&self, who: &T::AccountId) -> bool { @@ -737,8 +739,9 @@ impl BondedPool { fn ok_to_be_open(&self, new_funds: BalanceOf) -> Result<(), DispatchError> { ensure!(!self.is_destroying(), Error::::CanNotChangeState); - let bonded_balance = - T::Staking::active_stake(&self.bonded_account()).unwrap_or(Zero::zero()); + let bonded_balance = T::Staking::stake(&self.bonded_account()) + .map(|s| s.active) + .unwrap_or(Zero::zero()); ensure!(!bonded_balance.is_zero(), Error::::OverflowRisk); let points_to_balance_ratio_floor = self @@ -889,16 +892,11 @@ impl BondedPool { let points_issued = self.issue(amount); match ty { - BondType::Create => T::Staking::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::Staking::bond_extra(bonded_account, amount)?, + BondType::Later => T::Staking::bond_extra(&bonded_account, amount)?, } Ok(points_issued) @@ -1652,7 +1650,7 @@ pub mod pallet { // Unbond in the actual underlying nominator. let unbonding_balance = bonded_pool.dissolve(unbonding_points); - T::Staking::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) @@ -1955,7 +1953,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::Staking::nominate(bonded_pool.bonded_account(), validators) + T::Staking::nominate(&bonded_pool.bonded_account(), validators) } /// Set a new state for the pool. @@ -2127,7 +2125,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::Staking::chill(bonded_pool.bonded_account()) + T::Staking::chill(&bonded_pool.bonded_account()) } } @@ -2180,7 +2178,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::Staking::minimum_bond() + T::Staking::minimum_nominator_bond() .max(MinCreateBond::::get()) .max(MinJoinBond::::get()) .max(T::Currency::minimum_balance()) @@ -2207,7 +2205,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::Staking::total_stake(&bonded_account).unwrap_or_default(), + T::Staking::stake(&bonded_account).map(|s| s.total).unwrap_or_default(), Zero::zero() ); @@ -2482,7 +2480,8 @@ impl Pallet { let subs = SubPoolsStorage::::get(pool_id).unwrap_or_default(); let sum_unbonding_balance = subs.sum_unbonding_balance(); - let bonded_balance = T::Staking::active_stake(&pool_account).unwrap_or_default(); + let bonded_balance = + T::Staking::stake(&pool_account).map(|s| s.active).unwrap_or_default(); let total_balance = T::Currency::total_balance(&pool_account); assert!( From 0705029ae95568a5ea5b4db44665da19ce495740 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 18 Oct 2022 12:36:28 +0200 Subject: [PATCH 12/45] Fix fast-unstake tests --- Cargo.lock | 3 +++ frame/fast-unstake/Cargo.toml | 4 ++++ frame/fast-unstake/src/lib.rs | 6 +++--- frame/fast-unstake/src/mock.rs | 4 +++- frame/fast-unstake/src/tests.rs | 14 +++++++------- frame/staking/src/pallet/impls.rs | 2 +- primitives/staking/src/lib.rs | 4 +++- 7 files changed, 24 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index be6a3ef47ed52..2c1b427f0a2e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5496,7 +5496,10 @@ 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 19b79190ed4c1..f19b9f3d71586 100644 --- a/frame/fast-unstake/Cargo.toml +++ b/frame/fast-unstake/Cargo.toml @@ -33,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"] diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 8cad7acef4788..4cd883297f541 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -234,7 +234,7 @@ pub mod pallet { let ctrl = ensure_signed(origin)?; ensure!(ErasToCheckPerBlock::::get() != 0, >::CallNotAllowed); - let stash = T::Staking::stash(&ctrl)?; + let stash = T::Staking::stash_by_ctrl(&ctrl).map_err(|_| Error::::NotController)?; ensure!(!Queue::::contains_key(&stash), Error::::AlreadyQueued); ensure!( Head::::get().map_or(true, |UnstakeRequest { stash, .. }| stash != stash), @@ -242,7 +242,7 @@ pub mod pallet { ); // second part of the && is defensive. - ensure!(!T::Staking::is_unbonding(&stash), Error::::NotFullyBonded); + ensure!(T::Staking::is_unbonding(&stash), Error::::NotFullyBonded); // chill and fully unstake. T::Staking::chill(&stash)?; @@ -268,7 +268,7 @@ pub mod pallet { ensure!(ErasToCheckPerBlock::::get() != 0, >::CallNotAllowed); - let stash = T::Staking::stash(&ctrl)?; + let stash = T::Staking::stash_by_ctrl(&ctrl).map_err(|_| Error::::NotController)?; ensure!(Queue::::contains_key(&stash), Error::::NotQueued); ensure!( Head::::get().map_or(true, |UnstakeRequest { stash, .. }| stash != stash), diff --git a/frame/fast-unstake/src/mock.rs b/frame/fast-unstake/src/mock.rs index 71fc2d4ba905a..a6a4ab69b61f2 100644 --- a/frame/fast-unstake/src/mock.rs +++ b/frame/fast-unstake/src/mock.rs @@ -174,7 +174,9 @@ parameter_types! { impl fast_unstake::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Deposit = DepositAmount; - type DepositCurrency = Balances; + type Currency = Balances; + type CurrencyBalance = Balance; + 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/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index b32ee6b53bb8e..8b89795eea7ce 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1508,7 +1508,7 @@ impl StakingInterface for Pallet { Self::force_unstake(RawOrigin::Root.into(), who.clone(), num_slashing_spans) } - fn stash(controller: &Self::AccountId) -> Result { + fn stash_by_ctrl(controller: &Self::AccountId) -> Result { Self::ledger(controller) .map(|l| l.stash) .ok_or(Error::::NotController.into()) diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 4641012584a45..2873b62d54323 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -19,6 +19,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. +extern crate core; + use sp_runtime::{DispatchError, DispatchResult}; use sp_std::collections::btree_map::BTreeMap; @@ -99,7 +101,7 @@ pub trait StakingInterface { /// /// The controller abstraction is not permanent and might go away. Avoid using this as much as /// possible. - fn stash(controller: &Self::AccountId) -> Result; + fn stash_by_ctrl(controller: &Self::AccountId) -> Result; /// Number of eras that staked funds must remain bonded for. fn bonding_duration() -> EraIndex; From 39cf58056ab1ac4dbb303c1eb0f655afa79812a7 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 18 Oct 2022 13:56:37 +0200 Subject: [PATCH 13/45] Fix benches for fast-unstake --- frame/fast-unstake/Cargo.toml | 1 + frame/fast-unstake/src/benchmarking.rs | 39 ++++++++------------------ frame/staking/src/pallet/impls.rs | 19 +++++++++++++ primitives/staking/src/lib.rs | 10 +++++++ 4 files changed, 41 insertions(+), 28 deletions(-) diff --git a/frame/fast-unstake/Cargo.toml b/frame/fast-unstake/Cargo.toml index f19b9f3d71586..ea1abeb0b48c5 100644 --- a/frame/fast-unstake/Cargo.toml +++ b/frame/fast-unstake/Cargo.toml @@ -60,5 +60,6 @@ std = [ runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", "frame-system/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 b9f43fc87ab45..d85f0c8c8f157 100644 --- a/frame/fast-unstake/src/benchmarking.rs +++ b/frame/fast-unstake/src/benchmarking.rs @@ -26,21 +26,15 @@ use frame_support::{ traits::{Currency, EnsureOrigin, Get, Hooks}, }; use frame_system::RawOrigin; -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); @@ -52,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> { @@ -90,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()); }); } } @@ -136,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 v = x / T::Staking::bonding_duration(); + let u = T::Staking::bonding_duration(); 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/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 8b89795eea7ce..fa9fe9c24fbc1 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1593,6 +1593,25 @@ impl StakingInterface for Pallet { 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 + .map(|(who, value)| pallet_staking::IndividualExposure { who, value }) + .collect::>(); + let exposure = + pallet_staking::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/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 2873b62d54323..a7bb69f81f11c 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -178,4 +178,14 @@ pub trait StakingInterface { /// Get the nominations of a stash, if they are a nominator, `None` otherwise. #[cfg(feature = "runtime-benchmarks")] 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); } From 80935672d227496d322cb9a2328526d78b6397ea Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 18 Oct 2022 14:11:24 +0200 Subject: [PATCH 14/45] fix is_unbonding --- frame/fast-unstake/src/lib.rs | 3 +-- primitives/staking/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 4cd883297f541..1b8c93ec20891 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -241,8 +241,7 @@ pub mod pallet { Error::::AlreadyHead ); - // second part of the && is defensive. - ensure!(T::Staking::is_unbonding(&stash), Error::::NotFullyBonded); + ensure!(!T::Staking::is_unbonding(&stash), Error::::NotFullyBonded); // chill and fully unstake. T::Staking::chill(&stash)?; diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index a7bb69f81f11c..94e2a49b00f9a 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -117,8 +117,8 @@ pub trait StakingInterface { /// TODO: Possibly return a result here too for consistency. fn is_unbonding(who: &Self::AccountId) -> bool { match Self::stake(who) { - Ok(stake) if stake.active == stake.total => true, - _ => false, + Ok(stake) if stake.active == stake.total => false, + _ => true, } } From 30a5882ae2c3671fe7435cc895526f34de878d8a Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 18 Oct 2022 15:27:30 +0200 Subject: [PATCH 15/45] fix nomination pools --- frame/nomination-pools/src/mock.rs | 90 +++++++++++++++++++---------- frame/nomination-pools/src/tests.rs | 62 ++++++++++++-------- 2 files changed, 99 insertions(+), 53 deletions(-) diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 2673fc814c3a2..3f484a89cbfb2 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 { + todo!() + } + 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.clone()).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.clone(), 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 { + todo!() + } + + 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.clone() }), + (None, Some(v)) => Ok(Stake { total: v, active: v, stash: who.clone() }), + (Some(a), Some(b)) => Ok(Stake { total: a + b, active: b, stash: who.clone() }), + } + } + + fn election_ongoing() -> bool { + todo!() + } + + fn force_unstake(_who: Self::AccountId) -> sp_runtime::DispatchResult { + todo!() + } + + fn is_exposed_in_era(_who: &Self::AccountId, _era: &EraIndex) -> bool { + todo!() + } + + #[cfg(feature = "runtime-benchmarks")] + fn add_era_stakers( + _current_era: &EraIndex, + _stash: &Self::AccountId, + _exposures: Vec<(Self::AccountId, Self::Balance)>, + ) { + todo!() + } + + #[cfg(feature = "runtime-benchmarks")] + fn set_current_era(_era: EraIndex) { + todo!() + } } impl frame_system::Config for Runtime { diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 5074a7ffa695a..ff5cbf6448400 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -79,8 +79,8 @@ fn test_setup_works() { let reward_account = Pools::create_reward_account(last_pool); // the bonded_account should be bonded by the depositor's funds. - assert_eq!(StakingMock::active_stake(&bonded_account).unwrap(), 10); - assert_eq!(StakingMock::total_stake(&bonded_account).unwrap(), 10); + assert_eq!(StakingMock::stake(&bonded_account).map(|s| s.active).unwrap(), 10); + assert_eq!(StakingMock::stake(&bonded_account).map(|l| l.total).unwrap(), 10); // but not nominating yet. assert!(Nominations::get().is_none()); @@ -2368,7 +2368,7 @@ mod unbond { } ); - assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 0); + assert_eq!(StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), 0); }); } @@ -2415,7 +2415,10 @@ mod unbond { ] ); - assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 94); + assert_eq!( + StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), + 94 + ); assert_eq!( PoolMembers::::get(40).unwrap().unbonding_eras, member_unbonding_eras!(0 + 3 => 6) @@ -2443,7 +2446,10 @@ mod unbond { } } ); - assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 2); + assert_eq!( + StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), + 2 + ); assert_eq!( PoolMembers::::get(550).unwrap().unbonding_eras, member_unbonding_eras!(0 + 3 => 92) @@ -2486,7 +2492,10 @@ mod unbond { } } ); - assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 0); + assert_eq!( + StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), + 0 + ); assert_eq!(Balances::free_balance(&550), 550 + 550 + 92); assert_eq!( @@ -2614,7 +2623,10 @@ mod unbond { } } ); - assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 10); + assert_eq!( + StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), + 10 + ); assert_eq!( SubPoolsStorage::::get(1).unwrap(), SubPools { @@ -2724,7 +2736,7 @@ mod unbond { } } ); - assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 0); + assert_eq!(StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), 0); assert_eq!(*UnbondingBalanceMap::get().get(&default_bonded_account()).unwrap(), 10); }); } @@ -3083,18 +3095,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::stake(&default_bonded_account()).map(|l| l.active), Ok(5)); + assert_eq!(StakingMock::stake(&default_bonded_account()).map(|l| l.total), 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::stake(&default_bonded_account()).map(|l| l.active), Ok(5)); + assert_eq!(StakingMock::stake(&default_bonded_account()).map(|l| l.total), Ok(5)); assert_eq!(Balances::free_balance(&default_bonded_account()), 10); }); } @@ -3141,7 +3153,8 @@ mod withdraw_unbonded { ); StakingMock::set_bonded_balance( default_bonded_account(), - StakingMock::active_stake(&default_bonded_account()).unwrap() / 2, + StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap() / + 2, ); }; @@ -3270,7 +3283,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::stake(&default_bonded_account()).map(|l| l.total), Ok(300)); assert_ok!(fully_unbond_permissioned(40)); assert_ok!(fully_unbond_permissioned(550)); @@ -4074,12 +4087,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::stake(&next_pool_stash).map(|s| s.active), + 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 +4106,7 @@ mod create { PoolMembers::::get(11).unwrap(), PoolMember { pool_id: 2, - points: StakingMock::minimum_bond(), + points: StakingMock::minimum_nominator_bond(), ..Default::default() } ); @@ -4099,7 +4115,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 { @@ -4112,8 +4128,8 @@ mod create { } ); assert_eq!( - StakingMock::active_stake(&next_pool_stash).unwrap(), - StakingMock::minimum_bond() + StakingMock::stake(&next_pool_stash).map(|s| s.active).unwrap(), + StakingMock::minimum_nominator_bond() ); assert_eq!( RewardPools::::get(2).unwrap(), @@ -4142,7 +4158,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!( From 37107efacf011592608ce7dd6bd2eefb3a1afe29 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 19 Oct 2022 12:32:12 +0200 Subject: [PATCH 16/45] fix node code --- bin/node/runtime/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index f137b36eff036..22f5a40830753 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -585,7 +585,9 @@ 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 CurrencyBalance = Balance; + type Staking = pallet_staking::Pallet; type WeightInfo = (); } @@ -777,7 +779,7 @@ impl pallet_nomination_pools::Config for Runtime { type RewardCounter = FixedU128; type BalanceToU256 = BalanceToU256; type U256ToBalance = U256ToBalance; - type StakingInterface = pallet_staking::Pallet; + type Staking = pallet_staking::Pallet; type PostUnbondingPoolsWindow = PostUnbondPoolsWindow; type MaxMetadataLen = ConstU32<256>; type MaxUnbonding = ConstU32<8>; From 097a9d1327ad418e6096bf82df3f2c8238052378 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 19 Oct 2022 12:33:29 +0200 Subject: [PATCH 17/45] add mock comments --- frame/nomination-pools/src/mock.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 3f484a89cbfb2..08f93b4c7ed70 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -56,7 +56,7 @@ impl sp_staking::StakingInterface for StakingMock { } fn desired_validator_count() -> u32 { - todo!() + unimplemented!("method currently not used in testing") } fn current_era() -> EraIndex { @@ -113,7 +113,7 @@ impl sp_staking::StakingInterface for StakingMock { } fn stash_by_ctrl(_controller: &Self::AccountId) -> Result { - todo!() + unimplemented!("method currently not used in testing") } fn stake(who: &Self::AccountId) -> Result, DispatchError> { @@ -129,15 +129,15 @@ impl sp_staking::StakingInterface for StakingMock { } fn election_ongoing() -> bool { - todo!() + unimplemented!("method currently not used in testing") } fn force_unstake(_who: Self::AccountId) -> sp_runtime::DispatchResult { - todo!() + unimplemented!("method currently not used in testing") } fn is_exposed_in_era(_who: &Self::AccountId, _era: &EraIndex) -> bool { - todo!() + unimplemented!("method currently not used in testing") } #[cfg(feature = "runtime-benchmarks")] @@ -146,12 +146,12 @@ impl sp_staking::StakingInterface for StakingMock { _stash: &Self::AccountId, _exposures: Vec<(Self::AccountId, Self::Balance)>, ) { - todo!() + unimplemented!("method currently not used in testing") } #[cfg(feature = "runtime-benchmarks")] fn set_current_era(_era: EraIndex) { - todo!() + unimplemented!("method currently not used in testing") } } From 613a70132e3b4a9338b89bf41cf75430a754babc Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 19 Oct 2022 15:07:33 +0200 Subject: [PATCH 18/45] fix imports --- primitives/staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 94e2a49b00f9a..0d5246661cb5f 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -22,7 +22,7 @@ extern crate core; use sp_runtime::{DispatchError, DispatchResult}; -use sp_std::collections::btree_map::BTreeMap; +use sp_std::{collections::btree_map::BTreeMap, vec::Vec}; pub mod offence; From e785dd5504fbf58fe495e1856d8f58c653b7c813 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 19 Oct 2022 15:08:30 +0200 Subject: [PATCH 19/45] remove todo --- frame/staking/src/pallet/impls.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index fa9fe9c24fbc1..543ca07a5c251 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1535,15 +1535,6 @@ impl StakingInterface for Pallet { .ok_or(Error::::NotStash.into()) } - /// TODO: Find usages and depreacte those in favour of stake - // fn active_stake(who: &Self::AccountId) -> Option { - // Self::bonded(who).and_then(|c| Self::stake(c)).map(|l| l.active) - // } - // - // fn total_stake(who: &Self::AccountId) -> Option { - // Self::bonded(who).and_then(|c| Self::ledger(c)).map(|l| l.active) - // } - fn bond_extra(who: &Self::AccountId, extra: Self::Balance) -> DispatchResult { Self::bond_extra(RawOrigin::Signed(who.clone()).into(), extra) } From 03586d79ce69f7d963ffda541e69df747e049858 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 19 Oct 2022 15:21:51 +0200 Subject: [PATCH 20/45] more fixes --- frame/staking/src/pallet/impls.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 543ca07a5c251..1e0d5913ec775 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1592,10 +1592,9 @@ impl StakingInterface for Pallet { exposures: Vec<(Self::AccountId, Self::Balance)>, ) { let others = exposures - .map(|(who, value)| pallet_staking::IndividualExposure { who, value }) + .map(|(who, value)| IndividualExposure { who, value }) .collect::>(); - let exposure = - pallet_staking::Exposure { total: Default::default(), own: Default::default(), others }; + let exposure = Exposure { total: Default::default(), own: Default::default(), others }; Self::add_era_stakers(current_era.clone(), stash.clone(), &exposure) } From 20a4f295dd9ff942e1c3e9d9af212ef565d0fd6f Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 19 Oct 2022 16:10:22 +0200 Subject: [PATCH 21/45] more fixes --- frame/staking/src/pallet/impls.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 1e0d5913ec775..a9e9899b9761a 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1592,10 +1592,11 @@ impl StakingInterface for Pallet { exposures: Vec<(Self::AccountId, Self::Balance)>, ) { let others = exposures - .map(|(who, value)| IndividualExposure { who, value }) + .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) + Self::add_era_stakers(current_era.clone(), stash.clone(), exposure) } #[cfg(feature = "runtime-benchmarks")] From 9807cf292ccd21199bc1fa39e8bc626245478200 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 19 Oct 2022 16:32:22 +0200 Subject: [PATCH 22/45] bench fixes --- .../nomination-pools/benchmarking/src/lib.rs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 90f64d7abf7b4..b04c03afda8b7 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -182,7 +182,7 @@ impl ListScenario { self.origin1_member = Some(joiner.clone()); CurrencyOf::::make_free_balance_be(&joiner, amount * 2u32.into()); - let original_bonded = T::Staking::active_stake(&self.origin1).unwrap(); + let original_bonded = T::Staking::stake(&self.origin1).map(|s| s.active).unwrap(); // Unbond `amount` from the underlying pool account so when the member joins // we will maintain `current_bonded`. @@ -216,7 +216,7 @@ frame_benchmarking::benchmarks! { // setup the worst case list scenario. let scenario = ListScenario::::new(origin_weight, true)?; assert_eq!( - T::Staking::active_stake(&scenario.origin1).unwrap(), + T::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap(), origin_weight ); @@ -231,7 +231,7 @@ frame_benchmarking::benchmarks! { verify { assert_eq!(CurrencyOf::::free_balance(&joiner), joiner_free - max_additional); assert_eq!( - T::Staking::active_stake(&scenario.origin1).unwrap(), + T::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap(), scenario.dest_weight ); } @@ -246,7 +246,7 @@ frame_benchmarking::benchmarks! { }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::FreeBalance(extra)) verify { assert!( - T::Staking::active_stake(&scenario.origin1).unwrap() >= + T::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap() >= scenario.dest_weight ); } @@ -264,7 +264,7 @@ frame_benchmarking::benchmarks! { }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::Rewards) verify { assert!( - T::Staking::active_stake(&scenario.origin1).unwrap() >= + T::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap() >= scenario.dest_weight ); } @@ -311,7 +311,7 @@ frame_benchmarking::benchmarks! { whitelist_account!(member_id); }: _(RuntimeOrigin::Signed(member_id.clone()), member_id_lookup, all_points) verify { - let bonded_after = T::Staking::active_stake(&scenario.origin1).unwrap(); + let bonded_after = T::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap(); // We at least went down to the destination bag assert!(bonded_after <= scenario.dest_weight); let member = PoolMembers::::get( @@ -342,7 +342,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::Staking::active_stake(&pool_account).unwrap(), + T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), min_create_bond + min_join_bond ); assert_eq!(CurrencyOf::::free_balance(&joiner), min_join_bond); @@ -352,7 +352,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::Staking::active_stake(&pool_account).unwrap(), + T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), min_create_bond ); assert_eq!(pallet_staking::Ledger::::get(&pool_account).unwrap().unlocking.len(), 1); @@ -385,7 +385,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::Staking::active_stake(&pool_account).unwrap(), + T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), min_create_bond + min_join_bond ); assert_eq!(CurrencyOf::::free_balance(&joiner), min_join_bond); @@ -396,7 +396,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::Staking::active_stake(&pool_account).unwrap(), + T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), min_create_bond ); assert_eq!(pallet_staking::Ledger::::get(&pool_account).unwrap().unlocking.len(), 1); @@ -443,7 +443,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::Staking::active_stake(&pool_account).unwrap(), + T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), Zero::zero() ); assert_eq!( @@ -522,7 +522,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::Staking::active_stake(&Pools::::create_bonded_account(1)), + T::Staking::stake(&Pools::::create_bonded_account(1)).map(|s| s.active), Some(min_create_bond) ); } @@ -561,7 +561,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::Staking::active_stake(&Pools::::create_bonded_account(1)), + T::Staking::stake(&Pools::::create_bonded_account(1)).map(|s| s.active), Some(min_create_bond) ); } From 3639af8eda5e39838efa53e3c7ae04828311e10b Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 19 Oct 2022 16:48:41 +0200 Subject: [PATCH 23/45] more fixes --- frame/nomination-pools/benchmarking/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index b04c03afda8b7..c3fc7a2e1029b 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -132,16 +132,16 @@ impl ListScenario { sp_std::mem::forget(i); // Create accounts with the origin weight - let (pool_creator1, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); + let (pool_creator1, _) = create_pool_account::(USER_SEED + 1, origin_weight); T::Staking::nominate( - pool_origin1.clone(), + &pool_creator1, // 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); + let (pool_creator2, _) = create_pool_account::(USER_SEED + 2, origin_weight); T::Staking::nominate( - pool_origin2.clone(), + &pool_creator2, vec![account("random_validator", 0, USER_SEED)].clone(), )?; @@ -155,8 +155,8 @@ impl ListScenario { dest_weight_as_vote.try_into().map_err(|_| "could not convert u64 to Balance")?; // Create an account with the worst case destination weight - let (_, pool_dest1) = create_pool_account::(USER_SEED + 3, dest_weight); - T::Staking::nominate(pool_dest1.clone(), vec![account("random_validator", 0, USER_SEED)])?; + let (pool_creator3, _) = create_pool_account::(USER_SEED + 3, dest_weight); + T::Staking::nominate(&pool_creator3, 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); @@ -644,7 +644,7 @@ frame_benchmarking::benchmarks! { .map(|i| account("stash", USER_SEED, i)) .collect(); - assert_ok!(Pools::::nominate(RuntimeOrigin::Signed(depositor.clone()).into(), 1, validators)); + assert_ok!(Pools::::nominate(&pool_account, 1, validators)); assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_some()); whitelist_account!(depositor); From b7827d5b77deb531d62b162152709423ff72c870 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 20 Oct 2022 09:28:45 +0200 Subject: [PATCH 24/45] more fixes --- frame/nomination-pools/benchmarking/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index c3fc7a2e1029b..bbd5050f8f9fb 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -132,14 +132,14 @@ impl ListScenario { sp_std::mem::forget(i); // Create accounts with the origin weight - let (pool_creator1, _) = create_pool_account::(USER_SEED + 1, origin_weight); + let (pool_creator1, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); T::Staking::nominate( &pool_creator1, // NOTE: these don't really need to be validators. vec![account("random_validator", 0, USER_SEED)], )?; - let (pool_creator2, _) = create_pool_account::(USER_SEED + 2, origin_weight); + let (pool_creator2, pool_origin2) = create_pool_account::(USER_SEED + 2, origin_weight); T::Staking::nominate( &pool_creator2, vec![account("random_validator", 0, USER_SEED)].clone(), @@ -155,7 +155,7 @@ impl ListScenario { dest_weight_as_vote.try_into().map_err(|_| "could not convert u64 to Balance")?; // Create an account with the worst case destination weight - let (pool_creator3, _) = create_pool_account::(USER_SEED + 3, dest_weight); + let (pool_creator3, pool_dest1) = create_pool_account::(USER_SEED + 3, dest_weight); T::Staking::nominate(&pool_creator3, vec![account("random_validator", 0, USER_SEED)])?; let weight_of = pallet_staking::Pallet::::weight_of_fn(); From 5232b99f5691fb37f0029db8c767215805da4100 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 20 Oct 2022 09:31:53 +0200 Subject: [PATCH 25/45] import fix --- primitives/staking/src/lib.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 0d5246661cb5f..4fe1d9b765585 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -131,10 +131,7 @@ pub trait StakingInterface { -> DispatchResult; /// Have `who` nominate `validators`. - fn nominate( - who: &Self::AccountId, - validators: sp_std::vec::Vec, - ) -> DispatchResult; + fn nominate(who: &Self::AccountId, validators: Vec) -> DispatchResult; /// Chill `who`. fn chill(who: &Self::AccountId) -> DispatchResult; @@ -177,7 +174,7 @@ pub trait StakingInterface { /// 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( From f8814566ae935065a60fba0d70252c5481f8e1a2 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 20 Oct 2022 09:38:06 +0200 Subject: [PATCH 26/45] more fixes --- frame/nomination-pools/benchmarking/src/lib.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index bbd5050f8f9fb..8c2947fc2dff6 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -186,8 +186,7 @@ impl ListScenario { // Unbond `amount` from the underlying pool account so when the member joins // we will maintain `current_bonded`. - T::Staking::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| { @@ -523,7 +522,7 @@ frame_benchmarking::benchmarks! { ); assert_eq!( T::Staking::stake(&Pools::::create_bonded_account(1)).map(|s| s.active), - Some(min_create_bond) + Ok(min_create_bond) ); } @@ -562,7 +561,7 @@ frame_benchmarking::benchmarks! { ); assert_eq!( T::Staking::stake(&Pools::::create_bonded_account(1)).map(|s| s.active), - Some(min_create_bond) + Ok(min_create_bond) ); } @@ -644,7 +643,7 @@ frame_benchmarking::benchmarks! { .map(|i| account("stash", USER_SEED, i)) .collect(); - assert_ok!(Pools::::nominate(&pool_account, 1, validators)); + assert_ok!(Pools::::nominate(pool_account, 1, validators)); assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_some()); whitelist_account!(depositor); From c3cee71829d23e888900dbe47cb051b5c36bec26 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 20 Oct 2022 09:41:54 +0200 Subject: [PATCH 27/45] more bench fix --- frame/nomination-pools/benchmarking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 8c2947fc2dff6..c2c7290d8ae97 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -643,7 +643,7 @@ frame_benchmarking::benchmarks! { .map(|i| account("stash", USER_SEED, i)) .collect(); - assert_ok!(Pools::::nominate(pool_account, 1, validators)); + assert_ok!(Pools::::nominate(depositor, 1, validators)); assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_some()); whitelist_account!(depositor); From 068c565e436b691700ac57ecb2df7def487b1ad1 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 20 Oct 2022 09:46:50 +0200 Subject: [PATCH 28/45] refix --- frame/nomination-pools/benchmarking/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index c2c7290d8ae97..2cd0af059da47 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -132,16 +132,16 @@ impl ListScenario { sp_std::mem::forget(i); // Create accounts with the origin weight - let (pool_creator1, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); + let (_, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); T::Staking::nominate( - &pool_creator1, + &pool_origin1, // NOTE: these don't really need to be validators. vec![account("random_validator", 0, USER_SEED)], )?; - let (pool_creator2, pool_origin2) = create_pool_account::(USER_SEED + 2, origin_weight); + let (_, pool_origin2) = create_pool_account::(USER_SEED + 2, origin_weight); T::Staking::nominate( - &pool_creator2, + &pool_origin2, vec![account("random_validator", 0, USER_SEED)].clone(), )?; @@ -155,8 +155,8 @@ impl ListScenario { dest_weight_as_vote.try_into().map_err(|_| "could not convert u64 to Balance")?; // Create an account with the worst case destination weight - let (pool_creator3, pool_dest1) = create_pool_account::(USER_SEED + 3, dest_weight); - T::Staking::nominate(&pool_creator3, vec![account("random_validator", 0, USER_SEED)])?; + let (_, pool_dest1) = create_pool_account::(USER_SEED + 3, dest_weight); + 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); @@ -643,7 +643,7 @@ frame_benchmarking::benchmarks! { .map(|i| account("stash", USER_SEED, i)) .collect(); - assert_ok!(Pools::::nominate(depositor, 1, validators)); + assert_ok!(T::Staking::nominate(&pool_account, validators)); assert!(T::Staking::nominations(Pools::::create_bonded_account(1)).is_some()); whitelist_account!(depositor); From ff36f90ac006a3547fbca72658691ffc05f74be6 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 20 Oct 2022 09:50:34 +0200 Subject: [PATCH 29/45] refix --- frame/nomination-pools/benchmarking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 2cd0af059da47..cdaf670ca327f 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -132,7 +132,7 @@ impl ListScenario { sp_std::mem::forget(i); // Create accounts with the origin weight - let (_, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); + let (pool_creator1, pool_origin1) = create_pool_account::(USER_SEED + 1, origin_weight); T::Staking::nominate( &pool_origin1, // NOTE: these don't really need to be validators. From 6f862e4493ea71d3d3aa98f2c910443b39ce33ea Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 25 Oct 2022 13:35:27 +0200 Subject: [PATCH 30/45] Update primitives/staking/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- primitives/staking/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 4fe1d9b765585..715414bd8c1ad 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -19,7 +19,6 @@ //! A crate which contains primitives that are useful for implementation that uses staking //! approaches in general. Definitions related to sessions, slashing, etc go here. -extern crate core; use sp_runtime::{DispatchError, DispatchResult}; use sp_std::{collections::btree_map::BTreeMap, vec::Vec}; From bff58a46837f3318be2aebb34fac719992069ba1 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 20 Oct 2022 13:17:45 +0200 Subject: [PATCH 31/45] is_unbonding returns a result --- frame/fast-unstake/src/lib.rs | 2 +- primitives/staking/src/lib.rs | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 1b8c93ec20891..12a111c07b8a5 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -241,7 +241,7 @@ pub mod pallet { Error::::AlreadyHead ); - ensure!(!T::Staking::is_unbonding(&stash), Error::::NotFullyBonded); + ensure!(!T::Staking::is_unbonding(&stash)?, Error::::NotFullyBonded); // chill and fully unstake. T::Staking::chill(&stash)?; diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 715414bd8c1ad..a9a033a200757 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -113,12 +113,8 @@ pub trait StakingInterface { /// Returns the stake of `who`. fn stake(who: &Self::AccountId) -> Result, DispatchError>; - /// TODO: Possibly return a result here too for consistency. - fn is_unbonding(who: &Self::AccountId) -> bool { - match Self::stake(who) { - Ok(stake) if stake.active == stake.total => false, - _ => true, - } + fn is_unbonding(who: &Self::AccountId) -> Result { + Self::stake(who).map(|s| stake.active != stake.total) } fn fully_unbond(who: &Self::AccountId) -> DispatchResult { From bdb525e7b3eef6356b88aa731c690d4be7e860a9 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 25 Oct 2022 14:20:56 +0200 Subject: [PATCH 32/45] fix --- bin/node/runtime/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 22f5a40830753..be338d136cf33 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -587,7 +587,7 @@ impl pallet_fast_unstake::Config for Runtime { type Deposit = ConstU128<{ DOLLARS }>; type Currency = Balances; type CurrencyBalance = Balance; - type Staking = pallet_staking::Pallet; + type Staking = Staking; type WeightInfo = (); } @@ -779,7 +779,7 @@ impl pallet_nomination_pools::Config for Runtime { type RewardCounter = FixedU128; type BalanceToU256 = BalanceToU256; type U256ToBalance = U256ToBalance; - type Staking = pallet_staking::Pallet; + type Staking = Staking; type PostUnbondingPoolsWindow = PostUnbondPoolsWindow; type MaxMetadataLen = ConstU32<256>; type MaxUnbonding = ConstU32<8>; From 4bd57b15315b892722e9d9be90f22b6b5841a319 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 25 Oct 2022 14:30:04 +0200 Subject: [PATCH 33/45] review fixes --- bin/node/runtime/src/lib.rs | 1 - frame/fast-unstake/src/lib.rs | 44 ++++++++++++++-------------------- frame/fast-unstake/src/mock.rs | 1 - primitives/staking/src/lib.rs | 2 +- 4 files changed, 19 insertions(+), 29 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index be338d136cf33..05734bf22b725 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -586,7 +586,6 @@ impl pallet_fast_unstake::Config for Runtime { type ControlOrigin = frame_system::EnsureRoot; type Deposit = ConstU128<{ DOLLARS }>; type Currency = Balances; - type CurrencyBalance = Balance; type Staking = Staking; type WeightInfo = (); } diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index 12a111c07b8a5..0477bb251aa00 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -114,7 +114,7 @@ pub mod pallet { + TryInto>; /// The currency used for deposits. - type Currency: 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. @@ -123,18 +123,6 @@ pub mod pallet { /// The origin that can control this pallet. type ControlOrigin: frame_support::traits::EnsureOrigin; - /// Just the `Currency::Balance` type; we have this item to allow us to constrain it to - /// `From`. - type CurrencyBalance: sp_runtime::traits::AtLeast32BitUnsigned - + codec::FullCodec - + Copy - + MaybeSerializeDeserialize - + sp_std::fmt::Debug - + Default - + From - + TypeInfo - + MaxEncodedLen; - /// The access to staking functionality. type Staking: StakingInterface, AccountId = Self::AccountId>; @@ -234,23 +222,25 @@ pub mod pallet { let ctrl = ensure_signed(origin)?; ensure!(ErasToCheckPerBlock::::get() != 0, >::CallNotAllowed); - let stash = T::Staking::stash_by_ctrl(&ctrl).map_err(|_| Error::::NotController)?; - ensure!(!Queue::::contains_key(&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 != stash), + Head::::get() + .map_or(true, |UnstakeRequest { stash, .. }| stash_account != stash), Error::::AlreadyHead ); - ensure!(!T::Staking::is_unbonding(&stash)?, Error::::NotFullyBonded); + ensure!(!T::Staking::is_unbonding(&stash_account)?, Error::::NotFullyBonded); // chill and fully unstake. - T::Staking::chill(&stash)?; - T::Staking::fully_unbond(&stash)?; + T::Staking::chill(&stash_account)?; + T::Staking::fully_unbond(&stash_account)?; - T::Currency::reserve(&stash, T::Deposit::get())?; + T::Currency::reserve(&stash_account, T::Deposit::get())?; // enqueue them. - Queue::::insert(stash, T::Deposit::get()); + Queue::::insert(stash_account, T::Deposit::get()); Ok(()) } @@ -267,16 +257,18 @@ pub mod pallet { ensure!(ErasToCheckPerBlock::::get() != 0, >::CallNotAllowed); - let stash = T::Staking::stash_by_ctrl(&ctrl).map_err(|_| 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::Currency::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); diff --git a/frame/fast-unstake/src/mock.rs b/frame/fast-unstake/src/mock.rs index a6a4ab69b61f2..87e89d90d6304 100644 --- a/frame/fast-unstake/src/mock.rs +++ b/frame/fast-unstake/src/mock.rs @@ -175,7 +175,6 @@ impl fast_unstake::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Deposit = DepositAmount; type Currency = Balances; - type CurrencyBalance = Balance; type Staking = Staking; type ControlOrigin = frame_system::EnsureRoot; type WeightInfo = (); diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index a9a033a200757..6f7ffab383d5a 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -114,7 +114,7 @@ pub trait StakingInterface { fn stake(who: &Self::AccountId) -> Result, DispatchError>; fn is_unbonding(who: &Self::AccountId) -> Result { - Self::stake(who).map(|s| stake.active != stake.total) + Self::stake(who).map(|s| s.active != s.total) } fn fully_unbond(who: &Self::AccountId) -> DispatchResult { From f6040ecdd3e16208048c78eab9a82d0a5e4a33d2 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 25 Oct 2022 14:58:16 +0200 Subject: [PATCH 34/45] more review fixes --- .../nomination-pools/benchmarking/src/lib.rs | 26 +++++++++---------- frame/nomination-pools/src/lib.rs | 22 +++++++--------- frame/nomination-pools/src/tests.rs | 8 +++--- primitives/staking/src/lib.rs | 8 ++++++ 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index cdaf670ca327f..62b3074dbffc3 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -182,7 +182,7 @@ impl ListScenario { self.origin1_member = Some(joiner.clone()); CurrencyOf::::make_free_balance_be(&joiner, amount * 2u32.into()); - let original_bonded = T::Staking::stake(&self.origin1).map(|s| s.active).unwrap(); + let original_bonded = T::Staking::active_balance(&self.origin1).unwrap(); // Unbond `amount` from the underlying pool account so when the member joins // we will maintain `current_bonded`. @@ -215,7 +215,7 @@ frame_benchmarking::benchmarks! { // setup the worst case list scenario. let scenario = ListScenario::::new(origin_weight, true)?; assert_eq!( - T::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap(), + T::Staking::active_balance(&scenario.origin1).unwrap(), origin_weight ); @@ -230,7 +230,7 @@ frame_benchmarking::benchmarks! { verify { assert_eq!(CurrencyOf::::free_balance(&joiner), joiner_free - max_additional); assert_eq!( - T::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap(), + T::Staking::active_balance(&scenario.origin1).unwrap(), scenario.dest_weight ); } @@ -245,7 +245,7 @@ frame_benchmarking::benchmarks! { }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::FreeBalance(extra)) verify { assert!( - T::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap() >= + T::Staking::active_balance(&scenario.origin1).unwrap() >= scenario.dest_weight ); } @@ -263,7 +263,7 @@ frame_benchmarking::benchmarks! { }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::Rewards) verify { assert!( - T::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap() >= + T::Staking::active_balance(&scenario.origin1).unwrap() >= scenario.dest_weight ); } @@ -310,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::Staking::stake(&scenario.origin1).map(|s| s.active).unwrap(); + let bonded_after = T::Staking::active_balance(&scenario.origin1).unwrap(); // We at least went down to the destination bag assert!(bonded_after <= scenario.dest_weight); let member = PoolMembers::::get( @@ -341,7 +341,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), + T::Staking::active_balance(&pool_account).unwrap(), min_create_bond + min_join_bond ); assert_eq!(CurrencyOf::::free_balance(&joiner), min_join_bond); @@ -351,7 +351,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), + T::Staking::active_balance(&pool_account).unwrap(), min_create_bond ); assert_eq!(pallet_staking::Ledger::::get(&pool_account).unwrap().unlocking.len(), 1); @@ -384,7 +384,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), + T::Staking::active_balance(&pool_account).unwrap(), min_create_bond + min_join_bond ); assert_eq!(CurrencyOf::::free_balance(&joiner), min_join_bond); @@ -395,7 +395,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), + T::Staking::active_balance(&pool_account).unwrap(), min_create_bond ); assert_eq!(pallet_staking::Ledger::::get(&pool_account).unwrap().unlocking.len(), 1); @@ -442,7 +442,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::Staking::stake(&pool_account).map(|s| s.active).unwrap(), + T::Staking::active_balance(&pool_account).unwrap(), Zero::zero() ); assert_eq!( @@ -521,7 +521,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::Staking::stake(&Pools::::create_bonded_account(1)).map(|s| s.active), + T::Staking::active_balance(&Pools::::create_bonded_account(1)), Ok(min_create_bond) ); } @@ -560,7 +560,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::Staking::stake(&Pools::::create_bonded_account(1)).map(|s| s.active), + T::Staking::active_balance(&Pools::::create_bonded_account(1)), Ok(min_create_bond) ); } diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 149e0cef594e7..56f9fd0aef368 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -622,9 +622,8 @@ 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::Staking::stake(&self.bonded_account()) - .map(|s| s.active) - .unwrap_or(Zero::zero()); + let bonded_balance = + T::Staking::active_balance(&self.bonded_account()).unwrap_or(Zero::zero()); Pallet::::balance_to_point(bonded_balance, self.points, new_funds) } @@ -632,9 +631,8 @@ impl BondedPool { /// /// This is often used for unbonding. fn points_to_balance(&self, points: BalanceOf) -> BalanceOf { - let bonded_balance = T::Staking::stake(&self.bonded_account()) - .map(|s| s.active) - .unwrap_or(Zero::zero()); + let bonded_balance = + T::Staking::active_balance(&self.bonded_account()).unwrap_or(Zero::zero()); Pallet::::point_to_balance(bonded_balance, self.points, points) } @@ -685,7 +683,7 @@ impl BondedPool { fn transferrable_balance(&self) -> BalanceOf { let account = self.bonded_account(); T::Currency::free_balance(&account) - .saturating_sub(T::Staking::stake(&account).map(|s| s.active).unwrap_or_default()) + .saturating_sub(T::Staking::active_balance(&account).unwrap_or_default()) } fn is_root(&self, who: &T::AccountId) -> bool { @@ -739,9 +737,8 @@ impl BondedPool { fn ok_to_be_open(&self, new_funds: BalanceOf) -> Result<(), DispatchError> { ensure!(!self.is_destroying(), Error::::CanNotChangeState); - let bonded_balance = T::Staking::stake(&self.bonded_account()) - .map(|s| s.active) - .unwrap_or(Zero::zero()); + let bonded_balance = + T::Staking::active_balance(&self.bonded_account()).unwrap_or(Zero::zero()); ensure!(!bonded_balance.is_zero(), Error::::OverflowRisk); let points_to_balance_ratio_floor = self @@ -2205,7 +2202,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::Staking::stake(&bonded_account).map(|s| s.total).unwrap_or_default(), + T::Staking::total_balance(&bonded_account).unwrap_or_default(), Zero::zero() ); @@ -2480,8 +2477,7 @@ impl Pallet { let subs = SubPoolsStorage::::get(pool_id).unwrap_or_default(); let sum_unbonding_balance = subs.sum_unbonding_balance(); - let bonded_balance = - T::Staking::stake(&pool_account).map(|s| s.active).unwrap_or_default(); + let bonded_balance = T::Staking::active_balance(&pool_account).unwrap_or_default(); let total_balance = T::Currency::total_balance(&pool_account); assert!( diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index ff5cbf6448400..dbd8b6ad09b80 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -79,8 +79,8 @@ fn test_setup_works() { let reward_account = Pools::create_reward_account(last_pool); // the bonded_account should be bonded by the depositor's funds. - assert_eq!(StakingMock::stake(&bonded_account).map(|s| s.active).unwrap(), 10); - assert_eq!(StakingMock::stake(&bonded_account).map(|l| l.total).unwrap(), 10); + assert_eq!(StakingMock::active_balance(&bonded_account).unwrap(), 10); + assert_eq!(StakingMock::total_balance(&bonded_account).unwrap(), 10); // but not nominating yet. assert!(Nominations::get().is_none()); @@ -4088,7 +4088,7 @@ mod create { assert!(!RewardPools::::contains_key(2)); assert!(!PoolMembers::::contains_key(11)); assert_err!( - StakingMock::stake(&next_pool_stash).map(|s| s.active), + StakingMock::active_balance(&next_pool_stash), DispatchError::Other("balance not found") ); @@ -4128,7 +4128,7 @@ mod create { } ); assert_eq!( - StakingMock::stake(&next_pool_stash).map(|s| s.active).unwrap(), + StakingMock::active_balance(&next_pool_stash).unwrap(), StakingMock::minimum_nominator_bond() ); assert_eq!( diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 6f7ffab383d5a..60b301ef44c23 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -113,6 +113,14 @@ pub trait StakingInterface { /// Returns the stake of `who`. fn stake(who: &Self::AccountId) -> Result, DispatchError>; + fn total_balance(who: &Self::AccountId) -> Result { + Self::stake(who).map(|s| s.total) + } + + fn active_balance(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) } From 8c5604d42c8b51f41c2c98ea19b05b45341b4996 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 25 Oct 2022 15:00:47 +0200 Subject: [PATCH 35/45] more fixes --- frame/nomination-pools/src/tests.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index dbd8b6ad09b80..f8bae4e9450da 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -3097,16 +3097,16 @@ mod pool_withdraw_unbonded { // Given 10 unbond'ed directly against the pool account assert_ok!(StakingMock::unbond(&default_bonded_account(), 5)); // and the pool account only has 10 balance - assert_eq!(StakingMock::stake(&default_bonded_account()).map(|l| l.active), Ok(5)); - assert_eq!(StakingMock::stake(&default_bonded_account()).map(|l| l.total), Ok(10)); + assert_eq!(StakingMock::active_balance(&default_bonded_account()), Ok(5)); + assert_eq!(StakingMock::total_balance(&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::stake(&default_bonded_account()).map(|l| l.active), Ok(5)); - assert_eq!(StakingMock::stake(&default_bonded_account()).map(|l| l.total), Ok(5)); + assert_eq!(StakingMock::active_balance(&default_bonded_account()), Ok(5)); + assert_eq!(StakingMock::total_balance(&default_bonded_account()), Ok(5)); assert_eq!(Balances::free_balance(&default_bonded_account()), 10); }); } @@ -3283,7 +3283,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::stake(&default_bonded_account()).map(|l| l.total), Ok(300)); + assert_eq!(StakingMock::total_balance(&default_bonded_account()), Ok(300)); assert_ok!(fully_unbond_permissioned(40)); assert_ok!(fully_unbond_permissioned(550)); From 731fab2a8436700bdf6b9b9cfc2ac7bc600ed308 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Tue, 25 Oct 2022 15:02:21 +0200 Subject: [PATCH 36/45] more fixes --- frame/nomination-pools/src/tests.rs | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index f8bae4e9450da..425abf1aabe12 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -2368,7 +2368,7 @@ mod unbond { } ); - assert_eq!(StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), 0); + assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 0); }); } @@ -2415,10 +2415,7 @@ mod unbond { ] ); - assert_eq!( - StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), - 94 - ); + assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 94); assert_eq!( PoolMembers::::get(40).unwrap().unbonding_eras, member_unbonding_eras!(0 + 3 => 6) @@ -2446,10 +2443,7 @@ mod unbond { } } ); - assert_eq!( - StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), - 2 - ); + assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 2); assert_eq!( PoolMembers::::get(550).unwrap().unbonding_eras, member_unbonding_eras!(0 + 3 => 92) @@ -2492,10 +2486,7 @@ mod unbond { } } ); - assert_eq!( - StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), - 0 - ); + assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 0); assert_eq!(Balances::free_balance(&550), 550 + 550 + 92); assert_eq!( @@ -2623,10 +2614,7 @@ mod unbond { } } ); - assert_eq!( - StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), - 10 - ); + assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 10); assert_eq!( SubPoolsStorage::::get(1).unwrap(), SubPools { @@ -2736,7 +2724,7 @@ mod unbond { } } ); - assert_eq!(StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap(), 0); + assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 0); assert_eq!(*UnbondingBalanceMap::get().get(&default_bonded_account()).unwrap(), 10); }); } @@ -3153,8 +3141,7 @@ mod withdraw_unbonded { ); StakingMock::set_bonded_balance( default_bonded_account(), - StakingMock::stake(&default_bonded_account()).map(|l| l.active).unwrap() / - 2, + StakingMock::active_balance(&default_bonded_account()).unwrap() / 2, ); }; From b90082a337c8656f4b300535c201f9689aa78694 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 26 Oct 2022 09:03:22 +0200 Subject: [PATCH 37/45] Update frame/fast-unstake/src/benchmarking.rs Co-authored-by: Squirrel --- frame/fast-unstake/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/fast-unstake/src/benchmarking.rs b/frame/fast-unstake/src/benchmarking.rs index d85f0c8c8f157..762a59c96bcfa 100644 --- a/frame/fast-unstake/src/benchmarking.rs +++ b/frame/fast-unstake/src/benchmarking.rs @@ -121,8 +121,8 @@ benchmarks! { // number of eras multiplied by validators in that era. let x in (T::Staking::bonding_duration() * 1) .. (T::Staking::bonding_duration() * MAX_VALIDATORS); - let v = x / T::Staking::bonding_duration(); let u = T::Staking::bonding_duration(); + let v = x / u; ErasToCheckPerBlock::::put(u); T::Staking::set_current_era(u); From 1eee8a59ad4649183029468941f8bf8ea3e48aba Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 26 Oct 2022 10:43:41 +0200 Subject: [PATCH 38/45] remove redundant CurrencyBalance from nom-pools --- .../nomination-pools/benchmarking/src/mock.rs | 1 - frame/nomination-pools/src/lib.rs | 18 ++---------------- frame/nomination-pools/src/mock.rs | 1 - .../nomination-pools/test-staking/src/mock.rs | 1 - 4 files changed, 2 insertions(+), 19 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/mock.rs b/frame/nomination-pools/benchmarking/src/mock.rs index d82d071086e1e..db01989f2b563 100644 --- a/frame/nomination-pools/benchmarking/src/mock.rs +++ b/frame/nomination-pools/benchmarking/src/mock.rs @@ -157,7 +157,6 @@ 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; diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 56f9fd0aef368..6f12c21ec1809 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}; @@ -1133,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); @@ -1152,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. /// diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 08f93b4c7ed70..a21e58a7b370f 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -222,7 +222,6 @@ 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; diff --git a/frame/nomination-pools/test-staking/src/mock.rs b/frame/nomination-pools/test-staking/src/mock.rs index ece1798ade0e2..5758b884e348d 100644 --- a/frame/nomination-pools/test-staking/src/mock.rs +++ b/frame/nomination-pools/test-staking/src/mock.rs @@ -171,7 +171,6 @@ 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; From 1f4fad271565f420e0f515bc5cfd2bd6b7f15ad8 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 26 Oct 2022 10:47:58 +0200 Subject: [PATCH 39/45] remove CB --- bin/node/runtime/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 05734bf22b725..d7a363bd0f9c2 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -774,7 +774,6 @@ 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; From 90cec9f466f77aedd1ee136e13f3a2c05d98c04b Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 26 Oct 2022 13:15:43 +0200 Subject: [PATCH 40/45] rephrase --- primitives/staking/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 60b301ef44c23..5bb48e8cc8831 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -67,7 +67,8 @@ pub struct Stake { /// # 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. + /// 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. From b1560aaafdb7ab8904d3cc44279ff1eaabdb1f44 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Thu, 27 Oct 2022 08:25:36 +0200 Subject: [PATCH 41/45] Apply suggestions from code review --- frame/nomination-pools/src/lib.rs | 12 ++++++------ frame/nomination-pools/src/tests.rs | 28 ++++++++++++++-------------- primitives/staking/src/lib.rs | 4 ++-- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 6f12c21ec1809..e3ebf5d40b437 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -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::Staking::active_balance(&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::Staking::active_balance(&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::Staking::active_balance(&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::Staking::active_balance(&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 @@ -2188,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::Staking::total_balance(&bonded_account).unwrap_or_default(), + T::Staking::total_stake(&bonded_account).unwrap_or_default(), Zero::zero() ); @@ -2463,7 +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::Staking::active_balance(&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/tests.rs b/frame/nomination-pools/src/tests.rs index 425abf1aabe12..48030d018bf2a 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -79,8 +79,8 @@ fn test_setup_works() { let reward_account = Pools::create_reward_account(last_pool); // the bonded_account should be bonded by the depositor's funds. - assert_eq!(StakingMock::active_balance(&bonded_account).unwrap(), 10); - assert_eq!(StakingMock::total_balance(&bonded_account).unwrap(), 10); + assert_eq!(StakingMock::active_stake(&bonded_account).unwrap(), 10); + assert_eq!(StakingMock::total_stake(&bonded_account).unwrap(), 10); // but not nominating yet. assert!(Nominations::get().is_none()); @@ -2368,7 +2368,7 @@ mod unbond { } ); - assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 0); + assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 0); }); } @@ -2415,7 +2415,7 @@ mod unbond { ] ); - assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 94); + assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 94); assert_eq!( PoolMembers::::get(40).unwrap().unbonding_eras, member_unbonding_eras!(0 + 3 => 6) @@ -2443,7 +2443,7 @@ mod unbond { } } ); - assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 2); + assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 2); assert_eq!( PoolMembers::::get(550).unwrap().unbonding_eras, member_unbonding_eras!(0 + 3 => 92) @@ -2486,7 +2486,7 @@ mod unbond { } } ); - assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 0); + assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 0); assert_eq!(Balances::free_balance(&550), 550 + 550 + 92); assert_eq!( @@ -2614,7 +2614,7 @@ mod unbond { } } ); - assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 10); + assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 10); assert_eq!( SubPoolsStorage::::get(1).unwrap(), SubPools { @@ -2724,7 +2724,7 @@ mod unbond { } } ); - assert_eq!(StakingMock::active_balance(&default_bonded_account()).unwrap(), 0); + assert_eq!(StakingMock::active_stake(&default_bonded_account()).unwrap(), 0); assert_eq!(*UnbondingBalanceMap::get().get(&default_bonded_account()).unwrap(), 10); }); } @@ -3085,16 +3085,16 @@ mod pool_withdraw_unbonded { // Given 10 unbond'ed directly against the pool account assert_ok!(StakingMock::unbond(&default_bonded_account(), 5)); // and the pool account only has 10 balance - assert_eq!(StakingMock::active_balance(&default_bonded_account()), Ok(5)); - assert_eq!(StakingMock::total_balance(&default_bonded_account()), Ok(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_balance(&default_bonded_account()), Ok(5)); - assert_eq!(StakingMock::total_balance(&default_bonded_account()), Ok(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); }); } @@ -3141,7 +3141,7 @@ mod withdraw_unbonded { ); StakingMock::set_bonded_balance( default_bonded_account(), - StakingMock::active_balance(&default_bonded_account()).unwrap() / 2, + StakingMock::active_stake(&default_bonded_account()).unwrap() / 2, ); }; @@ -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_balance(&default_bonded_account()), Ok(300)); + assert_eq!(StakingMock::total_stake(&default_bonded_account()), Ok(300)); assert_ok!(fully_unbond_permissioned(40)); assert_ok!(fully_unbond_permissioned(550)); diff --git a/primitives/staking/src/lib.rs b/primitives/staking/src/lib.rs index 5bb48e8cc8831..703f0abe80458 100644 --- a/primitives/staking/src/lib.rs +++ b/primitives/staking/src/lib.rs @@ -114,11 +114,11 @@ pub trait StakingInterface { /// Returns the stake of `who`. fn stake(who: &Self::AccountId) -> Result, DispatchError>; - fn total_balance(who: &Self::AccountId) -> Result { + fn total_stake(who: &Self::AccountId) -> Result { Self::stake(who).map(|s| s.total) } - fn active_balance(who: &Self::AccountId) -> Result { + fn active_stake(who: &Self::AccountId) -> Result { Self::stake(who).map(|s| s.active) } From 75a36bbe20c6a15f488e253d8f45716775b5bbd5 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Thu, 27 Oct 2022 08:28:51 +0200 Subject: [PATCH 42/45] Update frame/nomination-pools/src/tests.rs --- frame/nomination-pools/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 48030d018bf2a..75c31c1f4d67f 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4075,7 +4075,7 @@ mod create { assert!(!RewardPools::::contains_key(2)); assert!(!PoolMembers::::contains_key(11)); assert_err!( - StakingMock::active_balance(&next_pool_stash), + StakingMock::active_stake(&next_pool_stash), DispatchError::Other("balance not found") ); From b3e843e56b58bd231d6afec921117f84b0ad7c5c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 27 Oct 2022 11:46:33 +0200 Subject: [PATCH 43/45] finish damn renamed --- .../nomination-pools/benchmarking/src/lib.rs | 26 +++++++++---------- frame/nomination-pools/src/lib.rs | 4 +-- frame/nomination-pools/src/tests.rs | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/frame/nomination-pools/benchmarking/src/lib.rs b/frame/nomination-pools/benchmarking/src/lib.rs index 62b3074dbffc3..9b063539152b7 100644 --- a/frame/nomination-pools/benchmarking/src/lib.rs +++ b/frame/nomination-pools/benchmarking/src/lib.rs @@ -182,7 +182,7 @@ impl ListScenario { self.origin1_member = Some(joiner.clone()); CurrencyOf::::make_free_balance_be(&joiner, amount * 2u32.into()); - let original_bonded = T::Staking::active_balance(&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`. @@ -215,7 +215,7 @@ frame_benchmarking::benchmarks! { // setup the worst case list scenario. let scenario = ListScenario::::new(origin_weight, true)?; assert_eq!( - T::Staking::active_balance(&scenario.origin1).unwrap(), + T::Staking::active_stake(&scenario.origin1).unwrap(), origin_weight ); @@ -230,7 +230,7 @@ frame_benchmarking::benchmarks! { verify { assert_eq!(CurrencyOf::::free_balance(&joiner), joiner_free - max_additional); assert_eq!( - T::Staking::active_balance(&scenario.origin1).unwrap(), + T::Staking::active_stake(&scenario.origin1).unwrap(), scenario.dest_weight ); } @@ -245,7 +245,7 @@ frame_benchmarking::benchmarks! { }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::FreeBalance(extra)) verify { assert!( - T::Staking::active_balance(&scenario.origin1).unwrap() >= + T::Staking::active_stake(&scenario.origin1).unwrap() >= scenario.dest_weight ); } @@ -263,7 +263,7 @@ frame_benchmarking::benchmarks! { }: bond_extra(RuntimeOrigin::Signed(scenario.creator1.clone()), BondExtra::Rewards) verify { assert!( - T::Staking::active_balance(&scenario.origin1).unwrap() >= + T::Staking::active_stake(&scenario.origin1).unwrap() >= scenario.dest_weight ); } @@ -310,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::Staking::active_balance(&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( @@ -341,7 +341,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::Staking::active_balance(&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); @@ -351,7 +351,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::Staking::active_balance(&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); @@ -384,7 +384,7 @@ frame_benchmarking::benchmarks! { // Sanity check join worked assert_eq!( - T::Staking::active_balance(&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); @@ -395,7 +395,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::Staking::active_balance(&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); @@ -442,7 +442,7 @@ frame_benchmarking::benchmarks! { // Sanity check that unbond worked assert_eq!( - T::Staking::active_balance(&pool_account).unwrap(), + T::Staking::active_stake(&pool_account).unwrap(), Zero::zero() ); assert_eq!( @@ -521,7 +521,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::Staking::active_balance(&Pools::::create_bonded_account(1)), + T::Staking::active_stake(&Pools::::create_bonded_account(1)), Ok(min_create_bond) ); } @@ -560,7 +560,7 @@ frame_benchmarking::benchmarks! { } ); assert_eq!( - T::Staking::active_balance(&Pools::::create_bonded_account(1)), + T::Staking::active_stake(&Pools::::create_bonded_account(1)), Ok(min_create_bond) ); } diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index e3ebf5d40b437..5173b2e7e7d21 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -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 { @@ -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. diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 75c31c1f4d67f..0c909b68152bb 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -4115,7 +4115,7 @@ mod create { } ); assert_eq!( - StakingMock::active_balance(&next_pool_stash).unwrap(), + StakingMock::active_stake(&next_pool_stash).unwrap(), StakingMock::minimum_nominator_bond() ); assert_eq!( From 963837b833a08ada4ac08d7356b838d848fc5d17 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 27 Oct 2022 13:03:44 +0200 Subject: [PATCH 44/45] clippy fix --- frame/nomination-pools/src/mock.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index a21e58a7b370f..963c01309c49c 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -79,7 +79,7 @@ impl sp_staking::StakingInterface for StakingMock { *x.get_mut(who).unwrap() = x.get_mut(who).unwrap().saturating_sub(amount); BondedBalanceMap::set(&x); let mut y = UnbondingBalanceMap::get(); - *y.entry(who.clone()).or_insert(Self::Balance::zero()) += amount; + *y.entry(who).or_insert(Self::Balance::zero()) += amount; UnbondingBalanceMap::set(&y); Ok(()) } @@ -98,7 +98,7 @@ impl sp_staking::StakingInterface for StakingMock { } fn bond(stash: &Self::AccountId, value: Self::Balance, _: &Self::AccountId) -> DispatchResult { - StakingMock::set_bonded_balance(stash.clone(), value); + StakingMock::set_bonded_balance(stash, value); Ok(()) } @@ -122,9 +122,9 @@ impl sp_staking::StakingInterface for StakingMock { 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.clone() }), - (None, Some(v)) => Ok(Stake { total: v, active: v, stash: who.clone() }), - (Some(a), Some(b)) => Ok(Stake { total: a + b, active: b, stash: who.clone() }), + (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 }), } } From cbed2f96b9b7becc9d4790066af6e69a7cb59325 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 27 Oct 2022 13:15:48 +0200 Subject: [PATCH 45/45] fix --- frame/nomination-pools/src/mock.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index 963c01309c49c..e1af456e31fd4 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -79,7 +79,7 @@ impl sp_staking::StakingInterface for StakingMock { *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(()) } @@ -98,7 +98,7 @@ impl sp_staking::StakingInterface for StakingMock { } fn bond(stash: &Self::AccountId, value: Self::Balance, _: &Self::AccountId) -> DispatchResult { - StakingMock::set_bonded_balance(stash, value); + StakingMock::set_bonded_balance(*stash, value); Ok(()) } @@ -122,9 +122,9 @@ impl sp_staking::StakingInterface for StakingMock { 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 }), + (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 }), } }