Skip to content

Commit

Permalink
move generics of election trait to associated types (paritytech#10475)
Browse files Browse the repository at this point in the history
* move generics of election trait to associated types

* fix doctest
  • Loading branch information
kianenigma authored Dec 16, 2021
1 parent 4db2f22 commit 7f68a8b
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 54 deletions.
7 changes: 2 additions & 5 deletions frame/bags-list/remote-tests/src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,8 @@ pub async fn execute<Runtime: crate::RuntimeT, Block: BlockT + DeserializeOwned>
<Runtime as pallet_staking::Config>::SortedListProvider::count(),
);

let voters = <pallet_staking::Pallet<Runtime> as ElectionDataProvider<
Runtime::AccountId,
Runtime::BlockNumber,
>>::voters(voter_limit)
.unwrap();
let voters =
<pallet_staking::Pallet<Runtime> as ElectionDataProvider>::voters(voter_limit).unwrap();

let mut voters_nominator_only = voters
.iter()
Expand Down
2 changes: 1 addition & 1 deletion frame/election-provider-multi-phase/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ frame_benchmarking::benchmarks! {
assert!(<Snapshot<T>>::get().is_some());
assert!(<SnapshotMetadata<T>>::get().is_some());
}: {
assert_ok!(<MultiPhase<T> as ElectionProvider<T::AccountId, T::BlockNumber>>::elect());
assert_ok!(<MultiPhase<T> as ElectionProvider>::elect());
} verify {
assert!(<MultiPhase<T>>::queued_solution().is_none());
assert!(<DesiredTargets<T>>::get().is_none());
Expand Down
26 changes: 15 additions & 11 deletions frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@
//! To generate an emergency solution, one must only provide one argument: [`Supports`]. This is
//! essentially a collection of elected winners for the election, and voters who support them. The
//! supports can be generated by any means. In the simplest case, it could be manual. For example,
//! in the case of massive network failure or misbehaviour, [`Config::ForceOrigin`] might decide to
//! in the case of massive network failure or misbehavior, [`Config::ForceOrigin`] might decide to
//! select only a small number of emergency winners (which would greatly restrict the next validator
//! set, if this pallet is used with `pallet-staking`). If the failure is for other technical
//! reasons, then a simple and safe way to generate supports is using the staking-miner binary
Expand Down Expand Up @@ -286,10 +286,7 @@ pub type SolutionTargetIndexOf<T> = <SolutionOf<T> as NposSolution>::TargetIndex
/// The accuracy of the election, when submitted from offchain. Derived from [`SolutionOf`].
pub type SolutionAccuracyOf<T> = <SolutionOf<T> as NposSolution>::Accuracy;
/// The fallback election type.
pub type FallbackErrorOf<T> = <<T as crate::Config>::Fallback as ElectionProvider<
<T as frame_system::Config>::AccountId,
<T as frame_system::Config>::BlockNumber,
>>::Error;
pub type FallbackErrorOf<T> = <<T as crate::Config>::Fallback as ElectionProvider>::Error;

/// Configuration for the benchmarks of the pallet.
pub trait BenchmarkingConfig {
Expand All @@ -312,7 +309,9 @@ pub trait BenchmarkingConfig {
/// A fallback implementation that transitions the pallet to the emergency phase.
pub struct NoFallback<T>(sp_std::marker::PhantomData<T>);

impl<T: Config> ElectionProvider<T::AccountId, T::BlockNumber> for NoFallback<T> {
impl<T: Config> ElectionProvider for NoFallback<T> {
type AccountId = T::AccountId;
type BlockNumber = T::BlockNumber;
type DataProvider = T::DataProvider;
type Error = &'static str;

Expand Down Expand Up @@ -654,7 +653,10 @@ pub mod pallet {
type MinerMaxLength: Get<u32>;

/// Something that will provide the election data.
type DataProvider: ElectionDataProvider<Self::AccountId, Self::BlockNumber>;
type DataProvider: ElectionDataProvider<
AccountId = Self::AccountId,
BlockNumber = Self::BlockNumber,
>;

/// The solution type.
type Solution: codec::Codec
Expand All @@ -669,8 +671,8 @@ pub mod pallet {

/// Configuration for the fallback
type Fallback: ElectionProvider<
Self::AccountId,
Self::BlockNumber,
AccountId = Self::AccountId,
BlockNumber = Self::BlockNumber,
DataProvider = Self::DataProvider,
>;

Expand Down Expand Up @@ -818,7 +820,7 @@ pub mod pallet {
// NOTE that this pallet does not really need to enforce this in runtime. The
// solution cannot represent any voters more than `LIMIT` anyhow.
assert_eq!(
<T::DataProvider as ElectionDataProvider<T::AccountId, T::BlockNumber>>::MAXIMUM_VOTES_PER_VOTER,
<T::DataProvider as ElectionDataProvider>::MAXIMUM_VOTES_PER_VOTER,
<SolutionOf<T> as NposSolution>::LIMIT as u32,
);
}
Expand Down Expand Up @@ -1492,7 +1494,9 @@ impl<T: Config> Pallet<T> {
}
}

impl<T: Config> ElectionProvider<T::AccountId, T::BlockNumber> for Pallet<T> {
impl<T: Config> ElectionProvider for Pallet<T> {
type AccountId = T::AccountId;
type BlockNumber = T::BlockNumber;
type Error = ElectionError<T>;
type DataProvider = T::DataProvider;

Expand Down
8 changes: 6 additions & 2 deletions frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,9 @@ impl onchain::Config for Runtime {
}

pub struct MockFallback;
impl ElectionProvider<AccountId, u64> for MockFallback {
impl ElectionProvider for MockFallback {
type AccountId = AccountId;
type BlockNumber = u64;
type Error = &'static str;
type DataProvider = StakingMock;

Expand Down Expand Up @@ -438,7 +440,9 @@ pub type Extrinsic = sp_runtime::testing::TestXt<Call, ()>;
pub struct ExtBuilder {}

pub struct StakingMock;
impl ElectionDataProvider<AccountId, u64> for StakingMock {
impl ElectionDataProvider for StakingMock {
type AccountId = AccountId;
type BlockNumber = u64;
const MAXIMUM_VOTES_PER_VOTER: u32 = <TestNposSolution as NposSolution>::LIMIT as u32;
fn targets(maybe_max_len: Option<usize>) -> data_provider::Result<Vec<AccountId>> {
let targets = Targets::get();
Expand Down
81 changes: 57 additions & 24 deletions frame/election-provider-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,24 @@
//!
//! pub trait Config: Sized {
//! type ElectionProvider: ElectionProvider<
//! AccountId,
//! BlockNumber,
//! DataProvider = Module<Self>,
//! AccountId = AccountId,
//! BlockNumber = BlockNumber,
//! DataProvider = Pallet<Self>,
//! >;
//! }
//!
//! pub struct Module<T: Config>(std::marker::PhantomData<T>);
//! pub struct Pallet<T: Config>(std::marker::PhantomData<T>);
//!
//! impl<T: Config> ElectionDataProvider<AccountId, BlockNumber> for Module<T> {
//! impl<T: Config> ElectionDataProvider for Pallet<T> {
//! type AccountId = AccountId;
//! type BlockNumber = BlockNumber;
//! const MAXIMUM_VOTES_PER_VOTER: u32 = 1;
//!
//! fn desired_targets() -> data_provider::Result<u32> {
//! Ok(1)
//! }
//! fn voters(maybe_max_len: Option<usize>)
//! -> data_provider::Result<Vec<(AccountId, VoteWeight, Vec<AccountId>)>>
//! -> data_provider::Result<Vec<(AccountId, VoteWeight, Vec<AccountId>)>>
//! {
//! Ok(Default::default())
//! }
Expand All @@ -124,10 +127,12 @@
//! pub struct GenericElectionProvider<T: Config>(std::marker::PhantomData<T>);
//!
//! pub trait Config {
//! type DataProvider: ElectionDataProvider<AccountId, BlockNumber>;
//! type DataProvider: ElectionDataProvider<AccountId=AccountId, BlockNumber = BlockNumber>;
//! }
//!
//! impl<T: Config> ElectionProvider<AccountId, BlockNumber> for GenericElectionProvider<T> {
//! impl<T: Config> ElectionProvider for GenericElectionProvider<T> {
//! type AccountId = AccountId;
//! type BlockNumber = BlockNumber;
//! type Error = &'static str;
//! type DataProvider = T::DataProvider;
//!
Expand All @@ -146,7 +151,7 @@
//!
//! struct Runtime;
//! impl generic_election_provider::Config for Runtime {
//! type DataProvider = data_provider_mod::Module<Runtime>;
//! type DataProvider = data_provider_mod::Pallet<Runtime>;
//! }
//!
//! impl data_provider_mod::Config for Runtime {
Expand Down Expand Up @@ -178,7 +183,13 @@ pub mod data_provider {
}

/// Something that can provide the data to an [`ElectionProvider`].
pub trait ElectionDataProvider<AccountId, BlockNumber> {
pub trait ElectionDataProvider {
/// The account identifier type.
type AccountId;

/// The block number type.
type BlockNumber;

/// Maximum number of votes per voter that this data provider is providing.
const MAXIMUM_VOTES_PER_VOTER: u32;

Expand All @@ -189,7 +200,7 @@ pub trait ElectionDataProvider<AccountId, BlockNumber> {
///
/// 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 targets(maybe_max_len: Option<usize>) -> data_provider::Result<Vec<AccountId>>;
fn targets(maybe_max_len: Option<usize>) -> data_provider::Result<Vec<Self::AccountId>>;

/// All possible voters for the election.
///
Expand All @@ -202,7 +213,7 @@ pub trait ElectionDataProvider<AccountId, BlockNumber> {
/// appropriate weight at the end of execution with the system pallet directly.
fn voters(
maybe_max_len: Option<usize>,
) -> data_provider::Result<Vec<(AccountId, VoteWeight, Vec<AccountId>)>>;
) -> data_provider::Result<Vec<(Self::AccountId, VoteWeight, Vec<Self::AccountId>)>>;

/// The number of targets to elect.
///
Expand All @@ -216,14 +227,14 @@ pub trait ElectionDataProvider<AccountId, BlockNumber> {
/// [`ElectionProvider::elect`].
///
/// This is only useful for stateful election providers.
fn next_election_prediction(now: BlockNumber) -> BlockNumber;
fn next_election_prediction(now: Self::BlockNumber) -> Self::BlockNumber;

/// Utility function only to be used in benchmarking scenarios, to be implemented optionally,
/// else a noop.
#[cfg(any(feature = "runtime-benchmarks", test))]
fn put_snapshot(
_voters: Vec<(AccountId, VoteWeight, Vec<AccountId>)>,
_targets: Vec<AccountId>,
_voters: Vec<(Self::AccountId, VoteWeight, Vec<Self::AccountId>)>,
_targets: Vec<Self::AccountId>,
_target_stake: Option<VoteWeight>,
) {
}
Expand All @@ -233,22 +244,29 @@ pub trait ElectionDataProvider<AccountId, BlockNumber> {
///
/// Same as `put_snapshot`, but can add a single voter one by one.
#[cfg(any(feature = "runtime-benchmarks", test))]
fn add_voter(_voter: AccountId, _weight: VoteWeight, _targets: Vec<AccountId>) {}
fn add_voter(_voter: Self::AccountId, _weight: VoteWeight, _targets: Vec<Self::AccountId>) {}

/// Utility function only to be used in benchmarking scenarios, to be implemented optionally,
/// else a noop.
///
/// Same as `put_snapshot`, but can add a single voter one by one.
#[cfg(any(feature = "runtime-benchmarks", test))]
fn add_target(_target: AccountId) {}
fn add_target(_target: Self::AccountId) {}

/// Clear all voters and targets.
#[cfg(any(feature = "runtime-benchmarks", test))]
fn clear() {}
}

/// An election data provider that should only be used for testing.
#[cfg(feature = "std")]
impl<AccountId, BlockNumber> ElectionDataProvider<AccountId, BlockNumber> for () {
pub struct TestDataProvider<X>(sp_std::marker::PhantomData<X>);

#[cfg(feature = "std")]
impl<AccountId, BlockNumber> ElectionDataProvider for TestDataProvider<(AccountId, BlockNumber)> {
type AccountId = AccountId;
type BlockNumber = BlockNumber;

const MAXIMUM_VOTES_PER_VOTER: u32 = 0;
fn targets(_maybe_max_len: Option<usize>) -> data_provider::Result<Vec<AccountId>> {
Ok(Default::default())
Expand All @@ -271,29 +289,44 @@ impl<AccountId, BlockNumber> ElectionDataProvider<AccountId, BlockNumber> for ()
/// 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<AccountId, BlockNumber> {
pub trait ElectionProvider {
/// The account identifier type.
type AccountId;

/// The block number type.
type BlockNumber;

/// The error type that is returned by the provider.
type Error: Debug;

/// The data provider of the election.
type DataProvider: ElectionDataProvider<AccountId, BlockNumber>;
type DataProvider: ElectionDataProvider<
AccountId = Self::AccountId,
BlockNumber = Self::BlockNumber,
>;

/// Elect a new set of winners.
///
/// 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.
fn elect() -> Result<Supports<AccountId>, Self::Error>;
fn elect() -> Result<Supports<Self::AccountId>, Self::Error>;
}

/// An election provider to be used only for testing.
#[cfg(feature = "std")]
impl<AccountId, BlockNumber> ElectionProvider<AccountId, BlockNumber> for () {
pub struct NoElection<X>(sp_std::marker::PhantomData<X>);

#[cfg(feature = "std")]
impl<AccountId, BlockNumber> ElectionProvider for NoElection<(AccountId, BlockNumber)> {
type AccountId = AccountId;
type BlockNumber = BlockNumber;
type Error = &'static str;
type DataProvider = ();
type DataProvider = TestDataProvider<(AccountId, BlockNumber)>;

fn elect() -> Result<Supports<AccountId>, Self::Error> {
Err("<() as ElectionProvider> cannot do anything.")
Err("<NoElection as ElectionProvider> cannot do anything.")
}
}

Expand Down
13 changes: 10 additions & 3 deletions frame/election-provider-support/src/onchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,15 @@ pub trait Config: frame_system::Config {
/// The accuracy used to compute the election:
type Accuracy: PerThing128;
/// Something that provides the data for election.
type DataProvider: ElectionDataProvider<Self::AccountId, Self::BlockNumber>;
type DataProvider: ElectionDataProvider<
AccountId = Self::AccountId,
BlockNumber = Self::BlockNumber,
>;
}

impl<T: Config> ElectionProvider<T::AccountId, T::BlockNumber> for OnChainSequentialPhragmen<T> {
impl<T: Config> ElectionProvider for OnChainSequentialPhragmen<T> {
type AccountId = T::AccountId;
type BlockNumber = T::BlockNumber;
type Error = Error;
type DataProvider = T::DataProvider;

Expand Down Expand Up @@ -160,7 +165,9 @@ mod tests {
use crate::data_provider;

pub struct DataProvider;
impl ElectionDataProvider<AccountId, BlockNumber> for DataProvider {
impl ElectionDataProvider for DataProvider {
type AccountId = AccountId;
type BlockNumber = BlockNumber;
const MAXIMUM_VOTES_PER_VOTER: u32 = 2;
fn voters(
_: Option<usize>,
Expand Down
4 changes: 3 additions & 1 deletion frame/staking/src/pallet/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,9 @@ impl<T: Config> Pallet<T> {
}
}

impl<T: Config> ElectionDataProvider<T::AccountId, BlockNumberFor<T>> for Pallet<T> {
impl<T: Config> ElectionDataProvider for Pallet<T> {
type AccountId = T::AccountId;
type BlockNumber = BlockNumberFor<T>;
const MAXIMUM_VOTES_PER_VOTER: u32 = T::MAX_NOMINATIONS;

fn desired_targets() -> data_provider::Result<u32> {
Expand Down
8 changes: 4 additions & 4 deletions frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,16 @@ pub mod pallet {

/// Something that provides the election functionality.
type ElectionProvider: frame_election_provider_support::ElectionProvider<
Self::AccountId,
Self::BlockNumber,
AccountId = Self::AccountId,
BlockNumber = Self::BlockNumber,
// we only accept an election provider that has staking as data provider.
DataProvider = Pallet<Self>,
>;

/// Something that provides the election functionality at genesis.
type GenesisElectionProvider: frame_election_provider_support::ElectionProvider<
Self::AccountId,
Self::BlockNumber,
AccountId = Self::AccountId,
BlockNumber = Self::BlockNumber,
DataProvider = Pallet<Self>,
>;

Expand Down
6 changes: 3 additions & 3 deletions frame/staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4013,7 +4013,7 @@ mod election_data_provider {
ExtBuilder::default().build_and_execute(|| {
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
assert_eq!(
<Staking as ElectionDataProvider<AccountId, BlockNumber>>::voters(None)
<Staking as ElectionDataProvider>::voters(None)
.unwrap()
.iter()
.find(|x| x.0 == 101)
Expand All @@ -4028,7 +4028,7 @@ mod election_data_provider {
// 11 is gone.
start_active_era(2);
assert_eq!(
<Staking as ElectionDataProvider<AccountId, BlockNumber>>::voters(None)
<Staking as ElectionDataProvider>::voters(None)
.unwrap()
.iter()
.find(|x| x.0 == 101)
Expand All @@ -4040,7 +4040,7 @@ mod election_data_provider {
// resubmit and it is back
assert_ok!(Staking::nominate(Origin::signed(100), vec![11, 21]));
assert_eq!(
<Staking as ElectionDataProvider<AccountId, BlockNumber>>::voters(None)
<Staking as ElectionDataProvider>::voters(None)
.unwrap()
.iter()
.find(|x| x.0 == 101)
Expand Down

0 comments on commit 7f68a8b

Please sign in to comment.