[Staking] Adds a round check at signed solution submission (paritytec…

…h#2690)

This PR adds a round check to the `Call::submit` extrinsic to make sure
that the solution submission has been prepared for the current election
round and avoid penalties for delayed submissions.

Related to
paritytech-secops/srlabs_findings#329

---------

Co-authored-by: command-bot <>

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

@@ -1024,6 +1024,7 @@ pub mod pallet {
 
 			// ensure solution is timely.
 			ensure!(Self::current_phase().is_signed(), Error::<T>::PreDispatchEarlySubmission);
+			ensure!(raw_solution.round == Self::round(), Error::<T>::PreDispatchDifferentRound);
 
 			// NOTE: this is the only case where having separate snapshot would have been better
 			// because could do just decode_len. But we can create abstractions to do this.
@@ -1197,6 +1198,8 @@ pub mod pallet {
 		BoundNotMet,
 		/// Submitted solution has too many winners
 		TooManyWinners,
+		/// Sumission was prepared for a different round.
+		PreDispatchDifferentRound,
 	}
 
 	#[pallet::validate_unsigned]

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

@@ -112,6 +112,15 @@ pub fn roll_to_with_ocw(n: BlockNumber) {
 	}
 }
 
+pub fn roll_to_round(n: u32) {
+	assert!(MultiPhase::round() <= n);
+
+	while MultiPhase::round() != n {
+		roll_to_signed();
+		assert_ok!(MultiPhase::elect());
+	}
+}
+
 pub struct TrimHelpers {
 	pub voters: Vec<VoterOf<Runtime>>,
 	pub assignments: Vec<IndexAssignmentOf<Runtime>>,

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

@@ -571,6 +571,40 @@ mod tests {
 	use frame_support::{assert_noop, assert_ok, assert_storage_noop};
 	use sp_runtime::Percent;
 
+	#[test]
+	fn cannot_submit_on_different_round() {
+		ExtBuilder::default().build_and_execute(|| {
+			// roll to a few rounds ahead.
+			roll_to_round(5);
+			assert_eq!(MultiPhase::round(), 5);
+
+			roll_to_signed();
+			assert_eq!(MultiPhase::current_phase(), Phase::Signed);
+
+			// create a temp snapshot only for this test.
+			MultiPhase::create_snapshot().unwrap();
+			let mut solution = raw_solution();
+
+			// try a solution prepared in a previous round.
+			solution.round = MultiPhase::round() - 1;
+
+			assert_noop!(
+				MultiPhase::submit(RuntimeOrigin::signed(10), Box::new(solution)),
+				Error::<Runtime>::PreDispatchDifferentRound,
+			);
+
+			// try a solution prepared in a later round (not expected to happen, but in any case).
+			MultiPhase::create_snapshot().unwrap();
+			let mut solution = raw_solution();
+			solution.round = MultiPhase::round() + 1;
+
+			assert_noop!(
+				MultiPhase::submit(RuntimeOrigin::signed(10), Box::new(solution)),
+				Error::<Runtime>::PreDispatchDifferentRound,
+			);
+		})
+	}
+
 	#[test]
 	fn cannot_submit_too_early() {
 		ExtBuilder::default().build_and_execute(|| {