Don't slash all outgoing members. (#7394)

* Don't slash all outgoing members.

* One more fix

* cargo run --release --features runtime-benchmarks --manifest-path bin/node/cli/Cargo.toml -- benchmark --chain dev --steps 50 --repeat 20 --extrinsic * --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output ./bin/node/runtime/src/weights --header ./HEADER --pallet pallet_elections_phragmen

Co-authored-by: Parity Benchmarking Bot <[email protected]>

Author: kianenigma

bin/node/runtime/src/weights/pallet_elections_phragmen.rs

@@ -1,6 +1,6 @@
 // This file is part of Substrate.
 
-// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd.
+// Copyright (C) 2020 Parity Technologies (UK) Ltd.
 // SPDX-License-Identifier: Apache-2.0
 
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -15,8 +15,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0-rc6
-
+//! Weights for pallet_elections_phragmen
+//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0
+//! DATE: 2020-10-26, STEPS: [50], REPEAT: 20, LOW RANGE: [], HIGH RANGE: []
+//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128
 #![allow(unused_parens)]
 #![allow(unused_imports)]
 
@@ -26,65 +28,65 @@ use sp_std::marker::PhantomData;
 pub struct WeightInfo<T>(PhantomData<T>);
 impl<T: frame_system::Trait> pallet_elections_phragmen::WeightInfo for WeightInfo<T> {
 	fn vote(v: u32, ) -> Weight {
-		(91_489_000 as Weight)
-			.saturating_add((199_000 as Weight).saturating_mul(v as Weight))
+		(91_991_000 as Weight)
+			.saturating_add((184_000 as Weight).saturating_mul(v as Weight))
 			.saturating_add(T::DbWeight::get().reads(5 as Weight))
 			.saturating_add(T::DbWeight::get().writes(2 as Weight))
 	}
 	fn vote_update(v: u32, ) -> Weight {
-		(56_511_000 as Weight)
-			.saturating_add((245_000 as Weight).saturating_mul(v as Weight))
+		(56_633_000 as Weight)
+			.saturating_add((228_000 as Weight).saturating_mul(v as Weight))
 			.saturating_add(T::DbWeight::get().reads(5 as Weight))
 			.saturating_add(T::DbWeight::get().writes(2 as Weight))
 	}
 	fn remove_voter() -> Weight {
-		(76_714_000 as Weight)
+		(76_890_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(2 as Weight))
 			.saturating_add(T::DbWeight::get().writes(2 as Weight))
 	}
 	fn report_defunct_voter_correct(c: u32, v: u32, ) -> Weight {
 		(0 as Weight)
-			.saturating_add((1_743_000 as Weight).saturating_mul(c as Weight))
-			.saturating_add((31_750_000 as Weight).saturating_mul(v as Weight))
+			.saturating_add((1_769_000 as Weight).saturating_mul(c as Weight))
+			.saturating_add((32_244_000 as Weight).saturating_mul(v as Weight))
 			.saturating_add(T::DbWeight::get().reads(7 as Weight))
 			.saturating_add(T::DbWeight::get().writes(3 as Weight))
 	}
 	fn report_defunct_voter_incorrect(c: u32, v: u32, ) -> Weight {
 		(0 as Weight)
-			.saturating_add((1_733_000 as Weight).saturating_mul(c as Weight))
-			.saturating_add((31_861_000 as Weight).saturating_mul(v as Weight))
+			.saturating_add((1_777_000 as Weight).saturating_mul(c as Weight))
+			.saturating_add((32_528_000 as Weight).saturating_mul(v as Weight))
 			.saturating_add(T::DbWeight::get().reads(6 as Weight))
 			.saturating_add(T::DbWeight::get().writes(2 as Weight))
 	}
 	fn submit_candidacy(c: u32, ) -> Weight {
-		(74_714_000 as Weight)
-			.saturating_add((315_000 as Weight).saturating_mul(c as Weight))
+		(75_137_000 as Weight)
+			.saturating_add((310_000 as Weight).saturating_mul(c as Weight))
 			.saturating_add(T::DbWeight::get().reads(3 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	fn renounce_candidacy_candidate(c: u32, ) -> Weight {
-		(50_408_000 as Weight)
-			.saturating_add((159_000 as Weight).saturating_mul(c as Weight))
+		(50_071_000 as Weight)
+			.saturating_add((184_000 as Weight).saturating_mul(c as Weight))
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	fn renounce_candidacy_members() -> Weight {
-		(79_626_000 as Weight)
+		(79_471_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(3 as Weight))
 			.saturating_add(T::DbWeight::get().writes(4 as Weight))
 	}
 	fn renounce_candidacy_runners_up() -> Weight {
-		(49_715_000 as Weight)
+		(49_740_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	fn remove_member_with_replacement() -> Weight {
-		(76_572_000 as Weight)
+		(76_973_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(4 as Weight))
 			.saturating_add(T::DbWeight::get().writes(5 as Weight))
 	}
 	fn remove_member_wrong_refund() -> Weight {
-		(8_777_000 as Weight)
+		(8_871_000 as Weight)
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 	}
 }

frame/elections-phragmen/src/lib.rs

@@ -973,18 +973,29 @@ impl<T: Trait> Module<T> {
 			);
 			T::ChangeMembers::set_prime(prime);
 
-			// outgoing members lose their bond.
-			let mut to_burn_bond = outgoing.to_vec();
+			// outgoing members who are no longer a runner-up lose their bond.
+			let mut to_burn_bond = outgoing
+				.iter()
+				.filter(|o| new_runners_up_ids_sorted.binary_search(o).is_err())
+				.cloned()
+				.collect::<Vec<_>>();
 
-			// compute the outgoing of runners up as well and append them to the `to_burn_bond`
+			// compute the outgoing of runners up as well and append them to the `to_burn_bond`, if
+			// they are not members.
 			{
 				let (_, outgoing) = T::ChangeMembers::compute_members_diff(
 					&new_runners_up_ids_sorted,
 					&old_runners_up_ids_sorted,
 				);
 				// 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);
+				to_burn_bond.extend(
+					outgoing
+						.iter()
+						.filter(|o| new_members_ids_sorted.binary_search(o).is_err())
+						.cloned()
+						.collect::<Vec<_>>()
+				);
 			}
 
 			// Burn loser bond. members list is sorted. O(NLogM) (N candidates, M members)
@@ -2876,4 +2887,86 @@ mod tests {
 			assert_eq!(balances(&3), (25, 5));
 		})
 	}
+
+	#[test]
+	fn member_to_runner_up_wont_slash() {
+		ExtBuilder::default().desired_runners_up(2).desired_members(1).build_and_execute(|| {
+			assert_ok!(submit_candidacy(Origin::signed(4)));
+			assert_ok!(submit_candidacy(Origin::signed(3)));
+			assert_ok!(submit_candidacy(Origin::signed(2)));
+
+
+			assert_ok!(vote(Origin::signed(4), vec![4], 40));
+			assert_ok!(vote(Origin::signed(3), vec![3], 30));
+			assert_ok!(vote(Origin::signed(2), vec![2], 20));
+
+			System::set_block_number(5);
+			Elections::end_block(System::block_number());
+
+			assert_eq!(Elections::members_ids(), vec![4]);
+			assert_eq!(Elections::runners_up_ids(), vec![2, 3]);
+
+			assert_eq!(balances(&4), (35, 5));
+			assert_eq!(balances(&3), (25, 5));
+			assert_eq!(balances(&2), (15, 5));
+
+			// this guy will shift everyone down.
+			assert_ok!(submit_candidacy(Origin::signed(5)));
+			assert_ok!(vote(Origin::signed(5), vec![5], 50));
+
+			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![3, 4]);
+
+			// 4 went from member to runner-up -- don't slash.
+			assert_eq!(balances(&4), (35, 5));
+			// 3 stayed runner-up -- don't slash.
+			assert_eq!(balances(&3), (25, 5));
+			// 2 was removed -- slash.
+			assert_eq!(balances(&2), (15, 2));
+		});
+	}
+
+	#[test]
+	fn runner_up_to_member_wont_slash() {
+		ExtBuilder::default().desired_runners_up(2).desired_members(1).build_and_execute(|| {
+			assert_ok!(submit_candidacy(Origin::signed(4)));
+			assert_ok!(submit_candidacy(Origin::signed(3)));
+			assert_ok!(submit_candidacy(Origin::signed(2)));
+
+
+			assert_ok!(vote(Origin::signed(4), vec![4], 40));
+			assert_ok!(vote(Origin::signed(3), vec![3], 30));
+			assert_ok!(vote(Origin::signed(2), vec![2], 20));
+
+			System::set_block_number(5);
+			Elections::end_block(System::block_number());
+
+			assert_eq!(Elections::members_ids(), vec![4]);
+			assert_eq!(Elections::runners_up_ids(), vec![2, 3]);
+
+			assert_eq!(balances(&4), (35, 5));
+			assert_eq!(balances(&3), (25, 5));
+			assert_eq!(balances(&2), (15, 5));
+
+			// swap some votes.
+			assert_ok!(vote(Origin::signed(4), vec![2], 40));
+			assert_ok!(vote(Origin::signed(2), vec![4], 20));
+
+			System::set_block_number(10);
+			Elections::end_block(System::block_number());
+
+			assert_eq!(Elections::members_ids(), vec![2]);
+			assert_eq!(Elections::runners_up_ids(), vec![4, 3]);
+
+			// 2 went from runner to member, don't slash
+			assert_eq!(balances(&2), (15, 5));
+			// 4 went from member to runner, don't slash
+			assert_eq!(balances(&4), (35, 5));
+			// 3 stayed the same
+			assert_eq!(balances(&3), (25, 5));
+		});
+	}
 }