diff --git a/Cargo.lock b/Cargo.lock index 6fb73b528ca7..3df437818d1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11551,6 +11551,7 @@ dependencies = [ "sp-npos-elections", "sp-runtime", "sp-staking", + "sp-std 14.0.0", "sp-tracing 16.0.0", ] diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 4c609c095a6f..1772a39c8259 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -590,6 +590,7 @@ parameter_types! { pub const TargetBagThresholds: &'static [u128] = &bag_thresholds::TARGET_THRESHOLDS; pub const VoterUpdateMode: pallet_stake_tracker::VoterUpdateMode = pallet_stake_tracker::VoterUpdateMode::Lazy; + pub const ScoreStrictUpdateThreshold: Option = Some(10); } type VoterBagsListInstance = pallet_bags_list::Instance1; @@ -616,6 +617,8 @@ impl pallet_stake_tracker::Config for Runtime { type VoterList = VoterList; type TargetList = TargetList; type VoterUpdateMode = VoterUpdateMode; + type ScoreStrictUpdateThreshold = ScoreStrictUpdateThreshold; + type WeightInfo = pallet_stake_tracker::weights::SubstrateWeight; } pallet_staking_reward_curve::build! { @@ -667,6 +670,8 @@ impl pallet_staking::Config for Runtime { type GenesisElectionProvider = onchain::OnChainExecution; type VoterList = VoterList; type TargetList = TargetList; + #[cfg(any(feature = "try-runtime", test))] + type TargetUnsettledApprovals = pallet_stake_tracker::UnsettledTargetScores; type NominationsQuota = pallet_staking::FixedNominationsQuota<{ MaxNominations::get() }>; type MaxUnlockingChunks = frame_support::traits::ConstU32<32>; type HistoryDepth = frame_support::traits::ConstU32<84>; diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 46df438074d1..d6aa5d5dc09c 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -697,6 +697,8 @@ impl pallet_staking::Config for Runtime { type GenesisElectionProvider = onchain::OnChainExecution; type VoterList = VoterList; type TargetList = TargetList; + #[cfg(any(feature = "try-runtime", test))] + type TargetUnsettledApprovals = pallet_stake_tracker::UnsettledTargetScores; type NominationsQuota = pallet_staking::FixedNominationsQuota; type MaxUnlockingChunks = ConstU32<32>; type MaxControllersInDeprecationBatch = MaxControllersInDeprecationBatch; @@ -709,6 +711,8 @@ impl pallet_staking::Config for Runtime { parameter_types! { pub const VoterUpdateMode: pallet_stake_tracker::VoterUpdateMode = pallet_stake_tracker::VoterUpdateMode::Lazy; + // disables the lazy approvals update. + pub const ScoreStrictUpdateThreshold: Option = None; } impl pallet_stake_tracker::Config for Runtime { @@ -717,6 +721,8 @@ impl pallet_stake_tracker::Config for Runtime { type VoterList = VoterList; type TargetList = TargetList; type VoterUpdateMode = VoterUpdateMode; + type ScoreStrictUpdateThreshold = ScoreStrictUpdateThreshold; + type WeightInfo = pallet_stake_tracker::weights::SubstrateWeight; } impl pallet_fast_unstake::Config for Runtime { @@ -2646,6 +2652,7 @@ mod benches { [pallet_session, SessionBench::] [pallet_society, Society] [pallet_staking, Staking] + [pallet_stake_tracker, StakeTracker] [pallet_state_trie_migration, StateTrieMigration] [pallet_sudo, Sudo] [frame_system, SystemBench::] diff --git a/substrate/frame/delegated-staking/src/mock.rs b/substrate/frame/delegated-staking/src/mock.rs index de9ff6f9f6f9..4f1aa8754c8f 100644 --- a/substrate/frame/delegated-staking/src/mock.rs +++ b/substrate/frame/delegated-staking/src/mock.rs @@ -110,6 +110,8 @@ impl pallet_staking::Config for Runtime { type GenesisElectionProvider = Self::ElectionProvider; type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; type TargetList = TargetBagsList; + #[cfg(any(feature = "try-runtime", test))] + type TargetUnsettledApprovals = pallet_stake_tracker::UnsettledTargetScores; type EventListeners = (StakeTracker, Pools, DelegatedStaking); } @@ -129,6 +131,7 @@ impl pallet_bags_list::Config for Runtime { parameter_types! { pub static VoterUpdateMode: pallet_stake_tracker::VoterUpdateMode = pallet_stake_tracker::VoterUpdateMode::Lazy; + pub static ScoreStrictUpdateThreshold: Option = None; } impl pallet_stake_tracker::Config for Runtime { @@ -137,6 +140,8 @@ impl pallet_stake_tracker::Config for Runtime { type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; type TargetList = TargetBagsList; type VoterUpdateMode = VoterUpdateMode; + type ScoreStrictUpdateThreshold = ScoreStrictUpdateThreshold; + type WeightInfo = (); } parameter_types! { diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index f71d907b5cc0..d04adca7147e 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -256,6 +256,7 @@ impl pallet_bags_list::Config for Runtime { parameter_types! { pub static UpdateMode: VoterUpdateMode = VoterUpdateMode::Lazy; + pub static ScoreStrictUpdateThreshold: Option = None; } impl pallet_stake_tracker::Config for Runtime { @@ -264,6 +265,8 @@ impl pallet_stake_tracker::Config for Runtime { type VoterList = VoterBagsList; type TargetList = TargetBagsList; type VoterUpdateMode = UpdateMode; + type ScoreStrictUpdateThreshold = ScoreStrictUpdateThreshold; + type WeightInfo = (); } pub struct BalanceToU256; @@ -330,6 +333,8 @@ impl pallet_staking::Config for Runtime { type VoterList = VoterBagsList; type NominationsQuota = pallet_staking::FixedNominationsQuota; type TargetList = TargetBagsList; + #[cfg(any(feature = "try-runtime", test))] + type TargetUnsettledApprovals = pallet_stake_tracker::UnsettledTargetScores; type MaxUnlockingChunks = MaxUnlockingChunks; type EventListeners = (StakeTracker, Pools); type WeightInfo = pallet_staking::weights::SubstrateWeight; diff --git a/substrate/frame/nomination-pools/src/mock.rs b/substrate/frame/nomination-pools/src/mock.rs index 6c0082073f68..4951b7532c4c 100644 --- a/substrate/frame/nomination-pools/src/mock.rs +++ b/substrate/frame/nomination-pools/src/mock.rs @@ -174,6 +174,10 @@ impl sp_staking::StakingInterface for StakingMock { Ok(()) } + fn validate(who: &Self::AccountId) -> sp_runtime::DispatchResult { + unimplemented!() + } + fn nominate(_: &Self::AccountId, nominations: Vec) -> DispatchResult { Nominations::set(&Some(nominations)); Ok(()) diff --git a/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs b/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs index b4f9330502be..18e57b6db197 100644 --- a/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs +++ b/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs @@ -108,6 +108,7 @@ impl pallet_staking::Config for Runtime { parameter_types! { pub static VoterUpdateMode: pallet_stake_tracker::VoterUpdateMode = pallet_stake_tracker::VoterUpdateMode::Lazy; + pub static ScoreStrictUpdateThreshold: Option = None; } impl pallet_stake_tracker::Config for Runtime { @@ -116,6 +117,8 @@ impl pallet_stake_tracker::Config for Runtime { type VoterList = VoterList; type TargetList = pallet_staking::UseValidatorsMap; type VoterUpdateMode = VoterUpdateMode; + type ScoreStrictUpdateThreshold = ScoreStrictUpdateThreshold; + type WeightInfo = (); } parameter_types! { diff --git a/substrate/frame/nomination-pools/test-transfer-stake/src/mock.rs b/substrate/frame/nomination-pools/test-transfer-stake/src/mock.rs index a1129d62cdf5..dd174fa97c06 100644 --- a/substrate/frame/nomination-pools/test-transfer-stake/src/mock.rs +++ b/substrate/frame/nomination-pools/test-transfer-stake/src/mock.rs @@ -100,6 +100,7 @@ impl pallet_staking::Config for Runtime { parameter_types! { pub static VoterUpdateMode: pallet_stake_tracker::VoterUpdateMode = pallet_stake_tracker::VoterUpdateMode::Lazy; + pub static ScoreStrictUpdateThreshold: Option = None; } impl pallet_stake_tracker::Config for Runtime { @@ -108,6 +109,8 @@ impl pallet_stake_tracker::Config for Runtime { type VoterList = VoterList; type TargetList = pallet_staking::UseValidatorsMap; type VoterUpdateMode = VoterUpdateMode; + type ScoreStrictUpdateThreshold = ScoreStrictUpdateThreshold; + type WeightInfo = (); } parameter_types! { diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 7592576d78fd..f1f1bd0c9066 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -22,6 +22,7 @@ use crate::{migrations::v13_stake_tracker as v13, ConfigOp, Pallet as Staking}; use testing_utils::*; use codec::Decode; +use frame_benchmarking::v2::*; use frame_election_provider_support::{bounds::DataProviderBounds, SortedListProvider}; use frame_support::{ assert_ok, @@ -31,18 +32,13 @@ use frame_support::{ traits::{Currency, Get, Imbalance, UnfilteredDispatchable}, weights::WeightMeter, }; +use frame_system::RawOrigin; use sp_runtime::{ traits::{Bounded, One, StaticLookup, TrailingZeroInput, Zero}, Perbill, Percent, Saturating, }; use sp_staking::{currency_to_vote::CurrencyToVote, SessionIndex, StakingInterface}; -pub use frame_benchmarking::v1::{ - account, impl_benchmark_test_suite, whitelist_account, whitelisted_caller, BenchmarkError, -}; -use frame_benchmarking::v2::*; -use frame_system::RawOrigin; - const SEED: u32 = 0; const MAX_SPANS: u32 = 100; const MAX_SLASHES: u32 = 1000; diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 17be5743ea1e..4faaf4280681 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -356,6 +356,8 @@ impl OnStakingUpdate for EventTracker { parameter_types! { pub static VoterUpdateMode: pallet_stake_tracker::VoterUpdateMode = pallet_stake_tracker::VoterUpdateMode::Lazy; + // disables the lazy approvals update. + pub static ScoreStrictUpdateThreshold: Option = None; } impl pallet_stake_tracker::Config for Test { @@ -364,6 +366,8 @@ impl pallet_stake_tracker::Config for Test { type VoterList = VoterBagsList; type TargetList = TargetBagsList; type VoterUpdateMode = VoterUpdateMode; + type ScoreStrictUpdateThreshold = ScoreStrictUpdateThreshold; + type WeightInfo = (); } // Disabling threshold for `UpToLimitDisablingStrategy` @@ -386,6 +390,8 @@ impl crate::pallet::pallet::Config for Test { type GenesisElectionProvider = Self::ElectionProvider; type VoterList = VoterBagsList; type TargetList = TargetBagsList; + #[cfg(any(feature = "try-runtime", test))] + type TargetUnsettledApprovals = pallet_stake_tracker::UnsettledTargetScores; type NominationsQuota = WeightedNominationsQuota<16>; type MaxUnlockingChunks = MaxUnlockingChunks; type HistoryDepth = HistoryDepth; @@ -559,6 +565,11 @@ impl ExtBuilder { MaxWinners::set(max); self } + pub fn stake_tracker_update_threshold(self, threshold: Option) -> Self { + ScoreStrictUpdateThreshold::set(threshold); + self + } + fn build(self) -> sp_io::TestExternalities { sp_tracing::try_init_simple(); let mut storage = frame_system::GenesisConfig::::default().build_storage().unwrap(); diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 7212816d1a08..415b704a631b 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -1993,6 +1993,11 @@ impl StakingInterface for Pallet { Self::nominate(RawOrigin::Signed(ctrl).into(), targets) } + fn validate(who: &Self::AccountId) -> DispatchResult { + let ctrl = Self::bonded(who).ok_or(Error::::NotStash)?; + Self::validate(RawOrigin::Signed(ctrl).into(), Default::default()) + } + fn desired_validator_count() -> u32 { ValidatorCount::::get() } @@ -2521,6 +2526,8 @@ impl Pallet { /// (active_validators + idle_validators + dangling_targets_score_with_score). pub fn do_try_state_approvals() -> Result<(), sp_runtime::TryRuntimeError> { use alloc::collections::{btree_map::BTreeMap, btree_set::BTreeSet}; + use pallet_stake_tracker::StakeImbalance; + let mut approvals_map: BTreeMap = BTreeMap::new(); // build map of approvals stakes from the `Nominators` storage map POV. @@ -2585,6 +2592,19 @@ impl Pallet { } } + // sync up current unsettled score to target's approvals. + for (target, imbalance) in T::TargetUnsettledApprovals::get().into_iter() { + if let Some(approvals) = approvals_map.get_mut(&target) { + match imbalance { + StakeImbalance::Positive(score) => *approvals -= score, + StakeImbalance::Negative(score) => *approvals += score, + StakeImbalance::Zero => (), + } + } else { + return Err("unsettled approval not in the target list".into()); + } + } + let mut mismatch_approvals = 0; // compare calculated approvals per target with target list state. diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index c83088e3e782..af76eb6a78ac 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -268,6 +268,19 @@ pub mod pallet { #[pallet::no_default] type TargetList: SortedListProvider>; + /// Getter for unsettled approvals scores of targets in stake-tracker. Only used for + /// testing and try-state. + #[cfg(any(feature = "try-runtime", test))] + #[pallet::no_default] + type TargetUnsettledApprovals: TypedGet< + Type = Vec<( + Self::AccountId, + pallet_stake_tracker::StakeImbalance< + >::Score, + >, + )>, + >; + /// The maximum number of `unlocking` chunks a [`StakingLedger`] can /// have. Effectively determines how many unique eras a staker may be /// unbonding in. diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 6243da7b8d0a..c3d579340d57 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -7853,6 +7853,7 @@ mod stake_tracker { use super::*; use frame_election_provider_support::ScoreProvider; use pallet_bags_list::Event as BagsEvent; + use pallet_stake_tracker::StakeImbalance; use sp_staking::{StakingAccount::*, StakingInterface}; // keep tests clean; @@ -8720,6 +8721,42 @@ mod stake_tracker { ); }) } + + #[test] + fn try_state_with_unsettled_score_works() { + ExtBuilder::default() + .stake_tracker_update_threshold(Some(50)) + .try_state(true) + .build_and_execute(|| { + assert_eq!(Staking::status(&11), Ok(StakerStatus::Validator)); + assert_eq!(TargetBagsList::score(&11), 1500); + + assert_ok!(Staking::bond( + RuntimeOrigin::signed(90), + 4000, + RewardDestination::Staked + )); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(90), vec![11])); + + let vote_weight = Staking::weight_of(&90); + + // confirm that vote weight of new nominator is lower than threshold. + assert!((vote_weight as u128) < ScoreStrictUpdateThreshold::get().unwrap()); + + // approvals of 42 did not updatee with nomination because new bond's vote weight + // was below the upadte threshold. + assert_eq!(TargetBagsList::score(&11), 1500); + + // unsettled score map has been updated. + assert_eq!( + pallet_stake_tracker::UnsettledTargetScores::::get(), + vec![(11, StakeImbalance::Positive(vote_weight as u128))] + ); + + // he try-state checks takes into consideration the unsetttled score. + assert!(Staking::do_try_state(System::block_number()).is_ok()); + }) + } } mod ledger { @@ -9318,7 +9355,8 @@ mod ledger_recovery { assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_333_before); // OK assert_eq!(Bonded::::get(&333), Some(444)); // OK assert!(Payee::::get(&333).is_some()); // OK - // however, ledger associated with its controller was killed. + + // however, ledger associated with its controller was killed. assert!(Ledger::::get(&444).is_none()); // NOK // side effects on 444 - ledger, bonded, payee, lock should be completely removed. diff --git a/substrate/frame/staking/stake-tracker/Cargo.toml b/substrate/frame/staking/stake-tracker/Cargo.toml index dffdb0d037d4..0da249ae99ec 100644 --- a/substrate/frame/staking/stake-tracker/Cargo.toml +++ b/substrate/frame/staking/stake-tracker/Cargo.toml @@ -19,6 +19,7 @@ codec = { features = [ scale-info = { features = ["derive", "serde"], workspace = true } +sp-std = { workspace = true } sp-runtime = { features = ["serde"], workspace = true } sp-staking = { features = ["serde"], workspace = true } @@ -56,6 +57,7 @@ std = [ "sp-runtime/std", "sp-runtime/std", "sp-staking/std", + "sp-std/std", "sp-tracing/std", ] diff --git a/substrate/frame/staking/stake-tracker/src/benchmarking.rs b/substrate/frame/staking/stake-tracker/src/benchmarking.rs new file mode 100644 index 000000000000..4a00e1d4460a --- /dev/null +++ b/substrate/frame/staking/stake-tracker/src/benchmarking.rs @@ -0,0 +1,53 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! # Stake Tracker Pallet benchmarking. + +use super::*; +use crate::{Pallet as StakeTracker, UnsettledTargetScore}; + +use frame_benchmarking::v2::*; +use frame_support::assert_ok; +use frame_system::RawOrigin; + +const SEED: u32 = 0; + +#[benchmarks] +mod benchmarks { + use super::*; + + #[benchmark] + fn settle() { + let caller = whitelisted_caller(); + let target: T::AccountId = account("target", 0, SEED); + + assert_ok!(StakeTracker::::setup_target_with_unsettled_score(&target)); + assert!(UnsettledTargetScore::::get(&target).is_some()); + + #[extrinsic_call] + _(RawOrigin::Signed(caller), target.clone()); + + assert!(UnsettledTargetScore::::get(target).is_none()); + } + + impl_benchmark_test_suite!( + StakeTracker, + crate::mock::ExtBuilder::default().set_update_threshold(Some(50)), + crate::mock::Test, + exec_name = build_and_execute + ); +} diff --git a/substrate/frame/staking/stake-tracker/src/lib.rs b/substrate/frame/staking/stake-tracker/src/lib.rs index 48121824fb84..eabba1fbe7b7 100644 --- a/substrate/frame/staking/stake-tracker/src/lib.rs +++ b/substrate/frame/staking/stake-tracker/src/lib.rs @@ -18,7 +18,8 @@ //! # Stake Tracker Pallet //! //! The stake-tracker pallet is responsible to keep track of the voter's stake and target's approval -//! voting in the staking system. +//! voting in the staking system. It provides an easy way to other pallets to query the list of +//! strictly sorted targets based on their approvals. //! //! ## Overview //! @@ -57,6 +58,17 @@ //! should handle that in some way (e.g. buffering events and implementing a multi-block event //! emitter). //! +//! ## Lazy target approvals' updates +//! +//! This pallet exposes a configuration that defines a threshold below which the target score +//! *should not* be updated automatically. The `Config::ScoreStrictUpdateThreshold` defines said +//! threshold. If a target stake update is below the threshold, the stake update is buffered in +//! the `UnsettledTargetScore` storage map while the target list is not affected. Multiple +//! approvals updates can be buffered for the same target. Calling `Call::settle` will setttle the +//! buffered approvals tally for a given target. +//! +//! Setting `Config::ScoreStrictUpdateThreshold` to `None` disables the stake approvals buffering. +//! //! ## Staker status and list invariants //! //! * A [`sp_staking::StakerStatus::Nominator`] is part of the voter list and its self-stake is the @@ -89,48 +101,108 @@ pub use pallet::*; +#[cfg(feature = "runtime-benchmarks")] +pub mod benchmarking; + +#[cfg(test)] +pub mod mock; +#[cfg(test)] +mod tests; + +pub mod weights; + extern crate alloc; use alloc::{collections::btree_map::BTreeMap, vec, vec::Vec}; -use frame_election_provider_support::SortedListProvider; +use frame_election_provider_support::{ExtendedBalance, SortedListProvider, VoteWeight}; use frame_support::{ defensive, pallet_prelude::*, - traits::{fungible::Inspect as FnInspect, Defensive, DefensiveSaturating}, + traits::{ + fungible::{Inspect as FnInspect, Mutate as FnMutate}, + Defensive, DefensiveSaturating, + }, }; +use frame_system::pallet_prelude::*; use sp_runtime::traits::Zero; use sp_staking::{ currency_to_vote::CurrencyToVote, OnStakingUpdate, Stake, StakerStatus, StakingInterface, }; - -#[cfg(test)] -pub(crate) mod mock; -#[cfg(test)] -mod tests; +use sp_std::ops::Add; +pub use weights::WeightInfo; /// The balance type of this pallet. pub type BalanceOf = <::Staking as StakingInterface>::Balance; /// The account ID of this pallet. pub type AccountIdOf = ::AccountId; +/// Score of a sorted list provider. +pub type ScoreOf = <::TargetList as SortedListProvider>>::Score; +/// List of current unsettled target scores. +pub type UnsettledList = Vec<(AccountIdOf, StakeImbalance)>; /// Represents a stake imbalance to be applied to a staker's score. -#[derive(Copy, Clone, Debug)] +#[derive(TypeInfo, Copy, Debug, Clone, Encode, Decode, PartialEq, MaxEncodedLen)] pub enum StakeImbalance { /// Represents the reduction of stake by `Score`. Negative(Score), /// Represents the increase of stake by `Score`. Positive(Score), + /// No stake imbalance. + Zero, +} + +impl Default for StakeImbalance { + fn default() -> Self { + Self::Positive(Zero::zero()) + } +} + +impl Add for StakeImbalance { + type Output = Self; + + fn add(self, other: Self) -> Self { + match (self, other) { + (Self::Positive(x), Self::Positive(y)) => Self::Positive(x.defensive_saturating_add(y)), + (Self::Negative(x), Self::Negative(y)) => Self::Negative(x.defensive_saturating_add(y)), + (Self::Positive(x), Self::Negative(y)) => + if x > y { + Self::Positive(x.defensive_saturating_sub(y)) + } else if x < y { + Self::Negative(y.defensive_saturating_sub(x)) + } else { + Self::Zero + }, + (Self::Negative(x), Self::Positive(y)) => + if x > y { + Self::Negative(x.defensive_saturating_sub(y)) + } else if x < y { + Self::Positive(y.defensive_saturating_sub(x)) + } else { + Self::Zero + }, + (Self::Zero, _) => other, + (_, Self::Zero) => self, + } + } } -impl StakeImbalance { - /// Constructor for a stake imbalance instance based on the previous and next score. - fn from(prev: Score, new: Score) -> Self { +impl StakeImbalance { + /// Constructor for a stake imbalance instance based on the previous and next unsigned value. + fn from(prev: V, new: V) -> Self { if prev > new { StakeImbalance::Negative(prev.defensive_saturating_sub(new)) } else { StakeImbalance::Positive(new.defensive_saturating_sub(prev)) } } + + /// Returns the unsigned value of a staking balance instance. + fn value(&self) -> V { + match self { + Self::Positive(score) | Self::Negative(score) => *score, + Self::Zero => Zero::zero(), + } + } } /// Defines the sorting mode of sorted list providers. @@ -152,7 +224,6 @@ impl VoterUpdateMode { #[frame_support::pallet] pub mod pallet { use crate::*; - use frame_election_provider_support::{ExtendedBalance, VoteWeight}; /// The current storage version. const STORAGE_VERSION: StorageVersion = StorageVersion::new(0); @@ -164,7 +235,8 @@ pub mod pallet { #[pallet::config] pub trait Config: frame_system::Config { /// The stake balance. - type Currency: FnInspect>; + type Currency: FnInspect> + + FnMutate; /// The staking interface. type Staking: StakingInterface; @@ -180,9 +252,63 @@ pub mod pallet { /// The voter list update mode. type VoterUpdateMode: Get; + + /// Score threshold which defines whether the approvals should be updated on buffered. + /// + /// If the approvals score to be updated is higher than `ScoreStrictUpdateThreshold`, + /// update the target list. Otherwise, buffer the update. + type ScoreStrictUpdateThreshold: Get>; + + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; + } + + /// Map with unsettled score for targets. + /// + /// This map keeps track of unsettled score for targets. + #[pallet::storage] + pub type UnsettledTargetScore = + StorageMap<_, Twox64Concat, T::AccountId, StakeImbalance>>; + + #[pallet::error] + #[derive(PartialEq)] + pub enum Error { + /// Account is not a target. + NotTarget, + /// No unsettled score for a given target. + NoScoreToSettle, + } + + #[pallet::call] + impl Pallet { + /// Settles a buffered target approvals imbalance. + #[pallet::call_index(0)] + #[pallet::weight(T::WeightInfo::settle())] + pub fn settle(origin: OriginFor, who: AccountIdOf) -> DispatchResult { + let _ = ensure_signed(origin)?; + + ensure!(T::TargetList::contains(&who), Error::::NotTarget); + ensure!(UnsettledTargetScore::::contains_key(&who), Error::::NoScoreToSettle); + + Self::do_settle(&who)?; + Ok(()) + } } impl Pallet { + /// Settles buffered score for a target, if it exists. + pub(crate) fn do_settle(who: &T::AccountId) -> Result<(), Error> { + UnsettledTargetScore::::try_mutate_exists(who, |maybe_imbalance| { + let imbalance = maybe_imbalance.ok_or(Error::::NoScoreToSettle)?; + + Self::update_target_score_unchecked(who, imbalance); + *maybe_imbalance = None; + Ok(()) + })?; + + Ok(()) + } + /// Updates the stake of a voter. /// /// NOTE: This method expects `nominations` to be deduplicated, otherwise the approvals @@ -254,7 +380,27 @@ pub mod pallet { ); } - // update target score. + // updates the target list OR buffers the score depending on whether the score + // imbalance is higher than the update threshold (if set). + if let Some(update_threshold) = T::ScoreStrictUpdateThreshold::get() { + if imbalance.value() > update_threshold { + Self::update_target_score_unchecked(who, imbalance); + } else { + // buffer the approvals update in `UnsettledTargetScore` map. + UnsettledTargetScore::::insert( + who, + UnsettledTargetScore::::get(who).unwrap_or_default().add(imbalance), + ); + } + } else { + Self::update_target_score_unchecked(who, imbalance); + } + } + + pub(crate) fn update_target_score_unchecked( + who: &T::AccountId, + imbalance: StakeImbalance, + ) { match imbalance { StakeImbalance::Positive(imbalance) => { let _ = T::TargetList::on_increase(who, imbalance).defensive_proof( @@ -280,6 +426,7 @@ pub mod pallet { defensive!("unexpected: unable to fetch score from staking interface of an existent staker"); } }, + StakeImbalance::Zero => (), // update not needed. }; } @@ -310,6 +457,33 @@ pub mod pallet { size_before != dedup.len() } + + #[cfg(feature = "runtime-benchmarks")] + pub(crate) fn setup_target_with_unsettled_score(target: &T::AccountId) -> DispatchResult { + // fund target account. + let mut balance = T::Currency::minimum_balance(); + for _ in 0..100 { + balance = balance + T::Currency::minimum_balance(); + } + let _ = T::Currency::set_balance(target, balance + T::Currency::minimum_balance()); + + ::bond(target, balance, target)?; + ::validate(target)?; + + UnsettledTargetScore::::insert(target, StakeImbalance::Positive(10)); + + Ok(()) + } + } +} + +/// Returns a vec with all the unsettled scores. +pub struct UnsettledTargetScores(PhantomData); +impl sp_runtime::traits::TypedGet for UnsettledTargetScores { + type Type = UnsettledList; + + fn get() -> Self::Type { + UnsettledTargetScore::::iter().collect::>() } } @@ -399,6 +573,9 @@ impl OnStakingUpdate> for Pallet { if score.is_zero() { let _ = T::TargetList::on_remove(who) .defensive_proof("target exists as per above; qed"); + + // ensure that the unsettled score list is cleaned. + let _ = UnsettledTargetScore::::take(who); } } else { // target is not part of the list. Given the contract with staking and the checks above, diff --git a/substrate/frame/staking/stake-tracker/src/mock.rs b/substrate/frame/staking/stake-tracker/src/mock.rs index b326ba52e697..ddbc02e3cdc2 100644 --- a/substrate/frame/staking/stake-tracker/src/mock.rs +++ b/substrate/frame/staking/stake-tracker/src/mock.rs @@ -31,7 +31,7 @@ type Block = frame_system::mocking::MockBlockU32; // Configure a mock runtime to test the pallet. #[frame_support::runtime] -mod runtime { +pub mod runtime { #[runtime::runtime] #[runtime::derive( RuntimeCall, @@ -98,8 +98,6 @@ const TARGET_THRESHOLDS: [u128; 9] = [100, 200, 300, 400, 500, 600, 700, 800, 90 parameter_types! { pub static VoterBagThresholds: &'static [VoteWeight] = &VOTER_THRESHOLDS; pub static TargetBagThresholds: &'static [u128] = &TARGET_THRESHOLDS; - - pub static VoterUpdateMode: crate::VoterUpdateMode = crate::VoterUpdateMode::Strict; } type VoterBagsListInstance = pallet_bags_list::Instance1; @@ -120,12 +118,20 @@ impl pallet_bags_list::Config for Test { type Score = u128; } +parameter_types! { + pub static VoterUpdateMode: crate::VoterUpdateMode = crate::VoterUpdateMode::Strict; + // disables the lazy approvals update. + pub static ScoreStrictUpdateThreshold: Option = None; +} + impl pallet_stake_tracker::Config for Test { type Currency = Balances; type Staking = StakingMock; type VoterList = VoterBagsList; type TargetList = TargetBagsList; type VoterUpdateMode = VoterUpdateMode; + type ScoreStrictUpdateThreshold = ScoreStrictUpdateThreshold; + type WeightInfo = (); } pub struct StakingMock {} @@ -194,7 +200,7 @@ impl StakingInterface for StakingMock { } fn minimum_validator_bond() -> Self::Balance { - unreachable!(); + 1 } fn stash_by_ctrl( @@ -216,7 +222,13 @@ impl StakingInterface for StakingMock { _value: Self::Balance, _payee: &Self::AccountId, ) -> sp_runtime::DispatchResult { - unreachable!(); + Ok(()) + } + + fn validate(who: &Self::AccountId) -> DispatchResult { + add_validator(*who, 10); + + Ok(()) } fn nominate( @@ -503,6 +515,11 @@ impl ExtBuilder { self } + pub fn set_update_threshold(self, threshold: Option) -> Self { + ScoreStrictUpdateThreshold::set(threshold); + self + } + pub fn build(self) -> sp_io::TestExternalities { sp_tracing::try_init_simple(); let storage = frame_system::GenesisConfig::::default().build_storage().unwrap(); diff --git a/substrate/frame/staking/stake-tracker/src/tests.rs b/substrate/frame/staking/stake-tracker/src/tests.rs index b53c670b0491..975c9ec43d2e 100644 --- a/substrate/frame/staking/stake-tracker/src/tests.rs +++ b/substrate/frame/staking/stake-tracker/src/tests.rs @@ -17,16 +17,172 @@ #![cfg(test)] -use crate::{mock::*, StakeImbalance}; +use crate::{mock::*, Error, StakeImbalance, UnsettledTargetScores}; use frame_election_provider_support::{ScoreProvider, SortedListProvider}; -use frame_support::assert_ok; +use frame_support::{assert_noop, assert_ok}; +use sp_runtime::traits::TypedGet; use sp_staking::{OnStakingUpdate, Stake, StakerStatus, StakingInterface}; // keeping tests clean. type A = AccountId; type B = Balance; +mod stake_imbalance { + use super::*; + + #[test] + fn stake_imbalance_arithmetic_works() { + use std::ops::Add; + + // 0 + 0 = 0 + assert_eq!(StakeImbalance::::Zero.add(StakeImbalance::Zero), StakeImbalance::Zero,); + + // 1 + 0 = 1 + assert_eq!( + StakeImbalance::Positive(1).add(StakeImbalance::Zero), + StakeImbalance::Positive(1), + ); + + // -1 + 0 = -1 + assert_eq!( + StakeImbalance::Negative(1).add(StakeImbalance::Zero), + StakeImbalance::Negative(1), + ); + + // -2 + 2 = 0 + assert_eq!( + StakeImbalance::Negative(2).add(StakeImbalance::Positive(2)), + StakeImbalance::Zero, + ); + + // 1 + 2 = 3 + assert_eq!( + StakeImbalance::Positive(1).add(StakeImbalance::Positive(2)), + StakeImbalance::Positive(3) + ); + + // 1 + (-2) = -1 + assert_eq!( + StakeImbalance::Positive(1).add(StakeImbalance::Negative(2)), + StakeImbalance::Negative(1) + ); + + // -1 + 2 = 1 + assert_eq!( + StakeImbalance::Negative(1).add(StakeImbalance::Positive(2)), + StakeImbalance::Positive(1) + ); + + // -1 + (-2) = -3 + assert_eq!( + StakeImbalance::Negative(1).add(StakeImbalance::Negative(2)), + StakeImbalance::Negative(3) + ); + } + + #[test] + fn updates_below_threshold_buffer_works() { + ExtBuilder::default().set_update_threshold(Some(10)).build_and_execute(|| { + add_validator(10, 100); + assert_eq!(TargetBagsList::get_score(&10).unwrap(), 100); + + // updating stake by 200 will reflect in the target list. + update_stake(10, 200, stake_of(10)); + assert_eq!(TargetBagsList::get_score(&10).unwrap(), 200); + // unsettled stake list is empty. + assert_eq!(UnsettledTargetScores::::get(), vec![]); + + // updating stake by +5 (below thresahold) will *not* reflect in the target list and + // update the unsettled stake list. + update_stake(10, 205, stake_of(10)); + // target list was not updated. + assert_eq!(TargetBagsList::get_score(&10).unwrap(), 200); + // unsettled stake list has the bufferd score for target 10. + assert_eq!( + UnsettledTargetScores::::get(), + vec![(10u64, StakeImbalance::Positive(5))] + ); + + // updating the stake by -7 is still below the threshold and thus target list is not + // affected. The buffered score will be updated accordingly. + update_stake(10, 198, stake_of(10)); + // target list was not updated. + assert_eq!(TargetBagsList::get_score(&10).unwrap(), 200); + // unsettled stake list has the bufferd score for target 10. + assert_eq!( + UnsettledTargetScores::::get(), + vec![(10u64, StakeImbalance::Negative(2))] + ); + + // finally, settling the buffered target score for 10 will affect the target's + // approvals. + assert_ok!(StakeTracker::do_settle(&10)); + // +5 + (-7) imbalance = (-2), thus approvals is initial approvals - 2. + assert_eq!(TargetBagsList::get_score(&10).unwrap(), 198); + // unsettled stake list is empty again. + assert_eq!(UnsettledTargetScores::::get(), vec![]); + }) + } + + #[test] + fn settle_buffered_score_works() { + ExtBuilder::default().set_update_threshold(Some(10)).build_and_execute(|| { + add_validator(10, 100); + assert_eq!(TargetBagsList::get_score(&10).unwrap(), 100); + assert_eq!(UnsettledTargetScores::::get(), vec![]); + + // updating stake by +5 (below thresahold) will *not* reflect in the target list and + // update the unsettled stake list. + update_stake(10, 105, stake_of(10)); + // target list was not updated. + assert_eq!(TargetBagsList::get_score(&10).unwrap(), 100); + // unsettled stake list has the bufferd score for target 10. + assert_eq!( + UnsettledTargetScores::::get(), + vec![(10u64, StakeImbalance::Positive(5))] + ); + + // calls `settle` extrinsic to settle the buffered score of 10. + assert_ok!(StakeTracker::settle(RuntimeOrigin::signed(1), 10)); + + // approvals were updated accordingly + assert_eq!(TargetBagsList::get_score(&10).unwrap(), 105); + // unsettled stake list is empty again. + assert_eq!(UnsettledTargetScores::::get(), vec![]); + + // calling `settle` with target account that does not have buffered score errors. + assert_noop!( + StakeTracker::settle(RuntimeOrigin::signed(1), 10), + Error::::NoScoreToSettle, + ); + }) + } + + #[test] + fn settle_buffered_score_error_works() { + ExtBuilder::default().set_update_threshold(Some(10)).build_and_execute(|| { + assert!(!TargetBagsList::contains(&100)); + + // calling `settle` of an account that does not exist in the target list errors. + assert_noop!( + StakeTracker::settle(RuntimeOrigin::signed(1), 100), + Error::::NotTarget, + ); + + add_validator(10, 100); + assert!(TargetBagsList::contains(&10)); + assert_eq!(UnsettledTargetScores::::get(), vec![]); + + // calling `settle` with target account that does not have buffered score errors. + assert_noop!( + StakeTracker::settle(RuntimeOrigin::signed(1), 10), + Error::::NoScoreToSettle, + ); + }) + } +} + #[test] fn setup_works() { ExtBuilder::default().build_and_execute(|| { diff --git a/substrate/frame/staking/stake-tracker/src/weights.rs b/substrate/frame/staking/stake-tracker/src/weights.rs new file mode 100644 index 000000000000..86ff3397cd33 --- /dev/null +++ b/substrate/frame/staking/stake-tracker/src/weights.rs @@ -0,0 +1,89 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Autogenerated weights for `pallet_staking` +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 +//! DATE: 2024-05-08, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! WORST CASE MAP SIZE: `1000000` +//! HOSTNAME: `runner-unxyhko3-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` + +// Executed Command: +// target/production/substrate-node +// benchmark +// pallet +// --steps=50 +// --repeat=20 +// --extrinsic=* +// --wasm-execution=compiled +// --heap-pages=4096 +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json +// --pallet=pallet_stake_tracker +// --chain=dev +// --header=./substrate/HEADER-APACHE2 +// --output=./substrate/frame/staking/stake-tracker/src/weights.rs +// --template=./substrate/.maintain/frame-weight-template.hbs + + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] +#![allow(missing_docs)] + +use frame_support::{traits::Get, weights::Weight}; +use core::marker::PhantomData; + +/// Weight functions needed for `pallet_stake_tracker`. +pub trait WeightInfo { + fn settle() -> Weight; +} + +pub struct SubstrateWeight(PhantomData); +impl WeightInfo for SubstrateWeight { + /// Storage: `TargetList::ListNodes` (r:1 w:1) + /// Proof: `TargetList::ListNodes` (`max_values`: None, `max_size`: Some(170), added: 2645, mode: `MaxEncodedLen`) + /// Storage: `StakeTracker::UnsettledScore` (r:1 w:1) + /// Proof: `StakeTracker::UnsettledScore` (`max_values`: None, `max_size`: Some(57), added: 2532, mode: `MaxEncodedLen`) + fn settle() -> Weight { + // Proof Size summary in bytes: + // Measured: `606` + // Estimated: `3635` + // Minimum execution time: 19_000_000 picoseconds. + Weight::from_parts(20_000_000, 0) + .saturating_add(Weight::from_parts(0, 3635)) + .saturating_add(T::DbWeight::get().reads(2)) + .saturating_add(T::DbWeight::get().writes(2)) + } +} + +// For backwards compatibility and tests. +impl WeightInfo for () { + /// Storage: `TargetList::ListNodes` (r:1 w:1) + /// Proof: `TargetList::ListNodes` (`max_values`: None, `max_size`: Some(170), added: 2645, mode: `MaxEncodedLen`) + /// Storage: `StakeTracker::UnsettledScore` (r:1 w:1) + /// Proof: `StakeTracker::UnsettledScore` (`max_values`: None, `max_size`: Some(57), added: 2532, mode: `MaxEncodedLen`) + fn settle() -> Weight { + // Proof Size summary in bytes: + // Measured: `606` + // Estimated: `3635` + // Minimum execution time: 19_000_000 picoseconds. + Weight::from_parts(20_000_000, 0) + .saturating_add(Weight::from_parts(0, 3635)) + } +} + diff --git a/substrate/primitives/staking/src/lib.rs b/substrate/primitives/staking/src/lib.rs index 8328dd664633..72014ecdb60b 100644 --- a/substrate/primitives/staking/src/lib.rs +++ b/substrate/primitives/staking/src/lib.rs @@ -324,6 +324,9 @@ pub trait StakingInterface { fn bond(who: &Self::AccountId, value: Self::Balance, payee: &Self::AccountId) -> DispatchResult; + /// Have `who` become a validator with default preferences. + fn validate(who: &Self::AccountId) -> DispatchResult; + /// Have `who` nominate `validators`. fn nominate(who: &Self::AccountId, validators: Vec) -> DispatchResult;