From e7f994d1e797420f252dd24714b029071ccbc46c Mon Sep 17 00:00:00 2001 From: Ankan <10196091+Ank4n@users.noreply.github.com> Date: Wed, 28 Sep 2022 22:52:16 +0200 Subject: [PATCH] bounding staking: `BoundedElectionProvider` trait (#12362) * add a bounded election provider trait * extract common trait election provider base * fmt * only bound the outer support vector * fix tests * docs * fix rust docs * fmt * fix rustdocs * docs * improve docs * small doc change --- .../election-provider-multi-phase/src/lib.rs | 16 ++-- .../election-provider-multi-phase/src/mock.rs | 5 +- frame/election-provider-support/src/lib.rs | 83 ++++++++++++------- .../election-provider-support/src/onchain.rs | 27 +++--- frame/fast-unstake/src/lib.rs | 5 +- frame/fast-unstake/src/mock.rs | 4 +- primitives/npos-elections/src/lib.rs | 14 ++-- 7 files changed, 99 insertions(+), 55 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index bba8139f38f44..649aec30c58b3 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -231,7 +231,8 @@ use codec::{Decode, Encode}; use frame_election_provider_support::{ - ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolution, + ElectionDataProvider, ElectionProvider, ElectionProviderBase, InstantElectionProvider, + NposSolution, }; use frame_support::{ dispatch::DispatchClass, @@ -289,7 +290,7 @@ pub type SolutionTargetIndexOf = as NposSolution>::TargetIndex pub type SolutionAccuracyOf = ::MinerConfig> as NposSolution>::Accuracy; /// The fallback election type. -pub type FallbackErrorOf = <::Fallback as ElectionProvider>::Error; +pub type FallbackErrorOf = <::Fallback as ElectionProviderBase>::Error; /// Configuration for the benchmarks of the pallet. pub trait BenchmarkingConfig { @@ -312,7 +313,7 @@ pub trait BenchmarkingConfig { /// A fallback implementation that transitions the pallet to the emergency phase. pub struct NoFallback(sp_std::marker::PhantomData); -impl ElectionProvider for NoFallback { +impl ElectionProviderBase for NoFallback { type AccountId = T::AccountId; type BlockNumber = T::BlockNumber; type DataProvider = T::DataProvider; @@ -321,7 +322,9 @@ impl ElectionProvider for NoFallback { fn ongoing() -> bool { false } +} +impl ElectionProvider for NoFallback { fn elect() -> Result, Self::Error> { // Do nothing, this will enable the emergency phase. Err("NoFallback.") @@ -1563,7 +1566,7 @@ impl Pallet { >::take() .ok_or(ElectionError::::NothingQueued) .or_else(|_| { - T::Fallback::elect() + ::elect() .map(|supports| ReadySolution { supports, score: Default::default(), @@ -1598,7 +1601,7 @@ impl Pallet { } } -impl ElectionProvider for Pallet { +impl ElectionProviderBase for Pallet { type AccountId = T::AccountId; type BlockNumber = T::BlockNumber; type Error = ElectionError; @@ -1610,7 +1613,9 @@ impl ElectionProvider for Pallet { _ => true, } } +} +impl ElectionProvider for Pallet { fn elect() -> Result, Self::Error> { match Self::do_elect() { Ok(supports) => { @@ -1627,7 +1632,6 @@ impl ElectionProvider for Pallet { } } } - /// convert a DispatchError to a custom InvalidTransaction with the inner code being the error /// number. pub fn dispatch_error_to_invalid(error: DispatchError) -> InvalidTransaction { diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 34aa2e1bbfc58..c1c53a3980676 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -297,7 +297,7 @@ impl onchain::Config for OnChainSeqPhragmen { } pub struct MockFallback; -impl ElectionProvider for MockFallback { +impl ElectionProviderBase for MockFallback { type AccountId = AccountId; type BlockNumber = u64; type Error = &'static str; @@ -306,7 +306,8 @@ impl ElectionProvider for MockFallback { fn ongoing() -> bool { false } - +} +impl ElectionProvider for MockFallback { fn elect() -> Result, Self::Error> { Self::elect_with_bounds(Bounded::max_value(), Bounded::max_value()) } diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 0bf62bd8c35cd..5ee65e102bd06 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -20,10 +20,11 @@ //! This crate provides two traits that could interact to enable extensible election functionality //! within FRAME pallets. //! -//! Something that will provide the functionality of election will implement [`ElectionProvider`], -//! whilst needing an associated [`ElectionProvider::DataProvider`], which needs to be fulfilled by -//! an entity implementing [`ElectionDataProvider`]. Most often, *the data provider is* the receiver -//! of the election, resulting in a diagram as below: +//! Something that will provide the functionality of election will implement +//! [`ElectionProvider`] and its parent-trait [`ElectionProviderBase`], whilst needing an +//! associated [`ElectionProviderBase::DataProvider`], which needs to be +//! fulfilled by an entity implementing [`ElectionDataProvider`]. Most often, *the data provider is* +//! the receiver of the election, resulting in a diagram as below: //! //! ```ignore //! ElectionDataProvider @@ -131,12 +132,16 @@ //! type DataProvider: ElectionDataProvider; //! } //! -//! impl ElectionProvider for GenericElectionProvider { +//! impl ElectionProviderBase for GenericElectionProvider { //! type AccountId = AccountId; //! type BlockNumber = BlockNumber; //! type Error = &'static str; //! type DataProvider = T::DataProvider; //! fn ongoing() -> bool { false } +//! +//! } +//! +//! impl ElectionProvider for GenericElectionProvider { //! fn elect() -> Result, Self::Error> { //! Self::DataProvider::electable_targets(None) //! .map_err(|_| "failed to elect") @@ -177,8 +182,8 @@ pub use frame_support::{traits::Get, weights::Weight, BoundedVec, RuntimeDebug}; /// Re-export some type as they are used in the interface. pub use sp_arithmetic::PerThing; pub use sp_npos_elections::{ - Assignment, BalancingConfig, ElectionResult, Error, ExtendedBalance, IdentifierT, PerThing128, - Support, Supports, VoteWeight, + Assignment, BalancingConfig, BoundedSupports, ElectionResult, Error, ExtendedBalance, + IdentifierT, PerThing128, Support, Supports, VoteWeight, }; pub use traits::NposSolution; @@ -349,12 +354,12 @@ pub trait ElectionDataProvider { fn clear() {} } -/// Something that can compute the result of an election and pass it back to the caller. +/// Base trait for [`ElectionProvider`] and [`BoundedElectionProvider`]. It is +/// meant to be used only with an extension trait that adds an election +/// functionality. /// -/// This trait only provides an interface to _request_ an election, i.e. -/// [`ElectionProvider::elect`]. That data required for the election need to be passed to the -/// implemented of this trait through [`ElectionProvider::DataProvider`]. -pub trait ElectionProvider { +/// Data can be bounded or unbounded and is fetched from [`Self::DataProvider`]. +pub trait ElectionProviderBase { /// The account identifier type. type AccountId; @@ -372,24 +377,39 @@ pub trait ElectionProvider { /// Indicate if this election provider is currently ongoing an asynchronous election or not. fn ongoing() -> bool; +} - /// Elect a new set of winners, without specifying any bounds on the amount of data fetched from - /// [`Self::DataProvider`]. An implementation could nonetheless impose its own custom limits. - /// - /// The result is returned in a target major format, namely as *vector of supports*. - /// - /// This should be implemented as a self-weighing function. The implementor should register its - /// appropriate weight at the end of execution with the system pallet directly. +/// Elect a new set of winners, bounded by `MaxWinners`. +/// +/// Returns a result in bounded, target major format, namely as +/// *BoundedVec<(AccountId, Vec), MaxWinners>*. +pub trait BoundedElectionProvider: ElectionProviderBase { + /// The upper bound on election winners. + type MaxWinners: Get; + /// Performs the election. This should be implemented as a self-weighing function. The + /// implementor should register its appropriate weight at the end of execution with the + /// system pallet directly. + fn elect() -> Result, Self::Error>; +} + +/// Same a [`BoundedElectionProvider`], but no bounds are imposed on the number +/// of winners. +/// +/// The result is returned in a target major format, namely as +///*Vec<(AccountId, Vec)>*. +pub trait ElectionProvider: ElectionProviderBase { + /// Performs the election. This should be implemented as a self-weighing + /// function, similar to [`BoundedElectionProvider::elect()`]. fn elect() -> Result, Self::Error>; } -/// A sub-trait of the [`ElectionProvider`] for cases where we need to be sure an election needs to -/// happen instantly, not asynchronously. +/// A sub-trait of the [`ElectionProvider`] for cases where we need to be sure +/// an election needs to happen instantly, not asynchronously. /// /// The same `DataProvider` is assumed to be used. /// -/// Consequently, allows for control over the amount of data that is being fetched from the -/// [`ElectionProvider::DataProvider`]. +/// Consequently, allows for control over the amount of data that is being +/// fetched from the [`ElectionProviderBase::DataProvider`]. pub trait InstantElectionProvider: ElectionProvider { /// Elect a new set of winners, but unlike [`ElectionProvider::elect`] which cannot enforce /// bounds, this trait method can enforce bounds on the amount of data provided by the @@ -410,7 +430,7 @@ pub trait InstantElectionProvider: ElectionProvider { pub struct NoElection(sp_std::marker::PhantomData); #[cfg(feature = "std")] -impl ElectionProvider +impl ElectionProviderBase for NoElection<(AccountId, BlockNumber, DataProvider)> where DataProvider: ElectionDataProvider, @@ -420,15 +440,22 @@ where type Error = &'static str; type DataProvider = DataProvider; - fn elect() -> Result, Self::Error> { - Err(" cannot do anything.") - } - fn ongoing() -> bool { false } } +#[cfg(feature = "std")] +impl ElectionProvider + for NoElection<(AccountId, BlockNumber, DataProvider)> +where + DataProvider: ElectionDataProvider, +{ + fn elect() -> Result, Self::Error> { + Err(" cannot do anything.") + } +} + /// A utility trait for something to implement `ElectionDataProvider` in a sensible way. /// /// This is generic over `AccountId` and it can represent a validator, a nominator, or any other diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 10c3519d03df6..88aa6ca7267a0 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -20,7 +20,8 @@ //! careful when using it onchain. use crate::{ - Debug, ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolver, WeightInfo, + Debug, ElectionDataProvider, ElectionProvider, ElectionProviderBase, InstantElectionProvider, + NposSolver, WeightInfo, }; use frame_support::{dispatch::DispatchClass, traits::Get}; use sp_npos_elections::*; @@ -133,15 +134,6 @@ fn elect_with( } impl ElectionProvider for UnboundedExecution { - type AccountId = ::AccountId; - type BlockNumber = ::BlockNumber; - type Error = Error; - type DataProvider = T::DataProvider; - - fn ongoing() -> bool { - false - } - fn elect() -> Result, Self::Error> { // This should not be called if not in `std` mode (and therefore neither in genesis nor in // testing) @@ -156,6 +148,17 @@ impl ElectionProvider for UnboundedExecution { } } +impl ElectionProviderBase for UnboundedExecution { + type AccountId = ::AccountId; + type BlockNumber = ::BlockNumber; + type Error = Error; + type DataProvider = T::DataProvider; + + fn ongoing() -> bool { + false + } +} + impl InstantElectionProvider for UnboundedExecution { fn elect_with_bounds( max_voters: usize, @@ -165,7 +168,7 @@ impl InstantElectionProvider for UnboundedExecution { } } -impl ElectionProvider for BoundedExecution { +impl ElectionProviderBase for BoundedExecution { type AccountId = ::AccountId; type BlockNumber = ::BlockNumber; type Error = Error; @@ -174,7 +177,9 @@ impl ElectionProvider for BoundedExecution { fn ongoing() -> bool { false } +} +impl ElectionProvider for BoundedExecution { fn elect() -> Result, Self::Error> { elect_with::(Some(T::VotersBound::get() as usize), Some(T::TargetsBound::get() as usize)) } diff --git a/frame/fast-unstake/src/lib.rs b/frame/fast-unstake/src/lib.rs index ed26d6b436e1d..8fdb7a79dd537 100644 --- a/frame/fast-unstake/src/lib.rs +++ b/frame/fast-unstake/src/lib.rs @@ -80,7 +80,7 @@ macro_rules! log { pub mod pallet { use super::*; use crate::types::*; - use frame_election_provider_support::ElectionProvider; + use frame_election_provider_support::ElectionProviderBase; use frame_support::{ pallet_prelude::*, traits::{Defensive, ReservableCurrency}, @@ -330,7 +330,8 @@ pub mod pallet { } } - if ::ElectionProvider::ongoing() { + if <::ElectionProvider as ElectionProviderBase>::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 diff --git a/frame/fast-unstake/src/mock.rs b/frame/fast-unstake/src/mock.rs index 4c4c5f9ff26fd..dc2c694d52956 100644 --- a/frame/fast-unstake/src/mock.rs +++ b/frame/fast-unstake/src/mock.rs @@ -104,7 +104,7 @@ parameter_types! { } pub struct MockElection; -impl frame_election_provider_support::ElectionProvider for MockElection { +impl frame_election_provider_support::ElectionProviderBase for MockElection { type AccountId = AccountId; type BlockNumber = BlockNumber; type DataProvider = Staking; @@ -113,7 +113,9 @@ impl frame_election_provider_support::ElectionProvider for MockElection { fn ongoing() -> bool { Ongoing::get() } +} +impl frame_election_provider_support::ElectionProvider for MockElection { fn elect() -> Result, Self::Error> { Err(()) } diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index dd2a9bf198f8d..514ded67ad38b 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -74,17 +74,16 @@ #![cfg_attr(not(feature = "std"), no_std)] +use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; use sp_arithmetic::{traits::Zero, Normalizable, PerThing, Rational128, ThresholdOrd}; -use sp_core::RuntimeDebug; +use sp_core::{bounded::BoundedVec, RuntimeDebug}; use sp_std::{ cell::RefCell, cmp::Ordering, collections::btree_map::BTreeMap, prelude::*, rc::Rc, vec, }; -use codec::{Decode, Encode, MaxEncodedLen}; -#[cfg(feature = "std")] -use serde::{Deserialize, Serialize}; - #[cfg(test)] mod mock; #[cfg(test)] @@ -451,6 +450,11 @@ impl Default for Support { /// The main advantage of this is that it is encodable. pub type Supports = Vec<(A, Support)>; +/// Same as `Supports` bounded by `MaxWinners`. +/// +/// To note, the inner `Support` is still unbounded. +pub type BoundedSupports = BoundedVec<(A, Support), MaxWinners>; + /// Linkage from a winner to their [`Support`]. /// /// This is more helpful than a normal [`Supports`] as it allows faster error checking.