Abstracts elections-phragmen pallet to use NposSolver (paritytech#12588)

* Abstracts elections-phragmen pallet to use NposSolver * Update frame/elections-phragmen/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/elections-phragmen/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * changes the name of the pallet; adds changelog * update changelog * Adds weight testing * Adds log macro_rules * renames elections-phragment dir to elections * weights rename * fixes typo in cargo toml * pre/post solve weight scafolding * refactor do_post_election * refactors into pre and post election solve for independent benchmarking * deconstructs PreElectionResults struct * updates benchmarking pre and post election solve; mock weights * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * addresses PR comments * adds pre_solve and post_sove weights * Adds comments on election pallet id param name change * ".git/.scripts/bench-bot.sh" pallet dev pallet_elections * Finishes pre-post solve weights * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Update frame/elections/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Addresses PR comments: no panic in on_init path; nits * Fixes node build * Implements approval voting to use as a `NposSolver` (paritytech#13367) * Implements the approval voting methods in sp_npos_elections * fmt * remove unecessary file * comment clarification * re-run weights * fix typo * updates MaxVoters in tests for integrity_tests to pass * Refactors election provider support benchmarks outside its own crate (paritytech#13431) * Refactors election provider support benchmarks outside its own crate --------- Co-authored-by: command-bot <> --------- Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: parity-processbot <> Co-authored-by: Ross Bulat <ross@parity.io>
gear-tech · Apr 22, 2023 · 779ed89 · 779ed89
1 parent 1115af3
commit 779ed89
Show file tree

Hide file tree

Showing 25 changed files with 978 additions and 284 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -98,10 +98,9 @@ members = [
 	"frame/democracy",
 	"frame/fast-unstake",
 	"frame/try-runtime",
-	"frame/elections-phragmen",
+	"frame/elections",
 	"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",

diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml
@@ -69,8 +69,7 @@ pallet-conviction-voting = { version = "4.0.0-dev", default-features = false, pa
 pallet-core-fellowship = { version = "4.0.0-dev", default-features = false, path = "../../../frame/core-fellowship" }
 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-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-elections = { version = "5.0.0-dev", default-features = false, path = "../../../frame/elections" }
 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" }
@@ -130,7 +129,6 @@ std = [
 	"sp-sandbox/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",
@@ -153,7 +151,7 @@ std = [
 	"pallet-conviction-voting/std",
 	"pallet-core-fellowship/std",
 	"pallet-democracy/std",
-	"pallet-elections-phragmen/std",
+	"pallet-elections/std",
 	"pallet-fast-unstake/std",
 	"frame-executive/std",
 	"pallet-nis/std",
@@ -228,6 +226,7 @@ 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",
@@ -242,8 +241,7 @@ runtime-benchmarks = [
 	"pallet-core-fellowship/runtime-benchmarks",
 	"pallet-democracy/runtime-benchmarks",
 	"pallet-election-provider-multi-phase/runtime-benchmarks",
-	"pallet-election-provider-support-benchmarking/runtime-benchmarks",
-	"pallet-elections-phragmen/runtime-benchmarks",
+	"pallet-elections/runtime-benchmarks",
 	"pallet-fast-unstake/runtime-benchmarks",
 	"pallet-nis/runtime-benchmarks",
 	"pallet-grandpa/runtime-benchmarks",
@@ -302,7 +300,7 @@ try-runtime = [
 	"pallet-core-fellowship/try-runtime",
 	"pallet-democracy/try-runtime",
 	"pallet-election-provider-multi-phase/try-runtime",
-	"pallet-elections-phragmen/try-runtime",
+	"pallet-elections/try-runtime",
 	"pallet-fast-unstake/try-runtime",
 	"pallet-nis/try-runtime",
 	"pallet-grandpa/try-runtime",

diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
@@ -24,7 +24,8 @@
 
 use codec::{Decode, Encode, MaxEncodedLen};
 use frame_election_provider_support::{
-	onchain, BalancingConfig, ElectionDataProvider, SequentialPhragmen, VoteWeight,
+	onchain, weights::SubstrateWeight, ApprovalVoting, BalancingConfig, ElectionDataProvider,
+	SequentialPhragmen, VoteWeight,
 };
 use frame_support::{
 	construct_runtime,
@@ -1023,18 +1024,21 @@ 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 ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect";
+	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";
 }
 
 // Make sure that there are no more than `MaxMembers` members elected via elections-phragmen.
 const_assert!(DesiredMembers::get() <= CouncilMaxMembers::get());
 
-impl pallet_elections_phragmen::Config for Runtime {
+impl pallet_elections::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
-	type PalletId = ElectionsPhragmenPalletId;
+	type PalletId = ElectionsPalletId;
 	type Currency = Balances;
 	type ChangeMembers = Council;
 	// NOTE: this implies that council's genesis members cannot be set directly and must come from
@@ -1052,7 +1056,9 @@ impl pallet_elections_phragmen::Config for Runtime {
 	type MaxVoters = MaxVoters;
 	type MaxVotesPerVoter = MaxVotesPerVoter;
 	type MaxCandidates = MaxCandidates;
-	type WeightInfo = pallet_elections_phragmen::weights::SubstrateWeight<Runtime>;
+	type ElectionSolver = ApprovalVoting<Self::AccountId, Perbill>;
+	type SolverWeightInfo = SubstrateWeight<Runtime>;
+	type WeightInfo = pallet_elections::weights::SubstrateWeight<Runtime>;
 }
 
 parameter_types! {
@@ -1761,7 +1767,7 @@ construct_runtime!(
 		Democracy: pallet_democracy,
 		Council: pallet_collective::<Instance1>,
 		TechnicalCommittee: pallet_collective::<Instance2>,
-		Elections: pallet_elections_phragmen,
+		Elections: pallet_elections,
 		TechnicalMembership: pallet_membership::<Instance1>,
 		Grandpa: pallet_grandpa,
 		Treasury: pallet_treasury,
@@ -1877,6 +1883,7 @@ 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]
@@ -1890,8 +1897,7 @@ mod benches {
 		[pallet_core_fellowship, CoreFellowship]
 		[pallet_democracy, Democracy]
 		[pallet_election_provider_multi_phase, ElectionProviderMultiPhase]
-		[pallet_election_provider_support_benchmarking, EPSBench::<Runtime>]
-		[pallet_elections_phragmen, Elections]
+		[pallet_elections, Elections]
 		[pallet_fast_unstake, FastUnstake]
 		[pallet_nis, Nis]
 		[pallet_grandpa, Grandpa]
@@ -2371,7 +2377,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 pallet_election_provider_support_benchmarking::Pallet as EPSBench;
+			use frame_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;
@@ -2394,14 +2400,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 pallet_election_provider_support_benchmarking::Pallet as EPSBench;
+			use frame_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 pallet_election_provider_support_benchmarking::Config for Runtime {}
+			impl frame_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 {}

diff --git a/frame/election-provider-multi-phase/Cargo.toml b/frame/election-provider-multi-phase/Cargo.toml
@@ -33,7 +33,6 @@ 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 }
 
@@ -50,7 +49,6 @@ 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",

diff --git a/frame/election-provider-support/Cargo.toml b/frame/election-provider-support/Cargo.toml
@@ -23,6 +23,8 @@ 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" }
@@ -41,6 +43,10 @@ std = [
 	"sp-core/std",
 	"sp-runtime/std",
 	"sp-std/std",
+
+	"frame-benchmarking?/std",
+]
+runtime-benchmarks = [
+	"frame-benchmarking/runtime-benchmarks",
 ]
-runtime-benchmarks = []
 try-runtime = []
diff --git a/frame/election-provider-support/benchmarking/Cargo.toml b/frame/election-provider-support/benchmarking/Cargo.toml
diff --git a/...-provider-support/benchmarking/src/lib.rs → ...tion-provider-support/src/benchmarking.rs b/...-provider-support/benchmarking/src/lib.rs → ...tion-provider-support/src/benchmarking.rs
@@ -18,12 +18,9 @@
 //! Election provider support pallet benchmarking.
 //! This is separated into its own crate to avoid bloating the size of the runtime.
 
-#![cfg(feature = "runtime-benchmarks")]
-#![cfg_attr(not(feature = "std"), no_std)]
-
+use crate::{ApprovalVoting, NposSolver, PhragMMS, SequentialPhragmen};
 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 {}
@@ -88,4 +85,17 @@ 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()
+		);
+	}
 }
diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs
@@ -199,6 +199,9 @@ pub use sp_arithmetic;
 #[doc(hidden)]
 pub use sp_std;
 
+#[cfg(feature = "runtime-benchmarks")]
+pub mod benchmarking;
+
 pub mod weights;
 pub use weights::WeightInfo;
 
@@ -660,6 +663,29 @@ 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>);
 

diff --git a/frame/election-provider-support/src/onchain.rs b/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::{ElectionProvider, PhragMMS, SequentialPhragmen};
+	use crate::{ApprovalVoting, ElectionProvider, PhragMMS, SequentialPhragmen};
 	use frame_support::{assert_noop, parameter_types, traits::ConstU32};
 	use sp_npos_elections::Support;
 	use sp_runtime::Perbill;
@@ -235,6 +235,7 @@ mod tests {
 
 	struct PhragmenParams;
 	struct PhragMMSParams;
+	struct ApprovalVotingParams;
 
 	parameter_types! {
 		pub static MaxWinners: u32 = 10;
@@ -261,6 +262,16 @@ 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};
 
@@ -333,4 +344,21 @@ 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)] })
+				]
+			)
+		})
+	}
 }
diff --git a/frame/elections-phragmen/CHANGELOG.md → frame/elections/CHANGELOG.md b/frame/elections-phragmen/CHANGELOG.md → frame/elections/CHANGELOG.md
@@ -4,6 +4,17 @@ All notable changes to this crate will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this crate adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [5.0.0] - UNRELEASED
+
+### Added
+
+### Changed
+Generalized the pallet to use `NposSolver` instead of hard coding the phragmen algorithm; Changed the name of the pallet from `pallet-elections-phragmen` to `pallet-elections`
+
+### Fixed
+
+### Security
+
 ## [4.0.0] - UNRELEASED
 
 ### Added