From 9353a2829d88ad620478d19ada000338d74274ef Mon Sep 17 00:00:00 2001
From: Xavier Lau <x@acg.box>
Date: Mon, 4 Nov 2024 20:16:49 +0800
Subject: [PATCH] Migrate pallet-election-provider-multi-phase benchmark to v2
 and improve doc (#6316)

Part of:

- #6202.

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Guillaume Thiolliere <guillaume.thiolliere@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
---
 prdoc/pr_6316.prdoc                           |   8 +
 .../src/benchmarking.rs                       | 438 +++++++++++-------
 2 files changed, 274 insertions(+), 172 deletions(-)
 create mode 100644 prdoc/pr_6316.prdoc

diff --git a/prdoc/pr_6316.prdoc b/prdoc/pr_6316.prdoc
new file mode 100644
index 00000000000..00ad8699ff8
--- /dev/null
+++ b/prdoc/pr_6316.prdoc
@@ -0,0 +1,8 @@
+title: Migrate pallet-election-provider-multi-phase benchmark to v2 and improve doc
+doc:
+- audience: Runtime Dev
+  description: |-
+    Migrate pallet-election-provider-multi-phase benchmark to v2 and improve doc
+crates:
+- name: pallet-election-provider-multi-phase
+  bump: patch
diff --git a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs
index 2a3994ff2aa..222e79ab99c 100644
--- a/substrate/frame/election-provider-multi-phase/src/benchmarking.rs
+++ b/substrate/frame/election-provider-multi-phase/src/benchmarking.rs
@@ -17,10 +17,9 @@
 
 //! Two phase election pallet benchmarking.
 
-use super::*;
-use crate::{unsigned::IndexAssignmentOf, Pallet as MultiPhase};
-use frame_benchmarking::account;
-use frame_election_provider_support::bounds::DataProviderBounds;
+use core::cmp::Reverse;
+use frame_benchmarking::{v2::*, BenchmarkError};
+use frame_election_provider_support::{bounds::DataProviderBounds, IndexAssignment};
 use frame_support::{
 	assert_ok,
 	traits::{Hooks, TryCollect},
@@ -31,6 +30,8 @@ use rand::{prelude::SliceRandom, rngs::SmallRng, SeedableRng};
 use sp_arithmetic::{per_things::Percent, traits::One};
 use sp_runtime::InnerOf;
 
+use crate::{unsigned::IndexAssignmentOf, *};
+
 const SEED: u32 = 999;
 
 /// Creates a **valid** solution with exactly the given size.
@@ -133,7 +134,7 @@ fn solution_with_size<T: Config>(
 		.map(|(voter, _stake, votes)| {
 			let percent_per_edge: InnerOf<SolutionAccuracyOf<T>> =
 				(100 / votes.len()).try_into().unwrap_or_else(|_| panic!("failed to convert"));
-			crate::unsigned::Assignment::<T> {
+			unsigned::Assignment::<T> {
 				who: voter.clone(),
 				distribution: votes
 					.iter()
@@ -190,140 +191,179 @@ fn set_up_data_provider<T: Config>(v: u32, t: u32) {
 	});
 }
 
-frame_benchmarking::benchmarks! {
-	on_initialize_nothing {
+#[benchmarks]
+mod benchmarks {
+	use super::*;
+
+	#[benchmark]
+	fn on_initialize_nothing() {
 		assert!(CurrentPhase::<T>::get().is_off());
-	}: {
-		MultiPhase::<T>::on_initialize(1u32.into());
-	} verify {
+
+		#[block]
+		{
+			Pallet::<T>::on_initialize(1_u32.into());
+		}
+
 		assert!(CurrentPhase::<T>::get().is_off());
 	}
 
-	on_initialize_open_signed {
+	#[benchmark]
+	fn on_initialize_open_signed() {
 		assert!(Snapshot::<T>::get().is_none());
 		assert!(CurrentPhase::<T>::get().is_off());
-	}: {
-		MultiPhase::<T>::phase_transition(Phase::Signed);
-	} verify {
+
+		#[block]
+		{
+			Pallet::<T>::phase_transition(Phase::Signed);
+		}
+
 		assert!(Snapshot::<T>::get().is_none());
 		assert!(CurrentPhase::<T>::get().is_signed());
 	}
 
-	on_initialize_open_unsigned {
+	#[benchmark]
+	fn on_initialize_open_unsigned() {
 		assert!(Snapshot::<T>::get().is_none());
 		assert!(CurrentPhase::<T>::get().is_off());
-	}: {
-		let now = frame_system::Pallet::<T>::block_number();
-		MultiPhase::<T>::phase_transition(Phase::Unsigned((true, now)));
-	} verify {
+
+		#[block]
+		{
+			let now = frame_system::Pallet::<T>::block_number();
+			Pallet::<T>::phase_transition(Phase::Unsigned((true, now)));
+		}
+
 		assert!(Snapshot::<T>::get().is_none());
 		assert!(CurrentPhase::<T>::get().is_unsigned());
 	}
 
-	finalize_signed_phase_accept_solution {
+	#[benchmark]
+	fn finalize_signed_phase_accept_solution() {
 		let receiver = account("receiver", 0, SEED);
-		let initial_balance = T::Currency::minimum_balance() + 10u32.into();
+		let initial_balance = T::Currency::minimum_balance() + 10_u32.into();
 		T::Currency::make_free_balance_be(&receiver, initial_balance);
 		let ready = Default::default();
-		let deposit: BalanceOf<T> = 10u32.into();
+		let deposit: BalanceOf<T> = 10_u32.into();
 
 		let reward: BalanceOf<T> = T::SignedRewardBase::get();
-		let call_fee: BalanceOf<T> = 30u32.into();
+		let call_fee: BalanceOf<T> = 30_u32.into();
 
 		assert_ok!(T::Currency::reserve(&receiver, deposit));
 		assert_eq!(T::Currency::free_balance(&receiver), T::Currency::minimum_balance());
-	}: {
-		MultiPhase::<T>::finalize_signed_phase_accept_solution(
-			ready,
-			&receiver,
-			deposit,
-			call_fee
-		)
-	} verify {
-		assert_eq!(
-			T::Currency::free_balance(&receiver),
-			initial_balance + reward + call_fee
-		);
-		assert_eq!(T::Currency::reserved_balance(&receiver), 0u32.into());
+
+		#[block]
+		{
+			Pallet::<T>::finalize_signed_phase_accept_solution(ready, &receiver, deposit, call_fee);
+		}
+
+		assert_eq!(T::Currency::free_balance(&receiver), initial_balance + reward + call_fee);
+		assert_eq!(T::Currency::reserved_balance(&receiver), 0_u32.into());
 	}
 
-	finalize_signed_phase_reject_solution {
+	#[benchmark]
+	fn finalize_signed_phase_reject_solution() {
 		let receiver = account("receiver", 0, SEED);
-		let initial_balance = T::Currency::minimum_balance() + 10u32.into();
-		let deposit: BalanceOf<T> = 10u32.into();
+		let initial_balance = T::Currency::minimum_balance() + 10_u32.into();
+		let deposit: BalanceOf<T> = 10_u32.into();
 		T::Currency::make_free_balance_be(&receiver, initial_balance);
 		assert_ok!(T::Currency::reserve(&receiver, deposit));
 
 		assert_eq!(T::Currency::free_balance(&receiver), T::Currency::minimum_balance());
-		assert_eq!(T::Currency::reserved_balance(&receiver), 10u32.into());
-	}: {
-		MultiPhase::<T>::finalize_signed_phase_reject_solution(&receiver, deposit)
-	} verify {
+		assert_eq!(T::Currency::reserved_balance(&receiver), 10_u32.into());
+
+		#[block]
+		{
+			Pallet::<T>::finalize_signed_phase_reject_solution(&receiver, deposit)
+		}
+
 		assert_eq!(T::Currency::free_balance(&receiver), T::Currency::minimum_balance());
-		assert_eq!(T::Currency::reserved_balance(&receiver), 0u32.into());
+		assert_eq!(T::Currency::reserved_balance(&receiver), 0_u32.into());
 	}
 
-	create_snapshot_internal {
-		// number of votes in snapshot.
-		let v in (T::BenchmarkingConfig::VOTERS[0]) .. T::BenchmarkingConfig::VOTERS[1];
-		// number of targets in snapshot.
-		let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1];
-
-		// we don't directly need the data-provider to be populated, but it is just easy to use it.
+	#[benchmark]
+	fn create_snapshot_internal(
+		// Number of votes in snapshot.
+		v: Linear<{ T::BenchmarkingConfig::VOTERS[0] }, { T::BenchmarkingConfig::VOTERS[1] }>,
+		// Number of targets in snapshot.
+		t: Linear<{ T::BenchmarkingConfig::TARGETS[0] }, { T::BenchmarkingConfig::TARGETS[1] }>,
+	) -> Result<(), BenchmarkError> {
+		// We don't directly need the data-provider to be populated, but it is just easy to use it.
 		set_up_data_provider::<T>(v, t);
-		// default bounds are unbounded.
+		// Default bounds are unbounded.
 		let targets = T::DataProvider::electable_targets(DataProviderBounds::default())?;
 		let voters = T::DataProvider::electing_voters(DataProviderBounds::default())?;
 		let desired_targets = T::DataProvider::desired_targets()?;
 		assert!(Snapshot::<T>::get().is_none());
-	}: {
-		MultiPhase::<T>::create_snapshot_internal(targets, voters, desired_targets)
-	} verify {
+
+		#[block]
+		{
+			Pallet::<T>::create_snapshot_internal(targets, voters, desired_targets)
+		}
+
 		assert!(Snapshot::<T>::get().is_some());
 		assert_eq!(SnapshotMetadata::<T>::get().ok_or("metadata missing")?.voters, v);
 		assert_eq!(SnapshotMetadata::<T>::get().ok_or("metadata missing")?.targets, t);
+
+		Ok(())
 	}
 
-	// a call to `<Pallet as ElectionProvider>::elect` where we only return the queued solution.
-	elect_queued {
-		// number of assignments, i.e. solution.len(). This means the active nominators, thus must be
-		// a subset of `v`.
-		let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
-		// number of desired targets. Must be a subset of `t`.
-		let d in (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. T::BenchmarkingConfig::DESIRED_TARGETS[1];
-
-		// number of votes in snapshot. Not dominant.
-		let v  = T::BenchmarkingConfig::VOTERS[1];
-		// number of targets in snapshot. Not dominant.
+	// A call to `<Pallet as ElectionProvider>::elect` where we only return the queued solution.
+	#[benchmark]
+	fn elect_queued(
+		// Number of assignments, i.e. `solution.len()`.
+		// This means the active nominators, thus must be a subset of `v`.
+		a: Linear<
+			{ T::BenchmarkingConfig::ACTIVE_VOTERS[0] },
+			{ T::BenchmarkingConfig::ACTIVE_VOTERS[1] },
+		>,
+		// Number of desired targets. Must be a subset of `t`.
+		d: Linear<
+			{ T::BenchmarkingConfig::DESIRED_TARGETS[0] },
+			{ T::BenchmarkingConfig::DESIRED_TARGETS[1] },
+		>,
+	) -> Result<(), BenchmarkError> {
+		// Number of votes in snapshot. Not dominant.
+		let v = T::BenchmarkingConfig::VOTERS[1];
+		// Number of targets in snapshot. Not dominant.
 		let t = T::BenchmarkingConfig::TARGETS[1];
 
 		let witness = SolutionOrSnapshotSize { voters: v, targets: t };
 		let raw_solution = solution_with_size::<T>(witness, a, d)?;
-		let ready_solution =
-			MultiPhase::<T>::feasibility_check(raw_solution, ElectionCompute::Signed)
-				.map_err(<&str>::from)?;
+		let ready_solution = Pallet::<T>::feasibility_check(raw_solution, ElectionCompute::Signed)
+			.map_err(<&str>::from)?;
 		CurrentPhase::<T>::put(Phase::Signed);
-		// assume a queued solution is stored, regardless of where it comes from.
+		// Assume a queued solution is stored, regardless of where it comes from.
 		QueuedSolution::<T>::put(ready_solution);
 
-		// these are set by the `solution_with_size` function.
+		// These are set by the `solution_with_size` function.
 		assert!(DesiredTargets::<T>::get().is_some());
 		assert!(Snapshot::<T>::get().is_some());
 		assert!(SnapshotMetadata::<T>::get().is_some());
-	}: {
-		assert_ok!(<MultiPhase<T> as ElectionProvider>::elect());
-	} verify {
+
+		let result;
+
+		#[block]
+		{
+			result = <Pallet<T> as ElectionProvider>::elect();
+		}
+
+		assert!(result.is_ok());
 		assert!(QueuedSolution::<T>::get().is_none());
 		assert!(DesiredTargets::<T>::get().is_none());
 		assert!(Snapshot::<T>::get().is_none());
 		assert!(SnapshotMetadata::<T>::get().is_none());
-		assert_eq!(CurrentPhase::<T>::get(), <Phase<frame_system::pallet_prelude::BlockNumberFor::<T>>>::Off);
+		assert_eq!(
+			CurrentPhase::<T>::get(),
+			<Phase<frame_system::pallet_prelude::BlockNumberFor::<T>>>::Off
+		);
+
+		Ok(())
 	}
 
-	submit {
-		// the queue is full and the solution is only better than the worse.
-		MultiPhase::<T>::create_snapshot().map_err(<&str>::from)?;
-		MultiPhase::<T>::phase_transition(Phase::Signed);
+	#[benchmark]
+	fn submit() -> Result<(), BenchmarkError> {
+		// The queue is full and the solution is only better than the worse.
+		Pallet::<T>::create_snapshot().map_err(<&str>::from)?;
+		Pallet::<T>::phase_transition(Phase::Signed);
 		Round::<T>::put(1);
 
 		let mut signed_submissions = SignedSubmissions::<T>::get();
@@ -331,7 +371,10 @@ frame_benchmarking::benchmarks! {
 		// Insert `max` submissions
 		for i in 0..(T::SignedMaxSubmissions::get() - 1) {
 			let raw_solution = RawSolution {
-				score: ElectionScore { minimal_stake: 10_000_000u128 + (i as u128), ..Default::default() },
+				score: ElectionScore {
+					minimal_stake: 10_000_000u128 + (i as u128),
+					..Default::default()
+				},
 				..Default::default()
 			};
 			let signed_submission = SignedSubmission {
@@ -344,67 +387,95 @@ frame_benchmarking::benchmarks! {
 		}
 		signed_submissions.put();
 
-		// this score will eject the weakest one.
+		// This score will eject the weakest one.
 		let solution = RawSolution {
 			score: ElectionScore { minimal_stake: 10_000_000u128 + 1, ..Default::default() },
 			..Default::default()
 		};
 
 		let caller = frame_benchmarking::whitelisted_caller();
-		let deposit = MultiPhase::<T>::deposit_for(
-			&solution,
-			SnapshotMetadata::<T>::get().unwrap_or_default(),
+		let deposit =
+			Pallet::<T>::deposit_for(&solution, SnapshotMetadata::<T>::get().unwrap_or_default());
+		T::Currency::make_free_balance_be(
+			&caller,
+			T::Currency::minimum_balance() * 1000u32.into() + deposit,
 		);
-		T::Currency::make_free_balance_be(&caller,  T::Currency::minimum_balance() * 1000u32.into() + deposit);
 
-	}: _(RawOrigin::Signed(caller), Box::new(solution))
-	verify {
-		assert!(MultiPhase::<T>::signed_submissions().len() as u32 == T::SignedMaxSubmissions::get());
-	}
+		#[extrinsic_call]
+		_(RawOrigin::Signed(caller), Box::new(solution));
 
-	submit_unsigned {
-		// number of votes in snapshot.
-		let v in (T::BenchmarkingConfig::VOTERS[0]) .. T::BenchmarkingConfig::VOTERS[1];
-		// number of targets in snapshot.
-		let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1];
-		// number of assignments, i.e. solution.len(). This means the active nominators, thus must be
-		// a subset of `v` component.
-		let a in
-			(T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
-		// number of desired targets. Must be a subset of `t` component.
-		let d in
-			(T::BenchmarkingConfig::DESIRED_TARGETS[0]) ..
-			T::BenchmarkingConfig::DESIRED_TARGETS[1];
+		assert!(Pallet::<T>::signed_submissions().len() as u32 == T::SignedMaxSubmissions::get());
 
+		Ok(())
+	}
+
+	#[benchmark]
+	fn submit_unsigned(
+		// Number of votes in snapshot.
+		v: Linear<{ T::BenchmarkingConfig::VOTERS[0] }, { T::BenchmarkingConfig::VOTERS[1] }>,
+		// Number of targets in snapshot.
+		t: Linear<{ T::BenchmarkingConfig::TARGETS[0] }, { T::BenchmarkingConfig::TARGETS[1] }>,
+		// Number of assignments, i.e. `solution.len()`.
+		// This means the active nominators, thus must be a subset of `v` component.
+		a: Linear<
+			{ T::BenchmarkingConfig::ACTIVE_VOTERS[0] },
+			{ T::BenchmarkingConfig::ACTIVE_VOTERS[1] },
+		>,
+		// Number of desired targets. Must be a subset of `t` component.
+		d: Linear<
+			{ T::BenchmarkingConfig::DESIRED_TARGETS[0] },
+			{ T::BenchmarkingConfig::DESIRED_TARGETS[1] },
+		>,
+	) -> Result<(), BenchmarkError> {
 		let witness = SolutionOrSnapshotSize { voters: v, targets: t };
 		let raw_solution = solution_with_size::<T>(witness, a, d)?;
 
 		assert!(QueuedSolution::<T>::get().is_none());
-		CurrentPhase::<T>::put(Phase::Unsigned((true, 1u32.into())));
-	}: _(RawOrigin::None, Box::new(raw_solution), witness)
-	verify {
+		CurrentPhase::<T>::put(Phase::Unsigned((true, 1_u32.into())));
+
+		#[extrinsic_call]
+		_(RawOrigin::None, Box::new(raw_solution), witness);
+
 		assert!(QueuedSolution::<T>::get().is_some());
+
+		Ok(())
 	}
 
 	// This is checking a valid solution. The worse case is indeed a valid solution.
-	feasibility_check {
-		// number of votes in snapshot.
-		let v in (T::BenchmarkingConfig::VOTERS[0]) .. T::BenchmarkingConfig::VOTERS[1];
-		// number of targets in snapshot.
-		let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1];
-		// number of assignments, i.e. solution.len(). This means the active nominators, thus must be
-		// a subset of `v` component.
-		let a in (T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
-		// number of desired targets. Must be a subset of `t` component.
-		let d in (T::BenchmarkingConfig::DESIRED_TARGETS[0]) .. T::BenchmarkingConfig::DESIRED_TARGETS[1];
-
+	#[benchmark]
+	fn feasibility_check(
+		// Number of votes in snapshot.
+		v: Linear<{ T::BenchmarkingConfig::VOTERS[0] }, { T::BenchmarkingConfig::VOTERS[1] }>,
+		// Number of targets in snapshot.
+		t: Linear<{ T::BenchmarkingConfig::TARGETS[0] }, { T::BenchmarkingConfig::TARGETS[1] }>,
+		// Number of assignments, i.e. `solution.len()`.
+		// This means the active nominators, thus must be a subset of `v` component.
+		a: Linear<
+			{ T::BenchmarkingConfig::ACTIVE_VOTERS[0] },
+			{ T::BenchmarkingConfig::ACTIVE_VOTERS[1] },
+		>,
+		// Number of desired targets. Must be a subset of `t` component.
+		d: Linear<
+			{ T::BenchmarkingConfig::DESIRED_TARGETS[0] },
+			{ T::BenchmarkingConfig::DESIRED_TARGETS[1] },
+		>,
+	) -> Result<(), BenchmarkError> {
 		let size = SolutionOrSnapshotSize { voters: v, targets: t };
 		let raw_solution = solution_with_size::<T>(size, a, d)?;
 
 		assert_eq!(raw_solution.solution.voter_count() as u32, a);
 		assert_eq!(raw_solution.solution.unique_targets().len() as u32, d);
-	}: {
-		assert!(MultiPhase::<T>::feasibility_check(raw_solution, ElectionCompute::Unsigned).is_ok());
+
+		let result;
+
+		#[block]
+		{
+			result = Pallet::<T>::feasibility_check(raw_solution, ElectionCompute::Unsigned);
+		}
+
+		assert!(result.is_ok());
+
+		Ok(())
 	}
 
 	// NOTE: this weight is not used anywhere, but the fact that it should succeed when execution in
@@ -419,20 +490,23 @@ frame_benchmarking::benchmarks! {
 	// This benchmark is doing more work than a raw call to `OffchainWorker_offchain_worker` runtime
 	// api call, since it is also setting up some mock data, which will itself exhaust the heap to
 	// some extent.
-	#[extra]
-	mine_solution_offchain_memory {
-		// number of votes in snapshot. Fixed to maximum.
+	#[benchmark(extra)]
+	fn mine_solution_offchain_memory() {
+		// Number of votes in snapshot. Fixed to maximum.
 		let v = T::BenchmarkingConfig::MINER_MAXIMUM_VOTERS;
-		// number of targets in snapshot. Fixed to maximum.
+		// Number of targets in snapshot. Fixed to maximum.
 		let t = T::BenchmarkingConfig::MAXIMUM_TARGETS;
 
 		set_up_data_provider::<T>(v, t);
 		let now = frame_system::Pallet::<T>::block_number();
 		CurrentPhase::<T>::put(Phase::Unsigned((true, now)));
-		MultiPhase::<T>::create_snapshot().unwrap();
-	}: {
-		// we can't really verify this as it won't write anything to state, check logs.
-		MultiPhase::<T>::offchain_worker(now)
+		Pallet::<T>::create_snapshot().unwrap();
+
+		#[block]
+		{
+			// we can't really verify this as it won't write anything to state, check logs.
+			Pallet::<T>::offchain_worker(now)
+		}
 	}
 
 	// NOTE: this weight is not used anywhere, but the fact that it should succeed when execution in
@@ -441,41 +515,48 @@ frame_benchmarking::benchmarks! {
 	// numbers.
 	//
 	// ONLY run this benchmark in isolation, and pass the `--extra` flag to enable it.
-	#[extra]
-	create_snapshot_memory {
-		// number of votes in snapshot. Fixed to maximum.
+	#[benchmark(extra)]
+	fn create_snapshot_memory() -> Result<(), BenchmarkError> {
+		// Number of votes in snapshot. Fixed to maximum.
 		let v = T::BenchmarkingConfig::SNAPSHOT_MAXIMUM_VOTERS;
-		// number of targets in snapshot. Fixed to maximum.
+		// Number of targets in snapshot. Fixed to maximum.
 		let t = T::BenchmarkingConfig::MAXIMUM_TARGETS;
 
 		set_up_data_provider::<T>(v, t);
 		assert!(Snapshot::<T>::get().is_none());
-	}: {
-		MultiPhase::<T>::create_snapshot().map_err(|_| "could not create snapshot")?;
-	} verify {
+
+		#[block]
+		{
+			Pallet::<T>::create_snapshot().map_err(|_| "could not create snapshot")?;
+		}
+
 		assert!(Snapshot::<T>::get().is_some());
 		assert_eq!(SnapshotMetadata::<T>::get().ok_or("snapshot missing")?.voters, v);
 		assert_eq!(SnapshotMetadata::<T>::get().ok_or("snapshot missing")?.targets, t);
-	}
 
-	#[extra]
-	trim_assignments_length {
-		// number of votes in snapshot.
-		let v in (T::BenchmarkingConfig::VOTERS[0]) .. T::BenchmarkingConfig::VOTERS[1];
-		// number of targets in snapshot.
-		let t in (T::BenchmarkingConfig::TARGETS[0]) .. T::BenchmarkingConfig::TARGETS[1];
-		// number of assignments, i.e. solution.len(). This means the active nominators, thus must be
-		// a subset of `v` component.
-		let a in
-			(T::BenchmarkingConfig::ACTIVE_VOTERS[0]) .. T::BenchmarkingConfig::ACTIVE_VOTERS[1];
-		// number of desired targets. Must be a subset of `t` component.
-		let d in
-			(T::BenchmarkingConfig::DESIRED_TARGETS[0]) ..
-			T::BenchmarkingConfig::DESIRED_TARGETS[1];
-		// Subtract this percentage from the actual encoded size
-		let f in 0 .. 95;
-		use frame_election_provider_support::IndexAssignment;
+		Ok(())
+	}
 
+	#[benchmark(extra)]
+	fn trim_assignments_length(
+		// Number of votes in snapshot.
+		v: Linear<{ T::BenchmarkingConfig::VOTERS[0] }, { T::BenchmarkingConfig::VOTERS[1] }>,
+		// Number of targets in snapshot.
+		t: Linear<{ T::BenchmarkingConfig::TARGETS[0] }, { T::BenchmarkingConfig::TARGETS[1] }>,
+		// Number of assignments, i.e. `solution.len()`.
+		// This means the active nominators, thus must be a subset of `v` component.
+		a: Linear<
+			{ T::BenchmarkingConfig::ACTIVE_VOTERS[0] },
+			{ T::BenchmarkingConfig::ACTIVE_VOTERS[1] },
+		>,
+		// Number of desired targets. Must be a subset of `t` component.
+		d: Linear<
+			{ T::BenchmarkingConfig::DESIRED_TARGETS[0] },
+			{ T::BenchmarkingConfig::DESIRED_TARGETS[1] },
+		>,
+		// Subtract this percentage from the actual encoded size.
+		f: Linear<0, 95>,
+	) -> Result<(), BenchmarkError> {
 		// Compute a random solution, then work backwards to get the lists of voters, targets, and
 		// assignments
 		let witness = SolutionOrSnapshotSize { voters: v, targets: t };
@@ -483,7 +564,9 @@ frame_benchmarking::benchmarks! {
 		let RoundSnapshot { voters, targets } = Snapshot::<T>::get().ok_or("snapshot missing")?;
 		let voter_at = helpers::voter_at_fn::<T::MinerConfig>(&voters);
 		let target_at = helpers::target_at_fn::<T::MinerConfig>(&targets);
-		let mut assignments = solution.into_assignment(voter_at, target_at).expect("solution generated by `solution_with_size` must be valid.");
+		let mut assignments = solution
+			.into_assignment(voter_at, target_at)
+			.expect("solution generated by `solution_with_size` must be valid.");
 
 		// make a voter cache and some helper functions for access
 		let cache = helpers::generate_voter_cache::<T::MinerConfig>(&voters);
@@ -491,12 +574,15 @@ frame_benchmarking::benchmarks! {
 		let target_index = helpers::target_index_fn::<T::MinerConfig>(&targets);
 
 		// sort assignments by decreasing voter stake
-		assignments.sort_by_key(|crate::unsigned::Assignment::<T> { who, .. }| {
-			let stake = cache.get(who).map(|idx| {
-				let (_, stake, _) = voters[*idx];
-				stake
-			}).unwrap_or_default();
-			core::cmp::Reverse(stake)
+		assignments.sort_by_key(|unsigned::Assignment::<T> { who, .. }| {
+			let stake = cache
+				.get(who)
+				.map(|idx| {
+					let (_, stake, _) = voters[*idx];
+					stake
+				})
+				.unwrap_or_default();
+			Reverse(stake)
 		});
 
 		let mut index_assignments = assignments
@@ -506,20 +592,26 @@ frame_benchmarking::benchmarks! {
 			.unwrap();
 
 		let encoded_size_of = |assignments: &[IndexAssignmentOf<T::MinerConfig>]| {
-			SolutionOf::<T::MinerConfig>::try_from(assignments).map(|solution| solution.encoded_size())
+			SolutionOf::<T::MinerConfig>::try_from(assignments)
+				.map(|solution| solution.encoded_size())
 		};
 
 		let desired_size = Percent::from_percent(100 - f.saturated_into::<u8>())
 			.mul_ceil(encoded_size_of(index_assignments.as_slice()).unwrap());
 		log!(trace, "desired_size = {}", desired_size);
-	}: {
-		crate::Miner::<T::MinerConfig>::trim_assignments_length(
-			desired_size.saturated_into(),
-			&mut index_assignments,
-			&encoded_size_of,
-		).unwrap();
-	} verify {
-		let solution = SolutionOf::<T::MinerConfig>::try_from(index_assignments.as_slice()).unwrap();
+
+		#[block]
+		{
+			Miner::<T::MinerConfig>::trim_assignments_length(
+				desired_size.saturated_into(),
+				&mut index_assignments,
+				&encoded_size_of,
+			)
+			.unwrap();
+		}
+
+		let solution =
+			SolutionOf::<T::MinerConfig>::try_from(index_assignments.as_slice()).unwrap();
 		let encoding = solution.encode();
 		log!(
 			trace,
@@ -528,11 +620,13 @@ frame_benchmarking::benchmarks! {
 		);
 		log!(trace, "actual encoded size = {}", encoding.len());
 		assert!(encoding.len() <= desired_size);
+
+		Ok(())
 	}
 
-	impl_benchmark_test_suite!(
-		MultiPhase,
-		crate::mock::ExtBuilder::default().build_offchainify(10).0,
-		crate::mock::Runtime,
-	);
+	impl_benchmark_test_suite! {
+		Pallet,
+		mock::ExtBuilder::default().build_offchainify(10).0,
+		mock::Runtime,
+	}
 }
-- 
GitLab