[stable2503] Backport #8585

docs/sdk/src/guides/your_first_node.rs

@@ -267,6 +267,7 @@ mod tests {
 	}
 
 	#[test]
+	#[ignore = "is flaky"]
 	fn works_with_different_block_times() {
 		test_runtime_preset(PARA_RUNTIME, 100, Some(DEV_RUNTIME_PRESET.into()));
 		test_runtime_preset(PARA_RUNTIME, 3000, Some(DEV_RUNTIME_PRESET.into()));

prdoc/pr_8585.prdoc

@@ -0,0 +1,15 @@
+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-phase
+  bump: minor
+- name: frame-election-provider-solution-type
+  bump: patch
+- name: frame-election-provider-support
+  bump: patch
+- name: sp-npos-elections
+  bump: major
+  validate: false

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

@@ -469,9 +469,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>,
 }

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

@@ -551,6 +551,13 @@ impl<T: MinerConfig> Miner<T> {
 
 		let is_trimmed = TrimmingStatus { weight: weight_trimmed, length: length_trimmed };
 
+		log_no_system!(
+			debug,
+			"feasible solution mined: trimmed? {:?}, score: {:?}, encoded size: {:?}",
+			is_trimmed,
+			score,
+			solution.encoded_size()
+		);
 		Ok((solution, score, size, is_trimmed))
 	}
 

substrate/frame/election-provider-support/solution-type/src/single_page.rs

@@ -153,9 +153,9 @@ pub(crate) fn generate(def: crate::SolutionDef) -> Result<TokenStream2> {
 				voter_at: impl Fn(Self::VoterIndex) -> Option<A>,
 				target_at: impl Fn(Self::TargetIndex) -> Option<A>,
 			) -> Result<_fepsp::Vec<_feps::Assignment<A, #weight_type>>, _feps::Error> {
-				let mut #assignment_name: _fepsp::Vec<_feps::Assignment<A, #weight_type>> = Default::default();
+				let mut #assignment_name: _fepsp::BTreeMap<Self::VoterIndex, _feps::Assignment<A, #weight_type>> = Default::default();
 				#into_impl
-				Ok(#assignment_name)
+				Ok(#assignment_name.into_values().collect())
 			}
 
 			fn voter_count(&self) -> usize {
@@ -426,13 +426,18 @@ pub(crate) fn into_impl(
 	let into_impl_single = {
 		let name = vote_field(1);
 		quote!(
-			for (voter_index, target_index) in self.#name {
-				#assignments.push(_feps::Assignment {
-					who: voter_at(voter_index).or_invalid_index()?,
-					distribution: vec![
-						(target_at(target_index).or_invalid_index()?, #per_thing::one())
-					],
-				})
+			for (voter_index, target_index) in self.#name {;
+				if #assignments.contains_key(&voter_index) {
+					return Err(_feps::Error::DuplicateVoter);
+				} else {
+					#assignments.insert(
+						voter_index,
+						_feps::Assignment {
+							who: voter_at(voter_index).or_invalid_index()?,
+							distribution: vec![(target_at(target_index).or_invalid_index()?, #per_thing::one())],
+						}
+					);
+				}
 			}
 		)
 	};
@@ -442,10 +447,21 @@ pub(crate) fn into_impl(
 			let name = vote_field(c);
 			quote!(
 				for (voter_index, inners, t_last_idx) in self.#name {
+					if #assignments.contains_key(&voter_index) {
+						return Err(_feps::Error::DuplicateVoter);
+					}
+
+					let mut targets_seen = _fepsp::BTreeSet::new();
+
 					let mut sum = #per_thing::zero();
 					let mut inners_parsed = inners
 						.iter()
 						.map(|(ref t_idx, p)| {
+							if targets_seen.contains(t_idx) {
+								return Err(_feps::Error::DuplicateTarget);
+							} else {
+								targets_seen.insert(t_idx);
+							}
 							sum = _fepsp::sp_arithmetic::traits::Saturating::saturating_add(sum, *p);
 							let target = target_at(*t_idx).or_invalid_index()?;
 							Ok((target, *p))
@@ -456,6 +472,13 @@ pub(crate) fn into_impl(
 						return Err(_feps::Error::SolutionWeightOverflow);
 					}
 
+					// check that the last target index is also unique.
+					if targets_seen.contains(&t_last_idx) {
+						return Err(_feps::Error::DuplicateTarget);
+					} else {
+						// no need to insert, we are done.
+					}
+
 					// defensive only. Since Percent doesn't have `Sub`.
 					let p_last = _fepsp::sp_arithmetic::traits::Saturating::saturating_sub(
 						#per_thing::one(),
@@ -464,10 +487,13 @@ pub(crate) fn into_impl(
 
 					inners_parsed.push((target_at(t_last_idx).or_invalid_index()?, p_last));
 
-					#assignments.push(_feps::Assignment {
-						who: voter_at(voter_index).or_invalid_index()?,
-						distribution: inners_parsed,
-					});
+					#assignments.insert(
+						voter_index,
+						_feps::Assignment {
+							who: voter_at(voter_index).or_invalid_index()?,
+							distribution: inners_parsed,
+						}
+					);
 				}
 			)
 		})

substrate/frame/election-provider-support/src/lib.rs

@@ -205,7 +205,10 @@ use sp_runtime::TryRuntimeError;
 // re-export for the solution macro, with the dependencies of the macro.
 #[doc(hidden)]
 pub mod private {
-	pub use alloc::{collections::btree_set::BTreeSet, vec::Vec};
+	pub use alloc::{
+		collections::{btree_map::BTreeMap, btree_set::BTreeSet},
+		vec::Vec,
+	};
 	pub use codec;
 	pub use scale_info;
 	pub use sp_arithmetic;

substrate/frame/election-provider-support/src/tests.rs

@@ -244,6 +244,65 @@ mod solution_type {
 		);
 	}
 
+	#[test]
+	fn prevents_target_duplicate_into_assignment() {
+		let voter_at = |a: u32| -> Option<AccountId> { Some(a as AccountId) };
+		let target_at = |a: u16| -> Option<AccountId> { Some(a as AccountId) };
+
+		// case 1: duplicate target in votes2.
+		let solution = TestSolution { votes2: vec![(0, [(1, p(50))], 1)], ..Default::default() };
+		assert_eq!(
+			solution.into_assignment(&voter_at, &target_at).unwrap_err(),
+			NposError::DuplicateTarget,
+		);
+
+		// case 2: duplicate target in votes3.
+		let solution =
+			TestSolution { votes3: vec![(0, [(1, p(25)), (2, p(50))], 1)], ..Default::default() };
+		assert_eq!(
+			solution.into_assignment(&voter_at, &target_at).unwrap_err(),
+			NposError::DuplicateTarget,
+		);
+	}
+
+	#[test]
+	fn prevents_voter_duplicate_into_assignment() {
+		let voter_at = |a: u32| -> Option<AccountId> { Some(a as AccountId) };
+		let target_at = |a: u16| -> Option<AccountId> { Some(a as AccountId) };
+
+		// case 1: there is a duplicate among two different fields
+		let solution = TestSolution {
+			// voter index 0 is present here
+			votes1: vec![(0, 0), (1, 0)],
+			// voter index 0 is also present here
+			votes2: vec![(0, [(1, p(50))], 2)],
+			..Default::default()
+		};
+
+		assert_eq!(
+			solution.into_assignment(&voter_at, &target_at).unwrap_err(),
+			NposError::DuplicateVoter,
+		);
+
+		// case 2: there is a duplicate in the same field
+		let solution = TestSolution { votes1: vec![(0, 0), (0, 1)], ..Default::default() };
+		assert_eq!(
+			solution.into_assignment(&voter_at, &target_at).unwrap_err(),
+			NposError::DuplicateVoter,
+		);
+
+		// case 2.1: there is a duplicate in the same fieild, a bit more complex
+		let solution = TestSolution {
+			votes1: vec![(0, 0)],
+			votes2: vec![(1, [(1, p(50))], 2), (1, [(3, p(50))], 4)],
+			..Default::default()
+		};
+		assert_eq!(
+			solution.into_assignment(&voter_at, &target_at).unwrap_err(),
+			NposError::DuplicateVoter,
+		);
+	}
+
 	#[test]
 	fn from_and_into_assignment_works() {
 		let voters = vec![2 as AccountId, 4, 1, 5, 3];

substrate/primitives/npos-elections/src/lib.rs

@@ -127,6 +127,10 @@ pub enum Error {
 	InvalidSupportEdge,
 	/// The number of voters is bigger than the `MaxVoters` bound.
 	TooManyVoters,
+	/// A duplicate voter was detected.
+	DuplicateVoter,
+	/// A duplicate target was detected.
+	DuplicateTarget,
 }
 
 /// A type which is used in the API of this crate as a numeric weight of a vote, most often the