From 70543557657f4dd80ccaaa7d5ba96913536e0a97 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Wed, 9 Mar 2022 23:49:17 +0000
Subject: [PATCH 01/21] Refund call fee for all non-invalid signed submissions

---
 .../election-provider-multi-phase/src/lib.rs  |  9 ++---
 .../src/signed.rs                             | 40 +++++++++++++------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
index 7e211c5ee9211..a601f6c383430 100644
--- a/frame/election-provider-multi-phase/src/lib.rs
+++ b/frame/election-provider-multi-phase/src/lib.rs
@@ -242,7 +242,7 @@ use frame_support::{
 use frame_system::{ensure_none, offchain::SendTransactionTypes};
 use scale_info::TypeInfo;
 use sp_arithmetic::{
-	traits::{CheckedAdd, Saturating, Zero},
+	traits::{CheckedAdd, Zero},
 	UpperOf,
 };
 use sp_npos_elections::{
@@ -987,14 +987,13 @@ pub mod pallet {
 
 			// create the submission
 			let deposit = Self::deposit_for(&raw_solution, size);
-			let reward = {
+			let call_fee = {
 				let call = Call::submit { raw_solution: raw_solution.clone() };
-				let call_fee = T::EstimateCallFee::estimate_call_fee(&call, None.into());
-				T::SignedRewardBase::get().saturating_add(call_fee)
+				T::EstimateCallFee::estimate_call_fee(&call, None.into())
 			};
 
 			let submission =
-				SignedSubmission { who: who.clone(), deposit, raw_solution: *raw_solution, reward };
+				SignedSubmission { who: who.clone(), deposit, raw_solution: *raw_solution, reward: T::SignedRewardBase::get(), call_fee };
 
 			// insert the submission if the queue has space or it's better than the weakest
 			// eject the weakest if the queue was full
diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index a233346b4fd77..d92dae6a47602 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -52,6 +52,8 @@ pub struct SignedSubmission<AccountId, Balance: HasCompact, Solution> {
 	pub raw_solution: RawSolution<Solution>,
 	/// The reward that should potentially be paid for this solution, if accepted.
 	pub reward: Balance,
+	// The estimated fee the origin paid to submit the solution.
+	pub call_fee: Balance,
 }
 
 impl<AccountId, Balance, Solution> Ord for SignedSubmission<AccountId, Balance, Solution>
@@ -361,7 +363,7 @@ impl<T: Config> Pallet<T> {
 			Self::snapshot_metadata().unwrap_or_default();
 
 		while let Some(best) = all_submissions.pop_last() {
-			let SignedSubmission { raw_solution, who, deposit, reward } = best;
+			let SignedSubmission { raw_solution, who, deposit, reward, call_fee } = best;
 			let active_voters = raw_solution.solution.voter_count() as u32;
 			let feasibility_weight = {
 				// defensive only: at the end of signed phase, snapshot will exits.
@@ -377,6 +379,7 @@ impl<T: Config> Pallet<T> {
 						&who,
 						deposit,
 						reward,
+						call_fee,
 					);
 					found_solution = true;
 
@@ -395,10 +398,9 @@ impl<T: Config> Pallet<T> {
 		// Any unprocessed solution is pointless to even consider. Feasible or malicious,
 		// they didn't end up being used. Unreserve the bonds.
 		let discarded = all_submissions.len();
-		for SignedSubmission { who, deposit, .. } in all_submissions.drain() {
-			let _remaining = T::Currency::unreserve(&who, deposit);
-			weight = weight.saturating_add(T::DbWeight::get().writes(1));
-			debug_assert!(_remaining.is_zero());
+		for SignedSubmission { who, deposit, call_fee, .. } in all_submissions.drain() {
+			Self::finalize_signed_phase_handle_refund(&who, deposit, call_fee);
+			weight = weight.saturating_add(T::DbWeight::get().writes(2));
 		}
 
 		debug_assert!(!SignedSubmissionIndices::<T>::exists());
@@ -424,6 +426,7 @@ impl<T: Config> Pallet<T> {
 		who: &T::AccountId,
 		deposit: BalanceOf<T>,
 		reward: BalanceOf<T>,
+		call_fee: BalanceOf<T>,
 	) {
 		// write this ready solution.
 		<QueuedSolution<T>>::put(ready_solution);
@@ -431,9 +434,7 @@ impl<T: Config> Pallet<T> {
 		// emit reward event
 		Self::deposit_event(crate::Event::Rewarded { account: who.clone(), value: reward });
 
-		// unreserve deposit.
-		let _remaining = T::Currency::unreserve(who, deposit);
-		debug_assert!(_remaining.is_zero());
+		Self::finalize_signed_phase_handle_refund(who, deposit, call_fee);
 
 		// Reward.
 		let positive_imbalance = T::Currency::deposit_creating(who, reward);
@@ -488,6 +489,21 @@ impl<T: Config> Pallet<T> {
 			.saturating_add(len_deposit)
 			.saturating_add(weight_deposit)
 	}
+
+	/// Unreserve `who`s deposit and refund their transaction fee.
+	fn finalize_signed_phase_handle_refund(
+		who: &T::AccountId,
+		deposit: BalanceOf<T>,
+		call_fee: BalanceOf<T>
+	) {
+		// Unreserve deposit
+		let _remaining = T::Currency::unreserve(who, deposit);
+		debug_assert!(_remaining.is_zero());
+
+		println!("{:?}=call_fee", call_fee);
+		// Refund fee
+		let _ = T::Currency::deposit_creating(who, call_fee);
+	}
 }
 
 #[cfg(test)]
@@ -598,8 +614,8 @@ mod tests {
 
 			// 99 is rewarded.
 			assert_eq!(balances(&99), (100 + 7 + 8, 0));
-			// 999 gets everything back.
-			assert_eq!(balances(&999), (100, 0));
+			// 999 gets everything back, including the call fee.
+			assert_eq!(balances(&999), (100 + 8, 0));
 		})
 	}
 
@@ -824,8 +840,8 @@ mod tests {
 			assert_eq!(balances(&99), (100 + 7 + 8, 0));
 			// 999 is slashed.
 			assert_eq!(balances(&999), (95, 0));
-			// 9999 gets everything back.
-			assert_eq!(balances(&9999), (100, 0));
+			// 9999 gets everything back, including the call fee.
+			assert_eq!(balances(&9999), (100 + 8, 0));
 		})
 	}
 

From ee3553d586c4c9ec0d7b32a4ef481153f84ef57f Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Wed, 9 Mar 2022 23:57:19 +0000
Subject: [PATCH 02/21] Clean up

---
 frame/election-provider-multi-phase/src/lib.rs    | 9 +++++++--
 frame/election-provider-multi-phase/src/signed.rs | 5 ++---
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
index a601f6c383430..56d7306cc1f41 100644
--- a/frame/election-provider-multi-phase/src/lib.rs
+++ b/frame/election-provider-multi-phase/src/lib.rs
@@ -992,8 +992,13 @@ pub mod pallet {
 				T::EstimateCallFee::estimate_call_fee(&call, None.into())
 			};
 
-			let submission =
-				SignedSubmission { who: who.clone(), deposit, raw_solution: *raw_solution, reward: T::SignedRewardBase::get(), call_fee };
+			let submission = SignedSubmission {
+				who: who.clone(),
+				deposit,
+				raw_solution: *raw_solution,
+				reward: T::SignedRewardBase::get(),
+				call_fee,
+			};
 
 			// insert the submission if the queue has space or it's better than the weakest
 			// eject the weakest if the queue was full
diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index d92dae6a47602..172d9fd7ffbbf 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -52,7 +52,7 @@ pub struct SignedSubmission<AccountId, Balance: HasCompact, Solution> {
 	pub raw_solution: RawSolution<Solution>,
 	/// The reward that should potentially be paid for this solution, if accepted.
 	pub reward: Balance,
-	// The estimated fee the origin paid to submit the solution.
+	// The estimated fee `who` paid to submit the solution.
 	pub call_fee: Balance,
 }
 
@@ -494,13 +494,12 @@ impl<T: Config> Pallet<T> {
 	fn finalize_signed_phase_handle_refund(
 		who: &T::AccountId,
 		deposit: BalanceOf<T>,
-		call_fee: BalanceOf<T>
+		call_fee: BalanceOf<T>,
 	) {
 		// Unreserve deposit
 		let _remaining = T::Currency::unreserve(who, deposit);
 		debug_assert!(_remaining.is_zero());
 
-		println!("{:?}=call_fee", call_fee);
 		// Refund fee
 		let _ = T::Currency::deposit_creating(who, call_fee);
 	}

From d25fa7b27f07e126ecdd7dd738f134280c2d6baf Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Thu, 10 Mar 2022 21:57:28 +0000
Subject: [PATCH 03/21] Fix benchmarks

---
 .../src/benchmarking.rs                           | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/benchmarking.rs b/frame/election-provider-multi-phase/src/benchmarking.rs
index 479afd9843386..c19eb58580121 100644
--- a/frame/election-provider-multi-phase/src/benchmarking.rs
+++ b/frame/election-provider-multi-phase/src/benchmarking.rs
@@ -227,13 +227,23 @@ frame_benchmarking::benchmarks! {
 		};
 		let deposit: BalanceOf<T> = 10u32.into();
 		let reward: BalanceOf<T> = 20u32.into();
+		let call_fee: BalanceOf<T> = 30u32.into();
 
 		assert_ok!(T::Currency::reserve(&receiver, deposit));
 		assert_eq!(T::Currency::free_balance(&receiver), initial_balance - 10u32.into());
 	}: {
-		<MultiPhase<T>>::finalize_signed_phase_accept_solution(ready, &receiver, deposit, reward)
+		<MultiPhase<T>>::finalize_signed_phase_accept_solution(
+			ready,
+			&receiver,
+			deposit,
+			reward,
+			call_fee
+		)
 	} verify {
-		assert_eq!(T::Currency::free_balance(&receiver), initial_balance + 20u32.into());
+		assert_eq!(
+			T::Currency::free_balance(&receiver),
+			initial_balance + 20u32.into() + 30u32.into()
+		);
 		assert_eq!(T::Currency::reserved_balance(&receiver), 0u32.into());
 	}
 
@@ -336,6 +346,7 @@ frame_benchmarking::benchmarks! {
 				who: account("submitters", i, SEED),
 				deposit: Default::default(),
 				reward: Default::default(),
+				call_fee: Default::default(),
 			};
 			signed_submissions.insert(signed_submission);
 		}

From c4fc0bdac36cf33f92467dcaafbd37e38897a3c2 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Thu, 10 Mar 2022 22:56:13 +0000
Subject: [PATCH 04/21] Remove reward from struct

---
 frame/election-provider-multi-phase/src/benchmarking.rs | 7 +++----
 frame/election-provider-multi-phase/src/lib.rs          | 1 -
 frame/election-provider-multi-phase/src/signed.rs       | 7 ++-----
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/benchmarking.rs b/frame/election-provider-multi-phase/src/benchmarking.rs
index c19eb58580121..32e2c6322fc9b 100644
--- a/frame/election-provider-multi-phase/src/benchmarking.rs
+++ b/frame/election-provider-multi-phase/src/benchmarking.rs
@@ -226,7 +226,8 @@ frame_benchmarking::benchmarks! {
 			compute: Default::default()
 		};
 		let deposit: BalanceOf<T> = 10u32.into();
-		let reward: BalanceOf<T> = 20u32.into();
+
+		let reward: BalanceOf<T> = T::SignedRewardBase::get();
 		let call_fee: BalanceOf<T> = 30u32.into();
 
 		assert_ok!(T::Currency::reserve(&receiver, deposit));
@@ -236,13 +237,12 @@ frame_benchmarking::benchmarks! {
 			ready,
 			&receiver,
 			deposit,
-			reward,
 			call_fee
 		)
 	} verify {
 		assert_eq!(
 			T::Currency::free_balance(&receiver),
-			initial_balance + 20u32.into() + 30u32.into()
+			initial_balance + reward + call_fee
 		);
 		assert_eq!(T::Currency::reserved_balance(&receiver), 0u32.into());
 	}
@@ -345,7 +345,6 @@ frame_benchmarking::benchmarks! {
 				raw_solution,
 				who: account("submitters", i, SEED),
 				deposit: Default::default(),
-				reward: Default::default(),
 				call_fee: Default::default(),
 			};
 			signed_submissions.insert(signed_submission);
diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
index 56d7306cc1f41..bc2417c742c14 100644
--- a/frame/election-provider-multi-phase/src/lib.rs
+++ b/frame/election-provider-multi-phase/src/lib.rs
@@ -996,7 +996,6 @@ pub mod pallet {
 				who: who.clone(),
 				deposit,
 				raw_solution: *raw_solution,
-				reward: T::SignedRewardBase::get(),
 				call_fee,
 			};
 
diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index 172d9fd7ffbbf..12f267210b02c 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -50,8 +50,6 @@ pub struct SignedSubmission<AccountId, Balance: HasCompact, Solution> {
 	pub deposit: Balance,
 	/// The raw solution itself.
 	pub raw_solution: RawSolution<Solution>,
-	/// The reward that should potentially be paid for this solution, if accepted.
-	pub reward: Balance,
 	// The estimated fee `who` paid to submit the solution.
 	pub call_fee: Balance,
 }
@@ -363,7 +361,7 @@ impl<T: Config> Pallet<T> {
 			Self::snapshot_metadata().unwrap_or_default();
 
 		while let Some(best) = all_submissions.pop_last() {
-			let SignedSubmission { raw_solution, who, deposit, reward, call_fee } = best;
+			let SignedSubmission { raw_solution, who, deposit, call_fee } = best;
 			let active_voters = raw_solution.solution.voter_count() as u32;
 			let feasibility_weight = {
 				// defensive only: at the end of signed phase, snapshot will exits.
@@ -378,7 +376,6 @@ impl<T: Config> Pallet<T> {
 						ready_solution,
 						&who,
 						deposit,
-						reward,
 						call_fee,
 					);
 					found_solution = true;
@@ -425,12 +422,12 @@ impl<T: Config> Pallet<T> {
 		ready_solution: ReadySolution<T::AccountId>,
 		who: &T::AccountId,
 		deposit: BalanceOf<T>,
-		reward: BalanceOf<T>,
 		call_fee: BalanceOf<T>,
 	) {
 		// write this ready solution.
 		<QueuedSolution<T>>::put(ready_solution);
 
+		let reward = T::SignedRewardBase::get();
 		// emit reward event
 		Self::deposit_event(crate::Event::Rewarded { account: who.clone(), value: reward });
 

From fbe67f2fb7a5b638ac7f72a53efc2fc22a846861 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Thu, 10 Mar 2022 23:27:24 +0000
Subject: [PATCH 05/21] WIP SignedMaxRefunds

---
 frame/election-provider-multi-phase/src/lib.rs    |  6 ++++++
 frame/election-provider-multi-phase/src/mock.rs   |  2 ++
 frame/election-provider-multi-phase/src/signed.rs | 11 +++++++++++
 3 files changed, 19 insertions(+)

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
index bc2417c742c14..4e25ff883cd99 100644
--- a/frame/election-provider-multi-phase/src/lib.rs
+++ b/frame/election-provider-multi-phase/src/lib.rs
@@ -628,6 +628,12 @@ pub mod pallet {
 		#[pallet::constant]
 		type SignedMaxWeight: Get<Weight>;
 
+		/// The maximum amount of unchecked solutions to refund the call fee for. If `None`, all
+		/// solutions will get a refund. Should either be `None` or a value less than
+		/// [`Self::SignedMaxSubmissions`]
+		#[pallet::constant]
+		type SignedMaxRefunds: Get<Option<u32>>;
+
 		/// Base reward for a signed solution
 		#[pallet::constant]
 		type SignedRewardBase: Get<BalanceOf<Self>>;
diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs
index 7c7034ac91a83..e60530b1b0575 100644
--- a/frame/election-provider-multi-phase/src/mock.rs
+++ b/frame/election-provider-multi-phase/src/mock.rs
@@ -252,6 +252,7 @@ parameter_types! {
 	pub static SignedPhase: BlockNumber = 10;
 	pub static UnsignedPhase: BlockNumber = 5;
 	pub static SignedMaxSubmissions: u32 = 5;
+	pub static SignedMaxRefunds: u32 = 1;
 	pub static SignedDepositBase: Balance = 5;
 	pub static SignedDepositByte: Balance = 0;
 	pub static SignedDepositWeight: Balance = 0;
@@ -404,6 +405,7 @@ impl crate::Config for Runtime {
 	type SignedDepositWeight = ();
 	type SignedMaxWeight = SignedMaxWeight;
 	type SignedMaxSubmissions = SignedMaxSubmissions;
+	type SignedMaxRefunds = SignedMaxRefunds;
 	type SlashHandler = ();
 	type RewardHandler = ();
 	type DataProvider = StakingMock;
diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index 12f267210b02c..ce5986fb040de 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -395,7 +395,18 @@ impl<T: Config> Pallet<T> {
 		// Any unprocessed solution is pointless to even consider. Feasible or malicious,
 		// they didn't end up being used. Unreserve the bonds.
 		let discarded = all_submissions.len();
+		let mut count = 0;
 		for SignedSubmission { who, deposit, call_fee, .. } in all_submissions.drain() {
+			if T::SignedMaxRefunds::get().map_or(true, |max| count < max) {
+				// Refund fee
+				let _ = T::Currency::deposit_creating(&who, call_fee);
+				count += 1;
+			}
+
+			// Unreserve deposit
+			let _remaining = T::Currency::unreserve(&who, deposit);
+			debug_assert!(_remaining.is_zero());
+
 			Self::finalize_signed_phase_handle_refund(&who, deposit, call_fee);
 			weight = weight.saturating_add(T::DbWeight::get().writes(2));
 		}

From 9e0490d7b912bdd402515ef9193495e06f9d9861 Mon Sep 17 00:00:00 2001
From: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Date: Sat, 12 Mar 2022 09:23:45 +0000
Subject: [PATCH 06/21] Apply suggestions from code review

---
 frame/election-provider-multi-phase/src/signed.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index ce5986fb040de..3a97076065ae9 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -395,12 +395,12 @@ impl<T: Config> Pallet<T> {
 		// Any unprocessed solution is pointless to even consider. Feasible or malicious,
 		// they didn't end up being used. Unreserve the bonds.
 		let discarded = all_submissions.len();
-		let mut count = 0;
+		let mut refunded = 0;
 		for SignedSubmission { who, deposit, call_fee, .. } in all_submissions.drain() {
-			if T::SignedMaxRefunds::get().map_or(true, |max| count < max) {
+			if T::SignedMaxRefunds::get().map_or(true, |max| refunded < max) {
 				// Refund fee
 				let _ = T::Currency::deposit_creating(&who, call_fee);
-				count += 1;
+				refunded += 1;
 			}
 
 			// Unreserve deposit

From 301902d02a8324c3f2791a71b6d0c255db1ae1f5 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Mon, 14 Mar 2022 13:33:39 +0000
Subject: [PATCH 07/21] Add test for ejected call_fee refunds

---
 .../election-provider-multi-phase/src/lib.rs  |  7 +-
 .../election-provider-multi-phase/src/mock.rs |  2 +-
 .../src/signed.rs                             | 66 ++++++++++++-------
 3 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
index 4e25ff883cd99..83e9083c85943 100644
--- a/frame/election-provider-multi-phase/src/lib.rs
+++ b/frame/election-provider-multi-phase/src/lib.rs
@@ -629,8 +629,7 @@ pub mod pallet {
 		type SignedMaxWeight: Get<Weight>;
 
 		/// The maximum amount of unchecked solutions to refund the call fee for. If `None`, all
-		/// solutions will get a refund. Should either be `None` or a value less than
-		/// [`Self::SignedMaxSubmissions`]
+		/// solutions, including ejected solutions, will get a refund.
 		#[pallet::constant]
 		type SignedMaxRefunds: Get<Option<u32>>;
 
@@ -1022,6 +1021,10 @@ pub mod pallet {
 			let ejected_a_solution = maybe_removed.is_some();
 			// if we had to remove the weakest solution, unreserve its deposit
 			if let Some(removed) = maybe_removed {
+				if T::SignedMaxRefunds::get().is_none() {
+					// There are no limits on how many solutions get their call fee refunded
+					let _ = T::Currency::deposit_creating(&removed.who, removed.call_fee);
+				}
 				let _remainder = T::Currency::unreserve(&removed.who, removed.deposit);
 				debug_assert!(_remainder.is_zero());
 			}
diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs
index e60530b1b0575..44ea6dd862505 100644
--- a/frame/election-provider-multi-phase/src/mock.rs
+++ b/frame/election-provider-multi-phase/src/mock.rs
@@ -252,7 +252,7 @@ parameter_types! {
 	pub static SignedPhase: BlockNumber = 10;
 	pub static UnsignedPhase: BlockNumber = 5;
 	pub static SignedMaxSubmissions: u32 = 5;
-	pub static SignedMaxRefunds: u32 = 1;
+	pub static SignedMaxRefunds: Option<u32> = Some(1);
 	pub static SignedDepositBase: Balance = 5;
 	pub static SignedDepositByte: Balance = 0;
 	pub static SignedDepositWeight: Balance = 0;
diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index ce5986fb040de..24cd911c5384d 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -395,19 +395,18 @@ impl<T: Config> Pallet<T> {
 		// Any unprocessed solution is pointless to even consider. Feasible or malicious,
 		// they didn't end up being used. Unreserve the bonds.
 		let discarded = all_submissions.len();
-		let mut count = 0;
+		let mut refund_count = 0;
+		let maybe_max_refunds =  T::SignedMaxRefunds::get();
 		for SignedSubmission { who, deposit, call_fee, .. } in all_submissions.drain() {
-			if T::SignedMaxRefunds::get().map_or(true, |max| count < max) {
+			if maybe_max_refunds.map_or(true, |max| refund_count < max) {
 				// Refund fee
 				let _ = T::Currency::deposit_creating(&who, call_fee);
-				count += 1;
+				refund_count += 1;
 			}
 
 			// Unreserve deposit
 			let _remaining = T::Currency::unreserve(&who, deposit);
 			debug_assert!(_remaining.is_zero());
-
-			Self::finalize_signed_phase_handle_refund(&who, deposit, call_fee);
 			weight = weight.saturating_add(T::DbWeight::get().writes(2));
 		}
 
@@ -442,10 +441,12 @@ impl<T: Config> Pallet<T> {
 		// emit reward event
 		Self::deposit_event(crate::Event::Rewarded { account: who.clone(), value: reward });
 
-		Self::finalize_signed_phase_handle_refund(who, deposit, call_fee);
+		// Unreserve deposit
+		let _remaining = T::Currency::unreserve(who, deposit);
+		debug_assert!(_remaining.is_zero());
 
-		// Reward.
-		let positive_imbalance = T::Currency::deposit_creating(who, reward);
+		// Reward and refund the call fee.
+		let positive_imbalance = T::Currency::deposit_creating(who, reward.saturating_add(call_fee));
 		T::RewardHandler::on_unbalanced(positive_imbalance);
 	}
 
@@ -497,20 +498,6 @@ impl<T: Config> Pallet<T> {
 			.saturating_add(len_deposit)
 			.saturating_add(weight_deposit)
 	}
-
-	/// Unreserve `who`s deposit and refund their transaction fee.
-	fn finalize_signed_phase_handle_refund(
-		who: &T::AccountId,
-		deposit: BalanceOf<T>,
-		call_fee: BalanceOf<T>,
-	) {
-		// Unreserve deposit
-		let _remaining = T::Currency::unreserve(who, deposit);
-		debug_assert!(_remaining.is_zero());
-
-		// Refund fee
-		let _ = T::Currency::deposit_creating(who, call_fee);
-	}
 }
 
 #[cfg(test)]
@@ -519,7 +506,7 @@ mod tests {
 	use crate::{
 		mock::{
 			balances, raw_solution, roll_to, ExtBuilder, MultiPhase, Origin, Runtime,
-			SignedMaxSubmissions, SignedMaxWeight,
+			SignedMaxSubmissions, SignedMaxWeight, Balances, SignedMaxRefunds
 		},
 		Error, Phase,
 	};
@@ -661,12 +648,15 @@ mod tests {
 			assert!(MultiPhase::current_phase().is_signed());
 
 			for s in 0..SignedMaxSubmissions::get() {
+				let account = 99 + s as u64;
+				Balances::make_free_balance_be(&account, 100);
 				// score is always getting better
 				let solution = RawSolution {
 					score: ElectionScore { minimal_stake: (5 + s).into(), ..Default::default() },
 					..Default::default()
 				};
-				assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
+				assert_ok!(MultiPhase::submit(Origin::signed(account), Box::new(solution)));
+				assert_eq!(balances(&account), (95, 5));
 			}
 
 			assert_eq!(
@@ -682,7 +672,7 @@ mod tests {
 				score: ElectionScore { minimal_stake: 20, ..Default::default() },
 				..Default::default()
 			};
-			assert_ok!(MultiPhase::submit(Origin::signed(99), Box::new(solution)));
+			assert_ok!(MultiPhase::submit(Origin::signed(999), Box::new(solution)));
 
 			// the one with score 5 was rejected, the new one inserted.
 			assert_eq!(
@@ -692,6 +682,32 @@ mod tests {
 					.collect::<Vec<_>>(),
 				vec![6, 7, 8, 9, 20]
 			);
+
+			// the submitter of the ejected solution does *not* get a call fee refund
+			assert_eq!(balances(&(99 + 0)), (100, 0));
+
+			// set the max refunds to None
+			SignedMaxRefunds::set(None);
+			assert!(<Runtime as Config>::SignedMaxRefunds::get().is_none());
+
+			// submit another solution to force an ejection
+			let solution = RawSolution {
+				score: ElectionScore { minimal_stake: 21, ..Default::default() },
+				..Default::default()
+			};
+			assert_ok!(MultiPhase::submit(Origin::signed(999), Box::new(solution)));
+
+			// the one with score 6 was rejected, the new one inserted.
+			assert_eq!(
+				MultiPhase::signed_submissions()
+					.iter()
+					.map(|s| s.raw_solution.score.minimal_stake)
+					.collect::<Vec<_>>(),
+				vec![7, 8, 9, 20, 21]
+			);
+
+			// the submitter of the ejected solution gets a call fee refund
+			assert_eq!(balances(&(99 + 1)), (100 + 8, 0));
 		})
 	}
 

From 13b88ba67f98fb777fdc61388eac392fa28fb412 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Mon, 14 Mar 2022 14:14:03 +0000
Subject: [PATCH 08/21] Add test for number of calls refunded

---
 .../src/signed.rs                             | 47 +++++++++++++++++--
 1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index 24cd911c5384d..b4841c00b4024 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -396,7 +396,7 @@ impl<T: Config> Pallet<T> {
 		// they didn't end up being used. Unreserve the bonds.
 		let discarded = all_submissions.len();
 		let mut refund_count = 0;
-		let maybe_max_refunds =  T::SignedMaxRefunds::get();
+		let maybe_max_refunds = T::SignedMaxRefunds::get();
 		for SignedSubmission { who, deposit, call_fee, .. } in all_submissions.drain() {
 			if maybe_max_refunds.map_or(true, |max| refund_count < max) {
 				// Refund fee
@@ -446,7 +446,8 @@ impl<T: Config> Pallet<T> {
 		debug_assert!(_remaining.is_zero());
 
 		// Reward and refund the call fee.
-		let positive_imbalance = T::Currency::deposit_creating(who, reward.saturating_add(call_fee));
+		let positive_imbalance =
+			T::Currency::deposit_creating(who, reward.saturating_add(call_fee));
 		T::RewardHandler::on_unbalanced(positive_imbalance);
 	}
 
@@ -505,8 +506,8 @@ mod tests {
 	use super::*;
 	use crate::{
 		mock::{
-			balances, raw_solution, roll_to, ExtBuilder, MultiPhase, Origin, Runtime,
-			SignedMaxSubmissions, SignedMaxWeight, Balances, SignedMaxRefunds
+			balances, raw_solution, roll_to, Balances, ExtBuilder, MultiPhase, Origin, Runtime,
+			SignedMaxRefunds, SignedMaxSubmissions, SignedMaxWeight,
 		},
 		Error, Phase,
 	};
@@ -641,6 +642,44 @@ mod tests {
 		})
 	}
 
+	#[test]
+	fn call_fee_refund_is_limited_by_signed_max_refunds() {
+		ExtBuilder::default().build_and_execute(|| {
+			roll_to(15);
+			assert!(MultiPhase::current_phase().is_signed());
+			assert_eq!(SignedMaxRefunds::get(), Some(1));
+			assert!(SignedMaxSubmissions::get() > 2);
+
+			for s in 0..SignedMaxSubmissions::get() {
+				let account = 99 + s as u64;
+				Balances::make_free_balance_be(&account, 100);
+				// score is always decreasing better
+				let mut solution = raw_solution();
+				solution.score.minimal_stake -= s as u128;
+
+				assert_ok!(MultiPhase::submit(Origin::signed(account), Box::new(solution)));
+				assert_eq!(balances(&account), (95, 5));
+			}
+
+			assert!(MultiPhase::finalize_signed_phase());
+
+			for s in 0..SignedMaxSubmissions::get() {
+				let account = 99 + s as u64;
+				// lower accounts have higher scores
+				if s == 0 {
+					// winning solution always gets call fee + reward
+					assert_eq!(balances(&account), (100 + 8 + 7, 0))
+				} else if s == 1 {
+					// 1 runner up gets there call fee refunded
+					assert_eq!(balances(&account), (100 + 8, 0))
+				} else {
+					// all other solutions don't get a call fee refunds
+					assert_eq!(balances(&account), (100, 0));
+				}
+			}
+		});
+	}
+
 	#[test]
 	fn weakest_is_removed_if_better_provided() {
 		ExtBuilder::default().build_and_execute(|| {

From dce1792e540cc885a648307efb63f44cff786488 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Mon, 14 Mar 2022 14:22:55 +0000
Subject: [PATCH 09/21] Account for read op in mutate

---
 frame/election-provider-multi-phase/src/signed.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index b4841c00b4024..2c38b30f262c1 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -407,7 +407,7 @@ impl<T: Config> Pallet<T> {
 			// Unreserve deposit
 			let _remaining = T::Currency::unreserve(&who, deposit);
 			debug_assert!(_remaining.is_zero());
-			weight = weight.saturating_add(T::DbWeight::get().writes(2));
+			weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 2));
 		}
 
 		debug_assert!(!SignedSubmissionIndices::<T>::exists());

From 7f7119982dbb5f43a32ee325348ceb097c2de2db Mon Sep 17 00:00:00 2001
From: Zeke Mostov <z.mostov@gmail.com>
Date: Mon, 14 Mar 2022 14:25:52 +0000
Subject: [PATCH 10/21] Apply suggestions from code review

---
 frame/election-provider-multi-phase/src/signed.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index 2c38b30f262c1..47220fb6ea863 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -441,7 +441,7 @@ impl<T: Config> Pallet<T> {
 		// emit reward event
 		Self::deposit_event(crate::Event::Rewarded { account: who.clone(), value: reward });
 
-		// Unreserve deposit
+		// Unreserve deposit.
 		let _remaining = T::Currency::unreserve(who, deposit);
 		debug_assert!(_remaining.is_zero());
 
@@ -653,7 +653,7 @@ mod tests {
 			for s in 0..SignedMaxSubmissions::get() {
 				let account = 99 + s as u64;
 				Balances::make_free_balance_be(&account, 100);
-				// score is always decreasing better
+				// score is always decreasing
 				let mut solution = raw_solution();
 				solution.score.minimal_stake -= s as u128;
 
@@ -670,10 +670,10 @@ mod tests {
 					// winning solution always gets call fee + reward
 					assert_eq!(balances(&account), (100 + 8 + 7, 0))
 				} else if s == 1 {
-					// 1 runner up gets there call fee refunded
+					// 1 runner up gets their call fee refunded
 					assert_eq!(balances(&account), (100 + 8, 0))
 				} else {
-					// all other solutions don't get a call fee refunds
+					// all other solutions don't get a call fee refund
 					assert_eq!(balances(&account), (100, 0));
 				}
 			}

From cc5da5eb69f6533db52568520a1b77a3c087e922 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Mon, 14 Mar 2022 16:50:45 +0000
Subject: [PATCH 11/21] Add to node runtime

---
 Cargo.lock                  | 2 +-
 bin/node/runtime/src/lib.rs | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Cargo.lock b/Cargo.lock
index 9f4d2f21cf9c1..82dea83813d0b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2196,7 +2196,7 @@ dependencies = [
 name = "frame-election-solution-type-fuzzer"
 version = "2.0.0-alpha.5"
 dependencies = [
- "clap 3.0.7",
+ "clap 3.1.6",
  "frame-election-provider-solution-type",
  "honggfuzz",
  "parity-scale-codec",
diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
index 20b718e2fa8f7..cbf18a17c6ebd 100644
--- a/bin/node/runtime/src/lib.rs
+++ b/bin/node/runtime/src/lib.rs
@@ -599,6 +599,7 @@ frame_election_provider_support::generate_solution_type!(
 
 parameter_types! {
 	pub MaxNominations: u32 = <NposSolution16 as sp_npos_elections::NposSolution>::LIMIT as u32;
+	pub SignedMaxRefunds: Option<u32> = Some(10);
 }
 
 /// The numbers configured here could always be more than the the maximum limits of staking pallet
@@ -661,6 +662,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
 	type SignedRewardBase = SignedRewardBase;
 	type SignedDepositBase = SignedDepositBase;
 	type SignedDepositByte = SignedDepositByte;
+	type SignedMaxRefunds = SignedMaxRefunds;
 	type SignedDepositWeight = ();
 	type SignedMaxWeight = MinerMaxWeight;
 	type SlashHandler = (); // burn slashes

From 0f20ee8870eb7bae53dac19f9ef4de5d70a3b40a Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Tue, 15 Mar 2022 16:30:30 +0000
Subject: [PATCH 12/21] Don't refund ejected solutions

---
 bin/node/runtime/src/lib.rs                   |  2 +-
 .../election-provider-multi-phase/src/lib.rs  |  6 +---
 .../election-provider-multi-phase/src/mock.rs |  2 +-
 .../src/signed.rs                             | 29 ++-----------------
 4 files changed, 6 insertions(+), 33 deletions(-)

diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
index cbf18a17c6ebd..b17372b8e60a8 100644
--- a/bin/node/runtime/src/lib.rs
+++ b/bin/node/runtime/src/lib.rs
@@ -599,7 +599,7 @@ frame_election_provider_support::generate_solution_type!(
 
 parameter_types! {
 	pub MaxNominations: u32 = <NposSolution16 as sp_npos_elections::NposSolution>::LIMIT as u32;
-	pub SignedMaxRefunds: Option<u32> = Some(10);
+	pub SignedMaxRefunds: u32 = 10;
 }
 
 /// The numbers configured here could always be more than the the maximum limits of staking pallet
diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
index 83e9083c85943..e54ad21149d41 100644
--- a/frame/election-provider-multi-phase/src/lib.rs
+++ b/frame/election-provider-multi-phase/src/lib.rs
@@ -631,7 +631,7 @@ pub mod pallet {
 		/// The maximum amount of unchecked solutions to refund the call fee for. If `None`, all
 		/// solutions, including ejected solutions, will get a refund.
 		#[pallet::constant]
-		type SignedMaxRefunds: Get<Option<u32>>;
+		type SignedMaxRefunds: Get<u32>;
 
 		/// Base reward for a signed solution
 		#[pallet::constant]
@@ -1021,10 +1021,6 @@ pub mod pallet {
 			let ejected_a_solution = maybe_removed.is_some();
 			// if we had to remove the weakest solution, unreserve its deposit
 			if let Some(removed) = maybe_removed {
-				if T::SignedMaxRefunds::get().is_none() {
-					// There are no limits on how many solutions get their call fee refunded
-					let _ = T::Currency::deposit_creating(&removed.who, removed.call_fee);
-				}
 				let _remainder = T::Currency::unreserve(&removed.who, removed.deposit);
 				debug_assert!(_remainder.is_zero());
 			}
diff --git a/frame/election-provider-multi-phase/src/mock.rs b/frame/election-provider-multi-phase/src/mock.rs
index 7f8fb0a193e11..f6fa1dead1db5 100644
--- a/frame/election-provider-multi-phase/src/mock.rs
+++ b/frame/election-provider-multi-phase/src/mock.rs
@@ -252,7 +252,7 @@ parameter_types! {
 	pub static SignedPhase: BlockNumber = 10;
 	pub static UnsignedPhase: BlockNumber = 5;
 	pub static SignedMaxSubmissions: u32 = 5;
-	pub static SignedMaxRefunds: Option<u32> = Some(1);
+	pub static SignedMaxRefunds: u32 = 1;
 	pub static SignedDepositBase: Balance = 5;
 	pub static SignedDepositByte: Balance = 0;
 	pub static SignedDepositWeight: Balance = 0;
diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index 47220fb6ea863..d61951f1e738f 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -396,9 +396,9 @@ impl<T: Config> Pallet<T> {
 		// they didn't end up being used. Unreserve the bonds.
 		let discarded = all_submissions.len();
 		let mut refund_count = 0;
-		let maybe_max_refunds = T::SignedMaxRefunds::get();
+		let max_refunds = T::SignedMaxRefunds::get();
 		for SignedSubmission { who, deposit, call_fee, .. } in all_submissions.drain() {
-			if maybe_max_refunds.map_or(true, |max| refund_count < max) {
+			if refund_count < max_refunds {
 				// Refund fee
 				let _ = T::Currency::deposit_creating(&who, call_fee);
 				refund_count += 1;
@@ -647,7 +647,7 @@ mod tests {
 		ExtBuilder::default().build_and_execute(|| {
 			roll_to(15);
 			assert!(MultiPhase::current_phase().is_signed());
-			assert_eq!(SignedMaxRefunds::get(), Some(1));
+			assert_eq!(SignedMaxRefunds::get(), 1);
 			assert!(SignedMaxSubmissions::get() > 2);
 
 			for s in 0..SignedMaxSubmissions::get() {
@@ -724,29 +724,6 @@ mod tests {
 
 			// the submitter of the ejected solution does *not* get a call fee refund
 			assert_eq!(balances(&(99 + 0)), (100, 0));
-
-			// set the max refunds to None
-			SignedMaxRefunds::set(None);
-			assert!(<Runtime as Config>::SignedMaxRefunds::get().is_none());
-
-			// submit another solution to force an ejection
-			let solution = RawSolution {
-				score: ElectionScore { minimal_stake: 21, ..Default::default() },
-				..Default::default()
-			};
-			assert_ok!(MultiPhase::submit(Origin::signed(999), Box::new(solution)));
-
-			// the one with score 6 was rejected, the new one inserted.
-			assert_eq!(
-				MultiPhase::signed_submissions()
-					.iter()
-					.map(|s| s.raw_solution.score.minimal_stake)
-					.collect::<Vec<_>>(),
-				vec![7, 8, 9, 20, 21]
-			);
-
-			// the submitter of the ejected solution gets a call fee refund
-			assert_eq!(balances(&(99 + 1)), (100 + 8, 0));
 		})
 	}
 

From d291d56365929a11cb320a39eabec4410bd88787 Mon Sep 17 00:00:00 2001
From: Zeke Mostov <z.mostov@gmail.com>
Date: Fri, 18 Mar 2022 14:11:39 +0000
Subject: [PATCH 13/21] Update frame/election-provider-multi-phase/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
---
 frame/election-provider-multi-phase/src/lib.rs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
index e54ad21149d41..056a7d5921bc7 100644
--- a/frame/election-provider-multi-phase/src/lib.rs
+++ b/frame/election-provider-multi-phase/src/lib.rs
@@ -628,8 +628,7 @@ pub mod pallet {
 		#[pallet::constant]
 		type SignedMaxWeight: Get<Weight>;
 
-		/// The maximum amount of unchecked solutions to refund the call fee for. If `None`, all
-		/// solutions, including ejected solutions, will get a refund.
+		/// The maximum amount of unchecked solutions to refund the call fee for.
 		#[pallet::constant]
 		type SignedMaxRefunds: Get<u32>;
 

From e60b055d4432b3636e8a6d3b2a3aca82e35891fb Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Fri, 18 Mar 2022 14:28:44 +0000
Subject: [PATCH 14/21] Inegrity test SignedMaxRefunds

---
 frame/election-provider-multi-phase/src/lib.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
index e54ad21149d41..79ee8fe7dbf06 100644
--- a/frame/election-provider-multi-phase/src/lib.rs
+++ b/frame/election-provider-multi-phase/src/lib.rs
@@ -852,6 +852,11 @@ pub mod pallet {
 				<T::DataProvider as ElectionDataProvider>::MaxVotesPerVoter::get(),
 				<SolutionOf<T> as NposSolution>::LIMIT as u32,
 			);
+
+			// While it won't cause any failures, setting `SignedMaxRefunds` gt
+			// `SignedMaxSubmissions` is a red flag that the developer does not understand how to
+			// configure this pallet.
+			assert!(T::SignedMaxSubmissions::get() >= T::SignedMaxRefunds::get());
 		}
 	}
 

From 4867e2feef6dd011c24a76863b9b4c3d1dce0f23 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Fri, 18 Mar 2022 15:15:56 +0000
Subject: [PATCH 15/21] Use reward handle to refund call fee

---
 frame/election-provider-multi-phase/src/signed.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index 0679d9b97bd9b..ec449e61b0211 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -401,7 +401,8 @@ impl<T: Config> Pallet<T> {
 		for SignedSubmission { who, deposit, call_fee, .. } in all_submissions.drain() {
 			if refund_count < max_refunds {
 				// Refund fee
-				let _ = T::Currency::deposit_creating(&who, call_fee);
+				let positive_imbalance = T::Currency::deposit_creating(&who, call_fee);
+				T::RewardHandler::on_unbalanced(positive_imbalance);
 				refund_count += 1;
 			}
 

From 9f8a136baa861cbb3a4480294c64012556efa239 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Thu, 7 Apr 2022 18:47:48 -0400
Subject: [PATCH 16/21] Fix node runtime build

---
 bin/node/runtime/src/lib.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
index 3759e70dbe67a..dd43112204595 100644
--- a/bin/node/runtime/src/lib.rs
+++ b/bin/node/runtime/src/lib.rs
@@ -600,7 +600,8 @@ frame_election_provider_support::generate_solution_type!(
 );
 
 parameter_types! {
-	pub MaxNominations: u32 = <NposSolution16 as sp_npos_elections::NposSolution>::LIMIT as u32;
+	pub MaxNominations: u32
+		= <NposSolution16 as frame_election_provider_support::NposSolution>::LIMIT as u32;
 	pub SignedMaxRefunds: u32 = 10;
 }
 

From 07d8c9d50cf5b8f92d04e134b8504167313b8d44 Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Fri, 8 Apr 2022 14:56:29 -0400
Subject: [PATCH 17/21] Drain in order of submission

---
 .../election-provider-multi-phase/src/lib.rs  |  2 +-
 .../src/signed.rs                             | 33 ++++++++++++++-----
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs
index 1a70f5886aab8..26376705aba61 100644
--- a/frame/election-provider-multi-phase/src/lib.rs
+++ b/frame/election-provider-multi-phase/src/lib.rs
@@ -242,7 +242,7 @@ use frame_support::{
 use frame_system::{ensure_none, offchain::SendTransactionTypes};
 use scale_info::TypeInfo;
 use sp_arithmetic::{
-	traits::{Bounded, CheckedAdd, Saturating, Zero},
+	traits::{Bounded, CheckedAdd, Zero},
 	UpperOf,
 };
 use sp_npos_elections::{
diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index ec449e61b0211..d4a5689e4bf39 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -235,20 +235,34 @@ impl<T: Config> SignedSubmissions<T> {
 	}
 
 	/// Empty the set of signed submissions, returning an iterator of signed submissions in
-	/// arbitrary order.
+	/// order of submission.
 	///
 	/// Note that if the iterator is dropped without consuming all elements, not all may be removed
 	/// from the underlying `SignedSubmissionsMap`, putting the storages into an invalid state.
 	///
 	/// Note that, like `put`, this function consumes `Self` and modifies storage.
-	fn drain(mut self) -> impl Iterator<Item = SignedSubmissionOf<T>> {
+	fn drain_submitted_order(mut self) -> impl Iterator<Item = SignedSubmissionOf<T>> {
+		let mut keys = SignedSubmissionsMap::<T>::iter_keys()
+			.filter(|k| {
+				if self.deletion_overlay.contains(k) {
+					// Remove submissions that should be deleted.
+					SignedSubmissionsMap::<T>::remove(k);
+					false
+				} else {
+					true
+				}
+			})
+			.chain(self.insertion_overlay.keys().map(|n| *n))
+			.collect::<Vec<_>>();
+		keys.sort();
+
 		SignedSubmissionIndices::<T>::kill();
 		SignedSubmissionNextIndex::<T>::kill();
-		let insertion_overlay = sp_std::mem::take(&mut self.insertion_overlay);
-		SignedSubmissionsMap::<T>::drain()
-			.filter(move |(k, _v)| !self.deletion_overlay.contains(k))
-			.map(|(_k, v)| v)
-			.chain(insertion_overlay.into_iter().map(|(_k, v)| v))
+		let _ = sp_std::mem::take(&mut self.deletion_overlay);
+
+		keys.into_iter().filter_map(move |index| {
+			SignedSubmissionsMap::<T>::take(index).or_else(|| self.insertion_overlay.remove(&index))
+		})
 	}
 
 	/// Decode the length of the signed submissions without actually reading the entire struct into
@@ -398,7 +412,10 @@ impl<T: Config> Pallet<T> {
 		let discarded = all_submissions.len();
 		let mut refund_count = 0;
 		let max_refunds = T::SignedMaxRefunds::get();
-		for SignedSubmission { who, deposit, call_fee, .. } in all_submissions.drain() {
+
+		for SignedSubmission { who, deposit, call_fee, .. } in
+			all_submissions.drain_submitted_order()
+		{
 			if refund_count < max_refunds {
 				// Refund fee
 				let positive_imbalance = T::Currency::deposit_creating(&who, call_fee);

From 1cf7847d2f9cf458b2c26acdd2be6ce45c946bc0 Mon Sep 17 00:00:00 2001
From: Zeke Mostov <z.mostov@gmail.com>
Date: Mon, 11 Apr 2022 16:45:02 -0400
Subject: [PATCH 18/21] Update
 frame/election-provider-multi-phase/src/signed.rs

---
 frame/election-provider-multi-phase/src/signed.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index d4a5689e4bf39..ee64254d6e52b 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -258,7 +258,6 @@ impl<T: Config> SignedSubmissions<T> {
 
 		SignedSubmissionIndices::<T>::kill();
 		SignedSubmissionNextIndex::<T>::kill();
-		let _ = sp_std::mem::take(&mut self.deletion_overlay);
 
 		keys.into_iter().filter_map(move |index| {
 			SignedSubmissionsMap::<T>::take(index).or_else(|| self.insertion_overlay.remove(&index))

From 9dd09743b929ce488acb6a66177c1dec3b374a1c Mon Sep 17 00:00:00 2001
From: emostov <32168567+emostov@users.noreply.github.com>
Date: Wed, 13 Apr 2022 12:38:16 -0700
Subject: [PATCH 19/21] save

---
 bin/node/runtime/src/lib.rs                       | 2 +-
 frame/election-provider-multi-phase/src/signed.rs | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
index 6617a3aaf3715..c1eb85ee22b5f 100644
--- a/bin/node/runtime/src/lib.rs
+++ b/bin/node/runtime/src/lib.rs
@@ -672,7 +672,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
 	type SignedRewardBase = SignedRewardBase;
 	type SignedDepositBase = SignedDepositBase;
 	type SignedDepositByte = SignedDepositByte;
-	type SignedMaxRefunds = SignedMaxRefunds;
+	type SignedMaxRefunds = ConstU32<3>;
 	type SignedDepositWeight = ();
 	type SignedMaxWeight = MinerMaxWeight;
 	type SlashHandler = (); // burn slashes
diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index ee64254d6e52b..074e967e68acf 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -38,6 +38,7 @@ use sp_std::{
 	cmp::Ordering,
 	collections::{btree_map::BTreeMap, btree_set::BTreeSet},
 	ops::Deref,
+	vec::Vec,
 };
 
 /// A raw, unchecked signed submission.

From 9482b3fe77d26c359d0ae3c655767b1fa79895a3 Mon Sep 17 00:00:00 2001
From: Zeke Mostov <z.mostov@gmail.com>
Date: Sat, 16 Apr 2022 16:16:22 -0700
Subject: [PATCH 20/21] Update
 frame/election-provider-multi-phase/src/signed.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
---
 frame/election-provider-multi-phase/src/signed.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index 074e967e68acf..e481d0740b088 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -253,7 +253,7 @@ impl<T: Config> SignedSubmissions<T> {
 					true
 				}
 			})
-			.chain(self.insertion_overlay.keys().map(|n| *n))
+			.chain(self.insertion_overlay.keys().copied()
 			.collect::<Vec<_>>();
 		keys.sort();
 

From 1976d4e9f9cfd68305d5332f564b9db6d4e14584 Mon Sep 17 00:00:00 2001
From: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Date: Sun, 17 Apr 2022 10:58:31 +0200
Subject: [PATCH 21/21] Update
 frame/election-provider-multi-phase/src/signed.rs

---
 frame/election-provider-multi-phase/src/signed.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/frame/election-provider-multi-phase/src/signed.rs b/frame/election-provider-multi-phase/src/signed.rs
index e481d0740b088..321d37714c345 100644
--- a/frame/election-provider-multi-phase/src/signed.rs
+++ b/frame/election-provider-multi-phase/src/signed.rs
@@ -253,7 +253,7 @@ impl<T: Config> SignedSubmissions<T> {
 					true
 				}
 			})
-			.chain(self.insertion_overlay.keys().copied()
+			.chain(self.insertion_overlay.keys().copied())
 			.collect::<Vec<_>>();
 		keys.sort();