Revert "Abstracts elections-phragmen pallet to use NposSolver (#12588)…

…" (#13451)

This reverts commit 4be5dd2.

Cargo.toml

@@ -97,9 +97,10 @@ members = [
 	"frame/democracy",
 	"frame/fast-unstake",
 	"frame/try-runtime",
-	"frame/elections",
+	"frame/elections-phragmen",
 	"frame/election-provider-multi-phase",
 	"frame/election-provider-support",
+	"frame/election-provider-support/benchmarking",
 	"frame/election-provider-support/solution-type",
 	"frame/election-provider-support/solution-type/fuzzer",
 	"frame/examples/basic",

bin/node/runtime/Cargo.toml

@@ -66,7 +66,8 @@ pallet-contracts-primitives = { version = "7.0.0", default-features = false, pat
 pallet-conviction-voting = { version = "4.0.0-dev", default-features = false, path = "../../../frame/conviction-voting" }
 pallet-democracy = { version = "4.0.0-dev", default-features = false, path = "../../../frame/democracy" }
 pallet-election-provider-multi-phase = { version = "4.0.0-dev", default-features = false, path = "../../../frame/election-provider-multi-phase" }
-pallet-elections = { version = "5.0.0-dev", default-features = false, path = "../../../frame/elections" }
+pallet-election-provider-support-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../../../frame/election-provider-support/benchmarking", optional = true }
+pallet-elections-phragmen = { version = "5.0.0-dev", default-features = false, path = "../../../frame/elections-phragmen" }
 pallet-fast-unstake = { version = "4.0.0-dev", default-features = false, path = "../../../frame/fast-unstake" }
 pallet-nis = { version = "4.0.0-dev", default-features = false, path = "../../../frame/nis" }
 pallet-grandpa = { version = "4.0.0-dev", default-features = false, path = "../../../frame/grandpa" }
@@ -124,6 +125,7 @@ with-tracing = ["frame-executive/with-tracing"]
 std = [
 	"pallet-whitelist/std",
 	"pallet-offences-benchmarking?/std",
+	"pallet-election-provider-support-benchmarking?/std",
 	"pallet-asset-tx-payment/std",
 	"frame-system-benchmarking?/std",
 	"frame-election-provider-support/std",
@@ -144,7 +146,7 @@ std = [
 	"pallet-contracts-primitives/std",
 	"pallet-conviction-voting/std",
 	"pallet-democracy/std",
-	"pallet-elections/std",
+	"pallet-elections-phragmen/std",
 	"pallet-fast-unstake/std",
 	"frame-executive/std",
 	"pallet-nis/std",
@@ -218,7 +220,6 @@ runtime-benchmarks = [
 	"frame-benchmarking-pallet-pov/runtime-benchmarks",
 	"frame-support/runtime-benchmarks",
 	"frame-system/runtime-benchmarks",
-	"frame-election-provider-support/runtime-benchmarks",
 	"sp-runtime/runtime-benchmarks",
 	"pallet-alliance/runtime-benchmarks",
 	"pallet-assets/runtime-benchmarks",
@@ -232,7 +233,8 @@ runtime-benchmarks = [
 	"pallet-conviction-voting/runtime-benchmarks",
 	"pallet-democracy/runtime-benchmarks",
 	"pallet-election-provider-multi-phase/runtime-benchmarks",
-	"pallet-elections/runtime-benchmarks",
+	"pallet-election-provider-support-benchmarking/runtime-benchmarks",
+	"pallet-elections-phragmen/runtime-benchmarks",
 	"pallet-fast-unstake/runtime-benchmarks",
 	"pallet-nis/runtime-benchmarks",
 	"pallet-grandpa/runtime-benchmarks",
@@ -289,7 +291,7 @@ try-runtime = [
 	"pallet-conviction-voting/try-runtime",
 	"pallet-democracy/try-runtime",
 	"pallet-election-provider-multi-phase/try-runtime",
-	"pallet-elections/try-runtime",
+	"pallet-elections-phragmen/try-runtime",
 	"pallet-fast-unstake/try-runtime",
 	"pallet-nis/try-runtime",
 	"pallet-grandpa/try-runtime",

bin/node/runtime/src/lib.rs

@@ -24,8 +24,7 @@
 
 use codec::{Decode, Encode, MaxEncodedLen};
 use frame_election_provider_support::{
-	onchain, weights::SubstrateWeight, ApprovalVoting, BalancingConfig, ElectionDataProvider,
-	SequentialPhragmen, VoteWeight,
+	onchain, BalancingConfig, ElectionDataProvider, SequentialPhragmen, VoteWeight,
 };
 use frame_support::{
 	construct_runtime,
@@ -1011,21 +1010,18 @@ parameter_types! {
 	pub const TermDuration: BlockNumber = 7 * DAYS;
 	pub const DesiredMembers: u32 = 13;
 	pub const DesiredRunnersUp: u32 = 7;
+	pub const MaxVotesPerVoter: u32 = 16;
 	pub const MaxVoters: u32 = 512;
 	pub const MaxCandidates: u32 = 64;
-	pub const MaxVotesPerVoter: u32 = 16;
-	// The ElectionsPalletId parameter name was changed along with the renaming of the elections
-	// pallet, but we keep the same lock ID to prevent a migration from current runtimes.
-	// Related to https://github.com/paritytech/substrate/issues/8250
-	pub const ElectionsPalletId: LockIdentifier = *b"phrelect";
+	pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect";
 }
 
 // Make sure that there are no more than `MaxMembers` members elected via elections-phragmen.
 const_assert!(DesiredMembers::get() <= CouncilMaxMembers::get());
 
-impl pallet_elections::Config for Runtime {
+impl pallet_elections_phragmen::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
-	type PalletId = ElectionsPalletId;
+	type PalletId = ElectionsPhragmenPalletId;
 	type Currency = Balances;
 	type ChangeMembers = Council;
 	// NOTE: this implies that council's genesis members cannot be set directly and must come from
@@ -1043,9 +1039,7 @@ impl pallet_elections::Config for Runtime {
 	type MaxVoters = MaxVoters;
 	type MaxVotesPerVoter = MaxVotesPerVoter;
 	type MaxCandidates = MaxCandidates;
-	type ElectionSolver = ApprovalVoting<Self::AccountId, Perbill>;
-	type SolverWeightInfo = SubstrateWeight<Runtime>;
-	type WeightInfo = pallet_elections::weights::SubstrateWeight<Runtime>;
+	type WeightInfo = pallet_elections_phragmen::weights::SubstrateWeight<Runtime>;
 }
 
 parameter_types! {
@@ -1743,7 +1737,7 @@ construct_runtime!(
 		Democracy: pallet_democracy,
 		Council: pallet_collective::<Instance1>,
 		TechnicalCommittee: pallet_collective::<Instance2>,
-		Elections: pallet_elections,
+		Elections: pallet_elections_phragmen,
 		TechnicalMembership: pallet_membership::<Instance1>,
 		Grandpa: pallet_grandpa,
 		Treasury: pallet_treasury,
@@ -1857,7 +1851,6 @@ mod benches {
 	frame_benchmarking::define_benchmarks!(
 		[frame_benchmarking, BaselineBench::<Runtime>]
 		[frame_benchmarking_pallet_pov, Pov]
-		[frame_election_provider_support, EPSBench::<Runtime>]
 		[pallet_alliance, Alliance]
 		[pallet_assets, Assets]
 		[pallet_babe, Babe]
@@ -1870,7 +1863,8 @@ mod benches {
 		[pallet_contracts, Contracts]
 		[pallet_democracy, Democracy]
 		[pallet_election_provider_multi_phase, ElectionProviderMultiPhase]
-		[pallet_elections, Elections]
+		[pallet_election_provider_support_benchmarking, EPSBench::<Runtime>]
+		[pallet_elections_phragmen, Elections]
 		[pallet_fast_unstake, FastUnstake]
 		[pallet_nis, Nis]
 		[pallet_grandpa, Grandpa]
@@ -2329,7 +2323,7 @@ impl_runtime_apis! {
 			// which is why we need these two lines below.
 			use pallet_session_benchmarking::Pallet as SessionBench;
 			use pallet_offences_benchmarking::Pallet as OffencesBench;
-			use frame_election_provider_support::benchmarking::Pallet as EPSBench;
+			use pallet_election_provider_support_benchmarking::Pallet as EPSBench;
 			use frame_system_benchmarking::Pallet as SystemBench;
 			use baseline::Pallet as BaselineBench;
 			use pallet_nomination_pools_benchmarking::Pallet as NominationPoolsBench;
@@ -2352,14 +2346,14 @@ impl_runtime_apis! {
 			// which is why we need these two lines below.
 			use pallet_session_benchmarking::Pallet as SessionBench;
 			use pallet_offences_benchmarking::Pallet as OffencesBench;
-			use frame_election_provider_support::benchmarking::Pallet as EPSBench;
+			use pallet_election_provider_support_benchmarking::Pallet as EPSBench;
 			use frame_system_benchmarking::Pallet as SystemBench;
 			use baseline::Pallet as BaselineBench;
 			use pallet_nomination_pools_benchmarking::Pallet as NominationPoolsBench;
 
 			impl pallet_session_benchmarking::Config for Runtime {}
 			impl pallet_offences_benchmarking::Config for Runtime {}
-			impl frame_election_provider_support::benchmarking::Config for Runtime {}
+			impl pallet_election_provider_support_benchmarking::Config for Runtime {}
 			impl frame_system_benchmarking::Config for Runtime {}
 			impl baseline::Config for Runtime {}
 			impl pallet_nomination_pools_benchmarking::Config for Runtime {}

frame/election-provider-multi-phase/Cargo.toml

@@ -33,6 +33,7 @@ frame-election-provider-support = { version = "4.0.0-dev", default-features = fa
 
 # Optional imports for benchmarking
 frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../benchmarking", optional = true }
+pallet-election-provider-support-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../election-provider-support/benchmarking", optional = true }
 rand = { version = "0.8.5", default-features = false, features = ["alloc", "small_rng"], optional = true }
 strum = { version = "0.24.1",  default-features = false, features = ["derive"], optional = true }
 
@@ -49,6 +50,7 @@ frame-benchmarking = { version = "4.0.0-dev", path = "../benchmarking" }
 [features]
 default = ["std"]
 std = [
+	"pallet-election-provider-support-benchmarking?/std",
 	"codec/std",
 	"scale-info/std",
 	"log/std",

frame/election-provider-support/Cargo.toml

@@ -23,8 +23,6 @@ sp-runtime = { version = "7.0.0", default-features = false, path = "../../primit
 sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" }
 sp-core = { version = "7.0.0", default-features = false, path = "../../primitives/core" }
 
-frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../benchmarking", optional = true }
-
 [dev-dependencies]
 rand = { version = "0.8.5", features = ["small_rng"] }
 sp-io = { version = "7.0.0", path = "../../primitives/io" }
@@ -43,10 +41,6 @@ std = [
 	"sp-core/std",
 	"sp-runtime/std",
 	"sp-std/std",
-
-	"frame-benchmarking?/std",
-]
-runtime-benchmarks = [
-	"frame-benchmarking/runtime-benchmarks",
 ]
+runtime-benchmarks = []
 try-runtime = []

frame/election-provider-support/benchmarking/Cargo.toml

@@ -0,0 +1,37 @@
+[package]
+name = "pallet-election-provider-support-benchmarking"
+version = "4.0.0-dev"
+authors = ["Parity Technologies <admin@parity.io>"]
+edition = "2021"
+license = "Apache-2.0"
+homepage = "https://substrate.io"
+repository = "https://github.com/paritytech/substrate/"
+description = "Benchmarking for election provider support onchain config trait"
+
+[package.metadata.docs.rs]
+targets = ["x86_64-unknown-linux-gnu"]
+
+[dependencies]
+codec = { package = "parity-scale-codec", version = "3.2.2", default-features = false, features = [
+	"derive",
+] }
+frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../../benchmarking" }
+frame-election-provider-support = { version = "4.0.0-dev", default-features = false, path = ".." }
+frame-system = { version = "4.0.0-dev", default-features = false, path = "../../system" }
+sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/npos-elections" }
+sp-runtime = { version = "7.0.0", default-features = false, path = "../../../primitives/runtime" }
+
+[features]
+default = ["std"]
+std = [
+	"codec/std",
+	"frame-benchmarking?/std",
+	"frame-election-provider-support/std",
+	"frame-system/std",
+	"sp-npos-elections/std",
+	"sp-runtime/std",
+]
+runtime-benchmarks = [
+	"frame-benchmarking/runtime-benchmarks",
+	"frame-election-provider-support/runtime-benchmarks",
+]

frame/election-provider-support/src/benchmarking.rs → frame/election-provider-support/benchmarking/src/lib.rs

@@ -18,9 +18,12 @@
 //! Election provider support pallet benchmarking.
 //! This is separated into its own crate to avoid bloating the size of the runtime.
 
-use crate::{ApprovalVoting, NposSolver, PhragMMS, SequentialPhragmen};
+#![cfg(feature = "runtime-benchmarks")]
+#![cfg_attr(not(feature = "std"), no_std)]
+
 use codec::Decode;
 use frame_benchmarking::v1::{benchmarks, Vec};
+use frame_election_provider_support::{NposSolver, PhragMMS, SequentialPhragmen};
 
 pub struct Pallet<T: Config>(frame_system::Pallet<T>);
 pub trait Config: frame_system::Config {}
@@ -85,17 +88,4 @@ benchmarks! {
 				::solve(d as usize, targets, voters).is_ok()
 		);
 	}
-
-	approval_voting {
-		let v in (VOTERS[0]) .. VOTERS[1];
-		let t in (TARGETS[0]) .. TARGETS[1];
-		let d in (VOTES_PER_VOTER[0]) .. VOTES_PER_VOTER[1];
-
-		let (voters, targets) = set_up_voters_targets::<T::AccountId>(v, t, d as usize);
-	}: {
-		assert!(
-			ApprovalVoting::<T::AccountId, sp_runtime::Perbill>
-				::solve(d as usize, targets, voters).is_ok()
-		);
-	}
 }

frame/election-provider-support/src/lib.rs

@@ -199,9 +199,6 @@ pub use sp_arithmetic;
 #[doc(hidden)]
 pub use sp_std;
 
-#[cfg(feature = "runtime-benchmarks")]
-pub mod benchmarking;
-
 pub mod weights;
 pub use weights::WeightInfo;
 
@@ -663,29 +660,6 @@ impl<AccountId: IdentifierT, Accuracy: PerThing128, Balancing: Get<Option<Balanc
 	}
 }
 
-/// A wrapper for [`sp_npos_elections::approval_voting()`] that implements [`NposSolver`]. See the
-/// documentation of [`sp_npos_elections::approval_voting()`] for more info.
-pub struct ApprovalVoting<AccountId, Accuracy>(sp_std::marker::PhantomData<(AccountId, Accuracy)>);
-
-impl<AccountId: IdentifierT, Accuracy: PerThing128> NposSolver
-	for ApprovalVoting<AccountId, Accuracy>
-{
-	type AccountId = AccountId;
-	type Accuracy = Accuracy;
-	type Error = sp_npos_elections::Error;
-	fn solve(
-		winners: usize,
-		targets: Vec<Self::AccountId>,
-		voters: Vec<(Self::AccountId, VoteWeight, impl IntoIterator<Item = Self::AccountId>)>,
-	) -> Result<ElectionResult<Self::AccountId, Self::Accuracy>, Self::Error> {
-		sp_npos_elections::approval_voting(winners, targets, voters)
-	}
-
-	fn weight<T: WeightInfo>(voters: u32, targets: u32, vote_degree: u32) -> Weight {
-		T::approval_voting(voters, targets, vote_degree)
-	}
-}
-
 /// A voter, at the level of abstraction of this crate.
 pub type Voter<AccountId, Bound> = (AccountId, VoteWeight, BoundedVec<AccountId, Bound>);
 

frame/election-provider-support/src/onchain.rs

@@ -185,7 +185,7 @@ impl<T: Config> ElectionProvider for OnChainExecution<T> {
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use crate::{ApprovalVoting, ElectionProvider, PhragMMS, SequentialPhragmen};
+	use crate::{ElectionProvider, PhragMMS, SequentialPhragmen};
 	use frame_support::{assert_noop, parameter_types, traits::ConstU32};
 	use sp_npos_elections::Support;
 	use sp_runtime::Perbill;
@@ -235,7 +235,6 @@ mod tests {
 
 	struct PhragmenParams;
 	struct PhragMMSParams;
-	struct ApprovalVotingParams;
 
 	parameter_types! {
 		pub static MaxWinners: u32 = 10;
@@ -262,16 +261,6 @@ mod tests {
 		type TargetsBound = ConstU32<400>;
 	}
 
-	impl Config for ApprovalVotingParams {
-		type System = Runtime;
-		type Solver = ApprovalVoting<AccountId, Perbill>;
-		type DataProvider = mock_data_provider::DataProvider;
-		type WeightInfo = ();
-		type MaxWinners = MaxWinners;
-		type VotersBound = ConstU32<600>;
-		type TargetsBound = ConstU32<400>;
-	}
-
 	mod mock_data_provider {
 		use frame_support::{bounded_vec, traits::ConstU32};
 
@@ -344,21 +333,4 @@ mod tests {
 			);
 		})
 	}
-
-	#[test]
-	fn onchain_approval_voting_works() {
-		sp_io::TestExternalities::new_empty().execute_with(|| {
-			DesiredTargets::set(3);
-
-			// note that the `OnChainExecution::elect` implementation normalizes the vote weights.
-			assert_eq!(
-				<OnChainExecution::<ApprovalVotingParams> as ElectionProvider>::elect().unwrap(),
-				vec![
-					(10, Support { total: 20, voters: vec![(1, 5), (3, 15)] }),
-					(20, Support { total: 15, voters: vec![(1, 5), (2, 10)] }),
-					(30, Support { total: 25, voters: vec![(2, 10), (3, 15)] })
-				]
-			)
-		})
-	}
 }