Add a bounded fallback on failed elections (#10988)

* Allow `pallet-election-provider` to accept smaller solutions, issue #9478 * Fixing a typo * Adding some more tests Removing a seemingly outdated comment * making it a URL * Updating test name as per suggestion Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Updating documentation to be more explicit And to follow the general guidelines Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Fixing formatting * `Fallback` now of type `InstantElectionProvider` Some cleanups * Allow `pallet-election-provider` to accept smaller solutions, issue #9478 * Fixing a typo * Adding some more tests Removing a seemingly outdated comment * making it a URL * Updating test name as per suggestion Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Updating documentation to be more explicit And to follow the general guidelines Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Fixing formatting * `Fallback` now of type `InstantElectionProvider` Some cleanups * Merging types into one type with generics * Removing `ConstUSize` and use `ConstU32` * cleaning up the code * deprecating `OnChainSequentialPhragmen` Renaming it to `UnboundedSequentialPhragmen` which should only be used at genesis and for testing. Use preferrably `BoundedOnChainSequentialPhragmen` * Amending docs * Adding some explicit imports * Implementing generic `BoundedOnchainExecution` Removing the deprecated `OnChainSequentialPhragmen` * Use the right Balancing strategy * Refactoring `onchain::Config` Creating `onchain::ExecutionConfig` * Merge master * fmt * Name cleanups after review suggestions * cosmetics * renaming `instant_elect` to `elect_with_bounds` Other corresponding changes as per @kianenigma feedback * `BoundedOnchainExecution` -> `BoundedExecution` And `UnboundedOnchainExecution` -> `UnboundedExecution` * feedback from kian * fmt + unneeded import Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: kianenigma <kian@parity.io>
paritytech · Mar 25, 2022 · 69079b5 · 69079b5
1 parent b8aea62
commit 69079b5
Show file tree

Hide file tree

Showing 11 changed files with 223 additions and 135 deletions.
diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
@@ -23,7 +23,7 @@
 #![recursion_limit = "256"]
 
 use codec::{Decode, Encode, MaxEncodedLen};
-use frame_election_provider_support::{onchain, ExtendedBalance, VoteWeight};
+use frame_election_provider_support::{onchain, ExtendedBalance, SequentialPhragmen, VoteWeight};
 use frame_support::{
 	construct_runtime,
 	pallet_prelude::Get,
@@ -557,7 +557,7 @@ impl pallet_staking::Config for Runtime {
 	type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator;
 	type OffendingValidatorsThreshold = OffendingValidatorsThreshold;
 	type ElectionProvider = ElectionProviderMultiPhase;
-	type GenesisElectionProvider = onchain::OnChainSequentialPhragmen<Self>;
+	type GenesisElectionProvider = onchain::UnboundedExecution<OnChainSeqPhragmen>;
 	type VoterList = BagsList;
 	type MaxUnlockingChunks = ConstU32<32>;
 	type WeightInfo = pallet_staking::weights::SubstrateWeight<Runtime>;
@@ -642,9 +642,19 @@ impl Get<Option<(usize, ExtendedBalance)>> for OffchainRandomBalancing {
 	}
 }
 
-impl onchain::Config for Runtime {
-	type Accuracy = Perbill;
-	type DataProvider = <Self as pallet_election_provider_multi_phase::Config>::DataProvider;
+pub struct OnChainSeqPhragmen;
+impl onchain::ExecutionConfig for OnChainSeqPhragmen {
+	type System = Runtime;
+	type Solver = SequentialPhragmen<
+		AccountId,
+		pallet_election_provider_multi_phase::SolutionAccuracyOf<Runtime>,
+	>;
+	type DataProvider = <Runtime as pallet_election_provider_multi_phase::Config>::DataProvider;
+}
+
+impl onchain::BoundedExecutionConfig for OnChainSeqPhragmen {
+	type VotersBound = ConstU32<20_000>;
+	type TargetsBound = ConstU32<2_000>;
 }
 
 impl pallet_election_provider_multi_phase::Config for Runtime {
@@ -668,13 +678,9 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
 	type RewardHandler = (); // nothing to do upon rewards
 	type DataProvider = Staking;
 	type Solution = NposSolution16;
-	type Fallback = pallet_election_provider_multi_phase::NoFallback<Self>;
-	type GovernanceFallback = onchain::OnChainSequentialPhragmen<Self>;
-	type Solver = frame_election_provider_support::SequentialPhragmen<
-		AccountId,
-		SolutionAccuracyOf<Self>,
-		OffchainRandomBalancing,
-	>;
+	type Fallback = onchain::BoundedExecution<OnChainSeqPhragmen>;
+	type GovernanceFallback = onchain::BoundedExecution<OnChainSeqPhragmen>;
+	type Solver = SequentialPhragmen<AccountId, SolutionAccuracyOf<Self>, OffchainRandomBalancing>;
 	type ForceOrigin = EnsureRootOrHalfCouncil;
 	type MaxElectableTargets = ConstU16<{ u16::MAX }>;
 	type MaxElectingVoters = MaxElectingVoters;
@@ -1899,6 +1905,7 @@ impl_runtime_apis! {
 #[cfg(test)]
 mod tests {
 	use super::*;
+	use frame_election_provider_support::NposSolution;
 	use frame_system::offchain::CreateSignedTransaction;
 	use sp_runtime::UpperOf;
 
@@ -1915,7 +1922,8 @@ mod tests {
 
 	#[test]
 	fn perbill_as_onchain_accuracy() {
-		type OnChainAccuracy = <Runtime as onchain::Config>::Accuracy;
+		type OnChainAccuracy =
+			<<Runtime as pallet_election_provider_multi_phase::Config>::Solution as NposSolution>::Accuracy;
 		let maximum_chain_accuracy: Vec<UpperOf<OnChainAccuracy>> = (0..MaxNominations::get())
 			.map(|_| <UpperOf<OnChainAccuracy>>::from(OnChainAccuracy::one().deconstruct()))
 			.collect();

diff --git 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;
+use frame_election_provider_support::{onchain, SequentialPhragmen};
 use frame_support::{
 	parameter_types,
 	traits::{ConstU128, ConstU32, ConstU64, GenesisBuild, KeyOwnerProofSystem, OnInitialize},
@@ -172,8 +172,10 @@ parameter_types! {
 	pub const OffendingValidatorsThreshold: Perbill = Perbill::from_percent(16);
 }
 
-impl onchain::Config for Test {
-	type Accuracy = Perbill;
+pub struct OnChainSeqPhragmen;
+impl onchain::ExecutionConfig for OnChainSeqPhragmen {
+	type System = Test;
+	type Solver = SequentialPhragmen<DummyValidatorId, Perbill>;
 	type DataProvider = Staking;
 }
 
@@ -195,7 +197,7 @@ impl pallet_staking::Config for Test {
 	type MaxNominatorRewardedPerValidator = ConstU32<64>;
 	type OffendingValidatorsThreshold = OffendingValidatorsThreshold;
 	type NextNewSession = Session;
-	type ElectionProvider = onchain::OnChainSequentialPhragmen<Self>;
+	type ElectionProvider = onchain::UnboundedExecution<OnChainSeqPhragmen>;
 	type GenesisElectionProvider = Self::ElectionProvider;
 	type VoterList = pallet_staking::UseNominatorsAndValidatorsMap<Self>;
 	type MaxUnlockingChunks = ConstU32<32>;

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
@@ -242,7 +242,7 @@ use frame_support::{
 use frame_system::{ensure_none, offchain::SendTransactionTypes};
 use scale_info::TypeInfo;
 use sp_arithmetic::{
-	traits::{CheckedAdd, Saturating, Zero},
+	traits::{Bounded, CheckedAdd, Saturating, Zero},
 	UpperOf,
 };
 use sp_npos_elections::{
@@ -323,10 +323,7 @@ impl<T: Config> ElectionProvider for NoFallback<T> {
 }
 
 impl<T: Config> InstantElectionProvider for NoFallback<T> {
-	fn instant_elect(
-		_: Option<usize>,
-		_: Option<usize>,
-	) -> Result<Supports<T::AccountId>, Self::Error> {
+	fn elect_with_bounds(_: usize, _: usize) -> Result<Supports<T::AccountId>, Self::Error> {
 		Err("NoFallback.")
 	}
 }
@@ -683,7 +680,7 @@ pub mod pallet {
 			+ TypeInfo;
 
 		/// Configuration for the fallback.
-		type Fallback: ElectionProvider<
+		type Fallback: InstantElectionProvider<
 			AccountId = Self::AccountId,
 			BlockNumber = Self::BlockNumber,
 			DataProvider = Self::DataProvider,
@@ -692,7 +689,7 @@ pub mod pallet {
 		/// Configuration of the governance-only fallback.
 		///
 		/// As a side-note, it is recommend for test-nets to use `type ElectionProvider =
-		/// OnChainSeqPhragmen<_>` if the test-net is not expected to have thousands of nominators.
+		/// BoundedExecution<_>` if the test-net is not expected to have thousands of nominators.
 		type GovernanceFallback: InstantElectionProvider<
 			AccountId = Self::AccountId,
 			BlockNumber = Self::BlockNumber,
@@ -1040,13 +1037,14 @@ pub mod pallet {
 			let maybe_max_voters = maybe_max_voters.map(|x| x as usize);
 			let maybe_max_targets = maybe_max_targets.map(|x| x as usize);
 
-			let supports =
-				T::GovernanceFallback::instant_elect(maybe_max_voters, maybe_max_targets).map_err(
-					|e| {
-						log!(error, "GovernanceFallback failed: {:?}", e);
-						Error::<T>::FallbackFailed
-					},
-				)?;
+			let supports = T::GovernanceFallback::elect_with_bounds(
+				maybe_max_voters.unwrap_or(Bounded::max_value()),
+				maybe_max_targets.unwrap_or(Bounded::max_value()),
+			)
+			.map_err(|e| {
+				log!(error, "GovernanceFallback failed: {:?}", e);
+				Error::<T>::FallbackFailed
+			})?;
 
 			let solution = ReadySolution {
 				supports,

diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs
@@ -272,11 +272,13 @@ parameter_types! {
 	pub static MaxElectableTargets: TargetIndex = TargetIndex::max_value();
 
 	pub static EpochLength: u64 = 30;
-	pub static OnChianFallback: bool = true;
+	pub static OnChainFallback: bool = true;
 }
 
-impl onchain::Config for Runtime {
-	type Accuracy = sp_runtime::Perbill;
+pub struct OnChainSeqPhragmen;
+impl onchain::ExecutionConfig for OnChainSeqPhragmen {
+	type System = Runtime;
+	type Solver = SequentialPhragmen<AccountId, SolutionAccuracyOf<Runtime>, Balancing>;
 	type DataProvider = StakingMock;
 }
 
@@ -288,11 +290,23 @@ impl ElectionProvider for MockFallback {
 	type DataProvider = StakingMock;
 
 	fn elect() -> Result<Supports<AccountId>, Self::Error> {
-		if OnChianFallback::get() {
-			onchain::OnChainSequentialPhragmen::<Runtime>::elect()
-				.map_err(|_| "OnChainSequentialPhragmen failed")
+		Self::elect_with_bounds(Bounded::max_value(), Bounded::max_value())
+	}
+}
+
+impl InstantElectionProvider for MockFallback {
+	fn elect_with_bounds(
+		max_voters: usize,
+		max_targets: usize,
+	) -> Result<Supports<Self::AccountId>, Self::Error> {
+		if OnChainFallback::get() {
+			onchain::UnboundedExecution::<OnChainSeqPhragmen>::elect_with_bounds(
+				max_voters,
+				max_targets,
+			)
+			.map_err(|_| "UnboundedExecution failed")
 		} else {
-			super::NoFallback::<Runtime>::elect()
+			super::NoFallback::<Runtime>::elect_with_bounds(max_voters, max_targets)
 		}
 	}
 }
@@ -532,7 +546,7 @@ impl ExtBuilder {
 		self
 	}
 	pub fn onchain_fallback(self, onchain: bool) -> Self {
-		<OnChianFallback>::set(onchain);
+		<OnChainFallback>::set(onchain);
 		self
 	}
 	pub fn miner_weight(self, weight: Weight) -> Self {

diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs
@@ -168,6 +168,7 @@
 
 pub mod onchain;
 pub mod traits;
+#[cfg(feature = "std")]
 use codec::{Decode, Encode};
 use frame_support::{traits::Get, BoundedVec, RuntimeDebug};
 use sp_runtime::traits::Bounded;
@@ -368,9 +369,10 @@ pub trait ElectionProvider {
 		BlockNumber = Self::BlockNumber,
 	>;
 
-	/// Elect a new set of winners.
+	/// 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.
 	///
-	/// The result is returned in a target major format, namely as vector of supports.
+	/// The result is returned in a target major format, namely as *vector of supports*.
 	///
 	/// This should be implemented as a self-weighing function. The implementor should register its
 	/// appropriate weight at the end of execution with the system pallet directly.
@@ -385,11 +387,17 @@ pub trait ElectionProvider {
 /// Consequently, allows for control over the amount of data that is being fetched from the
 /// [`ElectionProvider::DataProvider`].
 pub trait InstantElectionProvider: ElectionProvider {
-	/// Elect a new set of winners, instantly, with the given given limits set on the
+	/// Elect a new set of winners, but unlike [`ElectionProvider::elect`] which cannot enforce
+	/// bounds, this trait method can enforce bounds on the amount of data provided by the
 	/// `DataProvider`.
-	fn instant_elect(
-		maybe_max_voters: Option<usize>,
-		maybe_max_targets: Option<usize>,
+	///
+	/// An implementing type, if itself bounded, should choose the minimum of the two bounds to
+	/// choose the final value of `max_voters` and `max_targets`. In other words, an implementation
+	/// should guarantee that `max_voter` and `max_targets` provided to this method are absolutely
+	/// respected.
+	fn elect_with_bounds(
+		max_voters: usize,
+		max_targets: usize,
 	) -> Result<Supports<Self::AccountId>, Self::Error>;
 }