Use a better data structure for SignedSubmissions instead of Vec

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

@@ -273,19 +273,16 @@ frame_benchmarking::benchmarks! {
 		MultiPhase::<T>::on_initialize_open_signed().expect("should be ok to start signed phase");
 		<Round<T>>::put(1);
 
-		<SignedSubmissions<T>>::mutate(|queue| {
-			for i in 0..c {
-				let solution = RawSolution {
-					score: [(10_000_000 + i).into(), 0, 0],
-					..Default::default()
-				};
-				let signed_submission = SignedSubmission { solution, ..Default::default() };
-				queue.push(signed_submission);
-			}
-			// as of here, the queue is ordered worst-to-best.
-			// However, we have an invariant that it should be ordered best-to-worst
-			queue.reverse();
-		});
+		let mut signed_submissions = SignedSubmissions::<T>::get();
+		for i in 0..c {
+			let solution = RawSolution {
+				score: [(10_000_000 + i).into(), 0, 0],
+				..Default::default()
+			};
+			let signed_submission = SignedSubmission { solution, ..Default::default() };
+			signed_submissions.insert(signed_submission);
+		}
+		signed_submissions.put();
 
 		let caller = frame_benchmarking::whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller,  T::Currency::minimum_balance() * 10u32.into());

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

@@ -249,14 +249,16 @@ pub mod helpers;
 
 const LOG_TARGET: &'static str = "runtime::election-provider";
 
-pub mod unsigned;
 pub mod signed;
+pub mod unsigned;
 pub mod weights;
 
+pub use signed::{
+	BalanceOf, NegativeImbalanceOf, PositiveImbalanceOf, SignedSubmission, SignedSubmissionOf,
+	SignedSubmissions, SubmissionIndicesOf,
+};
 pub use weights::WeightInfo;
 
-pub use signed::{SignedSubmission, BalanceOf, NegativeImbalanceOf, PositiveImbalanceOf};
-
 /// The compact solution type used by this crate.
 pub type CompactOf<T> = <T as Config>::CompactSolution;
 
@@ -387,7 +389,7 @@ impl Default for ElectionCompute {
 ///
 /// Such a solution should never become effective in anyway before being checked by the
 /// `Pallet::feasibility_check`
-#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)]
+#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, PartialOrd, Ord)]
 pub struct RawSolution<C> {
 	/// Compact election edges.
 	pub compact: C,
@@ -562,6 +564,7 @@ pub mod pallet {
 		/// Maximum number of signed submissions that can be queued.
 		#[pallet::constant]
 		type SignedMaxSubmissions: Get<u32>;
+
 		/// Maximum weight of a signed solution.
 		///
 		/// This should probably be similar to [`Config::MinerMaxWeight`].
@@ -603,6 +606,7 @@ pub mod pallet {
 			+ Eq
 			+ Clone
 			+ sp_std::fmt::Debug
+			+ Ord
 			+ CompactSolution;
 
 		/// Accuracy used for fallback on-chain election.
@@ -881,20 +885,22 @@ pub mod pallet {
 
 			// ensure solution claims is better.
 			let mut signed_submissions = Self::signed_submissions();
-			let ejected_a_solution = signed_submissions.len()
-				== T::SignedMaxSubmissions::get().saturated_into::<usize>();
-			let index = Self::insert_submission(&who, &mut signed_submissions, solution, size)
-				.ok_or(Error::<T>::SignedQueueFull)?;
-
-			// collect deposit. Thereafter, the function cannot fail.
-			let deposit = signed_submissions
-				.get(index)
-				.map(|s| s.deposit)
-				.ok_or(Error::<T>::InvalidSubmissionIndex)?;
-			T::Currency::reserve(&who, deposit).map_err(|_| Error::<T>::SignedCannotPayDeposit)?;
-
-			// store the new signed submission.
-			<SignedSubmissions<T>>::put(signed_submissions);
+
+			let (maybe_deposit, ejected_a_solution) =
+				Self::insert_submission(&who, &mut signed_submissions, solution, size);
+			// it's an error if we neither inserted nor removed any submissions
+			ensure!(
+				(None, false) != (maybe_deposit, ejected_a_solution),
+				Error::<T>::SignedQueueFull,
+			);
+
+			if let Some(deposit_amount) = maybe_deposit {
+				// collect deposit. Thereafter, the function cannot fail.
+				T::Currency::reserve(&who, deposit_amount)
+					.map_err(|_| Error::<T>::SignedCannotPayDeposit)?;
+			}
+
+			signed_submissions.put();
 			Self::deposit_event(Event::SolutionStored(ElectionCompute::Signed, ejected_a_solution));
 			Ok(())
 		}
@@ -1048,14 +1054,34 @@ pub mod pallet {
 	#[pallet::getter(fn snapshot_metadata)]
 	pub type SnapshotMetadata<T: Config> = StorageValue<_, SolutionOrSnapshotSize>;
 
-	/// Sorted (worse -> best) list of unchecked, signed solutions.
+	/// The next index to be assigned to an incoming signed submission.
+	///
+	/// We can't just use `SignedSubmissionIndices.len()`, because that's a bounded set; past its
+	/// capacity, it will simply saturate. We can't just iterate over `SignedSubmissionsMap`,
+	/// because iteration is slow. Instead, we store the value here.
+	#[pallet::storage]
+	pub(crate) type SignedSubmissionNextIndex<T: Config> = StorageValue<_, u32, ValueQuery>;
+
+	/// A sorted, bounded set of `(score, 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
+	/// them one at a time instead of reading and decoding all of them at once.
+	#[pallet::storage]
+	pub(crate) type SignedSubmissionIndices<T: Config> =
+		StorageValue<_, SubmissionIndicesOf<T>, ValueQuery>;
+
+	/// Unchecked, signed solutions.
+	///
+	/// Together with `SubmissionIndices`, this stores a bounded set of `SignedSubmissions` while
+	/// allowing us to keep only a single one in memory at a time.
+	///
+	/// Twox note: the key of the map is an auto-incrementing index which users cannot inspect or
+	/// affect; we shouldn't need a cryptographically secure hasher.
 	#[pallet::storage]
-	#[pallet::getter(fn signed_submissions)]
-	pub type SignedSubmissions<T: Config> = StorageValue<
-		_,
-		Vec<SignedSubmission<T::AccountId, BalanceOf<T>, CompactOf<T>>>,
-		ValueQuery,
-	>;
+	pub(crate) type SignedSubmissionsMap<T: Config> =
+		StorageMap<_, Twox64Concat, u32, SignedSubmissionOf<T>, ValueQuery>;
 
 	/// The minimum score that each 'untrusted' solution must attain in order to be considered
 	/// feasible.