Make the number of nominations configurable (paritytech#8368)

* Base features and traits. * pallet and unsigned phase * Undo bad formattings. * some formatting cleanup. * Small self-cleanup. * Make it all build * self-review * Some doc tests. * Some changes from other PR * Fix session test * Update Cargo.lock * Update frame/election-provider-multi-phase/src/lib.rs Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> * Some review comments * Rename + make encode/decode * Do an assert as well, just in case. * Fix build * Update frame/election-provider-multi-phase/src/unsigned.rs Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> * Las comment * fix staking fuzzer. * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs * Add one last layer of feasibility check as well. * Last fixes to benchmarks * Some more docs. * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_election_provider_multi_phase --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/election-provider-multi-phase/src/weights.rs --template=./.maintain/frame-weight-template.hbs * Some nits * It all works * Some self cleanup * Update frame/staking/src/lib.rs Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com> * remove most todos. * Round of self-review. * Fix migration * clean macro * Revert wrong merge * Make the number of nominations configurable * Self reivew * renmae. Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> Co-authored-by: Parity Benchmarking Bot <admin@parity.io> Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>
Cerebellum-Network · AndreiNavoichyk · Nov 30, 2022 · Feb 24, 2021 · Feb 24, 2021 · Feb 24, 2021
commit 2f69b2d417cbee2c343e0bb6af20fc89e4dea213
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml
@@ -36,6 +36,7 @@ sp-keyring = { version = "3.0.0", optional = true, path = "../../../primitives/k
 sp-session = { version = "3.0.0", default-features = false, path = "../../../primitives/session" }
 sp-transaction-pool = { version = "3.0.0", default-features = false, path = "../../../primitives/transaction-pool" }
 sp-version = { version = "3.0.0", default-features = false, path = "../../../primitives/version" }
+sp-npos-elections = { version = "3.0.0", default-features = false, path = "../../../primitives/npos-elections" }
 
 # frame dependencies
 frame-executive = { version = "3.0.0", default-features = false, path = "../../../frame/executive" }
@@ -159,6 +160,7 @@ std = [
 	"pallet-vesting/std",
 	"log/std",
 	"frame-try-runtime/std",
+	"sp-npos-elections/std",
 ]
 runtime-benchmarks = [
 	"frame-benchmarking",

diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
@@ -471,6 +471,8 @@ parameter_types! {
 }
 
 impl pallet_staking::Config for Runtime {
+	const MAX_NOMINATIONS: u32 =
+		<NposCompactSolution16 as sp_npos_elections::CompactSolution>::LIMIT as u32;
 	type Currency = Balances;
 	type UnixTime = Timestamp;
 	type CurrencyToVote = U128CurrencyToVote;
@@ -515,6 +517,12 @@ parameter_types! {
 		.saturating_sub(BlockExecutionWeight::get());
 }
 
+sp_npos_elections::generate_solution_type!(
+	#[compact]
+	pub struct NposCompactSolution16::<u32, u16, sp_runtime::PerU16>(16)
+	// -------------------- ^^ <NominatorIndex, ValidatorIndex, Accuracy>
+);
+
 impl pallet_election_provider_multi_phase::Config for Runtime {
 	type Event = Event;
 	type Currency = Balances;
@@ -526,7 +534,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
 	type MinerTxPriority = MultiPhaseUnsignedPriority;
 	type DataProvider = Staking;
 	type OnChainAccuracy = Perbill;
-	type CompactSolution = pallet_staking::CompactAssignments;
+	type CompactSolution = NposCompactSolution16;
 	type Fallback = Fallback;
 	type WeightInfo = pallet_election_provider_multi_phase::weights::SubstrateWeight<Runtime>;
 	type BenchmarkingConfig = ();

diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs
@@ -193,6 +193,7 @@ impl onchain::Config for Test {
 }
 
 impl pallet_staking::Config for Test {
+	const MAX_NOMINATIONS: u32 = 16;
 	type RewardRemainder = ();
 	type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote;
 	type Event = Event;

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
@@ -678,6 +678,16 @@ pub mod pallet {
 			let _: UpperOf<CompactAccuracyOf<T>> = maximum_chain_accuracy
 				.iter()
 				.fold(Zero::zero(), |acc, x| acc.checked_add(x).unwrap());
+
+			// We only accept data provider who's maximum votes per voter matches our
+			// `T::CompactSolution`'s `LIMIT`.
+			//
+			// NOTE that this pallet does not really need to enforce this in runtime. The compact
+			// solution cannot represent any voters more than `LIMIT` anyhow.
+			assert_eq!(
+				<T::DataProvider as ElectionDataProvider<T::AccountId, T::BlockNumber>>::MAXIMUM_VOTES_PER_VOTER,
+				<CompactOf<T> as CompactSolution>::LIMIT as u32,
+			);
 		}
 	}
 

diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs
@@ -300,6 +300,7 @@ pub struct ExtBuilder {}
 
 pub struct StakingMock;
 impl ElectionDataProvider<AccountId, u64> for StakingMock {
+	const MAXIMUM_VOTES_PER_VOTER: u32 = <TestCompact as CompactSolution>::LIMIT as u32;
 	fn targets(maybe_max_len: Option<usize>) -> data_provider::Result<(Vec<AccountId>, Weight)> {
 		let targets = Targets::get();
 

diff --git a/frame/election-provider-support/src/lib.rs b/frame/election-provider-support/src/lib.rs
@@ -100,6 +100,7 @@
 //!     pub struct Module<T: Config>(std::marker::PhantomData<T>);
 //!
 //!     impl<T: Config> ElectionDataProvider<AccountId, BlockNumber> for Module<T> {
+//!         const MAXIMUM_VOTES_PER_VOTER: u32 = 1;
 //!         fn desired_targets() -> data_provider::Result<(u32, Weight)> {
 //!             Ok((1, 0))
 //!         }
@@ -180,6 +181,9 @@ pub mod data_provider {
 
 /// Something that can provide the data to an [`ElectionProvider`].
 pub trait ElectionDataProvider<AccountId, BlockNumber> {
+	/// Maximum number of votes per voter that this data provider is providing.
+	const MAXIMUM_VOTES_PER_VOTER: u32;
+
 	/// All possible targets for the election, i.e. the candidates.
 	///
 	/// If `maybe_max_len` is `Some(v)` then the resulting vector MUST NOT be longer than `v` items
@@ -226,6 +230,7 @@ pub trait ElectionDataProvider<AccountId, BlockNumber> {
 
 #[cfg(feature = "std")]
 impl<AccountId, BlockNumber> ElectionDataProvider<AccountId, BlockNumber> for () {
+	const MAXIMUM_VOTES_PER_VOTER: u32 = 0;
 	fn targets(_maybe_max_len: Option<usize>) -> data_provider::Result<(Vec<AccountId>, Weight)> {
 		Ok(Default::default())
 	}

diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs
@@ -125,6 +125,7 @@ mod tests {
 		pub struct DataProvider;
 
 		impl ElectionDataProvider<AccountId, BlockNumber> for DataProvider {
+			const MAXIMUM_VOTES_PER_VOTER: u32 = 2;
 			fn voters(
 				_: Option<usize>,
 			) -> data_provider::Result<(Vec<(AccountId, VoteWeight, Vec<AccountId>)>, Weight)> {

diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs
@@ -199,6 +199,7 @@ impl onchain::Config for Test {
 }
 
 impl pallet_staking::Config for Test {
+	const MAX_NOMINATIONS: u32 = 16;
 	type RewardRemainder = ();
 	type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote;
 	type Event = Event;

diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs
@@ -28,7 +28,10 @@ use frame_system::{RawOrigin, Pallet as System, Config as SystemConfig};
 use frame_benchmarking::{benchmarks, account, impl_benchmark_test_suite};
 use frame_support::traits::{Currency, OnInitialize, ValidatorSet, ValidatorSetWithIdentification};
 
-use sp_runtime::{Perbill, traits::{Convert, StaticLookup, Saturating, UniqueSaturatedInto}};
+use sp_runtime::{
+	Perbill,
+	traits::{Convert, StaticLookup, Saturating, UniqueSaturatedInto},
+};
 use sp_staking::offence::{ReportOffence, Offence, OffenceDetails};
 
 use pallet_balances::Config as BalancesConfig;
@@ -39,8 +42,8 @@ use pallet_offences::{Config as OffencesConfig, Module as Offences};
 use pallet_session::historical::{Config as HistoricalConfig, IdentificationTuple};
 use pallet_session::{Config as SessionConfig, SessionManager};
 use pallet_staking::{
-	Module as Staking, Config as StakingConfig, RewardDestination, ValidatorPrefs,
-	Exposure, IndividualExposure, MAX_NOMINATIONS, Event as StakingEvent
+	Module as Staking, Config as StakingConfig, RewardDestination, ValidatorPrefs, Exposure,
+	IndividualExposure, Event as StakingEvent,
 };
 
 const SEED: u32 = 0;
@@ -236,7 +239,7 @@ benchmarks! {
 		let r in 1 .. MAX_REPORTERS;
 		// we skip 1 offender, because in such case there is no slashing
 		let o in 2 .. MAX_OFFENDERS;
-		let n in 0 .. MAX_NOMINATORS.min(MAX_NOMINATIONS as u32);
+		let n in 0 .. MAX_NOMINATORS.min(<T as pallet_staking::Config>::MAX_NOMINATIONS);
 
 		// Make r reporters
 		let mut reporters = vec![];
@@ -310,7 +313,7 @@ benchmarks! {
 	}
 
 	report_offence_grandpa {
-		let n in 0 .. MAX_NOMINATORS.min(MAX_NOMINATIONS as u32);
+		let n in 0 .. MAX_NOMINATORS.min(<T as pallet_staking::Config>::MAX_NOMINATIONS);
 
 		// for grandpa equivocation reports the number of reporters
 		// and offenders is always 1
@@ -346,7 +349,7 @@ benchmarks! {
 	}
 
 	report_offence_babe {
-		let n in 0 .. MAX_NOMINATORS.min(MAX_NOMINATIONS as u32);
+		let n in 0 .. MAX_NOMINATORS.min(<T as pallet_staking::Config>::MAX_NOMINATIONS);
 
 		// for babe equivocation reports the number of reporters
 		// and offenders is always 1

diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs
@@ -158,6 +158,7 @@ impl onchain::Config for Test {
 }
 
 impl pallet_staking::Config for Test {
+	const MAX_NOMINATIONS: u32 = 16;
 	type Currency = Balances;
 	type UnixTime = pallet_timestamp::Pallet<Self>;
 	type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote;

diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs
@@ -35,7 +35,7 @@ use frame_system::RawOrigin;
 use pallet_session::{historical::Module as Historical, Module as Session, *};
 use pallet_staking::{
 	benchmarking::create_validator_with_nominators, testing_utils::create_validators,
-	MAX_NOMINATIONS, RewardDestination,
+	RewardDestination,
 };
 use sp_runtime::traits::{One, StaticLookup};
 
@@ -52,10 +52,10 @@ impl<T: Config> OnInitialize<T::BlockNumber> for Pallet<T> {
 
 benchmarks! {
 	set_keys {
-		let n = MAX_NOMINATIONS as u32;
+		let n = <T as pallet_staking::Config>::MAX_NOMINATIONS;
 		let (v_stash, _) = create_validator_with_nominators::<T>(
 			n,
-			MAX_NOMINATIONS as u32,
+			<T as pallet_staking::Config>::MAX_NOMINATIONS,
 			false,
 			RewardDestination::Staked,
 		)?;
@@ -68,10 +68,10 @@ benchmarks! {
 	}: _(RawOrigin::Signed(v_controller), keys, proof)
 
 	purge_keys {
-		let n = MAX_NOMINATIONS as u32;
+		let n = <T as pallet_staking::Config>::MAX_NOMINATIONS;
 		let (v_stash, _) = create_validator_with_nominators::<T>(
 			n,
-			MAX_NOMINATIONS as u32,
+			<T as pallet_staking::Config>::MAX_NOMINATIONS,
 			false,
 			RewardDestination::Staked
 		)?;

diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs
@@ -163,6 +163,7 @@ impl onchain::Config for Test {
 }
 
 impl pallet_staking::Config for Test {
+	const MAX_NOMINATIONS: u32 = 16;
 	type Currency = Balances;
 	type UnixTime = pallet_timestamp::Pallet<Self>;
 	type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote;

diff --git a/frame/staking/Cargo.toml b/frame/staking/Cargo.toml
@@ -17,7 +17,6 @@ static_assertions = "1.1.0"
 serde = { version = "1.0.101", optional = true }
 codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] }
 sp-std = { version = "3.0.0", default-features = false, path = "../../primitives/std" }
-sp-npos-elections = { version = "3.0.0", default-features = false, path = "../../primitives/npos-elections" }
 sp-io ={ version = "3.0.0", default-features = false, path = "../../primitives/io" }
 sp-runtime = { version = "3.0.0", default-features = false, path = "../../primitives/runtime" }
 sp-staking = { version = "3.0.0", default-features = false, path = "../../primitives/staking" }
@@ -54,7 +53,6 @@ std = [
 	"serde",
 	"codec/std",
 	"sp-std/std",
-	"sp-npos-elections/std",
 	"sp-io/std",
 	"frame-support/std",
 	"sp-runtime/std",

diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs
@@ -204,16 +204,20 @@ benchmarks! {
 	kick {
 		// scenario: we want to kick `k` nominators from nominating us (we are a validator).
 		// we'll assume that `k` is under 128 for the purposes of determining the slope.
-		// each nominator should have `MAX_NOMINATIONS` validators nominated, and our validator
+		// each nominator should have `T::MAX_NOMINATIONS` validators nominated, and our validator
 		// should be somewhere in there.
 		let k in 1 .. 128;
 
-		// these are the other validators; there are `MAX_NOMINATIONS - 1` of them, so there are a
-		// total of `MAX_NOMINATIONS` validators in the system.
-		let rest_of_validators = create_validators::<T>(MAX_NOMINATIONS as u32 - 1, 100)?;
+		// these are the other validators; there are `T::MAX_NOMINATIONS - 1` of them, so
+		// there are a total of `T::MAX_NOMINATIONS` validators in the system.
+		let rest_of_validators = create_validators::<T>(T::MAX_NOMINATIONS - 1, 100)?;
 
 		// this is the validator that will be kicking.
-		let (stash, controller) = create_stash_controller::<T>(MAX_NOMINATIONS as u32 - 1, 100, Default::default())?;
+		let (stash, controller) = create_stash_controller::<T>(
+			T::MAX_NOMINATIONS - 1,
+			100,
+			Default::default(),
+		)?;
 		let stash_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(stash.clone());
 
 		// they start validating.
@@ -224,7 +228,11 @@ benchmarks! {
 		let mut nominator_stashes = Vec::with_capacity(k as usize);
 		for i in 0 .. k {
 			// create a nominator stash.
-			let (n_stash, n_controller) = create_stash_controller::<T>(MAX_NOMINATIONS as u32 + i, 100, Default::default())?;
+			let (n_stash, n_controller) = create_stash_controller::<T>(
+				T::MAX_NOMINATIONS + i,
+				100,
+				Default::default(),
+			)?;
 
 			// bake the nominations; we first clone them from the rest of the validators.
 			let mut nominations = rest_of_validators.clone();
@@ -256,9 +264,9 @@ benchmarks! {
 		}
 	}
 
-	// Worst case scenario, MAX_NOMINATIONS
+	// Worst case scenario, T::MAX_NOMINATIONS
 	nominate {
-		let n in 1 .. MAX_NOMINATIONS as u32;
+		let n in 1 .. T::MAX_NOMINATIONS;
 		let (stash, controller) = create_stash_controller::<T>(n + 1, 100, Default::default())?;
 		let validators = create_validators::<T>(n, 100)?;
 		whitelist_account!(controller);
@@ -467,7 +475,13 @@ benchmarks! {
 		let v in 1 .. 10;
 		let n in 1 .. 100;
 
-		create_validators_with_nominators_for_era::<T>(v, n, MAX_NOMINATIONS, false, None)?;
+		create_validators_with_nominators_for_era::<T>(
+			v,
+			n,
+			<T as Config>::MAX_NOMINATIONS as usize,
+			false,
+			None,
+		)?;
 		let session_index = SessionIndex::one();
 	}: {
 		let validators = Staking::<T>::new_era(session_index).ok_or("`new_era` failed")?;
@@ -478,7 +492,13 @@ benchmarks! {
 	payout_all {
 		let v in 1 .. 10;
 		let n in 1 .. 100;
-		create_validators_with_nominators_for_era::<T>(v, n, MAX_NOMINATIONS, false, None)?;
+		create_validators_with_nominators_for_era::<T>(
+			v,
+			n,
+			<T as Config>::MAX_NOMINATIONS as usize,
+			false,
+			None,
+		)?;
 		// Start a new Era
 		let new_validators = Staking::<T>::new_era(SessionIndex::one()).unwrap();
 		assert!(new_validators.len() == v as usize);
@@ -548,7 +568,7 @@ benchmarks! {
 		// total number of slashing spans. Assigned to validators randomly.
 		let s in 1 .. 20;
 
-		let validators = create_validators_with_nominators_for_era::<T>(v, n, MAX_NOMINATIONS, false, None)?
+		let validators = create_validators_with_nominators_for_era::<T>(v, n, T::MAX_NOMINATIONS as usize, false, None)?
 			.into_iter()
 			.map(|v| T::Lookup::lookup(v).unwrap())
 			.collect::<Vec<_>>();
@@ -567,7 +587,7 @@ benchmarks! {
 		// number of nominator intention.
 		let n = 500;
 
-		let _ = create_validators_with_nominators_for_era::<T>(v, n, MAX_NOMINATIONS, false, None)?;
+		let _ = create_validators_with_nominators_for_era::<T>(v, n, T::MAX_NOMINATIONS as usize, false, None)?;
 	}: {
 		let targets = <Staking<T>>::get_npos_targets();
 		assert_eq!(targets.len() as u32, v);
@@ -586,8 +606,13 @@ mod tests {
 			let v = 10;
 			let n = 100;
 
-			create_validators_with_nominators_for_era::<Test>(v, n, MAX_NOMINATIONS, false, None)
-				.unwrap();
+			create_validators_with_nominators_for_era::<Test>(
+				v,
+				n,
+				<Test as Config>::MAX_NOMINATIONS as usize,
+				false,
+				None,
+			).unwrap();
 
 			let count_validators = Validators::<Test>::iter().count();
 			let count_nominators = Nominators::<Test>::iter().count();