Update elections-phragmen weight to WeightInfo (#7161)

* Update elections-phragmen weight to WeightInfo * Fix benchmark tests * Update weights * Update test * Update another thest :" * Weights from benchmarking machine ./substrate2/target/release/substrate benchmark --chain dev --steps 50 --repeat 20 --pallet pallet_elections_phragmen --extrinsic "*" --raw --execution=wasm --wasm-execution=compiled --output * Update weights from the benchmarking machine * Fix tests one last time Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
2026-05-30 02:21:04 +00:00 · 2020-09-22 14:06:15 +02:00
parent fc77d1d466
commit f70ef87a0c
7 changed files with 336 additions and 128 deletions
@@ -28,10 +28,18 @@ use crate::Module as Elections;

 const BALANCE_FACTOR: u32 = 250;
 const MAX_VOTERS: u32 = 500;
-const MAX_CANDIDATES: u32 = 100;
+const MAX_CANDIDATES: u32 = 200;

 type Lookup<T> = <<T as frame_system::Trait>::Lookup as StaticLookup>::Source;

+macro_rules! whitelist {
+	($acc:ident) => {
+		frame_benchmarking::benchmarking::add_to_whitelist(
+			frame_system::Account::<T>::hashed_key_for(&$acc).into()
+		);
+	};
+}
+
 /// grab new account with infinite balance.
 fn endowed_account<T: Trait>(name: &'static str, index: u32) -> T::AccountId {
 	let account: T::AccountId = account(name, index, 0);
@@ -40,6 +48,7 @@ fn endowed_account<T: Trait>(name: &'static str, index: u32) -> T::AccountId {
 	// important to increase the total issuance since T::CurrencyToVote will need it to be sane for
 	// phragmen to work.
 	T::Currency::issue(amount);
+
 	account
 }

@@ -102,10 +111,9 @@ fn submit_candidates_with_self_vote<T: Trait>(c: u32, prefix: &'static str)

 /// Submit one voter.
 fn submit_voter<T: Trait>(caller: T::AccountId, votes: Vec<T::AccountId>, stake: BalanceOf<T>)
-	-> Result<(), &'static str>
+	-> Result<(), sp_runtime::DispatchError>
 {
 	<Elections<T>>::vote(RawOrigin::Signed(caller).into(), votes, stake)
-		.map_err(|_| "failed to submit vote")
 }

 /// create `num_voter` voters who randomly vote for at most `votes` of `all_candidates` if
@@ -131,7 +139,7 @@ fn distribute_voters<T: Trait>(mut all_candidates: Vec<T::AccountId>, num_voters
 /// Fill the seats of members and runners-up up until `m`. Note that this might include either only
 /// members, or members and runners-up.
 fn fill_seats_up_to<T: Trait>(m: u32) -> Result<Vec<T::AccountId>, &'static str> {
-	let candidates = submit_candidates_with_self_vote::<T>(m, "fill_seats_up_to")?;
+	let _ = submit_candidates_with_self_vote::<T>(m, "fill_seats_up_to")?;
 	assert_eq!(<Elections<T>>::candidates().len() as u32, m, "wrong number of candidates.");
 	<Elections<T>>::do_phragmen();
 	assert_eq!(<Elections<T>>::candidates().len(), 0, "some candidates remaining.");
@@ -140,7 +148,13 @@ fn fill_seats_up_to<T: Trait>(m: u32) -> Result<Vec<T::AccountId>, &'static str>
 		m as usize,
 		"wrong number of members and runners-up",
 	);
-	Ok(candidates)
+	Ok(
+		<Elections<T>>::members()
+			.into_iter()
+			.map(|(x, _)| x)
+			.chain(<Elections<T>>::runners_up().into_iter().map(|(x, _)| x))
+			.collect()
+	)
 }

 /// removes all the storage items to reverse any genesis state.
@@ -152,50 +166,46 @@ fn clean<T: Trait>() {
 }

 benchmarks! {
-	_ {
-		// User account seed
-		let u in 0 .. 1000 => ();
-	}
+	_ {}

 	// -- Signed ones
 	vote {
-		let u in ...;
-		// we fix the number of voted candidates to max
-		let v = MAXIMUM_VOTE;
+		let v in 1 .. (MAXIMUM_VOTE as u32);
 		clean::<T>();

 		// create a bunch of candidates.
-		let all_candidates = submit_candidates::<T>(MAXIMUM_VOTE as u32, "candidates")?;
+		let all_candidates = submit_candidates::<T>(v, "candidates")?;

-		let caller = endowed_account::<T>("caller", u);
+		let caller = endowed_account::<T>("caller", 0);
 		let stake = default_stake::<T>(BALANCE_FACTOR);

 		// vote for all of them.
-		let votes = all_candidates.into_iter().take(v).collect();
+		let votes = all_candidates;

+		whitelist!(caller);
 	}: _(RawOrigin::Signed(caller), votes, stake)

 	vote_update {
-		let u in ...;
-		// we fix the number of voted candidates to max
-		let v = MAXIMUM_VOTE;
+		let v in 1 .. (MAXIMUM_VOTE as u32);
 		clean::<T>();

 		// create a bunch of candidates.
-		let all_candidates = submit_candidates::<T>(MAXIMUM_VOTE as u32, "candidates")?;
+		let all_candidates = submit_candidates::<T>(v, "candidates")?;

-		let caller = endowed_account::<T>("caller", u);
+		let caller = endowed_account::<T>("caller", 0);
 		let stake = default_stake::<T>(BALANCE_FACTOR);

 		// original votes.
-		let mut votes = all_candidates.into_iter().take(v).collect::<Vec<T::AccountId>>();
+		let mut votes = all_candidates;
 		submit_voter::<T>(caller.clone(), votes.clone(), stake)?;
+
 		// new votes.
 		votes.rotate_left(1);
+
+		whitelist!(caller);
 	}: vote(RawOrigin::Signed(caller), votes, stake)

 	remove_voter {
-		let u in ...;
 		// we fix the number of voted candidates to max
 		let v = MAXIMUM_VOTE as u32;
 		clean::<T>();
@@ -203,11 +213,12 @@ benchmarks! {
 		// create a bunch of candidates.
 		let all_candidates = submit_candidates::<T>(v, "candidates")?;

-		let caller = endowed_account::<T>("caller", u);
+		let caller = endowed_account::<T>("caller", 0);

 		let stake = default_stake::<T>(BALANCE_FACTOR);
 		submit_voter::<T>(caller.clone(), all_candidates, stake)?;

+		whitelist!(caller);
 	}: _(RawOrigin::Signed(caller))

 	report_defunct_voter_correct {
@@ -217,11 +228,11 @@ benchmarks! {
 		// number of candidates that the reported voter voted for. The worse case of search here is
 		// basically `c * v`.
 		let v in 1 .. (MAXIMUM_VOTE as u32);
-		// we fix the number of members to when members and runners-up to the desired. We'll be in
+		// we fix the number of members to the number of desired members and runners-up. We'll be in
 		// this state almost always.
 		let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
-		clean::<T>();

+		clean::<T>();
 		let stake = default_stake::<T>(BALANCE_FACTOR);

 		// create m members and runners combined.
@@ -231,8 +242,7 @@ benchmarks! {
 		let bailing_candidates = submit_candidates::<T>(v, "bailing_candidates")?;
 		let all_candidates = submit_candidates::<T>(c, "all_candidates")?;

-		// account 1 is the reporter and it doesn't matter how many it votes. But it has to be a
-		// voter.
+		// account 1 is the reporter and must be whitelisted, and a voter.
 		let account_1 = endowed_account::<T>("caller", 0);
 		submit_voter::<T>(
 			account_1.clone(),
@@ -248,7 +258,9 @@ benchmarks! {
 			stake,
 		)?;

-		// all the bailers go away.
+		// all the bailers go away. NOTE: we can simplify this. There's no need to create all these
+		// candidates and remove them. The defunct voter can just vote for random accounts as long
+		// as there are enough members (potential candidates).
 		bailing_candidates.into_iter().for_each(|b| {
 			let count = candidate_count::<T>();
 			assert!(<Elections<T>>::renounce_candidacy(
@@ -256,10 +268,13 @@ benchmarks! {
 				Renouncing::Candidate(count),
 			).is_ok());
 		});
-		let defunct = defunct_for::<T>(account_2.clone());
-	}: report_defunct_voter(RawOrigin::Signed(account_1.clone()), defunct)
+
+		let defunct_info = defunct_for::<T>(account_2.clone());
+		whitelist!(account_1);
+
+		assert!(<Elections<T>>::is_voter(&account_2));
+	}: report_defunct_voter(RawOrigin::Signed(account_1.clone()), defunct_info)
 	verify {
-		assert!(<Elections<T>>::is_voter(&account_1));
 		assert!(!<Elections<T>>::is_voter(&account_2));
 		#[cfg(test)]
 		{
@@ -276,7 +291,7 @@ benchmarks! {
 		// number of candidates that the reported voter voted for. The worse case of search here is
 		// basically `c * v`.
 		let v in 1 .. (MAXIMUM_VOTE as u32);
-		// we fix the number of members to when members and runners-up to the desired. We'll be in
+		// we fix the number of members to the number of desired members and runners-up. We'll be in
 		// this state almost always.
 		let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();

@@ -289,7 +304,7 @@ benchmarks! {
 		// create a bunch of candidates as well.
 		let all_candidates = submit_candidates::<T>(c, "candidates")?;

-		// account 1 is the reporter and it doesn't matter how many it votes.
+		// account 1 is the reporter and need to be whitelisted, and a voter.
 		let account_1 = endowed_account::<T>("caller", 0);
 		submit_voter::<T>(
 			account_1.clone(),
@@ -299,8 +314,9 @@ benchmarks! {

 		// account 2 votes for a bunch of crap, and finally a correct candidate.
 		let account_2 = endowed_account::<T>("caller_2", 1);
-		let mut invalid: Vec<T::AccountId> =
-			(0..(v-1)).map(|seed| account::<T::AccountId>("invalid", 0, seed).clone()).collect();
+		let mut invalid: Vec<T::AccountId> = (0..(v-1))
+			.map(|seed| account::<T::AccountId>("invalid", 0, seed).clone())
+			.collect();
 		invalid.push(all_candidates.last().unwrap().clone());
 		submit_voter::<T>(
 			account_2.clone(),
@@ -308,11 +324,11 @@ benchmarks! {
 			stake,
 		)?;

-		let defunct = defunct_for::<T>(account_2.clone());
-		// no one bails out. account_1 is slashed and removed as voter now.
-	}: report_defunct_voter(RawOrigin::Signed(account_1.clone()), defunct)
+		let defunct_info = defunct_for::<T>(account_2.clone());
+		whitelist!(account_1);
+	}: report_defunct_voter(RawOrigin::Signed(account_1.clone()), defunct_info)
 	verify {
-		assert!(!<Elections<T>>::is_voter(&account_1));
+		// account 2 is still a voter.
 		assert!(<Elections<T>>::is_voter(&account_2));
 		#[cfg(test)]
 		{
@@ -325,7 +341,7 @@ benchmarks! {
 	submit_candidacy {
 		// number of already existing candidates.
 		let c in 1 .. MAX_CANDIDATES;
-		// we fix the number of members to when members and runners-up to the desired. We'll be in
+		// we fix the number of members to the number of desired members and runners-up. We'll be in
 		// this state almost always.
 		let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();

@@ -340,6 +356,7 @@ benchmarks! {

 		// we assume worse case that: extrinsic is successful and candidate is not duplicate.
 		let candidate_account = endowed_account::<T>("caller", 0);
+		whitelist!(candidate_account);
 	}: _(RawOrigin::Signed(candidate_account.clone()), candidate_count::<T>())
 	verify {
 		#[cfg(test)]
@@ -355,7 +372,7 @@ benchmarks! {
 		// limited by the runtime bound, nonetheless we fill them by `m`.
 		// number of already existing candidates.
 		let c in 1 .. MAX_CANDIDATES;
-		// we fix the number of members to when members and runners-up to the desired. We'll be in
+		// we fix the number of members to the number of desired members and runners-up. We'll be in
 		// this state almost always.
 		let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();

@@ -367,6 +384,7 @@ benchmarks! {

 		let bailing = all_candidates[0].clone(); // Should be ("caller", 0)
 		let count = candidate_count::<T>();
+		whitelist!(bailing);
 	}: renounce_candidacy(RawOrigin::Signed(bailing), Renouncing::Candidate(count))
 	verify {
 		#[cfg(test)]
@@ -377,11 +395,10 @@ benchmarks! {
 		}
 	}

-	renounce_candidacy_member_runner_up {
+	renounce_candidacy_members {
 		// removing members and runners will be cheaper than a candidate.
 		// we fix the number of members to when members and runners-up to the desired. We'll be in
 		// this state almost always.
-		let u in ...;
 		let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
 		clean::<T>();

@@ -389,14 +406,34 @@ benchmarks! {
 		let members_and_runners_up = fill_seats_up_to::<T>(m)?;

 		let bailing = members_and_runners_up[0].clone();
-		let renouncing = if <Elections<T>>::is_member(&bailing) {
-			Renouncing::Member
-		} else if <Elections<T>>::is_runner_up(&bailing) {
-			Renouncing::RunnerUp
-		} else {
-			panic!("Bailing must be a member or runner-up for this bench to be sane.");
-		};
-	}: renounce_candidacy(RawOrigin::Signed(bailing.clone()), renouncing)
+		assert!(<Elections<T>>::is_member(&bailing));
+
+		whitelist!(bailing);
+	}: renounce_candidacy(RawOrigin::Signed(bailing.clone()), Renouncing::Member)
+	verify {
+		#[cfg(test)]
+		{
+			// reset members in between benchmark tests.
+			use crate::tests::MEMBERS;
+			MEMBERS.with(|m| *m.borrow_mut() = vec![]);
+		}
+	}
+
+	renounce_candidacy_runners_up {
+		// removing members and runners will be cheaper than a candidate.
+		// we fix the number of members to when members and runners-up to the desired. We'll be in
+		// this state almost always.
+		let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
+		clean::<T>();
+
+		// create m members and runners combined.
+		let members_and_runners_up = fill_seats_up_to::<T>(m)?;
+
+		let bailing = members_and_runners_up[T::DesiredMembers::get() as usize + 1].clone();
+		assert!(<Elections<T>>::is_runner_up(&bailing));
+
+		whitelist!(bailing);
+	}: renounce_candidacy(RawOrigin::Signed(bailing.clone()), Renouncing::RunnerUp)
 	verify {
 		#[cfg(test)]
 		{
@@ -407,6 +444,7 @@ benchmarks! {
 	}

 	// -- Root ones
+	#[extra] // this calls into phragmen and consumes a full block for now.
 	remove_member_without_replacement {
 		// worse case is when we remove a member and we have no runner as a replacement. This
 		// triggers phragmen again. The only parameter is how many candidates will compete for the
@@ -440,7 +478,6 @@ benchmarks! {
 	remove_member_with_replacement {
 		// easy case. We have a runner up. Nothing will have that much of an impact. m will be
 		// number of members and runners. There is always at least one runner.
-		let u in ...;
 		let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
 		clean::<T>();

@@ -461,7 +498,6 @@ benchmarks! {
 	remove_member_wrong_refund {
 		// The root call by mistake indicated that this will have no replacement, while it has!
 		// this has now consumed a lot of weight and need to refund.
-		let u in ...;
 		let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get();
 		clean::<T>();

@@ -484,6 +520,7 @@ benchmarks! {
 		}
 	}

+	#[extra]
 	on_initialize {
 		// if n % TermDuration is zero, then we run phragmen. The weight function must and should
 		// check this as it is cheap to do so. TermDuration is not a storage item, it is a constant
@@ -514,6 +551,7 @@ benchmarks! {
 		}
 	}

+	#[extra]
 	phragmen {
 		// This is just to focus on phragmen in the context of this module. We always select 20
 		// members, this is hard-coded in the runtime and cannot be trivially changed at this stage.
@@ -578,7 +616,11 @@ mod tests {
 		});

 		ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
-			assert_ok!(test_benchmark_renounce_candidacy_member_runner_up::<Test>());
+			assert_ok!(test_benchmark_renounce_candidacy_runners_up::<Test>());
+		});
+
+		ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
+			assert_ok!(test_benchmark_renounce_candidacy_members::<Test>());
 		});

 		ExtBuilder::default().desired_members(13).desired_runners_up(7).build_and_execute(|| {
@@ -0,0 +1,88 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// 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
+
+#![allow(unused_parens)]
+#![allow(unused_imports)]
+
+use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight};
+
+impl crate::WeightInfo for () {
+	fn vote(v: u32, ) -> Weight {
+		(91_489_000 as Weight)
+			.saturating_add((199_000 as Weight).saturating_mul(v as Weight))
+			.saturating_add(DbWeight::get().reads(5 as Weight))
+			.saturating_add(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))
+			.saturating_add(DbWeight::get().reads(5 as Weight))
+			.saturating_add(DbWeight::get().writes(2 as Weight))
+	}
+	fn remove_voter() -> Weight {
+		(76_714_000 as Weight)
+			.saturating_add(DbWeight::get().reads(2 as Weight))
+			.saturating_add(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(DbWeight::get().reads(7 as Weight))
+			.saturating_add(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(DbWeight::get().reads(6 as Weight))
+			.saturating_add(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))
+			.saturating_add(DbWeight::get().reads(3 as Weight))
+			.saturating_add(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))
+			.saturating_add(DbWeight::get().reads(1 as Weight))
+			.saturating_add(DbWeight::get().writes(1 as Weight))
+	}
+	fn renounce_candidacy_members() -> Weight {
+		(79_626_000 as Weight)
+			.saturating_add(DbWeight::get().reads(3 as Weight))
+			.saturating_add(DbWeight::get().writes(4 as Weight))
+	}
+	fn renounce_candidacy_runners_up() -> Weight {
+		(49_715_000 as Weight)
+			.saturating_add(DbWeight::get().reads(1 as Weight))
+			.saturating_add(DbWeight::get().writes(1 as Weight))
+	}
+	fn remove_member_with_replacement() -> Weight {
+		(76_572_000 as Weight)
+			.saturating_add(DbWeight::get().reads(4 as Weight))
+			.saturating_add(DbWeight::get().writes(5 as Weight))
+	}
+	fn remove_member_wrong_refund() -> Weight {
+		(8_777_000 as Weight)
+			.saturating_add(DbWeight::get().reads(1 as Weight))
+	}
+}
@@ -91,12 +91,12 @@ use sp_runtime::{
 };
 use frame_support::{
 	decl_storage, decl_event, ensure, decl_module, decl_error,
-	weights::{Weight, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}},
+	weights::Weight,
 	storage::{StorageMap, IterableStorageMap},
 	dispatch::{DispatchResultWithPostInfo, WithPostDispatchInfo},
 	traits::{
 		Currency, Get, LockableCurrency, LockIdentifier, ReservableCurrency, WithdrawReasons,
-		ChangeMembers, OnUnbalanced, WithdrawReason, Contains, BalanceStatus, InitializeMembers,
+		ChangeMembers, OnUnbalanced, WithdrawReason, Contains, InitializeMembers, BalanceStatus,
 		ContainsLengthBound,
 	}
 };
@@ -104,6 +104,7 @@ use sp_npos_elections::{build_support_map, ExtendedBalance, VoteWeight, Election
 use frame_system::{ensure_signed, ensure_root};

 mod benchmarking;
+mod default_weights;

 /// The maximum votes allowed per voter.
 pub const MAXIMUM_VOTE: usize = 16;
@@ -138,35 +139,17 @@ pub struct DefunctVoter<AccountId> {
 }

 pub trait WeightInfo {
-	fn vote(u: u32, ) -> Weight;
-	fn vote_update(u: u32, ) -> Weight;
-	fn remove_voter(u: u32, ) -> Weight;
+	fn vote(v: u32, ) -> Weight;
+	fn vote_update(v: u32, ) -> Weight;
+	fn remove_voter() -> Weight;
 	fn report_defunct_voter_correct(c: u32, v: u32, ) -> Weight;
 	fn report_defunct_voter_incorrect(c: u32, v: u32, ) -> Weight;
 	fn submit_candidacy(c: u32, ) -> Weight;
 	fn renounce_candidacy_candidate(c: u32, ) -> Weight;
-	fn renounce_candidacy_member_runner_up(u: u32, ) -> Weight;
-	fn remove_member_without_replacement(c: u32, ) -> Weight;
-	fn remove_member_with_replacement(u: u32, ) -> Weight;
-	fn remove_member_wrong_refund(u: u32, ) -> Weight;
-	fn on_initialize(c: u32, ) -> Weight;
-	fn phragmen(c: u32, v: u32, e: u32, ) -> Weight;
-}
-
-impl WeightInfo for () {
-	fn vote(_u: u32, ) -> Weight { 1_000_000_000 }
-	fn vote_update(_u: u32, ) -> Weight { 1_000_000_000 }
-	fn remove_voter(_u: u32, ) -> Weight { 1_000_000_000 }
-	fn report_defunct_voter_correct(_c: u32, _v: u32, ) -> Weight { 1_000_000_000 }
-	fn report_defunct_voter_incorrect(_c: u32, _v: u32, ) -> Weight { 1_000_000_000 }
-	fn submit_candidacy(_c: u32, ) -> Weight { 1_000_000_000 }
-	fn renounce_candidacy_candidate(_c: u32, ) -> Weight { 1_000_000_000 }
-	fn renounce_candidacy_member_runner_up(_u: u32, ) -> Weight { 1_000_000_000 }
-	fn remove_member_without_replacement(_c: u32, ) -> Weight { 1_000_000_000 }
-	fn remove_member_with_replacement(_u: u32, ) -> Weight { 1_000_000_000 }
-	fn remove_member_wrong_refund(_u: u32, ) -> Weight { 1_000_000_000 }
-	fn on_initialize(_c: u32, ) -> Weight { 1_000_000_000 }
-	fn phragmen(_c: u32, _v: u32, _e: u32, ) -> Weight { 1_000_000_000 }
+	fn renounce_candidacy_members() -> Weight;
+	fn renounce_candidacy_runners_up() -> Weight;
+	fn remove_member_with_replacement() -> Weight;
+	fn remove_member_wrong_refund() -> Weight;
 }

 pub trait Trait: frame_system::Trait {
@@ -350,13 +333,14 @@ decl_module! {
 		/// State reads:
 		/// 	- Candidates.len() + Members.len() + RunnersUp.len()
 		/// 	- Voting (is_voter)
+		/// 	- Lock
 		/// 	- [AccountBalance(who) (unreserve + total_balance)]
 		/// State writes:
 		/// 	- Voting
 		/// 	- Lock
 		/// 	- [AccountBalance(who) (unreserve -- only when creating a new voter)]
 		/// # </weight>
-		#[weight = 50 * WEIGHT_PER_MICROS + T::DbWeight::get().reads_writes(4, 2)]
+		#[weight = T::WeightInfo::vote(votes.len() as u32)]
 		fn vote(
 			origin,
 			votes: Vec<T::AccountId>,
@@ -412,7 +396,7 @@ decl_module! {
 		/// 	- Locks
 		/// 	- [AccountData(who)]
 		/// # </weight>
-		#[weight = 35 * WEIGHT_PER_MICROS + T::DbWeight::get().reads_writes(1, 2)]
+		#[weight = T::WeightInfo::remove_voter()]
 		fn remove_voter(origin) {
 			let who = ensure_signed(origin)?;
 			ensure!(Self::is_voter(&who), Error::<T>::MustBeVoter);
@@ -447,15 +431,14 @@ decl_module! {
 		/// 	- Voting(reporter || target)
 		/// Note: the db access is worse with respect to db, which is when the report is correct.
 		/// # </weight>
-		#[weight =
-			Weight::from(defunct.candidate_count).saturating_mul(2 * WEIGHT_PER_MICROS)
-			.saturating_add(Weight::from(defunct.vote_count).saturating_mul(19 * WEIGHT_PER_MICROS))
-			.saturating_add(T::DbWeight::get().reads_writes(6, 3))
-		]
+		#[weight = T::WeightInfo::report_defunct_voter_correct(
+			defunct.candidate_count,
+			defunct.vote_count,
+		)]
 		fn report_defunct_voter(
 			origin,
 			defunct: DefunctVoter<<T::Lookup as StaticLookup>::Source>,
-		) {
+		) -> DispatchResultWithPostInfo {
 			let reporter = ensure_signed(origin)?;
 			let target = T::Lookup::lookup(defunct.who)?;

@@ -482,19 +465,25 @@ decl_module! {
 			);

 			let valid = Self::is_defunct_voter(&votes);
-			if valid {
+			let maybe_refund = if valid {
 				// reporter will get the voting bond of the target
 				T::Currency::repatriate_reserved(&target, &reporter, T::VotingBond::get(), BalanceStatus::Free)?;
 				// remove the target. They are defunct.
 				Self::do_remove_voter(&target, false);
+				None
 			} else {
 				// slash the bond of the reporter.
 				let imbalance = T::Currency::slash_reserved(&reporter, T::VotingBond::get()).0;
 				T::BadReport::on_unbalanced(imbalance);
 				// remove the reporter.
 				Self::do_remove_voter(&reporter, false);
-			}
+				Some(T::WeightInfo::report_defunct_voter_incorrect(
+					defunct.candidate_count,
+					defunct.vote_count,
+				))
+			};
 			Self::deposit_event(RawEvent::VoterReported(target, reporter, valid));
+			Ok(maybe_refund.into())
 		}

 		/// Submit oneself for candidacy.
@@ -509,7 +498,6 @@ decl_module! {
 		/// Base weight = 33.33 µs
 		/// Complexity of candidate_count: 0.375 µs
 		/// State reads:
-		/// 	- Candidates.len()
 		/// 	- Candidates
 		/// 	- Members
 		/// 	- RunnersUp
@@ -518,11 +506,7 @@ decl_module! {
 		/// 	- [AccountBalance(who)]
 		/// 	- Candidates
 		/// # </weight>
-		#[weight =
-			(35 * WEIGHT_PER_MICROS)
-			.saturating_add(Weight::from(*candidate_count).saturating_mul(375 * WEIGHT_PER_NANOS))
-			.saturating_add(T::DbWeight::get().reads_writes(4, 1))
-		]
+		#[weight = T::WeightInfo::submit_candidacy(*candidate_count)]
 		fn submit_candidacy(origin, #[compact] candidate_count: u32) {
 			let who = ensure_signed(origin)?;

@@ -582,23 +566,11 @@ decl_module! {
 		/// 	State writes:
 		/// 		- RunnersUp (remove_and_replace_member),
 		/// 		- [AccountData(who) (unreserve)]
-		///
-		/// Weight note: The call into changeMembers need to be accounted for.
 		/// </weight>
 		#[weight =  match *renouncing {
-			Renouncing::Candidate(count) => {
-				(18 * WEIGHT_PER_MICROS)
-				.saturating_add(Weight::from(count).saturating_mul(235 * WEIGHT_PER_NANOS))
-				.saturating_add(T::DbWeight::get().reads_writes(1, 1))
-			},
-			Renouncing::Member => {
-				46 * WEIGHT_PER_MICROS +
-				T::DbWeight::get().reads_writes(2, 2)
-			},
-			Renouncing::RunnerUp => {
-				46 * WEIGHT_PER_MICROS +
-				T::DbWeight::get().reads_writes(1, 1)
-			}
+			Renouncing::Candidate(count) => T::WeightInfo::renounce_candidacy_candidate(count),
+			Renouncing::Member => T::WeightInfo::renounce_candidacy_members(),
+			Renouncing::RunnerUp => T::WeightInfo::renounce_candidacy_runners_up(),
 		}]
 		fn renounce_candidacy(origin, renouncing: Renouncing) {
 			let who = ensure_signed(origin)?;
@@ -659,7 +631,7 @@ decl_module! {
 		/// Else, since this is a root call and will go into phragmen, we assume full block for now.
 		/// # </weight>
 		#[weight = if *has_replacement {
-			50 * WEIGHT_PER_MICROS + T::DbWeight::get().reads_writes(3, 2)
+			T::WeightInfo::remove_member_with_replacement()
 		} else {
 			T::MaximumBlockWeight::get()
 		}]
@@ -677,7 +649,7 @@ decl_module! {
 				return Err(Error::<T>::InvalidReplacement.with_weight(
 					// refund. The weight value comes from a benchmark which is special to this.
 					//  5.751 µs
-					6 * WEIGHT_PER_MICROS + T::DbWeight::get().reads_writes(1, 0)
+					T::WeightInfo::remove_member_wrong_refund()
 				));
 			} // else, prediction was correct.

@@ -832,7 +804,9 @@ impl<T: Trait> Module<T> {
 	/// Reads Members, RunnersUp, Candidates and Voting(who) from database.
 	fn is_defunct_voter(votes: &[T::AccountId]) -> bool {
 		votes.iter().all(|v|
-			!Self::is_member(v) && !Self::is_runner_up(v) && !Self::is_candidate(v).is_ok()
+			!Self::is_member(v) &&
+			!Self::is_runner_up(v) &&
+			!Self::is_candidate(v).is_ok()
 		)
 	}

@@ -906,12 +880,12 @@ impl<T: Trait> Module<T> {

 		// used for prime election.
 		let voters_and_stakes = Voting::<T>::iter()
-			.map(|(voter, (stake, targets))| { (voter, stake, targets) })
+			.map(|(voter, (stake, votes))| { (voter, stake, votes) })
 			.collect::<Vec<_>>();
 		// used for phragmen.
 		let voters_and_votes = voters_and_stakes.iter()
 			.cloned()
-			.map(|(voter, stake, targets)| { (voter, to_votes(stake), targets)} )
+			.map(|(voter, stake, votes)| { (voter, to_votes(stake), votes)} )
 			.collect::<Vec<_>>();
 		let maybe_phragmen_result = sp_npos_elections::seq_phragmen::<T::AccountId, Perbill>(
 			num_to_elect,
@@ -976,8 +950,8 @@ impl<T: Trait> Module<T> {
 			// 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.iter().map(|c| (&c.0, BalanceOf::<T>::zero())).collect();
-			for (_, stake, targets) in voters_and_stakes.into_iter() {
-				for (vote_multiplier, who) in targets.iter()
+			for (_, stake, votes) in voters_and_stakes.into_iter() {
+				for (vote_multiplier, who) in votes.iter()
 					.enumerate()
 					.map(|(vote_position, who)| ((MAXIMUM_VOTE - vote_position) as u32, who))
 				{
@@ -2434,7 +2408,7 @@ mod tests {
 			assert_err_with_weight!(
 				Elections::remove_member(Origin::root(), 4, true),
 				Error::<Test>::InvalidReplacement,
-				Some(6000000),
+				Some(33777000), // only thing that matters for now is that it is NOT the full block.
 			);
 		});

@@ -2456,7 +2430,7 @@ mod tests {
 			assert_err_with_weight!(
 				Elections::remove_member(Origin::root(), 4, false),
 				Error::<Test>::InvalidReplacement,
-				Some(6000000) // only thing that matters for now is that it is NOT the full block.
+				Some(33777000) // only thing that matters for now is that it is NOT the full block.
 			);
 		});
 	}