Remove wrong assertion from phragmen (#4515)

* remove assertion

substrate/frame/nicks/src/lib.rs

@@ -145,7 +145,7 @@ decl_module! {
 		fn set_name(origin, name: Vec<u8>) {
 			let sender = ensure_signed(origin)?;
 
-			ensure!(name.len() >= T::MinLength::get(), Error::<T>::TooShort,);
+			ensure!(name.len() >= T::MinLength::get(), Error::<T>::TooShort);
 			ensure!(name.len() <= T::MaxLength::get(), Error::<T>::TooLong);
 
 			let deposit = if let Some((_, deposit)) = <NameOf<T>>::get(&sender) {

substrate/frame/staking/src/lib.rs

@@ -1528,18 +1528,30 @@ impl<T: Trait> Module<T> {
 			let mut slot_stake = BalanceOf::<T>::max_value();
 			for (c, s) in supports.into_iter() {
 				// build `struct exposure` from `support`
+				let mut others = Vec::new();
+				let mut own: BalanceOf<T> = Zero::zero();
+				let mut total: BalanceOf<T> = Zero::zero();
+				s.voters
+					.into_iter()
+					.map(|(who, value)| (who, to_balance(value)))
+					.for_each(|(who, value)| {
+						if who == c {
+							own = own.saturating_add(value);
+						} else {
+							others.push(IndividualExposure { who, value });
+						}
+						total = total.saturating_add(value);
+					});
 				let exposure = Exposure {
-					own: to_balance(s.own),
+					own,
+					others,
 					// This might reasonably saturate and we cannot do much about it. The sum of
 					// someone's stake might exceed the balance type if they have the maximum amount
 					// of balance and receive some support. This is super unlikely to happen, yet
 					// we simulate it in some tests.
-					total: to_balance(s.total),
+					total,
-					others: s.others
-						.into_iter()
-						.map(|(who, value)| IndividualExposure { who, value: to_balance(value) })
-						.collect::<Vec<IndividualExposure<_, _>>>(),
 				};
+
 				if exposure.total < slot_stake {
 					slot_stake = exposure.total;
 				}

substrate/primitives/phragmen/src/lib.rs

@@ -118,14 +118,12 @@ pub struct PhragmenResult<AccountId> {
 /// This, at the current version, resembles the `Exposure` defined in the staking SRML module, yet
 /// they do not necessarily have to be the same.
 #[derive(Default, RuntimeDebug)]
-#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize, Eq, PartialEq))]
 pub struct Support<AccountId> {
-	/// The amount of support as the effect of self-vote.
-	pub own: ExtendedBalance,
 	/// Total support.
 	pub total: ExtendedBalance,
 	/// Support from voters.
-	pub others: Vec<PhragmenStakedAssignment<AccountId>>,
+	pub voters: Vec<PhragmenStakedAssignment<AccountId>>,
 }
 
 /// A linkage from a candidate and its [`Support`].
@@ -368,20 +366,8 @@ pub fn build_support_map<Balance, AccountId, FS, C>(
 			// per-things to be sound.
 			let other_stake = *per_thing * nominator_stake;
 			if let Some(support) = supports.get_mut(c) {
-				if c == n {
+				support.voters.push((n.clone(), other_stake));
-					// This is a nomination from `n` to themselves. This will increase both the
+				support.total = support.total.saturating_add(other_stake);
-					// `own` and `total` field.
-					debug_assert!(*per_thing == Perbill::one()); // TODO: deal with this: do we want it?
-					support.own = support.own.saturating_add(other_stake);
-					support.total = support.total.saturating_add(other_stake);
-				} else {
-					// This is a nomination from `n` to someone else. Increase `total` and add an entry
-					// inside `others`.
-					// For an astronomically rich validator with more astronomically rich
-					// set of nominators, this might saturate.
-					support.total = support.total.saturating_add(other_stake);
-					support.others.push((n.clone(), other_stake));
-				}
 			}
 		}
 	}
@@ -450,8 +436,8 @@ fn do_equalize<Balance, AccountId, C>(
 	let budget = to_votes(budget_balance);
 
 	// Nothing to do. This voter had nothing useful.
-	// Defensive only. Assignment list should always be populated.
+	// Defensive only. Assignment list should always be populated. 1 might happen for self vote.
-	if elected_edges.is_empty() { return 0; }
+	if elected_edges.is_empty() || elected_edges.len() == 1 { return 0; }
 
 	let stake_used = elected_edges
 		.iter()
@@ -492,7 +478,7 @@ fn do_equalize<Balance, AccountId, C>(
 	elected_edges.iter_mut().for_each(|e| {
 		if let Some(support) = support_map.get_mut(&e.0) {
 			support.total = support.total.saturating_sub(e.1);
-			support.others.retain(|i_support| i_support.0 != *voter);
+			support.voters.retain(|i_support| i_support.0 != *voter);
 		}
 		e.1 = 0;
 	});
@@ -529,7 +515,7 @@ fn do_equalize<Balance, AccountId, C>(
 				.saturating_add(last_stake)
 				.saturating_sub(support.total);
 			support.total = support.total.saturating_add(e.1);
-			support.others.push((voter.clone(), e.1));
+			support.voters.push((voter.clone(), e.1));
 		}
 	});
 

substrate/primitives/phragmen/src/mock.rs

@@ -375,7 +375,7 @@ pub(crate) fn run_and_compare(
 	check_assignments(assignments);
 }
 
-pub(crate) fn build_support_map<FS>(
+pub(crate) fn build_support_map_float<FS>(
 	result: &mut _PhragmenResult<AccountId>,
 	stake_of: FS,
 ) -> _SupportMap<AccountId>

substrate/primitives/phragmen/src/tests.rs

@@ -19,7 +19,7 @@
 #![cfg(test)]
 
 use crate::mock::*;
-use crate::{elect, PhragmenResult};
+use crate::{elect, PhragmenResult, PhragmenStakedAssignment, build_support_map, Support, equalize};
 use substrate_test_utils::assert_eq_uvec;
 use sp_runtime::Perbill;
 
@@ -46,7 +46,7 @@ fn float_phragmen_poc_works() {
 		]
 	);
 
-	let mut support_map = build_support_map(&mut phragmen_result, &stake_of);
+	let mut support_map = build_support_map_float(&mut phragmen_result, &stake_of);
 
 	assert_eq!(
 		support_map.get(&2).unwrap(),
@@ -405,3 +405,90 @@ fn minimum_to_elect_is_respected() {
 
 	assert!(maybe_result.is_none());
 }
+
+#[test]
+fn self_votes_should_be_kept() {
+	let candidates = vec![5, 10, 20, 30];
+	let voters = vec![
+		(5, vec![5]),
+		(10, vec![10]),
+		(20, vec![20]),
+		(1, vec![10, 20])
+	];
+	let stake_of = create_stake_of(&[
+		(5, 5),
+		(10, 10),
+		(20, 20),
+		(1, 8),
+	]);
+
+	let result = elect::<_, _, _, TestCurrencyToVote>(
+		2,
+		2,
+		candidates,
+		voters,
+		&stake_of,
+	).unwrap();
+
+	assert_eq!(result.winners, vec![(20, 28), (10, 18)]);
+	assert_eq!(
+		result.assignments,
+		vec![
+			(10, vec![(10, Perbill::from_percent(100))]),
+			(20, vec![(20, Perbill::from_percent(100))]),
+			(1, vec![
+					(10, Perbill::from_percent(50)),
+					(20, Perbill::from_percent(50))
+				]
+			)
+		],
+	);
+
+	let mut supports = build_support_map::<
+		Balance,
+		AccountId,
+		_,
+		TestCurrencyToVote
+		>(
+			&result.winners.into_iter().map(|(who, _)| who).collect(),
+			&result.assignments,
+			&stake_of
+		);
+
+	assert_eq!(supports.get(&5u64), None);
+	assert_eq!(
+		supports.get(&10u64).unwrap(),
+		&Support { total: 14u128, voters: vec![(10u64, 10u128), (1u64, 4u128)] },
+	);
+	assert_eq!(
+		supports.get(&20u64).unwrap(),
+		&Support { total: 24u128, voters: vec![(20u64, 20u128), (1u64, 4u128)] },
+	);
+
+	let assignments = result.assignments;
+	let mut staked_assignments
+		: Vec<(AccountId, Vec<PhragmenStakedAssignment<AccountId>>)>
+		= Vec::with_capacity(assignments.len());
+	for (n, assignment) in assignments.iter() {
+		let mut staked_assignment
+			: Vec<PhragmenStakedAssignment<AccountId>>
+			= Vec::with_capacity(assignment.len());
+		let stake = stake_of(&n);
+		for (c, per_thing) in assignment.iter() {
+			let vote_stake = *per_thing * stake;
+			staked_assignment.push((c.clone(), vote_stake));
+		}
+		staked_assignments.push((n.clone(), staked_assignment));
+	}
+
+	equalize::<Balance, AccountId, TestCurrencyToVote, _>(staked_assignments, &mut supports, 0, 2usize, &stake_of);
+
+	assert_eq!(
+		supports.get(&10u64).unwrap(),
+		&Support { total: 18u128, voters: vec![(10u64, 10u128), (1u64, 8u128)] },
+	);
+	assert_eq!(
+		supports.get(&20u64).unwrap(),
+		&Support { total: 20u128, voters: vec![(20u64, 20u128)] },
+	);
+}