Undo phragmen merge

frame/elections-phragmen/src/lib.rs

@@ -891,18 +891,16 @@ impl<T: Trait> Module<T> {
 			num_to_elect,
 			0,
 			candidates,
-			voters_and_votes.clone(),
-			None,
-		).map(|ElectionResult { winners, assignments: _ }| {
-			// this is already sorted by id.
-			let old_members_ids_sorted = <Members<T>>::take().into_iter()
+			voters_and_votes,
+		);
+
+		if let Some(ElectionResult { winners, assignments }) = maybe_phragmen_result {
+			let old_members_ids = <Members<T>>::take().into_iter()
 				.map(|(m, _)| m)
 				.collect::<Vec<T::AccountId>>();
-			// this one needs a sort by id.
-			let mut old_runners_up_ids_sorted = <RunnersUp<T>>::take().into_iter()
+			let old_runners_up_ids = <RunnersUp<T>>::take().into_iter()
 				.map(|(r, _)| r)
 				.collect::<Vec<T::AccountId>>();
-			old_runners_up_ids_sorted.sort();
 
 			// filter out those who had literally no votes at all.
 			// NOTE: the need to do this is because all candidates, even those who have no
@@ -941,17 +939,17 @@ impl<T: Trait> Module<T> {
 
 			// split new set into winners and runners up.
 			let split_point = desired_seats.min(new_set_with_stake.len());
-			let mut new_members_sorted_by_id = (&new_set_with_stake[..split_point]).to_vec();
+			let mut new_members = (&new_set_with_stake[..split_point]).to_vec();
 
 			// save the runners up as-is. They are sorted based on desirability.
 			// save the members, sorted based on account id.
-			new_members_sorted_by_id.sort_by(|i, j| i.0.cmp(&j.0));
+			new_members.sort_by(|i, j| i.0.cmp(&j.0));
 
 			// Now we select a prime member using a [Borda count](https://en.wikipedia.org/wiki/Borda_count).
 			// We weigh everyone's vote for that new member by a multiplier based on the order
 			// of the votes. i.e. the first person a voter votes for gets a 16x multiplier,
 			// the next person gets a 15x multiplier, an so on... (assuming `MAXIMUM_VOTE` = 16)
-			let mut prime_votes: Vec<_> = new_members_sorted_by_id.iter().map(|c| (&c.0, BalanceOf::<T>::zero())).collect();
+			let mut prime_votes: Vec<_> = new_members.iter().map(|c| (&c.0, BalanceOf::<T>::zero())).collect();
 			for (_, stake, votes) in voters_and_stakes.into_iter() {
 				for (vote_multiplier, who) in votes.iter()
 					.enumerate()
@@ -969,58 +967,54 @@ impl<T: Trait> Module<T> {
 			// the person with the "highest" account id based on the sort above.
 			let prime = prime_votes.into_iter().max_by_key(|x| x.1).map(|x| x.0.clone());
 
-			// new_members_sorted_by_id is sorted by account id.
-			let new_members_ids_sorted = new_members_sorted_by_id
+			// new_members_ids is sorted by account id.
+			let new_members_ids = new_members
 				.iter()
 				.map(|(m, _)| m.clone())
 				.collect::<Vec<T::AccountId>>();
 
-			let new_runners_up_sorted_by_rank = &new_set_with_stake[split_point..]
+			let new_runners_up = &new_set_with_stake[split_point..]
 				.into_iter()
 				.cloned()
 				.rev()
 				.collect::<Vec<(T::AccountId, BalanceOf<T>)>>();
 			// new_runners_up remains sorted by desirability.
-			let mut new_runners_up_ids_sorted = new_runners_up_sorted_by_rank
+			let new_runners_up_ids = new_runners_up
 				.iter()
 				.map(|(r, _)| r.clone())
 				.collect::<Vec<T::AccountId>>();
-			new_runners_up_ids_sorted.sort();
 
 			// report member changes. We compute diff because we need the outgoing list.
 			let (incoming, outgoing) = T::ChangeMembers::compute_members_diff(
-				&new_members_ids_sorted,
-				&old_members_ids_sorted,
+				&new_members_ids,
+				&old_members_ids,
 			);
 			T::ChangeMembers::change_members_sorted(
 				&incoming,
 				&outgoing,
-				&new_members_ids_sorted,
+				&new_members_ids,
 			);
 			T::ChangeMembers::set_prime(prime);
 
-			// outgoing members lose their bond.
+			// outgoing candidates lose their bond.
 			let mut to_burn_bond = outgoing.to_vec();
 
 			// compute the outgoing of runners up as well and append them to the `to_burn_bond`
 			{
 				let (_, outgoing) = T::ChangeMembers::compute_members_diff(
-					&new_runners_up_ids_sorted,
-					&old_runners_up_ids_sorted,
+					&new_runners_up_ids,
+					&old_runners_up_ids,
 				);
-				// none of the ones computed to be outgoing must still be in the list.
-				debug_assert!(outgoing.iter().all(|o| !new_runners_up_ids_sorted.contains(o)));
 				to_burn_bond.extend(outgoing);
 			}
 
 			// Burn loser bond. members list is sorted. O(NLogM) (N candidates, M members)
-			// runner up list is also sorted. O(NLogK) given K runner ups. Overall: O(NLogM + N*K)
+			// runner up list is not sorted. O(K*N) given K runner ups. Overall: O(NLogM + N*K)
 			// both the member and runner counts are bounded.
 			exposed_candidates.into_iter().for_each(|c| {
 				// any candidate who is not a member and not a runner up.
-				if
-					new_members_ids_sorted.binary_search(&c).is_err() &&
-					new_runners_up_ids_sorted.binary_search(&c).is_err()
+				if new_members.binary_search_by_key(&c, |(m, _)| m.clone()).is_err()
+					&& !new_runners_up_ids.contains(&c)
 				{
 					let (imbalance, _) = T::Currency::slash_reserved(&c, T::CandidacyBond::get());
 					T::LoserCandidate::on_unbalanced(imbalance);
@@ -1033,10 +1027,13 @@ impl<T: Trait> Module<T> {
 				T::LoserCandidate::on_unbalanced(imbalance);
 			});
 
-			<Members<T>>::put(&new_members_sorted_by_id);
-			<RunnersUp<T>>::put(new_runners_up_sorted_by_rank);
+			<Members<T>>::put(&new_members);
+			<RunnersUp<T>>::put(new_runners_up);
 
-			Self::deposit_event(RawEvent::NewTerm(new_members_sorted_by_id.clone().to_vec()));
+			Self::deposit_event(RawEvent::NewTerm(new_members.clone().to_vec()));
+		} else {
+			Self::deposit_event(RawEvent::EmptyTerm);
+		}
 
 		// clean candidates.
 		<Candidates<T>>::kill();
@@ -1300,6 +1297,7 @@ mod tests {
 			self.genesis_members = members;
 			self
 		}
+		#[cfg(feature = "runtime-benchmarks")]
 		pub fn desired_members(mut self, count: u32) -> Self {
 			self.desired_members = count;
 			self
@@ -2820,38 +2818,4 @@ mod tests {
 			assert!(Elections::candidates().is_empty());
 		})
 	}
-
-	#[test]
-	fn unsorted_runners_up_are_detected() {
-		ExtBuilder::default().desired_runners_up(2).desired_members(1).build_and_execute(|| {
-			assert_ok!(submit_candidacy(Origin::signed(5)));
-			assert_ok!(submit_candidacy(Origin::signed(4)));
-			assert_ok!(submit_candidacy(Origin::signed(3)));
-
-
-			assert_ok!(vote(Origin::signed(5), vec![5], 50));
-			assert_ok!(vote(Origin::signed(4), vec![4], 5));
-			assert_ok!(vote(Origin::signed(3), vec![3], 15));
-
-			System::set_block_number(5);
-			Elections::end_block(System::block_number());
-
-			assert_eq!(Elections::members_ids(), vec![5]);
-			assert_eq!(Elections::runners_up_ids(), vec![4, 3]);
-
-			assert_ok!(submit_candidacy(Origin::signed(2)));
-			assert_ok!(vote(Origin::signed(2), vec![2], 10));
-
-			System::set_block_number(10);
-			Elections::end_block(System::block_number());
-
-			assert_eq!(Elections::members_ids(), vec![5]);
-			assert_eq!(Elections::runners_up_ids(), vec![2, 3]);
-
-			// 4 is outgoing runner-up. Slash candidacy bond.
-			assert_eq!(balances(&4), (35, 2));
-			// 3 stays.
-			assert_eq!(balances(&3), (25, 5));
-		})
-	}
 }