fix epmb solution duplicate issue + add remote mining apparatus to epm

prdoc/pr_8585.prdoc

@@ -0,0 +1,16 @@
+title: fix epmb solution duplicate issue + add remote mining apparatus to epm
+doc:
+- audience: Runtime Dev
+  description: |
+    Prevents the NPoS election process from accepting duplicate voters and targets.
+crates:
+- name: pallet-election-provider-multi-block
+  bump: patch
+- name: pallet-election-provider-multi-phase
+  bump: patch
+- name: frame-election-provider-solution-type
+  bump: patch
+- name: frame-election-provider-support
+  bump: patch
+- name: sp-npos-elections
+  bump: patch

substrate/frame/election-provider-multi-block/src/unsigned/miner.rs

@@ -1036,7 +1036,7 @@ mod trimming {
 						(40, Support { total: 40, voters: vec![(40, 40)] })
 					],
 					vec![
-						(30, Support { total: 11, voters: vec![(7, 7), (5, 2), (6, 2)] }),
+						(30, Support { total: 11, voters: vec![(5, 2), (6, 2), (7, 7)] }),
 						(40, Support { total: 7, voters: vec![(5, 3), (6, 4)] })
 					],
 					vec![(40, Support { total: 9, voters: vec![(2, 2), (3, 3), (4, 4)] })]
@@ -1080,7 +1080,7 @@ mod trimming {
 					// page only.
 					vec![(40, Support { total: 40, voters: vec![(40, 40)] })],
 					vec![
-						(30, Support { total: 11, voters: vec![(7, 7), (5, 2), (6, 2)] }),
+						(30, Support { total: 11, voters: vec![(5, 2), (6, 2), (7, 7)] }),
 						(40, Support { total: 7, voters: vec![(5, 3), (6, 4)] })
 					],
 					vec![(40, Support { total: 9, voters: vec![(2, 2), (3, 3), (4, 4)] })]
@@ -1122,7 +1122,7 @@ mod trimming {
 				vec![
 					vec![],
 					vec![
-						(30, Support { total: 11, voters: vec![(7, 7), (5, 2), (6, 2)] }),
+						(30, Support { total: 11, voters: vec![(5, 2), (6, 2), (7, 7)] }),
 						(40, Support { total: 7, voters: vec![(5, 3), (6, 4)] })
 					],
 					vec![(40, Support { total: 9, voters: vec![(2, 2), (3, 3), (4, 4)] })]
@@ -1160,11 +1160,11 @@ mod trimming {
 			assert!(VerifierPallet::queued_score().is_some());
 
 			assert_eq!(
-				dbg!(supports),
+				supports,
 				vec![
 					vec![],
 					vec![
-						(30, Support { total: 9, voters: vec![(7, 7), (6, 2)] }),
+						(30, Support { total: 9, voters: vec![(6, 2), (7, 7)] }),
 						(40, Support { total: 4, voters: vec![(6, 4)] })
 					],
 					vec![(40, Support { total: 9, voters: vec![(2, 2), (3, 3), (4, 4)] })]
@@ -1203,7 +1203,7 @@ mod trimming {
 						(40, Support { total: 40, voters: vec![(40, 40)] })
 					],
 					vec![
-						(30, Support { total: 9, voters: vec![(7, 7), (6, 2)] }),
+						(30, Support { total: 9, voters: vec![(6, 2), (7, 7)] }),
 						(40, Support { total: 9, voters: vec![(5, 5), (6, 4)] }) /* notice how
 						                                                          * 5's stake is
 						                                                          * re-distributed
@@ -1287,7 +1287,7 @@ mod trimming {
 						(40, Support { total: 40, voters: vec![(40, 40)] })
 					],
 					vec![
-						(30, Support { total: 14, voters: vec![(5, 5), (7, 7), (6, 2)] }),
+						(30, Support { total: 14, voters: vec![(5, 5), (6, 2), (7, 7)] }),
 						(40, Support { total: 4, voters: vec![(6, 4)] })
 					],
 					vec![(40, Support { total: 7, voters: vec![(3, 3), (4, 4)] })]
@@ -1412,12 +1412,12 @@ mod base_miner {
 			assert_eq!(
 				supports,
 				vec![vec![
-					(10, Support { total: 30, voters: vec![(1, 10), (8, 10), (4, 5), (5, 5)] }),
+					(10, Support { total: 30, voters: vec![(1, 10), (4, 5), (5, 5), (8, 10)] }),
 					(
 						40,
 						Support {
 							total: 40,
-							voters: vec![(2, 10), (3, 10), (6, 10), (4, 5), (5, 5)]
+							voters: vec![(2, 10), (3, 10), (4, 5), (5, 5), (6, 10)]
 						}
 					)
 				]]
@@ -1472,7 +1472,7 @@ mod base_miner {
 						// voter 6 (index 1) is backing 40 (index 3).
 						// voter 8 (index 3) is backing 10 (index 0)
 						votes1: vec![(1, 3), (3, 0)],
-						// voter 5 (index 0) is backing 40 (index 10) and 10 (index 0)
+						// voter 5 (index 0) is backing 40 (index 3) and 10 (index 0)
 						votes2: vec![(0, [(0, PerU16::from_parts(32768))], 3)],
 						..Default::default()
 					},
@@ -1500,8 +1500,8 @@ mod base_miner {
 				vec![
 					// page0, supports from voters 5, 6, 7, 8
 					vec![
-						(10, Support { total: 15, voters: vec![(8, 10), (5, 5)] }),
-						(40, Support { total: 15, voters: vec![(6, 10), (5, 5)] })
+						(10, Support { total: 15, voters: vec![(5, 5), (8, 10)] }),
+						(40, Support { total: 15, voters: vec![(5, 5), (6, 10)] })
 					],
 					// page1 supports from voters 1, 2, 3, 4
 					vec![
@@ -1591,13 +1591,13 @@ mod base_miner {
 					],
 					// page 1: 5, 6, 7, 8
 					vec![
-						(30, Support { total: 20, voters: vec![(7, 10), (5, 5), (6, 5)] }),
+						(30, Support { total: 20, voters: vec![(5, 5), (6, 5), (7, 10)] }),
 						(40, Support { total: 10, voters: vec![(5, 5), (6, 5)] })
 					],
 					// page 2: 1, 2, 3, 4
 					vec![
 						(30, Support { total: 5, voters: vec![(2, 5)] }),
-						(40, Support { total: 25, voters: vec![(3, 10), (4, 10), (2, 5)] })
+						(40, Support { total: 25, voters: vec![(2, 5), (3, 10), (4, 10)] })
 					]
 				]
 				.try_from_unbounded_paged()
@@ -1761,8 +1761,8 @@ mod base_miner {
 					vec![],
 					// supports from voters 5, 6, 7, 8
 					vec![
-						(10, Support { total: 15, voters: vec![(8, 10), (5, 5)] }),
-						(40, Support { total: 15, voters: vec![(6, 10), (5, 5)] })
+						(10, Support { total: 15, voters: vec![(5, 5), (8, 10)] }),
+						(40, Support { total: 15, voters: vec![(5, 5), (6, 10)] })
 					],
 					// supports from voters 1, 2, 3, 4
 					vec![

substrate/frame/election-provider-multi-block/src/verifier/impls.rs

@@ -898,7 +898,8 @@ pub fn feasibility_check_page_inner_with_snapshot<T: MinerConfig>(
 	let voter_index = helpers::voter_index_fn_usize::<T>(&cache);
 
 	// Then convert solution -> assignment. This will fail if any of the indices are
-	// gibberish.
+	// gibberish. It will also ensure each assignemnt (voter) is unique, and all targets within it
+	// are unique.
 	let assignments = partial_solution
 		.into_assignment(voter_at, target_at)
 		.map_err::<FeasibilityError, _>(Into::into)?;

substrate/frame/election-provider-multi-block/src/verifier/tests.rs

@@ -28,7 +28,7 @@ use frame_election_provider_support::Support;
 use frame_support::{assert_noop, assert_ok};
 use sp_core::bounded_vec;
 use sp_npos_elections::ElectionScore;
-use sp_runtime::{traits::Bounded, Perbill};
+use sp_runtime::{traits::Bounded, PerU16, Perbill};
 
 mod feasibility_check {
 	use super::*;
@@ -189,6 +189,52 @@ mod feasibility_check {
 			})
 	}
 
+	#[test]
+	fn prevents_duplicate_voter_index() {
+		ExtBuilder::verifier().pages(1).build_and_execute(|| {
+			roll_to_snapshot_created();
+
+			// let's build a manual, bogus solution with duplicate voters, on top of page 0 of
+			// snapshot (see `mock/staking.rs`).
+			let faulty_page = TestNposSolution {
+				// voter index 0 is giving 100% of stake to target index 0
+				votes1: vec![(0, 0)],
+				// and again 50% to target index 0 and target index 1. Both votes are "valid",
+				// as in they are in the snapshot.
+				votes2: vec![(0, [(0, PerU16::from_percent(50))], 1)],
+				..Default::default()
+			};
+
+			assert_noop!(
+				VerifierPallet::feasibility_check_page_inner(faulty_page, 0),
+				FeasibilityError::NposElection(
+					frame_election_provider_support::Error::DuplicateVoter
+				),
+			);
+		});
+	}
+
+	#[test]
+	fn prevents_duplicate_target_index() {
+		ExtBuilder::verifier().pages(1).build_and_execute(|| {
+			roll_to_snapshot_created();
+
+			// A bad solution with duplicate targets for a single voter in votes2.
+			let faulty_page = TestNposSolution {
+				// 50% to 0, and then the rest to 0 again, not valid.
+				votes2: vec![(0, [(0, PerU16::from_percent(50))], 0)],
+				..Default::default()
+			};
+
+			assert_noop!(
+				VerifierPallet::feasibility_check_page_inner(faulty_page, 0),
+				FeasibilityError::NposElection(
+					frame_election_provider_support::Error::DuplicateTarget
+				),
+			);
+		});
+	}
+
 	#[test]
 	fn heuristic_max_backers_per_winner_per_page() {
 		ExtBuilder::verifier().max_backers_per_winner(2).build_and_execute(|| {

substrate/frame/election-provider-multi-phase/Cargo.toml

@@ -34,6 +34,11 @@ frame-benchmarking = { optional = true, workspace = true }
 rand = { features = ["alloc", "small_rng"], optional = true, workspace = true }
 strum = { features = ["derive"], optional = true, workspace = true }
 
+# optional for remote testing
+hex = { workspace = true, default-features = true, optional = true }
+remote-externalities = { workspace = true, default-features = true, optional = true }
+tokio = { features = ["macros"], workspace = true, default-features = true, optional = true }
+
 [dev-dependencies]
 frame-benchmarking = { workspace = true, default-features = true }
 pallet-balances = { workspace = true, default-features = true }
@@ -42,7 +47,9 @@ rand = { workspace = true, default-features = true }
 sp-npos-elections = { workspace = true }
 sp-tracing = { workspace = true, default-features = true }
 
+
 [features]
+remote-mining = ["hex", "remote-externalities", "tokio"]
 default = ["std"]
 std = [
 	"codec/std",

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

@@ -279,6 +279,8 @@ use sp_runtime::TryRuntimeError;
 mod benchmarking;
 #[cfg(test)]
 mod mock;
+#[cfg(all(test, feature = "remote-mining"))]
+mod remote_mining;
 #[macro_use]
 pub mod helpers;
 
@@ -489,9 +491,9 @@ where
 /// These are stored together because they are often accessed together.
 #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, Default, TypeInfo)]
 #[scale_info(skip_type_params(T))]
-pub struct RoundSnapshot<AccountId, DataProvider> {
+pub struct RoundSnapshot<AccountId, VoterType> {
 	/// All of the voters.
-	pub voters: Vec<DataProvider>,
+	pub voters: Vec<VoterType>,
 	/// All of the targets.
 	pub targets: Vec<AccountId>,
 }