[NPoS] Remove better solution threshold for unsigned submissions (#2694)

closes https://github.com/paritytech-secops/srlabs_findings/issues/78.

Removes `BetterUnsignedThreshold` from pallet EPM. This will essentially
mean any solution submitted by the validator that is strictly better
than the current queued solution would be accepted.

The reason for having these thresholds is to limit number of solutions
submitted on-chain. However for unsigned submissions, the number of
solutions that could be submitted on average is limited even without
thresholding (calculation shown in the corresponding issue).

substrate/frame/election-provider-multi-phase/src/unsigned.rs

@@ -384,9 +384,8 @@ impl<T: Config> Pallet<T> {
 
 		// ensure score is being improved. Panic henceforth.
 		ensure!(
-			Self::queued_solution().map_or(true, |q: ReadySolution<_, _>| raw_solution
-				.score
-				.strict_threshold_better(q.score, T::BetterUnsignedThreshold::get())),
+			Self::queued_solution()
+				.map_or(true, |q: ReadySolution<_, _>| raw_solution.score > q.score),
 			Error::<T>::PreDispatchWeakSubmission,
 		);
 
@@ -1025,7 +1024,7 @@ mod tests {
 		bounded_vec,
 		offchain::storage_lock::{BlockAndTime, StorageLock},
 		traits::{Dispatchable, ValidateUnsigned, Zero},
-		ModuleError, PerU16, Perbill,
+		ModuleError, PerU16,
 	};
 
 	type Assignment = crate::unsigned::Assignment<Runtime>;
@@ -1360,7 +1359,7 @@ mod tests {
 			.desired_targets(1)
 			.add_voter(7, 2, bounded_vec![10])
 			.add_voter(8, 5, bounded_vec![10])
-			.better_unsigned_threshold(Perbill::from_percent(50))
+			.add_voter(9, 1, bounded_vec![10])
 			.build_and_execute(|| {
 				roll_to_unsigned();
 				assert!(MultiPhase::current_phase().is_unsigned());
@@ -1368,12 +1367,15 @@ mod tests {
 
 				// an initial solution
 				let result = ElectionResult {
-					// note: This second element of backing stake is not important here.
-					winners: vec![(10, 10)],
-					assignments: vec![Assignment {
-						who: 10,
-						distribution: vec![(10, PerU16::one())],
-					}],
+					winners: vec![(10, 12)],
+					assignments: vec![
+						Assignment { who: 10, distribution: vec![(10, PerU16::one())] },
+						Assignment {
+							who: 7,
+							// note: this percent doesn't even matter, in solution it is 100%.
+							distribution: vec![(10, PerU16::one())],
+						},
+					],
 				};
 
 				let RoundSnapshot { voters, targets } = MultiPhase::snapshot().unwrap();
@@ -1394,9 +1396,35 @@ mod tests {
 					Box::new(solution),
 					witness
 				));
-				assert_eq!(MultiPhase::queued_solution().unwrap().score.minimal_stake, 10);
+				assert_eq!(MultiPhase::queued_solution().unwrap().score.minimal_stake, 12);
 
-				// trial 1: a solution who's score is only 2, i.e. 20% better in the first element.
+				// trial 1: a solution who's minimal stake is 10, i.e. worse than the first solution
+				// of 12.
+				let result = ElectionResult {
+					winners: vec![(10, 10)],
+					assignments: vec![Assignment {
+						who: 10,
+						distribution: vec![(10, PerU16::one())],
+					}],
+				};
+				let (raw, score, _, _) = Miner::<Runtime>::prepare_election_result_with_snapshot(
+					result,
+					voters.clone(),
+					targets.clone(),
+					desired_targets,
+				)
+				.unwrap();
+				let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
+				// 10 is not better than 12
+				assert_eq!(solution.score.minimal_stake, 10);
+				// submitting this will actually panic.
+				assert_noop!(
+					MultiPhase::unsigned_pre_dispatch_checks(&solution),
+					Error::<Runtime>::PreDispatchWeakSubmission,
+				);
+
+				// trial 2: try resubmitting another solution with same score (12) as the queued
+				// solution.
 				let result = ElectionResult {
 					winners: vec![(10, 12)],
 					assignments: vec![
@@ -1408,6 +1436,7 @@ mod tests {
 						},
 					],
 				};
+
 				let (raw, score, _, _) = Miner::<Runtime>::prepare_election_result_with_snapshot(
 					result,
 					voters.clone(),
@@ -1416,15 +1445,45 @@ mod tests {
 				)
 				.unwrap();
 				let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
-				// 12 is not 50% more than 10
+				// 12 is not better than 12. We need score of atleast 13 to be accepted.
 				assert_eq!(solution.score.minimal_stake, 12);
+				// submitting this will panic.
 				assert_noop!(
 					MultiPhase::unsigned_pre_dispatch_checks(&solution),
 					Error::<Runtime>::PreDispatchWeakSubmission,
 				);
-				// submitting this will actually panic.
 
-				// trial 2: a solution who's score is only 7, i.e. 70% better in the first element.
+				// trial 3: a solution who's minimal stake is 13, i.e. 1 better than the queued
+				// solution of 12.
+				let result = ElectionResult {
+					winners: vec![(10, 12)],
+					assignments: vec![
+						Assignment { who: 10, distribution: vec![(10, PerU16::one())] },
+						Assignment { who: 7, distribution: vec![(10, PerU16::one())] },
+						Assignment { who: 9, distribution: vec![(10, PerU16::one())] },
+					],
+				};
+				let (raw, score, witness, _) =
+					Miner::<Runtime>::prepare_election_result_with_snapshot(
+						result,
+						voters.clone(),
+						targets.clone(),
+						desired_targets,
+					)
+					.unwrap();
+				let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
+				assert_eq!(solution.score.minimal_stake, 13);
+
+				// this should work
+				assert_ok!(MultiPhase::unsigned_pre_dispatch_checks(&solution));
+				assert_ok!(MultiPhase::submit_unsigned(
+					RuntimeOrigin::none(),
+					Box::new(solution),
+					witness
+				));
+
+				// trial 4: a solution who's minimal stake is 17, i.e. 4 better than the last
+				// soluton.
 				let result = ElectionResult {
 					winners: vec![(10, 12)],
 					assignments: vec![