Improve Payout Benchmarks in Staking (#7237)

* add additional checks

* fix wasm trap

* Wasm trap?

* Clean up validators before running payout

* fix line width

substrate/bin/node/runtime/src/lib.rs

@@ -436,7 +436,7 @@ parameter_types! {
 	pub const BondingDuration: pallet_staking::EraIndex = 24 * 28;
 	pub const SlashDeferDuration: pallet_staking::EraIndex = 24 * 7; // 1/4 the bonding duration.
 	pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE;
-	pub const MaxNominatorRewardedPerValidator: u32 = 64;
+	pub const MaxNominatorRewardedPerValidator: u32 = 256;
 	pub const ElectionLookahead: BlockNumber = EPOCH_DURATION_IN_BLOCKS / 4;
 	pub const MaxIterations: u32 = 10;
 	// 0.05%. The higher the value, the more strict solution acceptance becomes.

substrate/frame/session/benchmarking/src/lib.rs

@@ -55,7 +55,7 @@ benchmarks! {
 
 	set_keys {
 		let n = MAX_NOMINATIONS as u32;
-		let v_stash = create_validator_with_nominators::<T>(
+		let (v_stash, _) = create_validator_with_nominators::<T>(
 			n,
 			MAX_NOMINATIONS as u32,
 			false,
@@ -71,7 +71,12 @@ benchmarks! {
 
 	purge_keys {
 		let n = MAX_NOMINATIONS as u32;
-		let v_stash = create_validator_with_nominators::<T>(n, MAX_NOMINATIONS as u32, false, RewardDestination::Staked)?;
+		let (v_stash, _) = create_validator_with_nominators::<T>(
+			n,
+			MAX_NOMINATIONS as u32,
+			false,
+			RewardDestination::Staked
+		)?;
 		let v_controller = pallet_staking::Module::<T>::bonded(&v_stash).ok_or("not stash")?;
 		let keys = T::Keys::default();
 		let proof: Vec<u8> = vec![0,1,2,3];

substrate/frame/staking/src/benchmarking.rs

@@ -46,13 +46,13 @@ fn add_slashing_spans<T: Trait>(who: &T::AccountId, spans: u32) {
 }
 
 // This function generates one validator being nominated by n nominators, and returns the validator
-// stash account. It also starts an era and creates pending payouts.
+// stash account and the nominators' stash and controller. It also starts an era and creates pending payouts.
 pub fn create_validator_with_nominators<T: Trait>(
 	n: u32,
 	upper_bound: u32,
 	dead: bool,
 	destination: RewardDestination<T::AccountId>
-) -> Result<T::AccountId, &'static str> {
+) -> Result<(T::AccountId, Vec<(T::AccountId, T::AccountId)>), &'static str> {
 	let mut points_total = 0;
 	let mut points_individual = Vec::new();
 
@@ -66,15 +66,18 @@ pub fn create_validator_with_nominators<T: Trait>(
 	points_total += 10;
 	points_individual.push((v_stash.clone(), 10));
 
+	let mut nominators = Vec::new();
+
 	// Give the validator n nominators, but keep total users in the system the same.
 	for i in 0 .. upper_bound {
-		let (_n_stash, n_controller) = if !dead {
+		let (n_stash, n_controller) = if !dead {
 			create_stash_controller::<T>(u32::max_value() - i, 100, destination.clone())?
 		} else {
 			create_stash_and_dead_controller::<T>(u32::max_value() - i, 100, destination.clone())?
 		};
 		if i < n {
 			Staking::<T>::nominate(RawOrigin::Signed(n_controller.clone()).into(), vec![stash_lookup.clone()])?;
+			nominators.push((n_stash, n_controller));
 		}
 	}
 
@@ -84,6 +87,7 @@ pub fn create_validator_with_nominators<T: Trait>(
 	let new_validators = Staking::<T>::new_era(SessionIndex::one()).unwrap();
 
 	assert!(new_validators.len() == 1);
+	assert!(new_validators[0] == v_stash, "Our validator was not selected!");
 
 	// Give Era Points
 	let reward = EraRewardPoints::<T::AccountId> {
@@ -95,10 +99,12 @@ pub fn create_validator_with_nominators<T: Trait>(
 	ErasRewardPoints::<T>::insert(current_era, reward);
 
 	// Create reward pool
-	let total_payout = T::Currency::minimum_balance() * 1000.into();
+	let total_payout = T::Currency::minimum_balance()
+		.saturating_mul(upper_bound.into())
+		.saturating_mul(1000.into());
 	<ErasValidatorReward<T>>::insert(current_era, total_payout);
 
-	Ok(v_stash)
+	Ok((v_stash, nominators))
 }
 
 const USER_SEED: u32 = 999666;
@@ -280,7 +286,9 @@ benchmarks! {
 
 	payout_stakers_dead_controller {
 		let n in 1 .. T::MaxNominatorRewardedPerValidator::get() as u32;
-		let validator = create_validator_with_nominators::<T>(
+		// Clean up existing validators
+		Validators::<T>::remove_all();
+		let (validator, nominators) = create_validator_with_nominators::<T>(
 			n,
 			T::MaxNominatorRewardedPerValidator::get() as u32,
 			true,
@@ -294,19 +302,28 @@ benchmarks! {
 		let caller = whitelisted_caller();
 		let validator_controller = <Bonded<T>>::get(&validator).unwrap();
 		let balance_before = T::Currency::free_balance(&validator_controller);
+		for (_, controller) in &nominators {
+			let balance = T::Currency::free_balance(&controller);
+			ensure!(balance.is_zero(), "Controller has balance, but should be dead.");
+		}
 	}: payout_stakers(RawOrigin::Signed(caller), validator.clone(), current_era)
 	verify {
 		let balance_after = T::Currency::free_balance(&validator_controller);
-		assert!(
+		ensure!(
 			balance_before < balance_after,
-			"Balance of controller {:?} should have increased after payout.",
-			validator,
+			"Balance of validator controller should have increased after payout.",
 		);
+		for (_, controller) in &nominators {
+			let balance = T::Currency::free_balance(&controller);
+			ensure!(!balance.is_zero(), "Payout not given to controller.");
+		}
 	}
 
 	payout_stakers_alive_staked {
 		let n in 1 .. T::MaxNominatorRewardedPerValidator::get() as u32;
-		let validator = create_validator_with_nominators::<T>(
+		// Clean up existing validators
+		Validators::<T>::remove_all();
+		let (validator, nominators) = create_validator_with_nominators::<T>(
 			n,
 			T::MaxNominatorRewardedPerValidator::get() as u32,
 			false,
@@ -319,14 +336,25 @@ benchmarks! {
 
 		let caller = whitelisted_caller();
 		let balance_before = T::Currency::free_balance(&validator);
+		let mut nominator_balances_before = Vec::new();
+		for (stash, _) in &nominators {
+			let balance = T::Currency::free_balance(&stash);
+			nominator_balances_before.push(balance);
+		}
 	}: payout_stakers(RawOrigin::Signed(caller), validator.clone(), current_era)
 	verify {
 		let balance_after = T::Currency::free_balance(&validator);
-		assert!(
+		ensure!(
 			balance_before < balance_after,
-			"Balance of stash {:?} should have increased after payout.",
-			validator,
+			"Balance of validator stash should have increased after payout.",
 		);
+		for ((stash, _), balance_before) in nominators.iter().zip(nominator_balances_before.iter()) {
+			let balance_after = T::Currency::free_balance(&stash);
+			ensure!(
+				balance_before < &balance_after,
+				"Balance of nominator stash should have increased after payout.",
+			);
+		}
 	}
 
 	rebond {
@@ -700,13 +728,15 @@ mod tests {
 		ExtBuilder::default().has_stakers(false).build().execute_with(|| {
 			let n = 10;
 
-			let validator_stash = create_validator_with_nominators::<Test>(
+			let (validator_stash, nominators) = create_validator_with_nominators::<Test>(
 				n,
 				<Test as Trait>::MaxNominatorRewardedPerValidator::get() as u32,
 				false,
 				RewardDestination::Staked,
 			).unwrap();
 
+			assert_eq!(nominators.len() as u32, n);
+
 			let current_era = CurrentEra::get().unwrap();
 
 			let original_free_balance = Balances::free_balance(&validator_stash);
@@ -722,7 +752,7 @@ mod tests {
 		ExtBuilder::default().has_stakers(false).build().execute_with(|| {
 			let n = 10;
 
-			let validator_stash = create_validator_with_nominators::<Test>(
+			let (validator_stash, _nominators) = create_validator_with_nominators::<Test>(
 				n,
 				<Test as Trait>::MaxNominatorRewardedPerValidator::get() as u32,
 				false,