EPM: allow duplicate submissions (paritytech#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>

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,

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>);
 }
 

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)
+			}
+		}
+	}
+}

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),
 			],