From d233b4581e8b9c4ca81f0bd6abb638504ef90a59 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 29 Aug 2021 18:30:32 +0200 Subject: [PATCH 1/9] Recursive election provider as fallback --- bin/node/runtime/src/lib.rs | 8 +- .../election-provider-multi-phase/src/lib.rs | 146 +++++++++--------- .../election-provider-multi-phase/src/mock.rs | 47 ++++-- 3 files changed, 106 insertions(+), 95 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 909ff931756ad..cd83cb3caf5e9 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -42,7 +42,6 @@ use frame_system::{ pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; use pallet_contracts::weights::WeightInfo; -use pallet_election_provider_multi_phase::FallbackStrategy; use pallet_grandpa::{ fg_primitives, AuthorityId as GrandpaId, AuthorityList as GrandpaAuthorityList, }; @@ -527,9 +526,6 @@ parameter_types! { pub const SignedDepositBase: Balance = 1 * DOLLARS; pub const SignedDepositByte: Balance = 1 * CENTS; - // fallback: no on-chain fallback. - pub const Fallback: FallbackStrategy = FallbackStrategy::Nothing; - pub SolutionImprovementThreshold: Perbill = Perbill::from_rational(1u32, 10_000); // miner configs @@ -593,8 +589,8 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type DataProvider = Staking; type OnChainAccuracy = Perbill; type Solution = NposSolution16; - type Fallback = Fallback; - type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight; + type Fallback = pallet_election_provider_multi_phase::InitiateEmergencyPhase; + type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight; type ForceOrigin = EnsureRootOrHalfCouncil; type BenchmarkingConfig = BenchmarkConfig; } diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 96f54c4c03db5..8ae6c3721b12d 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -114,8 +114,8 @@ //! If we reach the end of both phases (i.e. call to [`ElectionProvider::elect`] happens) and no //! good solution is queued, then the fallback strategy [`pallet::Config::Fallback`] is used to //! determine what needs to be done. The on-chain election is slow, and contains no balancing or -//! reduction post-processing. See [`onchain::OnChainSequentialPhragmen`]. The -//! [`FallbackStrategy::Nothing`] just returns an error, and enables the [`Phase::Emergency`]. +//! reduction post-processing. [`InitiateEmergencyPhase`] enables the [`Phase::Emergency`] which is +//! a more *fail-safe* approach. //! //! ### Emergency Phase //! @@ -201,26 +201,9 @@ //! [`DesiredTargets`], no more, no less. Over time, we can change this to a [min, max] where any //! solution within this range is acceptable, where bigger solutions are prioritized. //! -//! **Recursive Fallback**: Currently, the fallback is a separate enum. A different and fancier way -//! of doing this would be to have the fallback be another -//! [`frame_election_provider_support::ElectionProvider`]. In this case, this pallet can even have -//! the on-chain election provider as fallback, or special _noop_ fallback that simply returns an -//! error, thus replicating [`FallbackStrategy::Nothing`]. In this case, we won't need the -//! additional config OnChainAccuracy either. -//! //! **Score based on (byte) size**: We should always prioritize small solutions over bigger ones, if //! there is a tie. Even more harsh should be to enforce the bound of the `reduce` algorithm. //! -//! **Make the number of nominators configurable from the runtime**. Remove `sp_npos_elections` -//! dependency from staking and the solution type. It should be generated at runtime, there -//! it should be encoded how many votes each nominators have. Essentially translate -//! to this pallet. -//! -//! **More accurate weight for error cases**: Both `ElectionDataProvider` and `ElectionProvider` -//! assume no weight is consumed in their functions, when operations fail with `Err`. This can -//! clearly be improved, but not a priority as we generally expect snapshot creation to fail only -//! due to extreme circumstances. -//! //! **Take into account the encode/decode weight in benchmarks.** Currently, we only take into //! account the weight of encode/decode in the `submit_unsigned` given its priority. Nonetheless, //! all operations on the solution and the snapshot are worthy of taking this into account. @@ -284,6 +267,11 @@ pub type SolutionTargetIndexOf = as NposSolution>::TargetIndex pub type SolutionAccuracyOf = as NposSolution>::Accuracy; /// The accuracy of the election, when computed on-chain. Equal to [`Config::OnChainAccuracy`]. pub type OnChainAccuracyOf = ::OnChainAccuracy; +/// The fallback election type. +pub type FallbackErrorOf = <::Fallback as ElectionProvider< + ::AccountId, + ::BlockNumber, +>>::Error; /// Wrapper type that implements the configurations needed for the on-chain backup. pub struct OnChainConfig(sp_std::marker::PhantomData); @@ -322,6 +310,21 @@ impl BenchmarkingConfig for () { const MAXIMUM_TARGETS: u32 = 2_000; } +/// A fallback implementation that transitions the pallet to the emergency phase. +pub struct InitiateEmergencyPhase(sp_std::marker::PhantomData); + +impl ElectionProvider for InitiateEmergencyPhase { + type DataProvider = T::DataProvider; + type Error = &'static str; + + fn elect() -> Result, Self::Error> { + log!(warn, "Entering emergency phase."); + CurrentPhase::::put(Phase::Emergency); + // We won't succeed, this is intentional. + Err("Emergency phase started.") + } +} + /// Current phase of the pallet. #[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug)] pub enum Phase { @@ -384,19 +387,6 @@ impl Phase { } } -/// A configuration for the pallet to indicate what should happen in the case of a fallback i.e. -/// reaching a call to `elect` with no good solution. -#[cfg_attr(test, derive(Clone))] -pub enum FallbackStrategy { - /// Run a on-chain sequential phragmen. - /// - /// This might burn the chain for a few minutes due to a stall, but is generally a safe - /// approach to maintain a sensible validator set. - OnChain, - /// Nothing. Return an error. - Nothing, -} - /// The type of `Computation` that provided this election data. #[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug)] pub enum ElectionCompute { @@ -406,6 +396,8 @@ pub enum ElectionCompute { Signed, /// Election was computed with an unsigned submission. Unsigned, + /// Election was computed using the fallback + Fallback, /// Election was computed with emergency status. Emergency, } @@ -485,34 +477,45 @@ pub struct SolutionOrSnapshotSize { /// Internal errors of the pallet. /// /// Note that this is different from [`pallet::Error`]. -#[derive(Debug, Eq, PartialEq)] +#[derive(frame_support::DebugNoBound)] #[cfg_attr(feature = "runtime-benchmarks", derive(strum_macros::IntoStaticStr))] -pub enum ElectionError { +pub enum ElectionError { /// An error happened in the feasibility check sub-system. Feasibility(FeasibilityError), /// An error in the miner (offchain) sub-system. Miner(unsigned::MinerError), - /// An error in the on-chain fallback. - OnChainFallback(onchain::Error), /// An error happened in the data provider. DataProvider(&'static str), - /// No fallback is configured. This is a special case. - NoFallbackConfigured, + /// An error nested in the fallback. + Fallback(FallbackErrorOf), } -impl From for ElectionError { - fn from(e: onchain::Error) -> Self { - ElectionError::OnChainFallback(e) +// NOTE: we have to do this manually because of the additional where clause needed on +// `FallbackErrorOf`. +#[cfg(test)] +impl PartialEq for ElectionError +where + FallbackErrorOf: PartialEq, +{ + fn eq(&self, other: &Self) -> bool { + use ElectionError::*; + match (self, other) { + (&Feasibility(ref x), &Feasibility(ref y)) if x == y => true, + (&Miner(ref x), &Miner(ref y)) if x == y => true, + (&DataProvider(ref x), &DataProvider(ref y)) if x == y => true, + (&Fallback(ref x), &Fallback(ref y)) if x == y => true, + _ => false, + } } } -impl From for ElectionError { +impl From for ElectionError { fn from(e: FeasibilityError) -> Self { ElectionError::Feasibility(e) } } -impl From for ElectionError { +impl From for ElectionError { fn from(e: unsigned::MinerError) -> Self { ElectionError::Miner(e) } @@ -666,7 +669,11 @@ pub mod pallet { type OnChainAccuracy: PerThing128; /// Configuration for the fallback - type Fallback: Get; + type Fallback: ElectionProvider< + Self::AccountId, + Self::BlockNumber, + DataProvider = Self::DataProvider, + >; /// 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. @@ -1301,7 +1308,7 @@ impl Pallet { /// /// Extracted for easier weight calculation. fn create_snapshot_external( - ) -> Result<(Vec, Vec>, u32), ElectionError> { + ) -> Result<(Vec, Vec>, u32), ElectionError> { let target_limit = >::max_value().saturated_into::(); let voter_limit = >::max_value().saturated_into::(); @@ -1331,7 +1338,7 @@ impl Pallet { /// /// This is a *self-weighing* function, it will register its own extra weight as /// [`DispatchClass::Mandatory`] with the system pallet. - pub fn create_snapshot() -> Result<(), ElectionError> { + pub fn create_snapshot() -> Result<(), ElectionError> { // this is self-weighing itself.. let (targets, voters, desired_targets) = Self::create_snapshot_external()?; @@ -1473,16 +1480,7 @@ impl Pallet { Self::kill_snapshot(); } - /// On-chain fallback of election. - fn onchain_fallback() -> Result, ElectionError> { - > as ElectionProvider< - T::AccountId, - T::BlockNumber, - >>::elect() - .map_err(Into::into) - } - - 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: // @@ -1493,15 +1491,10 @@ impl Pallet { let _ = Self::finalize_signed_phase(); >::take() .map_or_else( - || match T::Fallback::get() { - FallbackStrategy::OnChain => Self::onchain_fallback() - .map(|s| { - // onchain election incurs maximum block weight - Self::register_weight(T::BlockWeights::get().max_block); - (s, ElectionCompute::OnChain) - }) - .map_err(Into::into), - FallbackStrategy::Nothing => Err(ElectionError::NoFallbackConfigured), + || { + T::Fallback::elect() + .map_err(|fe| ElectionError::Fallback(fe)) + .map(|supports| (supports, ElectionCompute::Fallback)) }, |ReadySolution { supports, compute, .. }| Ok((supports, compute)), ) @@ -1533,7 +1526,7 @@ impl Pallet { } impl ElectionProvider for Pallet { - type Error = ElectionError; + type Error = ElectionError; type DataProvider = T::DataProvider; fn elect() -> Result, Self::Error> { @@ -1907,7 +1900,7 @@ mod tests { multi_phase_events(), vec![ Event::SignedPhaseStarted(1), - Event::ElectionFinalized(Some(ElectionCompute::OnChain)) + Event::ElectionFinalized(Some(ElectionCompute::Fallback)) ], ); // All storage items must be cleared. @@ -1959,14 +1952,12 @@ mod tests { #[test] fn fallback_strategy_works() { - ExtBuilder::default().fallback(FallbackStrategy::OnChain).build_and_execute(|| { - roll_to(15); - assert_eq!(MultiPhase::current_phase(), Phase::Signed); - + ExtBuilder::default().onchain_fallback(true).build_and_execute(|| { roll_to(25); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); - // Zilch solutions thus far. + // Zilch solutions thus far, but we get a result. + assert!(MultiPhase::queued_solution().is_none()); let supports = MultiPhase::elect().unwrap(); assert_eq!( @@ -1978,15 +1969,16 @@ mod tests { ) }); - ExtBuilder::default().fallback(FallbackStrategy::Nothing).build_and_execute(|| { - roll_to(15); - assert_eq!(MultiPhase::current_phase(), Phase::Signed); - + ExtBuilder::default().onchain_fallback(false).build_and_execute(|| { roll_to(25); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); // Zilch solutions thus far. - assert_eq!(MultiPhase::elect().unwrap_err(), ElectionError::NoFallbackConfigured); + assert!(MultiPhase::queued_solution().is_none()); + assert_eq!( + MultiPhase::elect().unwrap_err(), + ElectionError::Fallback("Emergency phase started.") + ); }) } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 03dc6985f313c..144113f6f5166 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -57,7 +57,7 @@ frame_support::construct_runtime!( pub(crate) type Balance = u64; pub(crate) type AccountId = u64; -pub(crate) type BlockNumber = u32; +pub(crate) type BlockNumber = u64; pub(crate) type VoterIndex = u32; pub(crate) type TargetIndex = u16; @@ -76,7 +76,7 @@ pub(crate) fn multi_phase_events() -> Vec> { } /// To from `now` to block `n`. -pub fn roll_to(n: u64) { +pub fn roll_to(n: BlockNumber) { let now = System::block_number(); for i in now + 1..=n { System::set_block_number(i); @@ -84,7 +84,7 @@ pub fn roll_to(n: u64) { } } -pub fn roll_to_with_ocw(n: u64) { +pub fn roll_to_with_ocw(n: BlockNumber) { let now = System::block_number(); for i in now + 1..=n { System::set_block_number(i); @@ -198,7 +198,7 @@ impl frame_system::Config for Runtime { type BaseCallFilter = frame_support::traits::Everything; type Origin = Origin; type Index = u64; - type BlockNumber = u64; + type BlockNumber = BlockNumber; type Call = Call; type Hash = H256; type Hashing = BlakeTwo256; @@ -252,10 +252,9 @@ parameter_types! { (40, 40, vec![40]), ]; - pub static Fallback: FallbackStrategy = FallbackStrategy::OnChain; pub static DesiredTargets: u32 = 2; - pub static SignedPhase: u64 = 10; - pub static UnsignedPhase: u64 = 5; + pub static SignedPhase: BlockNumber = 10; + pub static UnsignedPhase: BlockNumber = 5; pub static SignedMaxSubmissions: u32 = 5; pub static SignedDepositBase: Balance = 5; pub static SignedDepositByte: Balance = 0; @@ -271,6 +270,30 @@ parameter_types! { pub static MockWeightInfo: bool = false; pub static EpochLength: u64 = 30; + pub static OnChianFallback: bool = true; +} + +pub struct OnChainConfig; +impl onchain::Config for OnChainConfig { + type AccountId = AccountId; + type BlockNumber = BlockNumber; + type Accuracy = sp_runtime::Perbill; + type DataProvider = StakingMock; +} + +pub struct MockFallback; +impl ElectionProvider for MockFallback { + type Error = &'static str; + type DataProvider = StakingMock; + + fn elect() -> Result, Self::Error> { + if OnChianFallback::get() { + onchain::OnChainSequentialPhragmen::::elect() + .map_err(|_| "OnChainSequentialPhragmen failed") + } else { + super::InitiateEmergencyPhase::::elect() + } + } } // Hopefully this won't be too much of a hassle to maintain. @@ -376,7 +399,7 @@ impl crate::Config for Runtime { type WeightInfo = DualMockWeightInfo; type BenchmarkingConfig = (); type OnChainAccuracy = Perbill; - type Fallback = Fallback; + type Fallback = MockFallback; type ForceOrigin = frame_system::EnsureRoot; type Solution = TestNposSolution; } @@ -472,13 +495,13 @@ impl ExtBuilder { ::set(p); self } - pub fn phases(self, signed: u64, unsigned: u64) -> Self { + pub fn phases(self, signed: BlockNumber, unsigned: BlockNumber) -> Self { ::set(signed); ::set(unsigned); self } - pub fn fallback(self, fallback: FallbackStrategy) -> Self { - ::set(fallback); + pub fn onchain_fallback(self, onchain: bool) -> Self { + ::set(onchain); self } pub fn miner_weight(self, weight: Weight) -> Self { @@ -553,6 +576,6 @@ impl ExtBuilder { } } -pub(crate) fn balances(who: &u64) -> (u64, u64) { +pub(crate) fn balances(who: &AccountId) -> (Balance, Balance) { (Balances::free_balance(who), Balances::reserved_balance(who)) } From c05109d05fd3f5f5c8662f9b37f2be5569c4b359 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 30 Aug 2021 12:19:35 +0200 Subject: [PATCH 2/9] minor fix --- bin/node/runtime/src/lib.rs | 2 +- frame/election-provider-multi-phase/src/lib.rs | 16 ++++++++-------- frame/election-provider-multi-phase/src/mock.rs | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index cd83cb3caf5e9..52a2dff13a019 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -589,7 +589,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type DataProvider = Staking; type OnChainAccuracy = Perbill; type Solution = NposSolution16; - type Fallback = pallet_election_provider_multi_phase::InitiateEmergencyPhase; + type Fallback = pallet_election_provider_multi_phase::NoFallback; type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight; type ForceOrigin = EnsureRootOrHalfCouncil; type BenchmarkingConfig = BenchmarkConfig; diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 8ae6c3721b12d..3fff33dac0063 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -114,7 +114,7 @@ //! If we reach the end of both phases (i.e. call to [`ElectionProvider::elect`] happens) and no //! good solution is queued, then the fallback strategy [`pallet::Config::Fallback`] is used to //! determine what needs to be done. The on-chain election is slow, and contains no balancing or -//! reduction post-processing. [`InitiateEmergencyPhase`] enables the [`Phase::Emergency`] which is +//! reduction post-processing. [`NoFallback`] does nothing enables the [`Phase::Emergency`] which is //! a more *fail-safe* approach. //! //! ### Emergency Phase @@ -311,17 +311,15 @@ impl BenchmarkingConfig for () { } /// A fallback implementation that transitions the pallet to the emergency phase. -pub struct InitiateEmergencyPhase(sp_std::marker::PhantomData); +pub struct NoFallback(sp_std::marker::PhantomData); -impl ElectionProvider for InitiateEmergencyPhase { +impl ElectionProvider for NoFallback { type DataProvider = T::DataProvider; type Error = &'static str; fn elect() -> Result, Self::Error> { - log!(warn, "Entering emergency phase."); - CurrentPhase::::put(Phase::Emergency); - // We won't succeed, this is intentional. - Err("Emergency phase started.") + // Do nothing, this will enable the emergency phase. + Err("NoFallback.") } } @@ -1977,8 +1975,10 @@ mod tests { assert!(MultiPhase::queued_solution().is_none()); assert_eq!( MultiPhase::elect().unwrap_err(), - ElectionError::Fallback("Emergency phase started.") + ElectionError::Fallback("NoFallback.") ); + // phase is now emergency. + assert_eq!(MultiPhase::current_phase(), Phase::Emergency); }) } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 144113f6f5166..74803611678b9 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -291,7 +291,7 @@ impl ElectionProvider for MockFallback { onchain::OnChainSequentialPhragmen::::elect() .map_err(|_| "OnChainSequentialPhragmen failed") } else { - super::InitiateEmergencyPhase::::elect() + super::NoFallback::::elect() } } } From cc2598b787e2fa0cd19304aabf7c2b6b12f562f1 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 30 Aug 2021 12:50:17 +0200 Subject: [PATCH 3/9] Fix integrity tests --- bin/node/runtime/src/lib.rs | 21 +++++++-- .../election-provider-multi-phase/src/lib.rs | 47 ++++--------------- .../election-provider-multi-phase/src/mock.rs | 10 ++-- .../election-provider-support/src/onchain.rs | 17 +++++-- 4 files changed, 40 insertions(+), 55 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 52a2dff13a019..19e2a7c65dc06 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -486,6 +486,11 @@ parameter_types! { } use frame_election_provider_support::onchain; +impl onchain::Config for Runtime { + type Accuracy = Perbill; + type DataProvider = Staking; +} + impl pallet_staking::Config for Runtime { const MAX_NOMINATIONS: u32 = MAX_NOMINATIONS; type Currency = Balances; @@ -509,9 +514,7 @@ impl pallet_staking::Config for Runtime { type NextNewSession = Session; type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; type ElectionProvider = ElectionProviderMultiPhase; - type GenesisElectionProvider = onchain::OnChainSequentialPhragmen< - pallet_election_provider_multi_phase::OnChainConfig, - >; + type GenesisElectionProvider = onchain::OnChainSequentialPhragmen; type WeightInfo = pallet_staking::weights::SubstrateWeight; } @@ -587,7 +590,6 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type SlashHandler = (); // burn slashes type RewardHandler = (); // nothing to do upon rewards type DataProvider = Staking; - type OnChainAccuracy = Perbill; type Solution = NposSolution16; type Fallback = pallet_election_provider_multi_phase::NoFallback; type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight; @@ -1668,6 +1670,7 @@ impl_runtime_apis! { mod tests { use super::*; use frame_system::offchain::CreateSignedTransaction; + use sp_runtime::UpperOf; #[test] fn validate_transaction_submitter_bounds() { @@ -1680,6 +1683,16 @@ mod tests { is_submit_signed_transaction::(); } + #[test] + fn perbill_as_onchain_accuracy() { + type OnChainAccuracy = ::Accuracy; + let maximum_chain_accuracy: Vec> = (0..MAX_NOMINATIONS) + .map(|_| >::from(OnChainAccuracy::one().deconstruct())) + .collect(); + let _: UpperOf = + maximum_chain_accuracy.iter().fold(0, |acc, x| acc.checked_add(*x).unwrap()); + } + #[test] fn call_size() { assert!( diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 3fff33dac0063..83bb4d91d6dfb 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -146,13 +146,11 @@ //! //! ## Accuracy //! -//! The accuracy of the election is configured via two trait parameters. namely, -//! [`OnChainAccuracyOf`] dictates the accuracy used to compute the on-chain fallback election and -//! [`SolutionAccuracyOf`] is the accuracy that the submitted solutions must adhere to. +//! The accuracy of the election is configured via +//! [`SolutionAccuracyOf`] which is the accuracy that the submitted solutions must adhere to. //! -//! Note that both accuracies are of great importance. The offchain solution should be as small as -//! possible, reducing solutions size/weight. The on-chain solution can use more space for accuracy, -//! but should still be fast to prevent massively large blocks in case of a fallback. +//! Note that the accuracy is of great importance. The offchain solution should be as small as +//! possible, reducing solutions size/weight. //! //! ## Error types //! @@ -211,7 +209,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use codec::{Decode, Encode}; -use frame_election_provider_support::{onchain, ElectionDataProvider, ElectionProvider}; +use frame_election_provider_support::{ElectionDataProvider, ElectionProvider}; use frame_support::{ dispatch::DispatchResultWithPostInfo, ensure, @@ -224,8 +222,8 @@ use sp_arithmetic::{ UpperOf, }; use sp_npos_elections::{ - assignment_ratio_to_staked_normalized, ElectionScore, EvaluateSupport, NposSolution, - PerThing128, Supports, VoteWeight, + assignment_ratio_to_staked_normalized, ElectionScore, EvaluateSupport, NposSolution, Supports, + VoteWeight, }; use sp_runtime::{ traits::Bounded, @@ -265,23 +263,12 @@ pub type SolutionVoterIndexOf = as NposSolution>::VoterIndex; pub type SolutionTargetIndexOf = as NposSolution>::TargetIndex; /// The accuracy of the election, when submitted from offchain. Derived from [`SolutionOf`]. pub type SolutionAccuracyOf = as NposSolution>::Accuracy; -/// The accuracy of the election, when computed on-chain. Equal to [`Config::OnChainAccuracy`]. -pub type OnChainAccuracyOf = ::OnChainAccuracy; /// The fallback election type. pub type FallbackErrorOf = <::Fallback as ElectionProvider< ::AccountId, ::BlockNumber, >>::Error; -/// Wrapper type that implements the configurations needed for the on-chain backup. -pub struct OnChainConfig(sp_std::marker::PhantomData); -impl onchain::Config for OnChainConfig { - type AccountId = T::AccountId; - type BlockNumber = T::BlockNumber; - type Accuracy = T::OnChainAccuracy; - type DataProvider = T::DataProvider; -} - /// Configuration for the benchmarks of the pallet. pub trait BenchmarkingConfig { /// Range of voters. @@ -663,9 +650,6 @@ pub mod pallet { + Ord + NposSolution; - /// Accuracy used for fallback on-chain election. - type OnChainAccuracy: PerThing128; - /// Configuration for the fallback type Fallback: ElectionProvider< Self::AccountId, @@ -796,18 +780,6 @@ pub mod pallet { // Based on the requirements of [`sp_npos_elections::Assignment::try_normalize`]. let max_vote: usize = as NposSolution>::LIMIT; - // 1. Maximum sum of [ChainAccuracy; 16] must fit into `UpperOf`.. - let maximum_chain_accuracy: Vec>> = (0..max_vote) - .map(|_| { - >>::from( - >::one().deconstruct(), - ) - }) - .collect(); - let _: UpperOf> = maximum_chain_accuracy - .iter() - .fold(Zero::zero(), |acc, x| acc.checked_add(x).unwrap()); - // 2. Maximum sum of [SolutionAccuracy; 16] must fit into `UpperOf`. let maximum_chain_accuracy: Vec>> = (0..max_vote) .map(|_| { @@ -1973,10 +1945,7 @@ mod tests { // Zilch solutions thus far. assert!(MultiPhase::queued_solution().is_none()); - assert_eq!( - MultiPhase::elect().unwrap_err(), - ElectionError::Fallback("NoFallback.") - ); + assert_eq!(MultiPhase::elect().unwrap_err(), ElectionError::Fallback("NoFallback.")); // phase is now emergency. assert_eq!(MultiPhase::current_phase(), Phase::Emergency); }) diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 74803611678b9..58bba6208f96d 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -36,7 +36,7 @@ use sp_npos_elections::{ use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup}, - PerU16, + PerU16, RuntimeAppPublic, }; use std::{convert::TryFrom, sync::Arc}; @@ -273,10 +273,7 @@ parameter_types! { pub static OnChianFallback: bool = true; } -pub struct OnChainConfig; -impl onchain::Config for OnChainConfig { - type AccountId = AccountId; - type BlockNumber = BlockNumber; +impl onchain::Config for Runtime { type Accuracy = sp_runtime::Perbill; type DataProvider = StakingMock; } @@ -288,7 +285,7 @@ impl ElectionProvider for MockFallback { fn elect() -> Result, Self::Error> { if OnChianFallback::get() { - onchain::OnChainSequentialPhragmen::::elect() + onchain::OnChainSequentialPhragmen::::elect() .map_err(|_| "OnChainSequentialPhragmen failed") } else { super::NoFallback::::elect() @@ -398,7 +395,6 @@ impl crate::Config for Runtime { type DataProvider = StakingMock; type WeightInfo = DualMockWeightInfo; type BenchmarkingConfig = (); - type OnChainAccuracy = Perbill; type Fallback = MockFallback; type ForceOrigin = frame_system::EnsureRoot; type Solution = TestNposSolution; diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 8dcf8d4a87d05..b7157eadd8ce6 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -18,6 +18,7 @@ //! An implementation of [`ElectionProvider`] that does an on-chain sequential phragmen. use crate::{ElectionDataProvider, ElectionProvider}; +use frame_support::{traits::Get, weights::DispatchClass}; use sp_npos_elections::*; use sp_std::{collections::btree_map::BTreeMap, marker::PhantomData, prelude::*}; @@ -53,11 +54,11 @@ pub struct OnChainSequentialPhragmen(PhantomData); /// Configuration trait of [`OnChainSequentialPhragmen`]. /// /// Note that this is similar to a pallet traits, but [`OnChainSequentialPhragmen`] is not a pallet. -pub trait Config { - /// The account identifier type. - type AccountId: IdentifierT; - /// The block number type. - type BlockNumber; +/// +/// WARNING: the user of this pallet must ensure that the `Accuracy` type will work nicely with the +/// normalization operation done inside `seq_phragmen`. See +/// [`sp_npos_elections::assignment::try_normalize`] for more info. +pub trait Config: frame_system::Config { /// The accuracy used to compute the election: type Accuracy: PerThing128; /// Something that provides the data for election. @@ -89,6 +90,12 @@ impl ElectionProvider for OnChainSequen let staked = assignment_ratio_to_staked_normalized(assignments, &stake_of)?; let winners = to_without_backing(winners); + let weight = T::BlockWeights::get().max_block; + frame_system::Pallet::::register_extra_weight_unchecked( + weight, + DispatchClass::Mandatory, + ); + to_supports(&winners, &staked).map_err(Error::from) } } From a15cb01df8411d020b2d95dcbb48b13ff585b7c8 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Tue, 31 Aug 2021 14:03:55 +0100 Subject: [PATCH 4/9] Update frame/election-provider-multi-phase/src/lib.rs Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com> --- frame/election-provider-multi-phase/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 83bb4d91d6dfb..9600ea8152386 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -114,7 +114,7 @@ //! If we reach the end of both phases (i.e. call to [`ElectionProvider::elect`] happens) and no //! good solution is queued, then the fallback strategy [`pallet::Config::Fallback`] is used to //! determine what needs to be done. The on-chain election is slow, and contains no balancing or -//! reduction post-processing. [`NoFallback`] does nothing enables the [`Phase::Emergency`] which is +//! reduction post-processing. [`NoFallback`] does nothing and enables [`Phase::Emergency`], which is //! a more *fail-safe* approach. //! //! ### Emergency Phase From 8aa2afe871caf4961a17a9c7a8998ac3e088e134 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 1 Sep 2021 10:23:42 +0200 Subject: [PATCH 5/9] Fix everything --- Cargo.lock | 2 + frame/babe/src/mock.rs | 2 - .../election-provider-multi-phase/src/mock.rs | 4 +- frame/election-provider-support/Cargo.toml | 2 + .../election-provider-support/src/onchain.rs | 46 +++++++++++++++++-- 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 - 9 files changed, 48 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a613b7ba03686..b28e9763f6269 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1928,9 +1928,11 @@ name = "frame-election-provider-support" version = "4.0.0-dev" dependencies = [ "frame-support", + "frame-support-test", "frame-system", "parity-scale-codec", "sp-arithmetic", + "sp-core", "sp-npos-elections", "sp-runtime", "sp-std", diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index f4f1310e83566..bc0be32624cba 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -192,8 +192,6 @@ parameter_types! { } impl onchain::Config for Test { - type AccountId = ::AccountId; - type BlockNumber = ::BlockNumber; type Accuracy = Perbill; type DataProvider = Staking; } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 58bba6208f96d..e8c1091bdd67c 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -17,7 +17,7 @@ use super::*; use crate as multi_phase; -use frame_election_provider_support::{data_provider, ElectionDataProvider}; +use frame_election_provider_support::{data_provider, onchain, ElectionDataProvider}; pub use frame_support::{assert_noop, assert_ok}; use frame_support::{parameter_types, traits::Hooks, weights::Weight}; use multi_phase::unsigned::{IndexAssignmentOf, Voter}; @@ -36,7 +36,7 @@ use sp_npos_elections::{ use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup}, - PerU16, RuntimeAppPublic, + PerU16, }; use std::{convert::TryFrom, sync::Arc}; diff --git a/frame/election-provider-support/Cargo.toml b/frame/election-provider-support/Cargo.toml index c0d332315b020..a7af5093b61ad 100644 --- a/frame/election-provider-support/Cargo.toml +++ b/frame/election-provider-support/Cargo.toml @@ -23,6 +23,8 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys [dev-dependencies] sp-npos-elections = { version = "4.0.0-dev", path = "../../primitives/npos-elections" } sp-runtime = { version = "4.0.0-dev", path = "../../primitives/runtime" } +sp-core = { version = "4.0.0-dev", path = "../../primitives/core" } +frame-support-test = { version = "3.0.0", path = "../support/test" } [features] default = ["std"] diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index b7157eadd8ce6..30df567a8cbfe 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -107,11 +107,49 @@ mod tests { use sp_runtime::Perbill; type AccountId = u64; - type BlockNumber = u32; - struct Runtime; - impl Config for Runtime { - type AccountId = AccountId; + type BlockNumber = u64; + + pub type Header = sp_runtime::generic::Header; + pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; + pub type Block = sp_runtime::generic::Block; + + frame_support::construct_runtime!( + pub enum Runtime where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + System: frame_system::{Pallet, Call, Event}, + } + ); + + impl frame_system::Config for Runtime { + type SS58Prefix = (); + type BaseCallFilter = frame_support::traits::Everything; + type Origin = Origin; + type Index = AccountId; type BlockNumber = BlockNumber; + type Call = Call; + type Hash = sp_core::H256; + type Hashing = sp_runtime::traits::BlakeTwo256; + type AccountId = AccountId; + type Lookup = sp_runtime::traits::IdentityLookup; + type Header = sp_runtime::testing::Header; + type Event = (); + type BlockHashCount = (); + type DbWeight = (); + type BlockLength = (); + type BlockWeights = (); + type Version = (); + type PalletInfo = frame_support_test::PanicPalletInfo; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type SystemWeightInfo = (); + type OnSetCode = (); + } + + impl Config for Runtime { type Accuracy = Perbill; type DataProvider = mock_data_provider::DataProvider; } diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index b8d6f699f890c..26dda514516a3 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -194,8 +194,6 @@ parameter_types! { } impl onchain::Config for Test { - type AccountId = ::AccountId; - type BlockNumber = ::BlockNumber; type Accuracy = Perbill; type DataProvider = Staking; } diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index 431877c3a8f9d..c4fd88def0e33 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -151,8 +151,6 @@ parameter_types! { pub type Extrinsic = sp_runtime::testing::TestXt; impl onchain::Config for Test { - type AccountId = AccountId; - type BlockNumber = BlockNumber; type Accuracy = Perbill; type DataProvider = Staking; } diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index 9de4a0320d15d..c685db2bb2524 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -159,8 +159,6 @@ where } impl onchain::Config for Test { - type AccountId = AccountId; - type BlockNumber = BlockNumber; type Accuracy = sp_runtime::Perbill; type DataProvider = Staking; } diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 82eca58e5355e..0357fa05cb1dd 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -243,8 +243,6 @@ impl OnUnbalanced> for RewardRemainderMock { } impl onchain::Config for Test { - type AccountId = AccountId; - type BlockNumber = BlockNumber; type Accuracy = Perbill; type DataProvider = Staking; } From 3906c2cb68dfdb162fbc1e5cfc8a46dd1a03f5be Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 1 Sep 2021 10:31:15 +0200 Subject: [PATCH 6/9] fmt again --- frame/election-provider-multi-phase/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 9600ea8152386..e1288a1488d66 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -114,8 +114,8 @@ //! If we reach the end of both phases (i.e. call to [`ElectionProvider::elect`] happens) and no //! good solution is queued, then the fallback strategy [`pallet::Config::Fallback`] is used to //! determine what needs to be done. The on-chain election is slow, and contains no balancing or -//! reduction post-processing. [`NoFallback`] does nothing and enables [`Phase::Emergency`], which is -//! a more *fail-safe* approach. +//! reduction post-processing. [`NoFallback`] does nothing and enables [`Phase::Emergency`], which +//! is a more *fail-safe* approach. //! //! ### Emergency Phase //! From f2fa5572dbaf412364076c6e5628884ffb540465 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 1 Sep 2021 14:33:00 +0200 Subject: [PATCH 7/9] Fix test --- Cargo.lock | 2 +- frame/election-provider-support/Cargo.toml | 2 +- frame/election-provider-support/src/onchain.rs | 18 ++++++++++-------- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b28e9763f6269..6e2d0a71797ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1928,11 +1928,11 @@ name = "frame-election-provider-support" version = "4.0.0-dev" dependencies = [ "frame-support", - "frame-support-test", "frame-system", "parity-scale-codec", "sp-arithmetic", "sp-core", + "sp-io", "sp-npos-elections", "sp-runtime", "sp-std", diff --git a/frame/election-provider-support/Cargo.toml b/frame/election-provider-support/Cargo.toml index a7af5093b61ad..d713b98fcefa1 100644 --- a/frame/election-provider-support/Cargo.toml +++ b/frame/election-provider-support/Cargo.toml @@ -24,7 +24,7 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys sp-npos-elections = { version = "4.0.0-dev", path = "../../primitives/npos-elections" } sp-runtime = { version = "4.0.0-dev", path = "../../primitives/runtime" } sp-core = { version = "4.0.0-dev", path = "../../primitives/core" } -frame-support-test = { version = "3.0.0", path = "../support/test" } +sp-io = { version = "4.0.0-dev", path = "../../primitives/io" } [features] default = ["std"] diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 30df567a8cbfe..4685b9857023b 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -141,7 +141,7 @@ mod tests { type BlockLength = (); type BlockWeights = (); type Version = (); - type PalletInfo = frame_support_test::PanicPalletInfo; + type PalletInfo = PalletInfo; type AccountData = (); type OnNewAccount = (); type OnKilledAccount = (); @@ -185,12 +185,14 @@ mod tests { #[test] fn onchain_seq_phragmen_works() { - assert_eq!( - OnChainPhragmen::elect().unwrap(), - vec![ - (10, Support { total: 25, voters: vec![(1, 10), (3, 15)] }), - (30, Support { total: 35, voters: vec![(2, 20), (3, 15)] }) - ] - ); + sp_io::TestExternalities::new_empty().execute_with(|| { + assert_eq!( + OnChainPhragmen::elect().unwrap(), + vec![ + (10, Support { total: 25, voters: vec![(1, 10), (3, 15)] }), + (30, Support { total: 35, voters: vec![(2, 20), (3, 15)] }) + ] + ); + }) } } From 534b0ee68f4007213e7780a01f5da3a2fdbb1e2c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 10 Sep 2021 10:04:16 +0200 Subject: [PATCH 8/9] Fix state machine warning --- primitives/state-machine/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 032899faeb523..05d2c6d20ccee 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1579,7 +1579,8 @@ mod tests { let mut seed = [0; 16]; for i in 0..50u32 { let mut child_infos = Vec::new(); - &seed[0..4].copy_from_slice(&i.to_be_bytes()[..]); + let seed_partial = &mut seed[0..4]; + seed_partial.copy_from_slice(&i.to_be_bytes()[..]); let mut rand = SmallRng::from_seed(seed); let nb_child_trie = rand.next_u32() as usize % 25; From 10cacffe6223faba75d4c5e5394536196286a772 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 12 Sep 2021 09:28:23 +0200 Subject: [PATCH 9/9] Fix build --- frame/election-provider-support/src/onchain.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index f5151ac7ab83b..fb1ccfdfe2566 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -94,7 +94,7 @@ impl ElectionProvider for OnChainSequen DispatchClass::Mandatory, ); - to_supports(&staked).map_err(Error::from) + Ok(to_supports(&staked)) } }