From de627eec05a390bd5a9a698b5256cedb56891c47 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 24 Jul 2022 15:00:13 +0100 Subject: [PATCH 1/7] draft for Oliver --- .../election-provider-multi-phase/src/lib.rs | 14 ++- frame/election-provider-support/src/lib.rs | 90 ++++++++++++++++++- .../election-provider-support/src/onchain.rs | 32 ++++++- primitives/npos-elections/src/lib.rs | 47 ++++++---- 4 files changed, 160 insertions(+), 23 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index e1d3cb8ed5dee..5a829c1176e08 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -232,6 +232,7 @@ use codec::{Decode, Encode}; use frame_election_provider_support::{ ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolution, + BoundedSupportsOf }; use frame_support::{ dispatch::DispatchResultWithPostInfo, @@ -696,6 +697,10 @@ pub mod pallet { /// OCW election solution miner algorithm implementation. type Solver: NposSolver; + /// Maximum number of backers per winner that this pallet should, as its implementation of + /// `ElectionProvider` return. + type MaxBackersPerWinner: Get; + /// Origin that can control this pallet. Note that any action taken by this origin (such) /// as providing an emergency solution is not checked. Thus, it must be a trusted origin. type ForceOrigin: EnsureOrigin; @@ -1570,14 +1575,21 @@ impl ElectionProvider for Pallet { type AccountId = T::AccountId; type BlockNumber = T::BlockNumber; type Error = ElectionError; + type MaxBackersPerWinner = T::MaxBackersPerWinner; type DataProvider = T::DataProvider; - fn elect() -> Result, Self::Error> { + fn elect() -> Result, Self::Error> { match Self::do_elect() { Ok(supports) => { // All went okay, record the weight, put sign to be Off, clean snapshot, etc. Self::weigh_supports(&supports); Self::rotate_round(); + + // TODO: this is just to make it compile -- this should be checked inside of + // `feasibility_check`, and ReadySolution should already store a `BoundedSupports`. + use frame_election_provider_support::TruncateIntoBoundedSupports; + use sp_runtime::traits::Convert; + let supports = TruncateIntoBoundedSupports::::convert(supports); Ok(supports) }, Err(why) => { diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index eee865d0b737b..bcf1a96aedd2e 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -168,7 +168,11 @@ pub mod onchain; pub mod traits; -use sp_runtime::traits::{Bounded, Saturating, Zero}; +use codec::{Decode, Encode, MaxEncodedLen}; +use frame_support::{traits::ConstU32, DebugNoBound, DefaultNoBound, RuntimeDebugNoBound}; +use scale_info::TypeInfo; +use sp_npos_elections::EvaluateSupport; +use sp_runtime::traits::{Bounded, Convert, Saturating, Zero}; use sp_std::{fmt::Debug, prelude::*}; /// Re-export the solution generation macro. @@ -370,6 +374,12 @@ pub trait ElectionProvider { BlockNumber = Self::BlockNumber, >; + /// Maximum number of backers that each winner will have in any call to + /// [`ElectionProvider::elect`]. + /// + /// It is up to any particular implementation to know how it is going to achieve this property. + type MaxBackersPerWinner: Get; + /// 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. /// @@ -377,7 +387,7 @@ pub trait ElectionProvider { /// /// 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>; + fn elect() -> Result, Self::Error>; } /// A sub-trait of the [`ElectionProvider`] for cases where we need to be sure an election needs to @@ -416,8 +426,9 @@ where type BlockNumber = BlockNumber; type Error = &'static str; type DataProvider = DataProvider; + type MaxBackersPerWinner = ConstU32<0>; - fn elect() -> Result, Self::Error> { + fn elect() -> Result, Self::Error> { Err(" cannot do anything.") } } @@ -618,3 +629,76 @@ pub type Voter = (AccountId, VoteWeight, BoundedVec = Voter<::AccountId, ::MaxVotesPerVoter>; + +/// A bounded equivalent to [`sp_npos_elections::Support`]. +#[derive(Default, RuntimeDebug, Encode, Decode, scale_info::TypeInfo, MaxEncodedLen)] +#[codec(mel_bound(AccountId: MaxEncodedLen, Bound: Get))] +#[scale_info(skip_type_params(Bound))] +pub struct BoundedSupport> { + /// Total support. + pub total: ExtendedBalance, + /// Support from voters. + pub voters: BoundedVec<(AccountId, ExtendedBalance), Bound>, +} + +pub type BoundedSupportOf = BoundedSupport< + ::AccountId, + ::MaxBackersPerWinner, +>; + +pub type BoundedSupportsOf = Vec<(::AccountId, BoundedSupportOf)>; + +impl> sp_npos_elections::Backings for &BoundedSupport { + fn total(&self) -> ExtendedBalance { + self.total + } +} + +impl> PartialEq for BoundedSupport { + fn eq(&self, other: &Self) -> bool { + self.total == other.total && self.voters == other.voters + } +} +impl> Eq for BoundedSupport {} + +impl> Clone for BoundedSupport { + fn clone(&self) -> Self { + Self { voters: self.voters.clone(), total: self.total } + } +} + +impl> From> for Support { + fn from(b: BoundedSupport) -> Self { + Support { total: b.total, voters: b.voters.into_inner() } + } +} + +impl> TryFrom> + for BoundedSupport +{ + type Error = &'static str; + fn try_from(s: sp_npos_elections::Support) -> Result { + let voters = s.voters.try_into().map_err(|_| "voters bound not respected")?; + Ok(Self { voters, total: s.total }) + } +} + +pub struct SortIntoBoundedSupports(sp_std::marker::PhantomData); + +impl Convert, BoundedSupportsOf> + for SortIntoBoundedSupports +{ + fn convert(a: sp_npos_elections::Supports) -> BoundedSupportsOf { + todo!(); + } +} + +pub struct TruncateIntoBoundedSupports(sp_std::marker::PhantomData); + +impl Convert, BoundedSupportsOf> + for TruncateIntoBoundedSupports +{ + fn convert(a: sp_npos_elections::Supports) -> BoundedSupportsOf { + todo!(); + } +} diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 62e76c3888822..6c7f3b337e848 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -20,10 +20,12 @@ //! careful when using it onchain. use crate::{ - Debug, ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolver, WeightInfo, + BoundedSupport, BoundedSupportsOf, Debug, ElectionDataProvider, ElectionProvider, + InstantElectionProvider, NposSolver, WeightInfo, }; use frame_support::{traits::Get, weights::DispatchClass}; use sp_npos_elections::*; +use sp_runtime::traits::Convert; use sp_std::{collections::btree_map::BTreeMap, marker::PhantomData, prelude::*}; /// Errors of the on-chain election. @@ -77,6 +79,23 @@ pub trait Config { AccountId = ::AccountId, Error = sp_npos_elections::Error, >; + /// Maximum number of backers per winner to return. + type MaxBackersPerWinner: Get; + + /// Something that can convert the final `Supports` into a bounded version. + type Bounder: Convert< + sp_npos_elections::Supports<::AccountId>, + Vec< + ( + ::AccountId, + BoundedSupport< + ::AccountId, + Self::MaxBackersPerWinner, + > + ) + >, + >; + /// Something that provides the data for election. type DataProvider: ElectionDataProvider< AccountId = ::AccountId, @@ -129,6 +148,8 @@ fn elect_with( DispatchClass::Mandatory, ); + // TODO: not very efficient, but cleaner API. Ideally we would possibly trim while + // `to_supports`. Ok(to_supports(&staked)) } @@ -136,9 +157,10 @@ impl ElectionProvider for UnboundedExecution { type AccountId = ::AccountId; type BlockNumber = ::BlockNumber; type Error = Error; + type MaxBackersPerWinner = T::MaxBackersPerWinner; type DataProvider = T::DataProvider; - fn elect() -> Result, Self::Error> { + fn elect() -> Result, Self::Error> { // This should not be called if not in `std` mode (and therefore neither in genesis nor in // testing) if cfg!(not(feature = "std")) { @@ -148,7 +170,7 @@ impl ElectionProvider for UnboundedExecution { ); } - elect_with::(None, None) + elect_with::(None, None).map(|supports| T::Bounder::convert(supports)) } } @@ -165,10 +187,12 @@ impl ElectionProvider for BoundedExecution { type AccountId = ::AccountId; type BlockNumber = ::BlockNumber; type Error = Error; + type MaxBackersPerWinner = T::MaxBackersPerWinner; type DataProvider = T::DataProvider; - fn elect() -> Result, Self::Error> { + fn elect() -> Result, Self::Error> { elect_with::(Some(T::VotersBound::get() as usize), Some(T::TargetsBound::get() as usize)) + .map(|supports| T::Bounder::convert(supports)) } } diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index dd2a9bf198f8d..106b5c7bf81a6 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -444,6 +444,18 @@ impl Default for Support { } } +/// Generic representation of a support. +pub trait Backings { + /// The total backing of an individual winner. + fn total(&self) -> ExtendedBalance; +} + +impl Backings for &Support { + fn total(&self) -> ExtendedBalance { + self.total + } +} + /// A target-major representation of the the election outcome. /// /// Essentially a flat variant of [`SupportMap`]. @@ -494,23 +506,28 @@ pub trait EvaluateSupport { impl EvaluateSupport for Supports { fn evaluate(&self) -> ElectionScore { - let mut minimal_stake = ExtendedBalance::max_value(); - let mut sum_stake: ExtendedBalance = Zero::zero(); - // NOTE: The third element might saturate but fine for now since this will run on-chain and - // need to be fast. - let mut sum_stake_squared: ExtendedBalance = Zero::zero(); - - for (_, support) in self { - sum_stake = sum_stake.saturating_add(support.total); - let squared = support.total.saturating_mul(support.total); - sum_stake_squared = sum_stake_squared.saturating_add(squared); - if support.total < minimal_stake { - minimal_stake = support.total; - } - } + evaluate_support_core(self.iter().map(|(_, s)| s)) + } +} - ElectionScore { minimal_stake, sum_stake, sum_stake_squared } +/// Core implementation of how to evaluate a support (in the most generic form), exported as a +/// free-standing function for easy re-use. +pub fn evaluate_support_core(backings: impl Iterator) -> ElectionScore { + let mut minimal_stake = ExtendedBalance::max_value(); + let mut sum_stake: ExtendedBalance = Zero::zero(); + // NOTE: The third element might saturate but fine for now since this will run on-chain and + // need to be fast. + let mut sum_stake_squared: ExtendedBalance = Zero::zero(); + + for backing in backings { + sum_stake = sum_stake.saturating_add(backing.total()); + let squared = backing.total().saturating_mul(backing.total()); + sum_stake_squared = sum_stake_squared.saturating_add(squared); + if backing.total() < minimal_stake { + minimal_stake = backing.total(); + } } + ElectionScore { minimal_stake, sum_stake, sum_stake_squared } } /// Converts raw inputs to types used in this crate. From adce6cfe1285fc34c41a43d412821987ea20fd63 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 27 Jul 2022 17:38:34 +0200 Subject: [PATCH 2/7] Tests compile Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 1 + bin/node/runtime/Cargo.toml | 2 + bin/node/runtime/src/lib.rs | 10 +- frame/babe/src/mock.rs | 5 +- .../election-provider-multi-phase/src/lib.rs | 105 +++++++++++++----- .../election-provider-multi-phase/src/mock.rs | 12 +- .../src/signed.rs | 6 +- .../src/unsigned.rs | 5 +- .../solution-type/src/single_page.rs | 2 + frame/election-provider-support/src/lib.rs | 6 +- .../election-provider-support/src/onchain.rs | 54 +++++++-- frame/election-provider-support/src/traits.rs | 3 + frame/grandpa/src/mock.rs | 5 +- frame/offences/benchmarking/src/mock.rs | 6 +- frame/session/benchmarking/src/mock.rs | 6 +- frame/staking/src/mock.rs | 5 +- frame/staking/src/pallet/impls.rs | 6 +- frame/staking/src/pallet/mod.rs | 1 + frame/staking/src/tests.rs | 33 +++++- 19 files changed, 213 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3d5bfe5e51201..b48537861d5a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4858,6 +4858,7 @@ dependencies = [ "sp-core", "sp-inherents", "sp-io", + "sp-npos-elections", "sp-offchain", "sp-runtime", "sp-sandbox", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index ca971f29e93c9..9d422e810c668 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -40,6 +40,7 @@ sp-transaction-pool = { version = "4.0.0-dev", default-features = false, path = sp-version = { version = "5.0.0", default-features = false, path = "../../../primitives/version" } sp-io = { version = "6.0.0", default-features = false, path = "../../../primitives/io" } sp-sandbox = { version = "0.10.0-dev", default-features = false, path = "../../../primitives/sandbox" } +sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/npos-elections" } # frame dependencies frame-executive = { version = "4.0.0-dev", default-features = false, path = "../../../frame/executive" } @@ -189,6 +190,7 @@ std = [ "sp-io/std", "pallet-child-bounties/std", "pallet-alliance/std", + "sp-npos-elections/std", ] runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index b2efcb196787d..c34b6f5861a45 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -24,7 +24,9 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::{ - onchain, BalancingConfig, ElectionDataProvider, SequentialPhragmen, VoteWeight, + onchain, + onchain::{TruncatingBounder, TruncatingBounderOf}, + BalancingConfig, ElectionDataProvider, SequentialPhragmen, VoteWeight, }; use frame_support::{ construct_runtime, @@ -658,6 +660,10 @@ impl onchain::Config for OnChainSeqPhragmen { >; type DataProvider = ::DataProvider; type WeightInfo = frame_election_provider_support::weights::SubstrateWeight; + + // FIXME no idea what to use here + type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncatingBounderOf; } impl onchain::BoundedConfig for OnChainSeqPhragmen { @@ -711,6 +717,8 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type ForceOrigin = EnsureRootOrHalfCouncil; type MaxElectableTargets = ConstU16<{ u16::MAX }>; type MaxElectingVoters = MaxElectingVoters; + // TODO what to use here + type MaxBackersPerWinner = ConstU32<16>; type BenchmarkingConfig = ElectionProviderBenchmarkConfig; type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight; } diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 5677eb7e28e49..61323eb391054 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -19,7 +19,7 @@ use crate::{self as pallet_babe, Config, CurrentSlot}; use codec::Encode; -use frame_election_provider_support::{onchain, SequentialPhragmen}; +use frame_election_provider_support::{onchain, onchain::TruncatingBounderOf, SequentialPhragmen}; use frame_support::{ parameter_types, traits::{ConstU128, ConstU32, ConstU64, GenesisBuild, KeyOwnerProofSystem, OnInitialize}, @@ -178,6 +178,9 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen; type DataProvider = Staking; type WeightInfo = (); + // FIXME no idea what to use here + type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncatingBounderOf; } impl pallet_staking::Config for Test { diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 5a829c1176e08..5479d20c567b7 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -229,16 +229,17 @@ #![cfg_attr(not(feature = "std"), no_std)] -use codec::{Decode, Encode}; +use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::{ - ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolution, - BoundedSupportsOf + BoundedSupport, BoundedSupportsOf, ElectionDataProvider, ElectionProvider, + InstantElectionProvider, NposSolution, }; use frame_support::{ dispatch::DispatchResultWithPostInfo, ensure, traits::{Currency, Get, OnUnbalanced, ReservableCurrency}, weights::{DispatchClass, Weight}, + BoundedVec, DebugNoBound, }; use frame_system::{ensure_none, offchain::SendTransactionTypes}; use scale_info::TypeInfo; @@ -247,7 +248,8 @@ use sp_arithmetic::{ UpperOf, }; use sp_npos_elections::{ - assignment_ratio_to_staked_normalized, ElectionScore, EvaluateSupport, Supports, VoteWeight, + assignment_ratio_to_staked_normalized, ElectionScore, EvaluateSupport, Support, Supports, + VoteWeight, }; use sp_runtime::{ transaction_validity::{ @@ -318,8 +320,9 @@ impl ElectionProvider for NoFallback { type BlockNumber = T::BlockNumber; type DataProvider = T::DataProvider; type Error = &'static str; + type MaxBackersPerWinner = T::MaxBackersPerWinner; - fn elect() -> Result, Self::Error> { + fn elect() -> Result, Self::Error> { // Do nothing, this will enable the emergency phase. Err("NoFallback.") } @@ -394,7 +397,7 @@ impl Phase { } /// The type of `Computation` that provided this election data. -#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo)] +#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug, MaxEncodedLen, TypeInfo)] pub enum ElectionCompute { /// Election was computed on-chain. OnChain, @@ -437,14 +440,20 @@ impl Default for RawSolution { } } +use frame_support::{DefaultNoBound, EqNoBound, PartialEqNoBound}; + /// A checked solution, ready to be enacted. -#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default, TypeInfo)] -pub struct ReadySolution { +#[derive(PartialEqNoBound, Clone, Encode, Decode, RuntimeDebug, DefaultNoBound, TypeInfo)] +#[scale_info(skip_type_params(MaxBackersPerWinner))] +pub struct ReadySolution> +where + AccountId: PartialEq, +{ /// The final supports of the solution. /// /// This is target-major vector, storing each winners, total backing, and each individual /// backer. - pub supports: Supports, + pub supports: Vec<(AccountId, BoundedSupport)>, /// The score of the solution. /// /// This is needed to potentially challenge the solution. @@ -453,6 +462,23 @@ pub struct ReadySolution { pub compute: ElectionCompute, } +impl> From)>> + for ReadySolution +where + AccountId: PartialEq, +{ + fn from(supports: Vec<(AccountId, Support)>) -> Self { + Self { + supports: Default::default(), // FIXME @ggwpez + score: Default::default(), + compute: Default::default(), + } + } +} + +pub type ReadySolutionOf = + ReadySolution<::AccountId, ::MaxBackersPerWinner>; + /// A snapshot of all the data that is needed for en entire round. They are provided by /// [`ElectionDataProvider`] and are kept around until the round is finished. /// @@ -552,6 +578,7 @@ pub enum FeasibilityError { InvalidRound, /// Comparison against `MinimumUntrustedScore` failed. UntrustedScoreTooLow, + TooManyVotes, } impl From for FeasibilityError { @@ -682,6 +709,7 @@ pub mod pallet { AccountId = Self::AccountId, BlockNumber = Self::BlockNumber, DataProvider = Self::DataProvider, + MaxBackersPerWinner = Self::MaxBackersPerWinner, >; /// Configuration of the governance-only fallback. @@ -692,6 +720,7 @@ pub mod pallet { AccountId = Self::AccountId, BlockNumber = Self::BlockNumber, DataProvider = Self::DataProvider, + MaxBackersPerWinner = Self::MaxBackersPerWinner, >; /// OCW election solution miner algorithm implementation. @@ -947,11 +976,12 @@ pub mod pallet { // Note: we don't `rotate_round` at this point; the next call to // `ElectionProvider::elect` will succeed and take care of that. - let solution = ReadySolution { + let solution = ReadySolution::from( supports, - score: Default::default(), - compute: ElectionCompute::Emergency, - }; + /*score: Default::default(), + compute: ElectionCompute::Emergency, + };*/ + ); Self::deposit_event(Event::SolutionStored { election_compute: ElectionCompute::Emergency, @@ -1061,11 +1091,13 @@ pub mod pallet { Error::::FallbackFailed })?; - let solution = ReadySolution { + /*let solution = ReadySolution { supports, score: Default::default(), compute: ElectionCompute::Fallback, - }; + };*/ + // FIXME + let solution = ReadySolution::from(supports); Self::deposit_event(Event::SolutionStored { election_compute: ElectionCompute::Fallback, @@ -1200,7 +1232,7 @@ pub mod pallet { /// Current best solution, signed or unsigned, queued to be returned upon `elect`. #[pallet::storage] #[pallet::getter(fn queued_solution)] - pub type QueuedSolution = StorageValue<_, ReadySolution>; + pub type QueuedSolution = StorageValue<_, ReadySolutionOf>>; /// Snapshot data of the round. /// @@ -1434,7 +1466,7 @@ impl Pallet { pub fn feasibility_check( raw_solution: RawSolution>, compute: ElectionCompute, - ) -> Result, FeasibilityError> { + ) -> Result, FeasibilityError> { let RawSolution { solution, score, round } = raw_solution; // First, check round. @@ -1507,7 +1539,8 @@ impl Pallet { let known_score = supports.evaluate(); ensure!(known_score == score, FeasibilityError::InvalidScore); - Ok(ReadySolution { supports, compute, score }) + // FIXME @ggwpez + Ok(supports.into()) } /// Perform the tasks to be done after a new `elect` has been triggered: @@ -1526,7 +1559,7 @@ impl Pallet { Self::kill_snapshot(); } - fn do_elect() -> Result, ElectionError> { + fn do_elect() -> Result, ElectionError> { // We have to unconditionally try finalizing the signed phase here. There are only two // possibilities: // @@ -1542,7 +1575,7 @@ impl Pallet { .map_err(|fe| ElectionError::Fallback(fe)) .map(|supports| (supports, ElectionCompute::Fallback)) }, - |ReadySolution { supports, compute, .. }| Ok((supports, compute)), + |ReadySolutionOf:: { supports, compute, .. }| Ok((supports, compute)), ) .map(|(supports, compute)| { Self::deposit_event(Event::ElectionFinalized { election_compute: Some(compute) }); @@ -1561,7 +1594,7 @@ impl Pallet { } /// record the weight of the given `supports`. - fn weigh_supports(supports: &Supports) { + fn weigh_supports(supports: &BoundedSupportsOf) { let active_voters = supports .iter() .map(|(_, x)| x) @@ -1585,11 +1618,6 @@ impl ElectionProvider for Pallet { Self::weigh_supports(&supports); Self::rotate_round(); - // TODO: this is just to make it compile -- this should be checked inside of - // `feasibility_check`, and ReadySolution should already store a `BoundedSupports`. - use frame_election_provider_support::TruncateIntoBoundedSupports; - use sp_runtime::traits::Convert; - let supports = TruncateIntoBoundedSupports::::convert(supports); Ok(supports) }, Err(why) => { @@ -2031,6 +2059,7 @@ mod tests { #[test] fn fallback_strategy_works() { + use frame_support::bounded_vec; ExtBuilder::default().onchain_fallback(true).build_and_execute(|| { roll_to(25); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); @@ -2042,8 +2071,20 @@ mod tests { assert_eq!( supports, vec![ - (30, Support { total: 40, voters: vec![(2, 5), (4, 5), (30, 30)] }), - (40, Support { total: 60, voters: vec![(2, 5), (3, 10), (4, 5), (40, 40)] }) + ( + 30, + BoundedSupport { + total: 40, + voters: bounded_vec![(2, 5), (4, 5), (30, 30)] + } + ), + ( + 40, + BoundedSupport { + total: 60, + voters: bounded_vec![(2, 5), (3, 10), (4, 5), (40, 40)] + } + ) ] ) }); @@ -2198,6 +2239,14 @@ mod tests { }) } + #[test] + fn compare_ready_solution() { + let solution = ReadySolutionOf::::default(); + + assert_eq!(solution, solution); + assert!(solution == solution); + } + #[test] fn number_of_voters_allowed_2sec_block() { // Just a rough estimate with the substrate weights. diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 7eff70b47eba5..d6abeccdf004e 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -19,7 +19,7 @@ use super::*; use crate::{self as multi_phase, unsigned::MinerConfig}; use frame_election_provider_support::{ data_provider, - onchain::{self, UnboundedExecution}, + onchain::{self, TruncatingBounderOf, UnboundedExecution}, ElectionDataProvider, NposSolution, SequentialPhragmen, }; pub use frame_support::{assert_noop, assert_ok, pallet_prelude::GetDefault}; @@ -44,7 +44,7 @@ use sp_npos_elections::{ }; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, IdentityLookup}, + traits::{BlakeTwo256, Convert, IdentityLookup}, PerU16, }; use std::sync::Arc; @@ -292,6 +292,9 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen, Balancing>; type DataProvider = StakingMock; type WeightInfo = (); + // FIXME no idea what to use here + type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncatingBounderOf; } pub struct MockFallback; @@ -300,9 +303,11 @@ impl ElectionProvider for MockFallback { type BlockNumber = u64; type Error = &'static str; type DataProvider = StakingMock; + type MaxBackersPerWinner = ConstU32<16>; - fn elect() -> Result, Self::Error> { + fn elect() -> Result, Self::Error> { Self::elect_with_bounds(Bounded::max_value(), Bounded::max_value()) + .map(|supports| ::Bounder::convert(supports)) } } @@ -385,6 +390,7 @@ impl crate::Config for Runtime { type ForceOrigin = frame_system::EnsureRoot; type MaxElectingVoters = MaxElectingVoters; type MaxElectableTargets = MaxElectableTargets; + type MaxBackersPerWinner = ConstU32<16>; type MinerConfig = Self; type Solver = SequentialPhragmen, Balancing>; } diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs index eca75139f925a..aa9e16c2a5235 100644 --- a/frame/election-provider-multi-phase/src/signed.rs +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -19,8 +19,8 @@ use crate::{ unsigned::MinerConfig, Config, ElectionCompute, Pallet, QueuedSolution, RawSolution, - ReadySolution, SignedSubmissionIndices, SignedSubmissionNextIndex, SignedSubmissionsMap, - SolutionOf, SolutionOrSnapshotSize, Weight, WeightInfo, + ReadySolution, ReadySolutionOf, SignedSubmissionIndices, SignedSubmissionNextIndex, + SignedSubmissionsMap, SolutionOf, SolutionOrSnapshotSize, Weight, WeightInfo, }; use codec::{Decode, Encode, HasCompact}; use frame_election_provider_support::NposSolution; @@ -451,7 +451,7 @@ impl Pallet { /// /// Infallible pub fn finalize_signed_phase_accept_solution( - ready_solution: ReadySolution, + ready_solution: ReadySolutionOf, who: &T::AccountId, deposit: BalanceOf, call_fee: BalanceOf, diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index de25355f0ca5b..9a0ac714fc7e8 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -19,7 +19,8 @@ use crate::{ helpers, Call, Config, ElectionCompute, Error, FeasibilityError, Pallet, RawSolution, - ReadySolution, RoundSnapshot, SolutionAccuracyOf, SolutionOf, SolutionOrSnapshotSize, Weight, + ReadySolution, ReadySolutionOf, RoundSnapshot, SolutionAccuracyOf, SolutionOf, + SolutionOrSnapshotSize, Weight, }; use codec::Encode; use frame_election_provider_support::{NposSolution, NposSolver, PerThing128, VoteWeight}; @@ -351,7 +352,7 @@ impl Pallet { // ensure score is being improved. Panic henceforth. ensure!( - Self::queued_solution().map_or(true, |q: ReadySolution<_>| raw_solution + Self::queued_solution().map_or(true, |q: ReadySolutionOf| raw_solution .score .strict_threshold_better(q.score, T::BetterUnsignedThreshold::get())), Error::::PreDispatchWeakSubmission, diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index a7ccf5085d2b1..ff83a4d02df46 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -105,6 +105,8 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { use _feps::__OrInvalidIndex; impl _feps::NposSolution for #ident { const LIMIT: usize = #count; + + type Limit = sp_runtime::traits::ConstU32<{#count as u32}>; type VoterIndex = #voter_type; type TargetIndex = #target_type; type Accuracy = #weight_type; diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index bcf1a96aedd2e..924ea4aae9071 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -360,7 +360,7 @@ pub trait ElectionDataProvider { /// implemented of this trait through [`ElectionProvider::DataProvider`]. pub trait ElectionProvider { /// The account identifier type. - type AccountId; + type AccountId: Eq + Clone; /// The block number type. type BlockNumber; @@ -420,6 +420,7 @@ pub struct NoElection(sp_std::marker::PhantomData); impl ElectionProvider for NoElection<(AccountId, BlockNumber, DataProvider)> where + AccountId: Eq + Clone, DataProvider: ElectionDataProvider, { type AccountId = AccountId; @@ -646,6 +647,9 @@ pub type BoundedSupportOf = BoundedSupport< ::MaxBackersPerWinner, >; +pub type BoundedSupports = + Vec<(AccountId, BoundedSupport)>; +// todo transform pub type BoundedSupportsOf = Vec<(::AccountId, BoundedSupportOf)>; impl> sp_npos_elections::Backings for &BoundedSupport { diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 6c7f3b337e848..3f191bdf1b0c3 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -70,6 +70,36 @@ pub struct BoundedExecution(PhantomData); /// This can be very expensive to run frequently on-chain. Use with care. pub struct UnboundedExecution(PhantomData); +// TODO delete this, i did not see `TruncateIntoBoundedSupports` before :facepalm: +pub type TruncatingBounderOf = + TruncatingBounder<::AccountId, MaxBackersPerWinner>; +pub struct TruncatingBounder>( + sp_std::marker::PhantomData<(AccountId, MaxBackersPerWinner)>, +); + +impl> + Convert, Vec<(AccountId, BoundedSupport)>> + for TruncatingBounder +{ + /// Make conversion. + fn convert( + a: Supports, + ) -> Vec<(AccountId, BoundedSupport)> { + // TODO move to TruncateIntoBoundedSupports + a.into_iter() + .map(|(who, support)| { + ( + who, + BoundedSupport { + total: support.total, + voters: sp_runtime::BoundedVec::truncate_from(support.voters), + }, + ) + }) + .collect() + } +} + /// Configuration trait for an onchain election execution. pub trait Config { /// Needed for weight registration. @@ -85,15 +115,13 @@ pub trait Config { /// Something that can convert the final `Supports` into a bounded version. type Bounder: Convert< sp_npos_elections::Supports<::AccountId>, - Vec< - ( + Vec<( ::AccountId, BoundedSupport< ::AccountId, Self::MaxBackersPerWinner, - > - ) - >, + >, + )>, >; /// Something that provides the data for election. @@ -267,6 +295,9 @@ mod tests { type Solver = SequentialPhragmen; type DataProvider = mock_data_provider::DataProvider; type WeightInfo = (); + // FIXME no idea what to use here + type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncatingBounderOf; } impl BoundedConfig for PhragmenParams { @@ -279,6 +310,9 @@ mod tests { type Solver = PhragMMS; type DataProvider = mock_data_provider::DataProvider; type WeightInfo = (); + // FIXME no idea what to use here + type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncatingBounderOf; } impl BoundedConfig for PhragMMSParams { @@ -321,12 +355,13 @@ mod tests { #[test] fn onchain_seq_phragmen_works() { + use frame_support::bounded_vec; sp_io::TestExternalities::new_empty().execute_with(|| { assert_eq!( BoundedExecution::::elect().unwrap(), vec![ - (10, Support { total: 25, voters: vec![(1, 10), (3, 15)] }), - (30, Support { total: 35, voters: vec![(2, 20), (3, 15)] }) + (10, BoundedSupport { total: 25, voters: bounded_vec![(1, 10), (3, 15)] }), + (30, BoundedSupport { total: 35, voters: bounded_vec![(2, 20), (3, 15)] }) ] ); }) @@ -334,12 +369,13 @@ mod tests { #[test] fn onchain_phragmms_works() { + use frame_support::bounded_vec; sp_io::TestExternalities::new_empty().execute_with(|| { assert_eq!( BoundedExecution::::elect().unwrap(), vec![ - (10, Support { total: 25, voters: vec![(1, 10), (3, 15)] }), - (30, Support { total: 35, voters: vec![(2, 20), (3, 15)] }) + (10, BoundedSupport { total: 25, voters: bounded_vec![(1, 10), (3, 15)] }), + (30, BoundedSupport { total: 35, voters: bounded_vec![(2, 20), (3, 15)] }) ] ); }) diff --git a/frame/election-provider-support/src/traits.rs b/frame/election-provider-support/src/traits.rs index ed812e2e0f2c4..189dc1950f4ac 100644 --- a/frame/election-provider-support/src/traits.rs +++ b/frame/election-provider-support/src/traits.rs @@ -22,6 +22,7 @@ use codec::Encode; use scale_info::TypeInfo; use sp_arithmetic::traits::{Bounded, UniqueSaturatedInto}; use sp_npos_elections::{ElectionScore, Error, EvaluateSupport}; +use sp_runtime::traits::Get; use sp_std::{fmt::Debug, prelude::*}; /// An opaque index-based, NPoS solution type. @@ -32,6 +33,8 @@ where /// The maximum number of votes that are allowed. const LIMIT: usize; + type Limit: Get; + /// The voter type. Needs to be an index (convert to usize). type VoterIndex: UniqueSaturatedInto + TryInto diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index 5e6c955c441c5..7872f1072948d 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -22,7 +22,7 @@ use crate::{self as pallet_grandpa, AuthorityId, AuthorityList, Config, ConsensusLog}; use ::grandpa as finality_grandpa; use codec::Encode; -use frame_election_provider_support::{onchain, SequentialPhragmen}; +use frame_election_provider_support::{onchain, onchain::TruncatingBounderOf, SequentialPhragmen}; use frame_support::{ parameter_types, traits::{ @@ -182,6 +182,9 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen; type DataProvider = Staking; type WeightInfo = (); + // FIXME no idea what to use here + type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncatingBounderOf; } impl pallet_staking::Config for Test { diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index d51a81b1212c0..d2516ff0fd4c5 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -20,7 +20,7 @@ #![cfg(test)] use super::*; -use frame_election_provider_support::{onchain, SequentialPhragmen}; +use frame_election_provider_support::{onchain, onchain::TruncatingBounderOf, SequentialPhragmen}; use frame_support::{ parameter_types, traits::{ConstU32, ConstU64}, @@ -154,6 +154,10 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen; type DataProvider = Staking; type WeightInfo = (); + + // FIXME no idea what to use here + type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncatingBounderOf; } impl pallet_staking::Config for Test { diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index c777f2c56de3a..616d9a0c9d39c 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -19,7 +19,7 @@ #![cfg(test)] -use frame_election_provider_support::{onchain, SequentialPhragmen}; +use frame_election_provider_support::{onchain, onchain::TruncatingBounderOf, SequentialPhragmen}; use frame_support::{ parameter_types, traits::{ConstU32, ConstU64}, @@ -160,6 +160,10 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen; type DataProvider = Staking; type WeightInfo = (); + + // FIXME no idea what to use here + type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncatingBounderOf; } impl pallet_staking::Config for Test { diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index bd2d8cdc32ce9..a827d15127b20 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -19,7 +19,7 @@ use crate::{self as pallet_staking, *}; use frame_election_provider_support::{ - onchain, SequentialPhragmen, SortedListProvider, VoteWeight, + onchain, onchain::TruncatingBounderOf, SequentialPhragmen, SortedListProvider, VoteWeight, }; use frame_support::{ assert_ok, parameter_types, @@ -255,6 +255,9 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen; type DataProvider = Staking; type WeightInfo = (); + // FIXME no idea what to use here + type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncatingBounderOf; } pub struct MockReward {} diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 7656eec80a5ff..6670157f9caf6 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, BoundedSupportsOf, ElectionDataProvider, ElectionProvider, ScoreProvider, + SortedListProvider, Supports, VoteWeight, VoterOf, }; use frame_support::{ pallet_prelude::*, @@ -526,7 +526,7 @@ impl Pallet { /// Consume a set of [`Supports`] from [`sp_npos_elections`] and collect them into a /// [`Exposure`]. fn collect_exposures( - supports: Supports, + supports: BoundedSupportsOf, ) -> Vec<(T::AccountId, Exposure>)> { let total_issuance = T::Currency::total_issuance(); let to_currency = |e: frame_election_provider_support::ExtendedBalance| { diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index e53464195de23..f0304d801dd20 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -118,6 +118,7 @@ pub mod pallet { AccountId = Self::AccountId, BlockNumber = Self::BlockNumber, DataProvider = Pallet, + MaxBackersPerWinner = ::MaxBackersPerWinner, >; /// Maximum number of nominations per nominator. diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index b76126f0c5d04..ae0c7f3add471 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -18,7 +18,9 @@ //! Tests for the module. use super::{ConfigOp, Event, MaxUnlockingChunks, *}; -use frame_election_provider_support::{ElectionProvider, SortedListProvider, Support}; +use frame_election_provider_support::{ + BoundedSupport, ElectionProvider, SortedListProvider, Support, +}; use frame_support::{ assert_noop, assert_ok, assert_storage_noop, bounded_vec, dispatch::WithPostDispatchInfo, @@ -1963,8 +1965,20 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider() { assert_eq!( supports, vec![ - (21, Support { total: 1800, voters: vec![(21, 1000), (1, 400), (3, 400)] }), - (31, Support { total: 2200, voters: vec![(31, 1000), (1, 600), (3, 600)] }) + ( + 21, + BoundedSupport { + total: 1800, + voters: bounded_vec![(21, 1000), (1, 400), (3, 400)] + } + ), + ( + 31, + BoundedSupport { + total: 2200, + voters: bounded_vec![(31, 1000), (1, 600), (3, 600)] + } + ) ], ); }); @@ -2007,8 +2021,17 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() { assert_eq!( supports, vec![ - (11, Support { total: 1500, voters: vec![(11, 1000), (1, 500)] }), - (21, Support { total: 2500, voters: vec![(21, 1000), (1, 500), (3, 1000)] }) + ( + 11, + BoundedSupport { total: 1500, voters: bounded_vec![(11, 1000), (1, 500)] } + ), + ( + 21, + BoundedSupport { + total: 2500, + voters: bounded_vec![(21, 1000), (1, 500), (3, 1000)] + } + ) ], ); }); From 3a25c092ebe88cb35c3a97773276b18c42421bab Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 27 Jul 2022 17:49:22 +0200 Subject: [PATCH 3/7] Benchmarks compile Signed-off-by: Oliver Tale-Yazdi --- frame/election-provider-multi-phase/src/benchmarking.rs | 5 +++-- frame/election-provider-multi-phase/src/lib.rs | 7 +++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-multi-phase/src/benchmarking.rs b/frame/election-provider-multi-phase/src/benchmarking.rs index a8195df7305ff..da13cf38c3fce 100644 --- a/frame/election-provider-multi-phase/src/benchmarking.rs +++ b/frame/election-provider-multi-phase/src/benchmarking.rs @@ -310,7 +310,7 @@ frame_benchmarking::benchmarks! { assert!(>::get().is_some()); assert!(>::get().is_some()); }: { - assert_ok!( as ElectionProvider>::elect()); + assert!( as ElectionProvider>::elect().is_ok()); } verify { assert!(>::queued_solution().is_none()); assert!(>::get().is_none()); @@ -404,7 +404,8 @@ frame_benchmarking::benchmarks! { assert_eq!(raw_solution.solution.voter_count() as u32, a); assert_eq!(raw_solution.solution.unique_targets().len() as u32, d); }: { - assert_ok!(>::feasibility_check(raw_solution, ElectionCompute::Unsigned)); + // TODO @ggwpez Why does format! work but not this? + assert!(>::feasibility_check(raw_solution, ElectionCompute::Unsigned).is_ok()); } // NOTE: this weight is not used anywhere, but the fact that it should succeed when execution in diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 5479d20c567b7..3542b7ac413ca 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -2247,6 +2247,13 @@ mod tests { assert!(solution == solution); } + #[test] + fn debug_ready_solution() { + let solution = ReadySolutionOf::::default(); + + format!("{:?}", solution); + } + #[test] fn number_of_voters_allowed_2sec_block() { // Just a rough estimate with the substrate weights. From 6f5fdb74bc8dc72a34a414218a4cfa0478a1ee71 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 27 Jul 2022 19:23:24 +0200 Subject: [PATCH 4/7] Cleanup Signed-off-by: Oliver Tale-Yazdi --- bin/node/runtime/src/lib.rs | 13 ++-- frame/babe/src/mock.rs | 8 +- .../src/benchmarking.rs | 2 +- .../election-provider-multi-phase/src/lib.rs | 75 +++++++------------ .../election-provider-multi-phase/src/mock.rs | 12 ++- .../src/signed.rs | 4 +- .../src/unsigned.rs | 5 +- .../solution-type/src/single_page.rs | 1 - frame/election-provider-support/src/lib.rs | 38 +++++++--- .../election-provider-support/src/onchain.rs | 43 +++-------- frame/election-provider-support/src/traits.rs | 3 - frame/grandpa/src/mock.rs | 8 +- frame/offences/benchmarking/src/mock.rs | 8 +- frame/session/benchmarking/src/mock.rs | 8 +- frame/staking/src/mock.rs | 7 +- frame/staking/src/pallet/impls.rs | 2 +- frame/staking/src/tests.rs | 5 +- 17 files changed, 112 insertions(+), 130 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index c34b6f5861a45..1b46bb9c95e05 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -24,9 +24,8 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::{ - onchain, - onchain::{TruncatingBounder, TruncatingBounderOf}, - BalancingConfig, ElectionDataProvider, SequentialPhragmen, VoteWeight, + onchain, onchain::TruncateIntoBoundedSupportsOf, BalancingConfig, ElectionDataProvider, + SequentialPhragmen, TruncateIntoBoundedSupports, VoteWeight, }; use frame_support::{ construct_runtime, @@ -661,9 +660,9 @@ impl onchain::Config for OnChainSeqPhragmen { type DataProvider = ::DataProvider; type WeightInfo = frame_election_provider_support::weights::SubstrateWeight; - // FIXME no idea what to use here + // TODO no idea what to use here type MaxBackersPerWinner = ConstU32<16>; - type Bounder = TruncatingBounderOf; + type Bounder = TruncateIntoBoundedSupportsOf; } impl onchain::BoundedConfig for OnChainSeqPhragmen { @@ -719,6 +718,10 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type MaxElectingVoters = MaxElectingVoters; // TODO what to use here type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncateIntoBoundedSupports< + ::AccountId, + Self::MaxBackersPerWinner, + >; type BenchmarkingConfig = ElectionProviderBenchmarkConfig; type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight; } diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 61323eb391054..71c8b1a9bd605 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -19,7 +19,9 @@ use crate::{self as pallet_babe, Config, CurrentSlot}; use codec::Encode; -use frame_election_provider_support::{onchain, onchain::TruncatingBounderOf, SequentialPhragmen}; +use frame_election_provider_support::{ + onchain, onchain::TruncateIntoBoundedSupportsOf, SequentialPhragmen, +}; use frame_support::{ parameter_types, traits::{ConstU128, ConstU32, ConstU64, GenesisBuild, KeyOwnerProofSystem, OnInitialize}, @@ -178,9 +180,9 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen; type DataProvider = Staking; type WeightInfo = (); - // FIXME no idea what to use here + // TODO no idea what to use here type MaxBackersPerWinner = ConstU32<16>; - type Bounder = TruncatingBounderOf; + type Bounder = TruncateIntoBoundedSupportsOf; } impl pallet_staking::Config for Test { diff --git a/frame/election-provider-multi-phase/src/benchmarking.rs b/frame/election-provider-multi-phase/src/benchmarking.rs index da13cf38c3fce..0f6b715c0330d 100644 --- a/frame/election-provider-multi-phase/src/benchmarking.rs +++ b/frame/election-provider-multi-phase/src/benchmarking.rs @@ -404,7 +404,7 @@ frame_benchmarking::benchmarks! { assert_eq!(raw_solution.solution.voter_count() as u32, a); assert_eq!(raw_solution.solution.unique_targets().len() as u32, d); }: { - // TODO @ggwpez Why does format! work but not this? + // TODO @ggwpez Why does format! work but not assert_ok? assert!(>::feasibility_check(raw_solution, ElectionCompute::Unsigned).is_ok()); } diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 3542b7ac413ca..5f2f223030000 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -239,7 +239,7 @@ use frame_support::{ ensure, traits::{Currency, Get, OnUnbalanced, ReservableCurrency}, weights::{DispatchClass, Weight}, - BoundedVec, DebugNoBound, + DefaultNoBound, PartialEqNoBound, }; use frame_system::{ensure_none, offchain::SendTransactionTypes}; use scale_info::TypeInfo; @@ -248,10 +248,10 @@ use sp_arithmetic::{ UpperOf, }; use sp_npos_elections::{ - assignment_ratio_to_staked_normalized, ElectionScore, EvaluateSupport, Support, Supports, - VoteWeight, + assignment_ratio_to_staked_normalized, ElectionScore, EvaluateSupport, Supports, VoteWeight, }; use sp_runtime::{ + traits::Convert, transaction_validity::{ InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, TransactionValidityError, ValidTransaction, @@ -440,8 +440,6 @@ impl Default for RawSolution { } } -use frame_support::{DefaultNoBound, EqNoBound, PartialEqNoBound}; - /// A checked solution, ready to be enacted. #[derive(PartialEqNoBound, Clone, Encode, Decode, RuntimeDebug, DefaultNoBound, TypeInfo)] #[scale_info(skip_type_params(MaxBackersPerWinner))] @@ -462,20 +460,7 @@ where pub compute: ElectionCompute, } -impl> From)>> - for ReadySolution -where - AccountId: PartialEq, -{ - fn from(supports: Vec<(AccountId, Support)>) -> Self { - Self { - supports: Default::default(), // FIXME @ggwpez - score: Default::default(), - compute: Default::default(), - } - } -} - +/// Convenience wrapper to create a [`ReadySolution`] from a [`ElectionProvider`]. pub type ReadySolutionOf = ReadySolution<::AccountId, ::MaxBackersPerWinner>; @@ -730,6 +715,19 @@ pub mod pallet { /// `ElectionProvider` return. type MaxBackersPerWinner: Get; + /// Something that can convert the final `Supports` into a bounded version. + /// TODO put in trait and reuse in onchain. + type Bounder: Convert< + sp_npos_elections::Supports<::AccountId>, + Vec<( + ::AccountId, + BoundedSupport< + ::AccountId, + Self::MaxBackersPerWinner, + >, + )>, + >; + /// Origin that can control this pallet. Note that any action taken by this origin (such) /// as providing an emergency solution is not checked. Thus, it must be a trusted origin. type ForceOrigin: EnsureOrigin; @@ -976,12 +974,11 @@ pub mod pallet { // Note: we don't `rotate_round` at this point; the next call to // `ElectionProvider::elect` will succeed and take care of that. - let solution = ReadySolution::from( - supports, - /*score: Default::default(), - compute: ElectionCompute::Emergency, - };*/ - ); + let solution = ReadySolution { + supports: T::Bounder::convert(supports), + score: Default::default(), + compute: ElectionCompute::Emergency, + }; Self::deposit_event(Event::SolutionStored { election_compute: ElectionCompute::Emergency, @@ -1091,13 +1088,11 @@ pub mod pallet { Error::::FallbackFailed })?; - /*let solution = ReadySolution { - supports, + let solution = ReadySolution { + supports: T::Bounder::convert(supports), score: Default::default(), compute: ElectionCompute::Fallback, - };*/ - // FIXME - let solution = ReadySolution::from(supports); + }; Self::deposit_event(Event::SolutionStored { election_compute: ElectionCompute::Fallback, @@ -1539,8 +1534,7 @@ impl Pallet { let known_score = supports.evaluate(); ensure!(known_score == score, FeasibilityError::InvalidScore); - // FIXME @ggwpez - Ok(supports.into()) + Ok(ReadySolution { supports: T::Bounder::convert(supports), score, compute }) } /// Perform the tasks to be done after a new `elect` has been triggered: @@ -1848,7 +1842,7 @@ mod tests { }; use frame_election_provider_support::ElectionProvider; use frame_support::{assert_noop, assert_ok}; - use sp_npos_elections::{BalancingConfig, Support}; + use sp_npos_elections::BalancingConfig; #[test] fn phase_rotation_works() { @@ -2239,21 +2233,6 @@ mod tests { }) } - #[test] - fn compare_ready_solution() { - let solution = ReadySolutionOf::::default(); - - assert_eq!(solution, solution); - assert!(solution == solution); - } - - #[test] - fn debug_ready_solution() { - let solution = ReadySolutionOf::::default(); - - format!("{:?}", solution); - } - #[test] fn number_of_voters_allowed_2sec_block() { // Just a rough estimate with the substrate weights. diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index d6abeccdf004e..f7b0028019572 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -19,8 +19,8 @@ use super::*; use crate::{self as multi_phase, unsigned::MinerConfig}; use frame_election_provider_support::{ data_provider, - onchain::{self, TruncatingBounderOf, UnboundedExecution}, - ElectionDataProvider, NposSolution, SequentialPhragmen, + onchain::{self, TruncateIntoBoundedSupportsOf, UnboundedExecution}, + ElectionDataProvider, NposSolution, SequentialPhragmen, TruncateIntoBoundedSupports, }; pub use frame_support::{assert_noop, assert_ok, pallet_prelude::GetDefault}; use frame_support::{ @@ -292,9 +292,9 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen, Balancing>; type DataProvider = StakingMock; type WeightInfo = (); - // FIXME no idea what to use here + // TODO no idea what to use here type MaxBackersPerWinner = ConstU32<16>; - type Bounder = TruncatingBounderOf; + type Bounder = TruncateIntoBoundedSupportsOf; } pub struct MockFallback; @@ -391,6 +391,10 @@ impl crate::Config for Runtime { type MaxElectingVoters = MaxElectingVoters; type MaxElectableTargets = MaxElectableTargets; type MaxBackersPerWinner = ConstU32<16>; + type Bounder = TruncateIntoBoundedSupports< + ::AccountId, + Self::MaxBackersPerWinner, + >; type MinerConfig = Self; type Solver = SequentialPhragmen, Balancing>; } diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs index aa9e16c2a5235..5e0d7574cc027 100644 --- a/frame/election-provider-multi-phase/src/signed.rs +++ b/frame/election-provider-multi-phase/src/signed.rs @@ -19,8 +19,8 @@ use crate::{ unsigned::MinerConfig, Config, ElectionCompute, Pallet, QueuedSolution, RawSolution, - ReadySolution, ReadySolutionOf, SignedSubmissionIndices, SignedSubmissionNextIndex, - SignedSubmissionsMap, SolutionOf, SolutionOrSnapshotSize, Weight, WeightInfo, + ReadySolutionOf, SignedSubmissionIndices, SignedSubmissionNextIndex, SignedSubmissionsMap, + SolutionOf, SolutionOrSnapshotSize, Weight, WeightInfo, }; use codec::{Decode, Encode, HasCompact}; use frame_election_provider_support::NposSolution; diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 9a0ac714fc7e8..be725a3f31c56 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -19,8 +19,7 @@ use crate::{ helpers, Call, Config, ElectionCompute, Error, FeasibilityError, Pallet, RawSolution, - ReadySolution, ReadySolutionOf, RoundSnapshot, SolutionAccuracyOf, SolutionOf, - SolutionOrSnapshotSize, Weight, + ReadySolutionOf, RoundSnapshot, SolutionAccuracyOf, SolutionOf, SolutionOrSnapshotSize, Weight, }; use codec::Encode; use frame_election_provider_support::{NposSolution, NposSolver, PerThing128, VoteWeight}; @@ -874,7 +873,7 @@ mod tests { assert!(::pre_dispatch(&call).is_ok()); // set a better score - let ready = ReadySolution { + let ready = crate::ReadySolution { score: ElectionScore { minimal_stake: 10, ..Default::default() }, ..Default::default() }; diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index ff83a4d02df46..43e68acd9d563 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -106,7 +106,6 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { impl _feps::NposSolution for #ident { const LIMIT: usize = #count; - type Limit = sp_runtime::traits::ConstU32<{#count as u32}>; type VoterIndex = #voter_type; type TargetIndex = #target_type; type Accuracy = #weight_type; diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index 924ea4aae9071..a150efbb4c0d1 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -169,9 +169,7 @@ pub mod onchain; pub mod traits; use codec::{Decode, Encode, MaxEncodedLen}; -use frame_support::{traits::ConstU32, DebugNoBound, DefaultNoBound, RuntimeDebugNoBound}; -use scale_info::TypeInfo; -use sp_npos_elections::EvaluateSupport; +use frame_support::traits::ConstU32; use sp_runtime::traits::{Bounded, Convert, Saturating, Zero}; use sp_std::{fmt::Debug, prelude::*}; @@ -649,8 +647,12 @@ pub type BoundedSupportOf = BoundedSupport< pub type BoundedSupports = Vec<(AccountId, BoundedSupport)>; -// todo transform -pub type BoundedSupportsOf = Vec<(::AccountId, BoundedSupportOf)>; + +/// Convenience to create a [`BoundedSupport`] from a [`ElectionProvider`]. +pub type BoundedSupportsOf = BoundedSupports< + ::AccountId, + ::MaxBackersPerWinner, +>; impl> sp_npos_elections::Backings for &BoundedSupport { fn total(&self) -> ExtendedBalance { @@ -697,12 +699,28 @@ impl Convert, Bou } } -pub struct TruncateIntoBoundedSupports(sp_std::marker::PhantomData); +/// Converts an unbounded support into a bounded support by truncating it. +pub struct TruncateIntoBoundedSupports>( + sp_std::marker::PhantomData<(AccountId, MaxBackersPerWinner)>, +); -impl Convert, BoundedSupportsOf> - for TruncateIntoBoundedSupports +impl> + Convert, BoundedSupports> + for TruncateIntoBoundedSupports { - fn convert(a: sp_npos_elections::Supports) -> BoundedSupportsOf { - todo!(); + fn convert( + a: sp_npos_elections::Supports, + ) -> BoundedSupports { + a.into_iter() + .map(|(who, support)| { + ( + who, + BoundedSupport { + total: support.total, + voters: BoundedVec::truncate_from(support.voters), + }, + ) + }) + .collect() } } diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 3f191bdf1b0c3..0d3c721533174 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -70,35 +70,11 @@ pub struct BoundedExecution(PhantomData); /// This can be very expensive to run frequently on-chain. Use with care. pub struct UnboundedExecution(PhantomData); -// TODO delete this, i did not see `TruncateIntoBoundedSupports` before :facepalm: -pub type TruncatingBounderOf = - TruncatingBounder<::AccountId, MaxBackersPerWinner>; -pub struct TruncatingBounder>( - sp_std::marker::PhantomData<(AccountId, MaxBackersPerWinner)>, -); - -impl> - Convert, Vec<(AccountId, BoundedSupport)>> - for TruncatingBounder -{ - /// Make conversion. - fn convert( - a: Supports, - ) -> Vec<(AccountId, BoundedSupport)> { - // TODO move to TruncateIntoBoundedSupports - a.into_iter() - .map(|(who, support)| { - ( - who, - BoundedSupport { - total: support.total, - voters: sp_runtime::BoundedVec::truncate_from(support.voters), - }, - ) - }) - .collect() - } -} +/// Convenience alias to create a [`TruncateIntoBoundedSupports`] from a [`Config`]. +pub type TruncateIntoBoundedSupportsOf = crate::TruncateIntoBoundedSupports< + <::System as frame_system::Config>::AccountId, + ::MaxBackersPerWinner, +>; /// Configuration trait for an onchain election execution. pub trait Config { @@ -241,7 +217,6 @@ mod tests { use super::*; use crate::{PhragMMS, SequentialPhragmen}; use frame_support::traits::ConstU32; - use sp_npos_elections::Support; use sp_runtime::Perbill; type AccountId = u64; type BlockNumber = u64; @@ -295,9 +270,9 @@ mod tests { type Solver = SequentialPhragmen; type DataProvider = mock_data_provider::DataProvider; type WeightInfo = (); - // FIXME no idea what to use here + // TODO no idea what to use here type MaxBackersPerWinner = ConstU32<16>; - type Bounder = TruncatingBounderOf; + type Bounder = TruncateIntoBoundedSupportsOf; } impl BoundedConfig for PhragmenParams { @@ -310,9 +285,9 @@ mod tests { type Solver = PhragMMS; type DataProvider = mock_data_provider::DataProvider; type WeightInfo = (); - // FIXME no idea what to use here + // TODO no idea what to use here type MaxBackersPerWinner = ConstU32<16>; - type Bounder = TruncatingBounderOf; + type Bounder = TruncateIntoBoundedSupportsOf; } impl BoundedConfig for PhragMMSParams { diff --git a/frame/election-provider-support/src/traits.rs b/frame/election-provider-support/src/traits.rs index 189dc1950f4ac..ed812e2e0f2c4 100644 --- a/frame/election-provider-support/src/traits.rs +++ b/frame/election-provider-support/src/traits.rs @@ -22,7 +22,6 @@ use codec::Encode; use scale_info::TypeInfo; use sp_arithmetic::traits::{Bounded, UniqueSaturatedInto}; use sp_npos_elections::{ElectionScore, Error, EvaluateSupport}; -use sp_runtime::traits::Get; use sp_std::{fmt::Debug, prelude::*}; /// An opaque index-based, NPoS solution type. @@ -33,8 +32,6 @@ where /// The maximum number of votes that are allowed. const LIMIT: usize; - type Limit: Get; - /// The voter type. Needs to be an index (convert to usize). type VoterIndex: UniqueSaturatedInto + TryInto diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index 7872f1072948d..ae6ea6c5eb1e5 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -22,7 +22,9 @@ use crate::{self as pallet_grandpa, AuthorityId, AuthorityList, Config, ConsensusLog}; use ::grandpa as finality_grandpa; use codec::Encode; -use frame_election_provider_support::{onchain, onchain::TruncatingBounderOf, SequentialPhragmen}; +use frame_election_provider_support::{ + onchain, onchain::TruncateIntoBoundedSupportsOf, SequentialPhragmen, +}; use frame_support::{ parameter_types, traits::{ @@ -182,9 +184,9 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen; type DataProvider = Staking; type WeightInfo = (); - // FIXME no idea what to use here + // TODO no idea what to use here type MaxBackersPerWinner = ConstU32<16>; - type Bounder = TruncatingBounderOf; + type Bounder = TruncateIntoBoundedSupportsOf; } impl pallet_staking::Config for Test { diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index d2516ff0fd4c5..b5666d7cba067 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -20,7 +20,9 @@ #![cfg(test)] use super::*; -use frame_election_provider_support::{onchain, onchain::TruncatingBounderOf, SequentialPhragmen}; +use frame_election_provider_support::{ + onchain, onchain::TruncateIntoBoundedSupportsOf, SequentialPhragmen, +}; use frame_support::{ parameter_types, traits::{ConstU32, ConstU64}, @@ -155,9 +157,9 @@ impl onchain::Config for OnChainSeqPhragmen { type DataProvider = Staking; type WeightInfo = (); - // FIXME no idea what to use here + // TODO no idea what to use here type MaxBackersPerWinner = ConstU32<16>; - type Bounder = TruncatingBounderOf; + type Bounder = TruncateIntoBoundedSupportsOf; } impl pallet_staking::Config for Test { diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index 616d9a0c9d39c..84ebe14cf812a 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -19,7 +19,9 @@ #![cfg(test)] -use frame_election_provider_support::{onchain, onchain::TruncatingBounderOf, SequentialPhragmen}; +use frame_election_provider_support::{ + onchain, onchain::TruncateIntoBoundedSupportsOf, SequentialPhragmen, +}; use frame_support::{ parameter_types, traits::{ConstU32, ConstU64}, @@ -161,9 +163,9 @@ impl onchain::Config for OnChainSeqPhragmen { type DataProvider = Staking; type WeightInfo = (); - // FIXME no idea what to use here + // TODO no idea what to use here type MaxBackersPerWinner = ConstU32<16>; - type Bounder = TruncatingBounderOf; + type Bounder = TruncateIntoBoundedSupportsOf; } impl pallet_staking::Config for Test { diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index a827d15127b20..afbf7bcb01f01 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -19,7 +19,8 @@ use crate::{self as pallet_staking, *}; use frame_election_provider_support::{ - onchain, onchain::TruncatingBounderOf, SequentialPhragmen, SortedListProvider, VoteWeight, + onchain, onchain::TruncateIntoBoundedSupportsOf, SequentialPhragmen, SortedListProvider, + VoteWeight, }; use frame_support::{ assert_ok, parameter_types, @@ -255,9 +256,9 @@ impl onchain::Config for OnChainSeqPhragmen { type Solver = SequentialPhragmen; type DataProvider = Staking; type WeightInfo = (); - // FIXME no idea what to use here + // TODO no idea what to use here type MaxBackersPerWinner = ConstU32<16>; - type Bounder = TruncatingBounderOf; + type Bounder = TruncateIntoBoundedSupportsOf; } pub struct MockReward {} diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 6670157f9caf6..d1971bb38f58a 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -19,7 +19,7 @@ use frame_election_provider_support::{ data_provider, BoundedSupportsOf, ElectionDataProvider, ElectionProvider, ScoreProvider, - SortedListProvider, Supports, VoteWeight, VoterOf, + SortedListProvider, VoteWeight, VoterOf, }; use frame_support::{ pallet_prelude::*, diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index ae0c7f3add471..cc63ef33766a1 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -16,11 +16,10 @@ // limitations under the License. //! Tests for the module. +#![cfg(test)] use super::{ConfigOp, Event, MaxUnlockingChunks, *}; -use frame_election_provider_support::{ - BoundedSupport, ElectionProvider, SortedListProvider, Support, -}; +use frame_election_provider_support::{BoundedSupport, ElectionProvider, SortedListProvider}; use frame_support::{ assert_noop, assert_ok, assert_storage_noop, bounded_vec, dispatch::WithPostDispatchInfo, From aa63ebc14eb85c22b9eb846814be1e76f4e5eb81 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 27 Jul 2022 19:37:44 +0200 Subject: [PATCH 5/7] More cleanup Signed-off-by: Oliver Tale-Yazdi --- frame/election-provider-multi-phase/src/lib.rs | 10 ++++------ .../solution-type/src/single_page.rs | 1 - 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 5f2f223030000..4928f501c795c 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -231,7 +231,7 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_election_provider_support::{ - BoundedSupport, BoundedSupportsOf, ElectionDataProvider, ElectionProvider, + BoundedSupport, BoundedSupports, BoundedSupportsOf, ElectionDataProvider, ElectionProvider, InstantElectionProvider, NposSolution, }; use frame_support::{ @@ -451,7 +451,7 @@ where /// /// This is target-major vector, storing each winners, total backing, and each individual /// backer. - pub supports: Vec<(AccountId, BoundedSupport)>, + pub supports: BoundedSupports, /// The score of the solution. /// /// This is needed to potentially challenge the solution. @@ -563,7 +563,6 @@ pub enum FeasibilityError { InvalidRound, /// Comparison against `MinimumUntrustedScore` failed. UntrustedScoreTooLow, - TooManyVotes, } impl From for FeasibilityError { @@ -1569,7 +1568,7 @@ impl Pallet { .map_err(|fe| ElectionError::Fallback(fe)) .map(|supports| (supports, ElectionCompute::Fallback)) }, - |ReadySolutionOf:: { supports, compute, .. }| Ok((supports, compute)), + |ReadySolution { supports, compute, .. }| Ok((supports, compute)), ) .map(|(supports, compute)| { Self::deposit_event(Event::ElectionFinalized { election_compute: Some(compute) }); @@ -1841,7 +1840,7 @@ mod tests { Phase, }; use frame_election_provider_support::ElectionProvider; - use frame_support::{assert_noop, assert_ok}; + use frame_support::{assert_noop, assert_ok, bounded_vec}; use sp_npos_elections::BalancingConfig; #[test] @@ -2053,7 +2052,6 @@ mod tests { #[test] fn fallback_strategy_works() { - use frame_support::bounded_vec; ExtBuilder::default().onchain_fallback(true).build_and_execute(|| { roll_to(25); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); diff --git a/frame/election-provider-support/solution-type/src/single_page.rs b/frame/election-provider-support/solution-type/src/single_page.rs index 43e68acd9d563..a7ccf5085d2b1 100644 --- a/frame/election-provider-support/solution-type/src/single_page.rs +++ b/frame/election-provider-support/solution-type/src/single_page.rs @@ -105,7 +105,6 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result { use _feps::__OrInvalidIndex; impl _feps::NposSolution for #ident { const LIMIT: usize = #count; - type VoterIndex = #voter_type; type TargetIndex = #target_type; type Accuracy = #weight_type; From 9cbbe833c8469dfb49730db03a0603008c2ff1de Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 27 Jul 2022 19:54:28 +0200 Subject: [PATCH 6/7] Cleanup Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 1 - bin/node/runtime/Cargo.toml | 2 -- bin/node/runtime/src/lib.rs | 4 ++-- frame/babe/src/mock.rs | 2 +- frame/election-provider-multi-phase/src/lib.rs | 5 +---- frame/election-provider-multi-phase/src/mock.rs | 6 +++--- frame/election-provider-support/src/onchain.rs | 4 ++-- frame/grandpa/src/mock.rs | 2 +- frame/offences/benchmarking/src/mock.rs | 2 +- frame/session/benchmarking/src/mock.rs | 2 +- frame/staking/src/mock.rs | 2 +- 11 files changed, 13 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b48537861d5a6..3d5bfe5e51201 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4858,7 +4858,6 @@ dependencies = [ "sp-core", "sp-inherents", "sp-io", - "sp-npos-elections", "sp-offchain", "sp-runtime", "sp-sandbox", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 9d422e810c668..ca971f29e93c9 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -40,7 +40,6 @@ sp-transaction-pool = { version = "4.0.0-dev", default-features = false, path = sp-version = { version = "5.0.0", default-features = false, path = "../../../primitives/version" } sp-io = { version = "6.0.0", default-features = false, path = "../../../primitives/io" } sp-sandbox = { version = "0.10.0-dev", default-features = false, path = "../../../primitives/sandbox" } -sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/npos-elections" } # frame dependencies frame-executive = { version = "4.0.0-dev", default-features = false, path = "../../../frame/executive" } @@ -190,7 +189,6 @@ std = [ "sp-io/std", "pallet-child-bounties/std", "pallet-alliance/std", - "sp-npos-elections/std", ] runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 1b46bb9c95e05..07dc3f2b144a3 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -661,7 +661,7 @@ impl onchain::Config for OnChainSeqPhragmen { type WeightInfo = frame_election_provider_support::weights::SubstrateWeight; // TODO no idea what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupportsOf; } @@ -717,7 +717,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type MaxElectableTargets = ConstU16<{ u16::MAX }>; type MaxElectingVoters = MaxElectingVoters; // TODO what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupports< ::AccountId, Self::MaxBackersPerWinner, diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 71c8b1a9bd605..867c2507410b2 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -181,7 +181,7 @@ impl onchain::Config for OnChainSeqPhragmen { type DataProvider = Staking; type WeightInfo = (); // TODO no idea what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupportsOf; } diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 4928f501c795c..f398759c1f27c 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -443,10 +443,7 @@ impl Default for RawSolution { /// A checked solution, ready to be enacted. #[derive(PartialEqNoBound, Clone, Encode, Decode, RuntimeDebug, DefaultNoBound, TypeInfo)] #[scale_info(skip_type_params(MaxBackersPerWinner))] -pub struct ReadySolution> -where - AccountId: PartialEq, -{ +pub struct ReadySolution> { /// The final supports of the solution. /// /// This is target-major vector, storing each winners, total backing, and each individual diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index f7b0028019572..d17f0ece1821f 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -293,7 +293,7 @@ impl onchain::Config for OnChainSeqPhragmen { type DataProvider = StakingMock; type WeightInfo = (); // TODO no idea what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupportsOf; } @@ -303,7 +303,7 @@ impl ElectionProvider for MockFallback { type BlockNumber = u64; type Error = &'static str; type DataProvider = StakingMock; - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; fn elect() -> Result, Self::Error> { Self::elect_with_bounds(Bounded::max_value(), Bounded::max_value()) @@ -390,7 +390,7 @@ impl crate::Config for Runtime { type ForceOrigin = frame_system::EnsureRoot; type MaxElectingVoters = MaxElectingVoters; type MaxElectableTargets = MaxElectableTargets; - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupports< ::AccountId, Self::MaxBackersPerWinner, diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 0d3c721533174..01506d00f0097 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -271,7 +271,7 @@ mod tests { type DataProvider = mock_data_provider::DataProvider; type WeightInfo = (); // TODO no idea what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupportsOf; } @@ -286,7 +286,7 @@ mod tests { type DataProvider = mock_data_provider::DataProvider; type WeightInfo = (); // TODO no idea what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupportsOf; } diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index ae6ea6c5eb1e5..24b0100a817b0 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -185,7 +185,7 @@ impl onchain::Config for OnChainSeqPhragmen { type DataProvider = Staking; type WeightInfo = (); // TODO no idea what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupportsOf; } diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index b5666d7cba067..6dd5725af1bfa 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -158,7 +158,7 @@ impl onchain::Config for OnChainSeqPhragmen { type WeightInfo = (); // TODO no idea what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupportsOf; } diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index 84ebe14cf812a..69ee71e0d7e52 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -164,7 +164,7 @@ impl onchain::Config for OnChainSeqPhragmen { type WeightInfo = (); // TODO no idea what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupportsOf; } diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index afbf7bcb01f01..7a7e9bfd643a0 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -257,7 +257,7 @@ impl onchain::Config for OnChainSeqPhragmen { type DataProvider = Staking; type WeightInfo = (); // TODO no idea what to use here - type MaxBackersPerWinner = ConstU32<16>; + type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; type Bounder = TruncateIntoBoundedSupportsOf; } From b21f3ad9d693cd563934807abc339b511c705965 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 28 Jul 2022 15:58:34 +0200 Subject: [PATCH 7/7] Review fixes Signed-off-by: Oliver Tale-Yazdi --- bin/node/runtime/src/lib.rs | 4 -- .../election-provider-multi-phase/src/lib.rs | 55 ++++++++++++------- .../election-provider-multi-phase/src/mock.rs | 8 +-- frame/election-provider-support/src/lib.rs | 2 +- .../election-provider-support/src/onchain.rs | 18 +++--- 5 files changed, 47 insertions(+), 40 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 07dc3f2b144a3..cb7ee7c2277d6 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -718,10 +718,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type MaxElectingVoters = MaxElectingVoters; // TODO what to use here type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; - type Bounder = TruncateIntoBoundedSupports< - ::AccountId, - Self::MaxBackersPerWinner, - >; type BenchmarkingConfig = ElectionProviderBenchmarkConfig; type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight; } diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index f398759c1f27c..d79498d443975 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -329,7 +329,7 @@ impl ElectionProvider for NoFallback { } impl InstantElectionProvider for NoFallback { - fn elect_with_bounds(_: usize, _: usize) -> Result, Self::Error> { + fn elect_with_bounds(_: usize, _: usize) -> Result, Self::Error> { Err("NoFallback.") } } @@ -457,6 +457,27 @@ pub struct ReadySolution> { pub compute: ElectionCompute, } +/// Try to convert the supports of a solution into a [`ReadySolution`]. +/// +/// Errors if any of the supports has more than [`MaxBackersPerWinner`] backers. +impl> + TryFrom<(Supports, ElectionScore, ElectionCompute)> + for ReadySolution +{ + type Error = &'static str; + + fn try_from( + supports: (Supports, ElectionScore, ElectionCompute), + ) -> Result { + let mut bounded_supports = BoundedSupports::default(); + for (winner, backing) in supports.0.into_iter() { + bounded_supports.push((winner, backing.try_into()?)); + } + + Ok(Self { supports: bounded_supports, score: supports.1, compute: supports.2 }) + } +} + /// Convenience wrapper to create a [`ReadySolution`] from a [`ElectionProvider`]. pub type ReadySolutionOf = ReadySolution<::AccountId, ::MaxBackersPerWinner>; @@ -560,6 +581,8 @@ pub enum FeasibilityError { InvalidRound, /// Comparison against `MinimumUntrustedScore` failed. UntrustedScoreTooLow, + /// There are too many backers. + TooManyBackers, } impl From for FeasibilityError { @@ -711,19 +734,6 @@ pub mod pallet { /// `ElectionProvider` return. type MaxBackersPerWinner: Get; - /// Something that can convert the final `Supports` into a bounded version. - /// TODO put in trait and reuse in onchain. - type Bounder: Convert< - sp_npos_elections::Supports<::AccountId>, - Vec<( - ::AccountId, - BoundedSupport< - ::AccountId, - Self::MaxBackersPerWinner, - >, - )>, - >; - /// Origin that can control this pallet. Note that any action taken by this origin (such) /// as providing an emergency solution is not checked. Thus, it must be a trusted origin. type ForceOrigin: EnsureOrigin; @@ -970,11 +980,10 @@ pub mod pallet { // Note: we don't `rotate_round` at this point; the next call to // `ElectionProvider::elect` will succeed and take care of that. - let solution = ReadySolution { - supports: T::Bounder::convert(supports), - score: Default::default(), - compute: ElectionCompute::Emergency, - }; + let solution: ReadySolution<_, _> = + (supports, Default::default(), ElectionCompute::Emergency) + .try_into() + .map_err(|_| Error::::TooManyBackersPerWinner)?; Self::deposit_event(Event::SolutionStored { election_compute: ElectionCompute::Emergency, @@ -1085,7 +1094,7 @@ pub mod pallet { })?; let solution = ReadySolution { - supports: T::Bounder::convert(supports), + supports, score: Default::default(), compute: ElectionCompute::Fallback, }; @@ -1150,6 +1159,8 @@ pub mod pallet { CallNotAllowed, /// The fallback failed FallbackFailed, + /// The solution has too many backers per winner. + TooManyBackersPerWinner, } #[pallet::validate_unsigned] @@ -1530,7 +1541,9 @@ impl Pallet { let known_score = supports.evaluate(); ensure!(known_score == score, FeasibilityError::InvalidScore); - Ok(ReadySolution { supports: T::Bounder::convert(supports), score, compute }) + (supports, score, compute) + .try_into() + .map_err(|_| FeasibilityError::TooManyBackers) } /// Perform the tasks to be done after a new `elect` has been triggered: diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index d17f0ece1821f..fcaf0608a94ca 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -307,7 +307,6 @@ impl ElectionProvider for MockFallback { fn elect() -> Result, Self::Error> { Self::elect_with_bounds(Bounded::max_value(), Bounded::max_value()) - .map(|supports| ::Bounder::convert(supports)) } } @@ -315,7 +314,7 @@ impl InstantElectionProvider for MockFallback { fn elect_with_bounds( max_voters: usize, max_targets: usize, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { if OnChainFallback::get() { onchain::UnboundedExecution::::elect_with_bounds( max_voters, @@ -390,11 +389,8 @@ impl crate::Config for Runtime { type ForceOrigin = frame_system::EnsureRoot; type MaxElectingVoters = MaxElectingVoters; type MaxElectableTargets = MaxElectableTargets; + // TODO what to use here type MaxBackersPerWinner = ConstU32<{ u32::MAX }>; - type Bounder = TruncateIntoBoundedSupports< - ::AccountId, - Self::MaxBackersPerWinner, - >; type MinerConfig = Self; type Solver = SequentialPhragmen, Balancing>; } diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs index a150efbb4c0d1..61a063c4fa91f 100644 --- a/frame/election-provider-support/src/lib.rs +++ b/frame/election-provider-support/src/lib.rs @@ -407,7 +407,7 @@ pub trait InstantElectionProvider: ElectionProvider { fn elect_with_bounds( max_voters: usize, max_targets: usize, - ) -> Result, Self::Error>; + ) -> Result, Self::Error>; } /// An election provider to be used only for testing. diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 01506d00f0097..c91db46bbf475 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -20,8 +20,8 @@ //! careful when using it onchain. use crate::{ - BoundedSupport, BoundedSupportsOf, Debug, ElectionDataProvider, ElectionProvider, - InstantElectionProvider, NposSolver, WeightInfo, + BoundedSupport, BoundedSupports, BoundedSupportsOf, Debug, ElectionDataProvider, + ElectionProvider, InstantElectionProvider, NposSolver, WeightInfo, }; use frame_support::{traits::Get, weights::DispatchClass}; use sp_npos_elections::*; @@ -119,7 +119,10 @@ pub trait BoundedConfig: Config { fn elect_with( maybe_max_voters: Option, maybe_max_targets: Option, -) -> Result::AccountId>, Error> { +) -> Result< + BoundedSupports<::AccountId, T::MaxBackersPerWinner>, + Error, +> { let voters = T::DataProvider::electing_voters(maybe_max_voters).map_err(Error::DataProvider)?; let targets = T::DataProvider::electable_targets(maybe_max_targets).map_err(Error::DataProvider)?; @@ -154,7 +157,7 @@ fn elect_with( // TODO: not very efficient, but cleaner API. Ideally we would possibly trim while // `to_supports`. - Ok(to_supports(&staked)) + Ok(to_supports(&staked)).map(|s| T::Bounder::convert(s)) } impl ElectionProvider for UnboundedExecution { @@ -174,7 +177,7 @@ impl ElectionProvider for UnboundedExecution { ); } - elect_with::(None, None).map(|supports| T::Bounder::convert(supports)) + elect_with::(None, None) } } @@ -182,7 +185,7 @@ impl InstantElectionProvider for UnboundedExecution { fn elect_with_bounds( max_voters: usize, max_targets: usize, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { elect_with::(Some(max_voters), Some(max_targets)) } } @@ -196,7 +199,6 @@ impl ElectionProvider for BoundedExecution { fn elect() -> Result, Self::Error> { elect_with::(Some(T::VotersBound::get() as usize), Some(T::TargetsBound::get() as usize)) - .map(|supports| T::Bounder::convert(supports)) } } @@ -204,7 +206,7 @@ impl InstantElectionProvider for BoundedExecution { fn elect_with_bounds( max_voters: usize, max_targets: usize, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { elect_with::( Some(max_voters.min(T::VotersBound::get() as usize)), Some(max_targets.min(T::TargetsBound::get() as usize)),