diff --git a/Cargo.lock b/Cargo.lock index 9e6e30cf2d00..1ed1027f8348 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4271,6 +4271,7 @@ name = "node-runtime" version = "2.0.1" dependencies = [ "frame-benchmarking", + "frame-election-provider-support", "frame-executive", "frame-support", "frame-system", diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index f0cad60f2614..d93b54d9424a 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -46,6 +46,7 @@ frame-system = { version = "3.0.0", default-features = false, path = "../../../f frame-system-benchmarking = { version = "3.0.0", default-features = false, path = "../../../frame/system/benchmarking", optional = true } frame-system-rpc-runtime-api = { version = "3.0.0", default-features = false, path = "../../../frame/system/rpc/runtime-api/" } frame-try-runtime = { version = "0.9.0", default-features = false, path = "../../../frame/try-runtime", optional = true } +frame-election-provider-support = { version = "3.0.0", default-features = false, path = "../../../frame/election-provider-support" } pallet-assets = { version = "3.0.0", default-features = false, path = "../../../frame/assets" } pallet-authority-discovery = { version = "3.0.0", default-features = false, path = "../../../frame/authority-discovery" } pallet-authorship = { version = "3.0.0", default-features = false, path = "../../../frame/authorship" } @@ -115,6 +116,7 @@ std = [ "pallet-democracy/std", "pallet-elections-phragmen/std", "frame-executive/std", + "frame-election-provider-support/std", "pallet-gilt/std", "pallet-grandpa/std", "pallet-im-online/std", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 448867b25cb1..41761605db82 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -504,10 +504,6 @@ parameter_types! { pub const SignedPhase: u32 = EPOCH_DURATION_IN_BLOCKS / 4; pub const UnsignedPhase: u32 = EPOCH_DURATION_IN_BLOCKS / 4; - // fallback: no need to do on-chain phragmen initially. - pub const Fallback: pallet_election_provider_multi_phase::FallbackStrategy = - pallet_election_provider_multi_phase::FallbackStrategy::OnChain; - pub SolutionImprovementThreshold: Perbill = Perbill::from_rational(1u32, 10_000); // miner configs @@ -543,7 +539,9 @@ impl pallet_election_provider_multi_phase::Config for Runtime { type DataProvider = Staking; type OnChainAccuracy = Perbill; type CompactSolution = NposCompactSolution16; - type Fallback = Fallback; + type Fallback = frame_election_provider_support::onchain::OnChainSequentialPhragmen< + pallet_election_provider_multi_phase::OnChainConfig, + >; type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight; type BenchmarkingConfig = (); } diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 17978566a858..6272c5df145c 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -112,9 +112,10 @@ //! //! 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`] should probably only be used for testing, and returns an error. +//! determine what needs to be done. A fallback strategy is an [`ElectionProvider`] itself. If +//! on-chain phragmen is to be used as fallback, this pallet is an [`ElectionDataProvider`] itself, +//! and can act as the data provider for the fallback. This is likely to be more efficient than +//! querying the original source of the election data (i.e. staking) again. //! //! ## Feasible Solution (correct solution) //! @@ -184,13 +185,6 @@ //! [`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. //! @@ -198,11 +192,6 @@ //! this pallet as well. The `OFFCHAIN_REPEAT` also needs to become an adjustable parameter of the //! pallet. //! -//! **Make the number of nominators configurable from the runtime**. Remove `sp_npos_elections` -//! dependency from staking and the compact 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 @@ -222,7 +211,7 @@ use frame_support::{ weights::Weight, }; use frame_system::{ensure_none, offchain::SendTransactionTypes}; -use frame_election_provider_support::{ElectionDataProvider, ElectionProvider, onchain}; +use frame_election_provider_support::{ElectionDataProvider, ElectionProvider, data_provider, onchain}; use sp_npos_elections::{ assignment_ratio_to_staked_normalized, is_score_better, CompactSolution, ElectionScore, EvaluateSupport, PerThing128, Supports, VoteWeight, @@ -235,7 +224,7 @@ use sp_runtime::{ DispatchError, PerThing, Perbill, RuntimeDebug, SaturatedConversion, traits::Bounded, }; -use sp_std::prelude::*; +use sp_std::{prelude::*, marker::PhantomData}; use sp_arithmetic::{ UpperOf, traits::{Zero, CheckedAdd}, @@ -268,16 +257,6 @@ pub type CompactAccuracyOf = as CompactSolution>::Accuracy; /// The accuracy of the election, when computed on-chain. Equal to [`Config::OnChainAccuracy`]. pub type OnChainAccuracyOf = ::OnChainAccuracy; -/// Wrapper type that implements the configurations needed for the on-chain backup. -struct OnChainConfig(sp_std::marker::PhantomData); -impl onchain::Config for OnChainConfig { - type AccountId = T::AccountId; - type BlockNumber = T::BlockNumber; - type BlockWeights = T::BlockWeights; - type Accuracy = T::OnChainAccuracy; - type DataProvider = T::DataProvider; -} - /// Configuration for the benchmarks of the pallet. pub trait BenchmarkingConfig { /// Range of voters. @@ -342,24 +321,11 @@ 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, RuntimeDebug)] pub enum ElectionCompute { - /// Election was computed on-chain. - OnChain, + /// Election was computed using the fallback method. + Fallback, /// Election was computed with a signed submission. Signed, /// Election was computed with an unsigned submission. @@ -368,7 +334,7 @@ pub enum ElectionCompute { impl Default for ElectionCompute { fn default() -> Self { - ElectionCompute::OnChain + ElectionCompute::Fallback } } @@ -447,18 +413,10 @@ pub enum ElectionError { 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, -} - -impl From for ElectionError { - fn from(e: onchain::Error) -> Self { - ElectionError::OnChainFallback(e) - } + /// The fallback election provider also failed. + FallbackFailed, } impl From for ElectionError { @@ -555,8 +513,13 @@ pub mod pallet { /// Accuracy used for fallback on-chain election. type OnChainAccuracy: PerThing128; - /// Configuration for the fallback - type Fallback: Get; + /// The fallback. We only accept another election provider that uses us as the data + /// provider. + type Fallback: ElectionProvider< + Self::AccountId, + Self::BlockNumber, + DataProvider = Pallet, + >; /// The configuration of benchmarking. type BenchmarkingConfig: BenchmarkingConfig; @@ -1068,23 +1031,17 @@ impl Pallet { } /// On-chain fallback of election. - fn onchain_fallback() -> Result<(Supports, Weight), ElectionError> { - > as ElectionProvider< - T::AccountId, - T::BlockNumber, - >>::elect() - .map_err(Into::into) + fn fallback() -> Result<(Supports, Weight), ElectionError> { + >::elect().map_err(|e| { + log!(error, "fallback election failed due to: {:?}", e); + ElectionError::FallbackFailed + }) } fn do_elect() -> Result<(Supports, Weight), ElectionError> { >::take() .map_or_else( - || match T::Fallback::get() { - FallbackStrategy::OnChain => Self::onchain_fallback() - .map(|(s, w)| (s, w, ElectionCompute::OnChain)) - .map_err(Into::into), - FallbackStrategy::Nothing => Err(ElectionError::NoFallbackConfigured), - }, + || Self::fallback().map(|(s, w)| (s, w, ElectionCompute::Fallback)), |ReadySolution { supports, compute, .. }| Ok(( supports, T::WeightInfo::elect_queued(), @@ -1120,6 +1077,71 @@ impl ElectionProvider for Pallet { } } +fn maybe_trim(items: Vec, maybe_size: Option) -> Vec { + if let Some(size) = maybe_size { + items.into_iter().take(size).collect::>() + } else { + items + } +} + +impl ElectionDataProvider for Pallet { + const MAXIMUM_VOTES_PER_VOTER: u32 = T::DataProvider::MAXIMUM_VOTES_PER_VOTER; + + fn desired_targets() -> data_provider::Result<(u32, Weight)> { + Self::desired_targets().map(|t| (t, T::DbWeight::get().reads(1))).ok_or("No snapshot") + } + + fn voters( + maybe_max_len: Option, + ) -> data_provider::Result<(Vec<(T::AccountId, VoteWeight, Vec)>, Weight)> { + Self::snapshot() + .map(|s| (maybe_trim(s.voters, maybe_max_len), T::DbWeight::get().reads(1))) + .ok_or("No snapshot") + } + + fn targets(maybe_max_len: Option) -> data_provider::Result<(Vec, Weight)> { + Self::snapshot() + .map(|s| (maybe_trim(s.targets, maybe_max_len), T::DbWeight::get().reads(1))) + .ok_or("No snapshot") + } + + fn next_election_prediction(now: T::BlockNumber) -> T::BlockNumber { + T::DataProvider::next_election_prediction(now) + } + + #[cfg(any(feature = "runtime-benchmarks", test))] + fn put_snapshot( + _: Vec<(T::AccountId, VoteWeight, Vec)>, + _: Vec, + _: Option, + ) { + unreachable!() + } +} + +/// A struct that can be used the the fallback election of this pallet in cases where no fallback is +/// needed. +pub struct NoFallback(PhantomData); +impl ElectionProvider for NoFallback { + type Error = &'static str; + type DataProvider = Pallet; + + fn elect() -> Result<(Supports, Weight), Self::Error> { + Err("'NoFallback' will not perform an election.") + } +} + +/// 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 BlockWeights = T::BlockWeights; + type Accuracy = T::OnChainAccuracy; + type DataProvider = Pallet; +} + /// convert a DispatchError to a custom InvalidTransaction with the inner code being the error /// number. pub fn dispatch_error_to_invalid(error: DispatchError) -> InvalidTransaction { @@ -1138,7 +1160,7 @@ mod feasibility_check { use super::{mock::*, *}; - const COMPUTE: ElectionCompute = ElectionCompute::OnChain; + const COMPUTE: ElectionCompute = ElectionCompute::Fallback; #[test] fn snapshot_is_there() { @@ -1304,8 +1326,7 @@ mod feasibility_check { #[cfg(test)] mod tests { use super::{mock::*, Event, *}; - use frame_election_provider_support::ElectionProvider; - use sp_npos_elections::Support; + use frame_election_provider_support::{ElectionProvider, Support}; #[test] fn phase_rotation_works() { @@ -1433,9 +1454,11 @@ mod tests { roll_to(30); assert!(MultiPhase::current_phase().is_off()); - // this module is now only capable of doing on-chain backup. - assert_ok!(MultiPhase::elect()); + // this module is only capable of the fallback, which in this case cannot work either + // because no snapshot exists. + assert!(MultiPhase::elect().is_err()); + // Phase resets anyhow. assert!(MultiPhase::current_phase().is_off()); }); } @@ -1462,7 +1485,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. @@ -1476,7 +1499,7 @@ mod tests { #[test] fn fallback_strategy_works() { - ExtBuilder::default().fallback(FallbackStrategy::OnChain).build_and_execute(|| { + ExtBuilder::default().fallback(NothingOrOnChain::OnChain).build_and_execute(|| { roll_to(15); assert_eq!(MultiPhase::current_phase(), Phase::Signed); @@ -1486,6 +1509,7 @@ mod tests { // zilch solutions thus far. let (supports, _) = MultiPhase::elect().unwrap(); + // but the fallback works. assert_eq!( supports, vec![ @@ -1495,21 +1519,21 @@ mod tests { ) }); - ExtBuilder::default().fallback(FallbackStrategy::Nothing).build_and_execute(|| { + ExtBuilder::default().fallback(NothingOrOnChain::Nothing).build_and_execute(|| { roll_to(15); assert_eq!(MultiPhase::current_phase(), Phase::Signed); roll_to(25); assert_eq!(MultiPhase::current_phase(), Phase::Unsigned((true, 25))); - // zilch solutions thus far. - assert_eq!(MultiPhase::elect().unwrap_err(), ElectionError::NoFallbackConfigured); + // zilch solutions thus far, and fallback will also do nothing. + assert_eq!(MultiPhase::elect().unwrap_err(), ElectionError::FallbackFailed); }) } #[test] fn snapshot_creation_fails_if_too_big() { - ExtBuilder::default().build_and_execute(|| { + ExtBuilder::default().fallback(NothingOrOnChain::OnChain).build_and_execute(|| { Targets::set((0..(TargetIndex::max_value() as AccountId) + 1).collect::>()); // signed phase failed to open. @@ -1520,10 +1544,9 @@ mod tests { roll_to(25); assert_eq!(MultiPhase::current_phase(), Phase::Off); - // on-chain backup works though. + // on-chain backup cannot work either roll_to(29); - let (supports, _) = MultiPhase::elect().unwrap(); - assert!(supports.len() > 0); + assert!(MultiPhase::elect().is_err()); }) } diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs index 5a0a83354b26..d0e53834dd2b 100644 --- a/frame/election-provider-multi-phase/src/mock.rs +++ b/frame/election-provider-multi-phase/src/mock.rs @@ -195,7 +195,6 @@ 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; @@ -269,6 +268,32 @@ impl multi_phase::weights::WeightInfo for DualMockWeightInfo { } } +#[derive(Clone)] +pub enum NothingOrOnChain { + Nothing, + OnChain, +} + +parameter_types! { + pub static FallbackMode: NothingOrOnChain = NothingOrOnChain::OnChain; +} + +pub struct MultiFacadeElectionProvider; +impl ElectionProvider for MultiFacadeElectionProvider { + type Error = (); + type DataProvider = Pallet; + + fn elect() -> Result<(Supports, Weight), Self::Error> { + match FallbackMode::get() { + NothingOrOnChain::Nothing => NoFallback::::elect().map_err(|_| ()), + NothingOrOnChain::OnChain => { + onchain::OnChainSequentialPhragmen::>::elect() + .map_err(|_| ()) + } + } + } +} + impl crate::Config for Runtime { type Event = Event; type Currency = Balances; @@ -282,7 +307,7 @@ impl crate::Config for Runtime { type WeightInfo = DualMockWeightInfo; type BenchmarkingConfig = (); type OnChainAccuracy = Perbill; - type Fallback = Fallback; + type Fallback = MultiFacadeElectionProvider; type CompactSolution = TestCompact; } @@ -355,8 +380,8 @@ impl ExtBuilder { ::set(unsigned); self } - pub fn fallback(self, fallback: FallbackStrategy) -> Self { - ::set(fallback); + pub fn fallback(self, fallback: NothingOrOnChain) -> Self { + ::set(fallback); self } pub fn miner_weight(self, weight: Weight) -> Self {