Recursive election provider as fallback (#9648)

* Recursive election provider as fallback

* minor fix

* Fix integrity tests

* Update frame/election-provider-multi-phase/src/lib.rs

Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>

* Fix everything

* fmt again

* Fix test

* Fix state machine warning

* Fix build

Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>
This commit is contained in:
Kian Paimani
2021-09-12 15:38:32 +01:00
committed by GitHub
parent c09d52ead7
commit 056fd9b8a8
12 changed files with 190 additions and 157 deletions
@@ -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. [`NoFallback`] does nothing and enables [`Phase::Emergency`], which
//! is a more *fail-safe* approach.
//!
//! ### Emergency Phase
//!
@@ -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
//!
@@ -201,26 +199,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
//! <https://github.com/paritytech/substrate/pull/7929> 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.
@@ -228,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,
@@ -241,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,
@@ -282,17 +263,11 @@ pub type SolutionVoterIndexOf<T> = <SolutionOf<T> as NposSolution>::VoterIndex;
pub type SolutionTargetIndexOf<T> = <SolutionOf<T> as NposSolution>::TargetIndex;
/// The accuracy of the election, when submitted from offchain. Derived from [`SolutionOf`].
pub type SolutionAccuracyOf<T> = <SolutionOf<T> as NposSolution>::Accuracy;
/// The accuracy of the election, when computed on-chain. Equal to [`Config::OnChainAccuracy`].
pub type OnChainAccuracyOf<T> = <T as Config>::OnChainAccuracy;
/// Wrapper type that implements the configurations needed for the on-chain backup.
pub struct OnChainConfig<T: Config>(sp_std::marker::PhantomData<T>);
impl<T: Config> onchain::Config for OnChainConfig<T> {
type AccountId = T::AccountId;
type BlockNumber = T::BlockNumber;
type Accuracy = T::OnChainAccuracy;
type DataProvider = T::DataProvider;
}
/// The fallback election type.
pub type FallbackErrorOf<T> = <<T as crate::Config>::Fallback as ElectionProvider<
<T as frame_system::Config>::AccountId,
<T as frame_system::Config>::BlockNumber,
>>::Error;
/// Configuration for the benchmarks of the pallet.
pub trait BenchmarkingConfig {
@@ -322,6 +297,19 @@ impl BenchmarkingConfig for () {
const MAXIMUM_TARGETS: u32 = 2_000;
}
/// A fallback implementation that transitions the pallet to the emergency phase.
pub struct NoFallback<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> ElectionProvider<T::AccountId, T::BlockNumber> for NoFallback<T> {
type DataProvider = T::DataProvider;
type Error = &'static str;
fn elect() -> Result<Supports<T::AccountId>, Self::Error> {
// Do nothing, this will enable the emergency phase.
Err("NoFallback.")
}
}
/// Current phase of the pallet.
#[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, Debug)]
pub enum Phase<Bn> {
@@ -384,19 +372,6 @@ impl<Bn: PartialEq + Eq> Phase<Bn> {
}
}
/// 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 +381,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,24 +462,35 @@ pub struct SolutionOrSnapshotSize {
/// Internal errors of the pallet.
///
/// Note that this is different from [`pallet::Error`].
#[derive(frame_support::DebugNoBound, frame_support::PartialEqNoBound)]
#[derive(frame_support::DebugNoBound)]
#[cfg_attr(feature = "runtime-benchmarks", derive(strum_macros::IntoStaticStr))]
pub enum ElectionError<T: Config> {
/// An error happened in the feasibility check sub-system.
Feasibility(FeasibilityError),
/// An error in the miner (offchain) sub-system.
Miner(unsigned::MinerError<T>),
/// 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<T>),
}
impl<T: Config> From<onchain::Error> for ElectionError<T> {
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<T>`.
#[cfg(test)]
impl<T: Config> PartialEq for ElectionError<T>
where
FallbackErrorOf<T>: 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,
}
}
}
@@ -657,11 +645,12 @@ pub mod pallet {
+ Ord
+ NposSolution;
/// Accuracy used for fallback on-chain election.
type OnChainAccuracy: PerThing128;
/// Configuration for the fallback
type Fallback: Get<FallbackStrategy>;
type Fallback: ElectionProvider<
Self::AccountId,
Self::BlockNumber,
DataProvider = Self::DataProvider,
>;
/// OCW election solution miner algorithm implementation.
type Solver: NposSolver<AccountId = Self::AccountId>;
@@ -789,18 +778,6 @@ pub mod pallet {
// Based on the requirements of [`sp_npos_elections::Assignment::try_normalize`].
let max_vote: usize = <SolutionOf<T> as NposSolution>::LIMIT;
// 1. Maximum sum of [ChainAccuracy; 16] must fit into `UpperOf<ChainAccuracy>`..
let maximum_chain_accuracy: Vec<UpperOf<OnChainAccuracyOf<T>>> = (0..max_vote)
.map(|_| {
<UpperOf<OnChainAccuracyOf<T>>>::from(
<OnChainAccuracyOf<T>>::one().deconstruct(),
)
})
.collect();
let _: UpperOf<OnChainAccuracyOf<T>> = maximum_chain_accuracy
.iter()
.fold(Zero::zero(), |acc, x| acc.checked_add(x).unwrap());
// 2. Maximum sum of [SolutionAccuracy; 16] must fit into `UpperOf<OffchainAccuracy>`.
let maximum_chain_accuracy: Vec<UpperOf<SolutionAccuracyOf<T>>> = (0..max_vote)
.map(|_| {
@@ -1455,15 +1432,6 @@ impl<T: Config> Pallet<T> {
Self::kill_snapshot();
}
/// On-chain fallback of election.
fn onchain_fallback() -> Result<Supports<T::AccountId>, ElectionError<T>> {
<onchain::OnChainSequentialPhragmen<OnChainConfig<T>> as ElectionProvider<
T::AccountId,
T::BlockNumber,
>>::elect()
.map_err(Into::into)
}
fn do_elect() -> Result<Supports<T::AccountId>, ElectionError<T>> {
// We have to unconditionally try finalizing the signed phase here. There are only two
// possibilities:
@@ -1475,15 +1443,10 @@ impl<T: Config> Pallet<T> {
let _ = Self::finalize_signed_phase();
<QueuedSolution<T>>::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)),
)
@@ -1889,7 +1852,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.
@@ -1941,14 +1904,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!(
@@ -1960,15 +1921,15 @@ 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("NoFallback."));
// phase is now emergency.
assert_eq!(MultiPhase::current_phase(), Phase::Emergency);
})
}