EPM: allow duplicate submissions (#12237)

* allow for duplicate signed submissions

* Fix a bunch of things, seems all good now

* fmt

* Fix

* Update frame/election-provider-multi-phase/src/signed.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* Update frame/election-provider-multi-phase/src/signed.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* add migratin

* fmt

* comment typo

* some review comments

* fix bench

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Co-authored-by: Ross Bulat <ross@parity.io>

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

@@ -320,21 +320,14 @@ frame_benchmarking::benchmarks! {
 	}
 
 	submit {
-		// the solution will be worse than all of them meaning the score need to be checked against
-		// ~ log2(c)
-		let solution = RawSolution {
-			score: ElectionScore { minimal_stake: 10_000_000u128 - 1, ..Default::default() },
-			..Default::default()
-		};
-
+		// the queue is full and the solution is only better than the worse.
 		<MultiPhase<T>>::create_snapshot().map_err(<&str>::from)?;
 		MultiPhase::<T>::on_initialize_open_signed();
 		<Round<T>>::put(1);
 
 		let mut signed_submissions = SignedSubmissions::<T>::get();
 
-		// Insert `max - 1` submissions because the call to `submit` will insert another
-		// submission and the score is worse then the previous scores.
+		// Insert `max` submissions
 		for i in 0..(T::SignedMaxSubmissions::get() - 1) {
 			let raw_solution = RawSolution {
 				score: ElectionScore { minimal_stake: 10_000_000u128 + (i as u128), ..Default::default() },
@@ -350,6 +343,12 @@ frame_benchmarking::benchmarks! {
 		}
 		signed_submissions.put();
 
+		// this score will eject the weakest one.
+		let solution = RawSolution {
+			score: ElectionScore { minimal_stake: 10_000_000u128 + 1, ..Default::default() },
+			..Default::default()
+		};
+
 		let caller = frame_benchmarking::whitelisted_caller();
 		let deposit = MultiPhase::<T>::deposit_for(
 			&solution,

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

@@ -267,6 +267,7 @@ pub mod helpers;
 
 const LOG_TARGET: &str = "runtime::election-provider";
 
+pub mod migrations;
 pub mod signed;
 pub mod unsigned;
 pub mod weights;
@@ -1265,8 +1266,8 @@ pub mod pallet {
 	#[pallet::storage]
 	pub type SignedSubmissionNextIndex<T: Config> = StorageValue<_, u32, ValueQuery>;
 
-	/// A sorted, bounded set of `(score, index)`, where each `index` points to a value in
-	/// `SignedSubmissions`.
+	/// A sorted, bounded vector of `(score, block_number, index)`, where each `index` points to a
+	/// value in `SignedSubmissions`.
 	///
 	/// We never need to process more than a single signed submission at a time. Signed submissions
 	/// can be quite large, so we're willing to pay the cost of multiple database accesses to access
@@ -1296,9 +1297,14 @@ pub mod pallet {
 	#[pallet::getter(fn minimum_untrusted_score)]
 	pub type MinimumUntrustedScore<T: Config> = StorageValue<_, ElectionScore>;
 
+	/// The current storage version.
+	///
+	/// v1: https://github.com/paritytech/substrate/pull/12237/
+	const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
+
 	#[pallet::pallet]
-	#[pallet::generate_store(pub(super) trait Store)]
 	#[pallet::without_storage_info]
+	#[pallet::storage_version(STORAGE_VERSION)]
 	pub struct Pallet<T>(PhantomData<T>);
 }
 

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

@@ -0,0 +1,78 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2022 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+pub mod v1 {
+	use frame_support::{
+		storage::unhashed,
+		traits::{Defensive, GetStorageVersion, OnRuntimeUpgrade},
+		BoundedVec,
+	};
+	use sp_std::collections::btree_map::BTreeMap;
+
+	use crate::*;
+	pub struct MigrateToV1<T>(sp_std::marker::PhantomData<T>);
+	impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
+		fn on_runtime_upgrade() -> Weight {
+			let current = Pallet::<T>::current_storage_version();
+			let onchain = Pallet::<T>::on_chain_storage_version();
+
+			log!(
+				info,
+				"Running migration with current storage version {:?} / onchain {:?}",
+				current,
+				onchain
+			);
+
+			if current == 1 && onchain == 0 {
+				if SignedSubmissionIndices::<T>::exists() {
+					// This needs to be tested at a both a block height where this value exists, and
+					// when it doesn't.
+					let now = frame_system::Pallet::<T>::block_number();
+					let map = unhashed::get::<BTreeMap<ElectionScore, u32>>(
+						&SignedSubmissionIndices::<T>::hashed_key(),
+					)
+					.defensive_unwrap_or_default();
+					let vector = map
+						.into_iter()
+						.map(|(score, index)| (score, now, index))
+						.collect::<Vec<_>>();
+
+					log!(
+						debug,
+						"{:?} SignedSubmissionIndices read from storage (max: {:?})",
+						vector.len(),
+						T::SignedMaxSubmissions::get()
+					);
+
+					// defensive-only, assuming a constant `SignedMaxSubmissions`.
+					let bounded = BoundedVec::<_, _>::truncate_from(vector);
+					SignedSubmissionIndices::<T>::put(bounded);
+
+					log!(info, "SignedSubmissionIndices existed and got migrated");
+				} else {
+					log!(info, "SignedSubmissionIndices did NOT exist.");
+				}
+
+				current.put::<Pallet<T>>();
+				T::DbWeight::get().reads_writes(2, 1)
+			} else {
+				log!(info, "Migration did not execute. This probably should be removed");
+				T::DbWeight::get().reads(1)
+			}
+		}
+	}
+}

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

@@ -571,6 +571,12 @@ impl ExtBuilder {
 			balances: vec![
 				// bunch of account for submitting stuff only.
 				(99, 100),
+				(100, 100),
+				(101, 100),
+				(102, 100),
+				(103, 100),
+				(104, 100),
+				(105, 100),
 				(999, 100),
 				(9999, 100),
 			],

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

@@ -24,11 +24,11 @@ use crate::{
 };
 use codec::{Decode, Encode, HasCompact};
 use frame_election_provider_support::NposSolution;
-use frame_support::{
-	storage::bounded_btree_map::BoundedBTreeMap,
-	traits::{defensive_prelude::*, Currency, Get, OnUnbalanced, ReservableCurrency},
+use frame_support::traits::{
+	defensive_prelude::*, Currency, Get, OnUnbalanced, ReservableCurrency,
 };
 use sp_arithmetic::traits::SaturatedConversion;
+use sp_core::bounded::BoundedVec;
 use sp_npos_elections::ElectionScore;
 use sp_runtime::{
 	traits::{Saturating, Zero},
@@ -37,7 +37,6 @@ use sp_runtime::{
 use sp_std::{
 	cmp::Ordering,
 	collections::{btree_map::BTreeMap, btree_set::BTreeSet},
-	ops::Deref,
 	vec::Vec,
 };
 
@@ -99,8 +98,12 @@ pub type SignedSubmissionOf<T> = SignedSubmission<
 	<<T as crate::Config>::MinerConfig as MinerConfig>::Solution,
 >;
 
-pub type SubmissionIndicesOf<T> =
-	BoundedBTreeMap<ElectionScore, u32, <T as Config>::SignedMaxSubmissions>;
+/// Always sorted vector of a score, submitted at the given block number, which can be found at the
+/// given index (`u32`) of the `SignedSubmissionsMap`.
+pub type SubmissionIndicesOf<T> = BoundedVec<
+	(ElectionScore, <T as frame_system::Config>::BlockNumber, u32),
+	<T as Config>::SignedMaxSubmissions,
+>;
 
 /// Outcome of [`SignedSubmissions::insert`].
 pub enum InsertResult<T: Config> {
@@ -126,6 +129,16 @@ pub struct SignedSubmissions<T: Config> {
 }
 
 impl<T: Config> SignedSubmissions<T> {
+	/// `true` if the structure is empty.
+	pub fn is_empty(&self) -> bool {
+		self.indices.is_empty()
+	}
+
+	/// Get the length of submitted solutions.
+	pub fn len(&self) -> usize {
+		self.indices.len()
+	}
+
 	/// Get the signed submissions from storage.
 	pub fn get() -> Self {
 		let submissions = SignedSubmissions {
@@ -134,10 +147,12 @@ impl<T: Config> SignedSubmissions<T> {
 			insertion_overlay: BTreeMap::new(),
 			deletion_overlay: BTreeSet::new(),
 		};
+
 		// validate that the stored state is sane
 		debug_assert!(submissions
 			.indices
-			.values()
+			.iter()
+			.map(|(_, _, index)| index)
 			.copied()
 			.max()
 			.map_or(true, |max_idx| submissions.next_idx > max_idx,));
@@ -155,7 +170,8 @@ impl<T: Config> SignedSubmissions<T> {
 			.map_or(true, |max_idx| self.next_idx > max_idx,));
 		debug_assert!(self
 			.indices
-			.values()
+			.iter()
+			.map(|(_, _, index)| index)
 			.copied()
 			.max()
 			.map_or(true, |max_idx| self.next_idx > max_idx,));
@@ -174,9 +190,9 @@ impl<T: Config> SignedSubmissions<T> {
 	/// Get the submission at a particular index.
 	fn get_submission(&self, index: u32) -> Option<SignedSubmissionOf<T>> {
 		if self.deletion_overlay.contains(&index) {
-			// Note: can't actually remove the item from the insertion overlay (if present)
-			// because we don't want to use `&mut self` here. There may be some kind of
-			// `RefCell` optimization possible here in the future.
+			// Note: can't actually remove the item from the insertion overlay (if present) because
+			// we don't want to use `&mut self` here. There may be some kind of `RefCell`
+			// optimization possible here in the future.
 			None
 		} else {
 			self.insertion_overlay
@@ -188,27 +204,30 @@ impl<T: Config> SignedSubmissions<T> {
 
 	/// Perform three operations:
 	///
-	/// - Remove a submission (identified by score)
-	/// - Insert a new submission (identified by score and insertion index)
-	/// - Return the submission which was removed.
+	/// - Remove the solution at the given position of `self.indices`.
+	/// - Insert a new submission (identified by score and insertion index), if provided.
+	/// - Return the submission which was removed, if any.
 	///
-	/// Note: in the case that `weakest_score` is not present in `self.indices`, this will return
-	/// `None` without inserting the new submission and without further notice.
-	///
-	/// Note: this does not enforce any ordering relation between the submission removed and that
-	/// inserted.
+	/// The call site must ensure that `remove_pos` is a valid index. If otherwise, `None` is
+	/// silently returned.
 	///
 	/// Note: this doesn't insert into `insertion_overlay`, the optional new insertion must be
-	/// inserted into  `insertion_overlay` to keep the variable `self` in a valid state.
+	/// inserted into `insertion_overlay` to keep the variable `self` in a valid state.
 	fn swap_out_submission(
 		&mut self,
-		remove_score: ElectionScore,
-		insert: Option<(ElectionScore, u32)>,
+		remove_pos: usize,
+		insert: Option<(ElectionScore, T::BlockNumber, u32)>,
 	) -> Option<SignedSubmissionOf<T>> {
-		let remove_index = self.indices.remove(&remove_score)?;
-		if let Some((insert_score, insert_idx)) = insert {
+		if remove_pos >= self.indices.len() {
+			return None
+		}
+
+		// safe: index was just checked in the line above.
+		let (_, _, remove_index) = self.indices.remove(remove_pos);
+
+		if let Some((insert_score, block_number, insert_idx)) = insert {
 			self.indices
-				.try_insert(insert_score, insert_idx)
+				.try_push((insert_score, block_number, insert_idx))
 				.expect("just removed an item, we must be under capacity; qed");
 		}
 
@@ -222,20 +241,17 @@ impl<T: Config> SignedSubmissions<T> {
 		})
 	}
 
+	/// Remove the signed submission with the highest score from the set.
+	pub fn pop_last(&mut self) -> Option<SignedSubmissionOf<T>> {
+		let best_index = self.indices.len().checked_sub(1)?;
+		self.swap_out_submission(best_index, None)
+	}
+
 	/// Iterate through the set of signed submissions in order of increasing score.
 	pub fn iter(&self) -> impl '_ + Iterator<Item = SignedSubmissionOf<T>> {
-		self.indices.iter().filter_map(move |(_score, &idx)| {
-			let maybe_submission = self.get_submission(idx);
-			if maybe_submission.is_none() {
-				log!(
-					error,
-					"SignedSubmissions internal state is invalid (idx {}); \
-					there is a logic error in code handling signed solution submissions",
-					idx,
-				)
-			}
-			maybe_submission
-		})
+		self.indices
+			.iter()
+			.filter_map(move |(_score, _bn, idx)| self.get_submission(*idx).defensive())
 	}
 
 	/// Empty the set of signed submissions, returning an iterator of signed submissions in
@@ -283,68 +299,54 @@ impl<T: Config> SignedSubmissions<T> {
 	/// to `is_score_better`, we do not change anything.
 	pub fn insert(&mut self, submission: SignedSubmissionOf<T>) -> InsertResult<T> {
 		// verify the expectation that we never reuse an index
-		debug_assert!(!self.indices.values().any(|&idx| idx == self.next_idx));
+		debug_assert!(!self.indices.iter().map(|(_, _, x)| x).any(|&idx| idx == self.next_idx));
+		let block_number = frame_system::Pallet::<T>::block_number();
 
-		let weakest = match self.indices.try_insert(submission.raw_solution.score, self.next_idx) {
-			Ok(Some(prev_idx)) => {
-				// a submission of equal score was already present in the set;
-				// no point editing the actual backing map as we know that the newer solution can't
-				// be better than the old. However, we do need to put the old value back.
-				self.indices
-					.try_insert(submission.raw_solution.score, prev_idx)
-					.expect("didn't change the map size; qed");
-				return InsertResult::NotInserted
-			},
-			Ok(None) => {
-				// successfully inserted into the set; no need to take out weakest member
-				None
-			},
-			Err((insert_score, insert_idx)) => {
-				// could not insert into the set because it is full.
-				// note that we short-circuit return here in case the iteration produces `None`.
-				// If there wasn't a weakest entry to remove, then there must be a capacity of 0,
-				// which means that we can't meaningfully proceed.
-				let weakest_score = match self.indices.iter().next() {
+		let maybe_weakest = match self.indices.try_push((
+			submission.raw_solution.score,
+			block_number,
+			self.next_idx,
+		)) {
+			Ok(_) => None,
+			Err(_) => {
+				// the queue is full -- if this is better, insert it.
+				let weakest_score = match self.indices.iter().next().defensive() {
 					None => return InsertResult::NotInserted,
-					Some((score, _)) => *score,
+					Some((score, _, _)) => *score,
 				};
 				let threshold = T::BetterSignedThreshold::get();
 
 				// if we haven't improved on the weakest score, don't change anything.
-				if !insert_score.strict_threshold_better(weakest_score, threshold) {
+				if !submission.raw_solution.score.strict_threshold_better(weakest_score, threshold)
+				{
 					return InsertResult::NotInserted
 				}
 
-				self.swap_out_submission(weakest_score, Some((insert_score, insert_idx)))
+				self.swap_out_submission(
+					0, // swap out the worse one, which is always index 0.
+					Some((submission.raw_solution.score, block_number, self.next_idx)),
+				)
 			},
 		};
 
+		// this is the ONLY place that we insert, and we sort post insertion. If scores are the
+		// same, we sort based on reverse of submission block number.
+		self.indices
+			.sort_by(|(score1, bn1, _), (score2, bn2, _)| match score1.cmp(score2) {
+				Ordering::Equal => bn1.cmp(&bn2).reverse(),
+				x => x,
+			});
+
 		// we've taken out the weakest, so update the storage map and the next index
 		debug_assert!(!self.insertion_overlay.contains_key(&self.next_idx));
 		self.insertion_overlay.insert(self.next_idx, submission);
 		debug_assert!(!self.deletion_overlay.contains(&self.next_idx));
 		self.next_idx += 1;
-		match weakest {
+		match maybe_weakest {
 			Some(weakest) => InsertResult::InsertedEjecting(weakest),
 			None => InsertResult::Inserted,
 		}
 	}
-
-	/// Remove the signed submission with the highest score from the set.
-	pub fn pop_last(&mut self) -> Option<SignedSubmissionOf<T>> {
-		let (score, _) = self.indices.iter().rev().next()?;
-		// deref in advance to prevent mutable-immutable borrow conflict
-		let score = *score;
-		self.swap_out_submission(score, None)
-	}
-}
-
-impl<T: Config> Deref for SignedSubmissions<T> {
-	type Target = SubmissionIndicesOf<T>;
-
-	fn deref(&self) -> &Self::Target {
-		&self.indices
-	}
 }
 
 impl<T: Config> Pallet<T> {
@@ -379,6 +381,12 @@ impl<T: Config> Pallet<T> {
 			Self::snapshot_metadata().unwrap_or_default();
 
 		while let Some(best) = all_submissions.pop_last() {
+			log!(
+				debug,
+				"finalized_signed: trying to verify from {:?} score {:?}",
+				best.who,
+				best.raw_solution.score
+			);
 			let SignedSubmission { raw_solution, who, deposit, call_fee } = best;
 			let active_voters = raw_solution.solution.voter_count() as u32;
 			let feasibility_weight = {
@@ -386,6 +394,7 @@ impl<T: Config> Pallet<T> {
 				let desired_targets = Self::desired_targets().defensive_unwrap_or_default();
 				T::WeightInfo::feasibility_check(voters, targets, active_voters, desired_targets)
 			};
+
 			// the feasibility check itself has some weight
 			weight = weight.saturating_add(feasibility_weight);
 			match Self::feasibility_check(raw_solution, ElectionCompute::Signed) {
@@ -397,12 +406,14 @@ impl<T: Config> Pallet<T> {
 						call_fee,
 					);
 					found_solution = true;
+					log!(debug, "finalized_signed: found a valid solution");
 
 					weight = weight
 						.saturating_add(T::WeightInfo::finalize_signed_phase_accept_solution());
 					break
 				},
 				Err(_) => {
+					log!(warn, "finalized_signed: invalid signed submission found, slashing.");
 					Self::finalize_signed_phase_reject_solution(&who, deposit);
 					weight = weight
 						.saturating_add(T::WeightInfo::finalize_signed_phase_reject_solution());
@@ -526,14 +537,7 @@ impl<T: Config> Pallet<T> {
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use crate::{
-		mock::{
-			balances, multi_phase_events, raw_solution, roll_to, roll_to_signed, Balances,
-			ExtBuilder, MockedWeightInfo, MultiPhase, Runtime, RuntimeOrigin, SignedMaxRefunds,
-			SignedMaxSubmissions, SignedMaxWeight,
-		},
-		Error, Event, Perbill, Phase,
-	};
+	use crate::{mock::*, ElectionCompute, Error, Event, Perbill, Phase};
 	use frame_support::{assert_noop, assert_ok, assert_storage_noop};
 
 	#[test]
@@ -868,8 +872,8 @@ mod tests {
 	}
 
 	#[test]
-	fn replace_weakest_works() {
-		ExtBuilder::default().build_and_execute(|| {
+	fn replace_weakest_by_score_works() {
+		ExtBuilder::default().signed_max_submission(3).build_and_execute(|| {
 			roll_to_signed();
 			assert!(MultiPhase::current_phase().is_signed());
 
@@ -893,7 +897,7 @@ mod tests {
 					.iter()
 					.map(|s| s.raw_solution.score.minimal_stake)
 					.collect::<Vec<_>>(),
-				vec![4, 6, 7, 8, 9],
+				vec![4, 6, 7],
 			);
 
 			// better.
@@ -909,7 +913,7 @@ mod tests {
 					.iter()
 					.map(|s| s.raw_solution.score.minimal_stake)
 					.collect::<Vec<_>>(),
-				vec![5, 6, 7, 8, 9],
+				vec![5, 6, 7],
 			);
 		})
 	}
@@ -946,7 +950,8 @@ mod tests {
 	}
 
 	#[test]
-	fn equally_good_solution_is_not_accepted() {
+	fn equally_good_solution_is_not_accepted_when_queue_full() {
+		// because in ordering of solutions, an older solution has higher priority and should stay.
 		ExtBuilder::default().signed_max_submission(3).build_and_execute(|| {
 			roll_to_signed();
 			assert!(MultiPhase::current_phase().is_signed());
@@ -958,6 +963,7 @@ mod tests {
 				};
 				assert_ok!(MultiPhase::submit(RuntimeOrigin::signed(99), Box::new(solution)));
 			}
+
 			assert_eq!(
 				MultiPhase::signed_submissions()
 					.iter()
@@ -978,6 +984,99 @@ mod tests {
 		})
 	}
 
+	#[test]
+	fn equally_good_solution_is_accepted_when_queue_not_full() {
+		// because in ordering of solutions, an older solution has higher priority and should stay.
+		ExtBuilder::default().signed_max_submission(3).build_and_execute(|| {
+			roll_to(15);
+			assert!(MultiPhase::current_phase().is_signed());
+
+			let solution = RawSolution {
+				score: ElectionScore { minimal_stake: 5, ..Default::default() },
+				..Default::default()
+			};
+			assert_ok!(MultiPhase::submit(RuntimeOrigin::signed(99), Box::new(solution)));
+
+			assert_eq!(
+				MultiPhase::signed_submissions()
+					.iter()
+					.map(|s| (s.who, s.raw_solution.score.minimal_stake,))
+					.collect::<Vec<_>>(),
+				vec![(99, 5)]
+			);
+
+			roll_to(16);
+			let solution = RawSolution {
+				score: ElectionScore { minimal_stake: 5, ..Default::default() },
+				..Default::default()
+			};
+			assert_ok!(MultiPhase::submit(RuntimeOrigin::signed(999), Box::new(solution)));
+
+			assert_eq!(
+				MultiPhase::signed_submissions()
+					.iter()
+					.map(|s| (s.who, s.raw_solution.score.minimal_stake,))
+					.collect::<Vec<_>>(),
+				vec![(999, 5), (99, 5)]
+			);
+
+			let solution = RawSolution {
+				score: ElectionScore { minimal_stake: 6, ..Default::default() },
+				..Default::default()
+			};
+			assert_ok!(MultiPhase::submit(RuntimeOrigin::signed(9999), Box::new(solution)));
+
+			assert_eq!(
+				MultiPhase::signed_submissions()
+					.iter()
+					.map(|s| (s.who, s.raw_solution.score.minimal_stake,))
+					.collect::<Vec<_>>(),
+				vec![(999, 5), (99, 5), (9999, 6)]
+			);
+		})
+	}
+
+	#[test]
+	fn all_equal_score() {
+		// because in ordering of solutions, an older solution has higher priority and should stay.
+		ExtBuilder::default().signed_max_submission(3).build_and_execute(|| {
+			roll_to(15);
+			assert!(MultiPhase::current_phase().is_signed());
+
+			for i in 0..SignedMaxSubmissions::get() {
+				roll_to((15 + i).into());
+				let solution = raw_solution();
+				assert_ok!(MultiPhase::submit(
+					RuntimeOrigin::signed(100 + i as AccountId),
+					Box::new(solution)
+				));
+			}
+
+			assert_eq!(
+				MultiPhase::signed_submissions()
+					.iter()
+					.map(|s| (s.who, s.raw_solution.score.minimal_stake))
+					.collect::<Vec<_>>(),
+				vec![(102, 40), (101, 40), (100, 40)]
+			);
+
+			roll_to(25);
+
+			// The first one that will actually get verified is the last one.
+			assert_eq!(
+				multi_phase_events(),
+				vec![
+					Event::SignedPhaseStarted { round: 1 },
+					Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false },
+					Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false },
+					Event::SolutionStored { compute: ElectionCompute::Signed, prev_ejected: false },
+					Event::Rewarded { account: 100, value: 7 },
+					Event::UnsignedPhaseStarted { round: 1 }
+				]
+			);
+		})
+	}
+
 	#[test]
 	fn all_in_one_signed_submission_scenario() {
 		// a combination of:
@@ -991,6 +1090,7 @@ mod tests {
 			assert_eq!(balances(&99), (100, 0));
 			assert_eq!(balances(&999), (100, 0));
 			assert_eq!(balances(&9999), (100, 0));
+
 			let solution = raw_solution();
 
 			// submit a correct one.