Dont Convert Benchmark Error Too Early (#9743)

* dont convert benchmark error

* fix warning

* fix more warnings

* more fixes

* more fixes

* add missing feature deps

* Update frame/election-provider-multi-phase/src/benchmarking.rs

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* Update frame/election-provider-multi-phase/src/benchmarking.rs

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* use from

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

substrate/frame/benchmarking/src/lib.rs

@@ -644,7 +644,7 @@ macro_rules! benchmark_backend {
 				&self,
 				components: &[($crate::BenchmarkParameter, u32)],
 				verify: bool
-			) -> Result<$crate::Box<dyn FnOnce() -> Result<(), $crate::BenchmarkError>>, &'static str> {
+			) -> Result<$crate::Box<dyn FnOnce() -> Result<(), $crate::BenchmarkError>>, $crate::BenchmarkError> {
 				$(
 					// Prepare instance
 					let $param = components.iter()
@@ -717,7 +717,7 @@ macro_rules! selected_benchmark {
 				&self,
 				components: &[($crate::BenchmarkParameter, u32)],
 				verify: bool
-			) -> Result<$crate::Box<dyn FnOnce() -> Result<(), $crate::BenchmarkError>>, &'static str> {
+			) -> Result<$crate::Box<dyn FnOnce() -> Result<(), $crate::BenchmarkError>>, $crate::BenchmarkError> {
 				match self {
 					$(
 						Self::$bench => <
@@ -1246,7 +1246,7 @@ macro_rules! impl_benchmark_test_suite {
 									}
 								}
 							},
-							Ok(Ok(_)) => (),
+							Ok(Ok(())) => (),
 						}
 					}
 					assert!(!anything_failed);

substrate/frame/benchmarking/src/tests.rs

@@ -41,6 +41,7 @@ mod pallet_test {
 	pub trait Config: frame_system::Config {
 		type LowerBound: Get<u32>;
 		type UpperBound: Get<u32>;
+		type MaybeItem: Get<Option<u32>>;
 	}
 
 	#[pallet::storage]
@@ -112,11 +113,13 @@ impl frame_system::Config for Test {
 parameter_types! {
 	pub const LowerBound: u32 = 1;
 	pub const UpperBound: u32 = 100;
+	pub const MaybeItem: Option<u32> = None;
 }
 
 impl pallet_test::Config for Test {
 	type LowerBound = LowerBound;
 	type UpperBound = UpperBound;
+	type MaybeItem = MaybeItem;
 }
 
 fn new_test_ext() -> sp_io::TestExternalities {
@@ -218,6 +221,13 @@ mod benchmarks {
 				}
 			))?;
 		}
+
+		skip_benchmark {
+			let value = T::MaybeItem::get().ok_or(BenchmarkError::Skip)?;
+		}: {
+			// This should never be reached.
+			assert!(value > 100);
+		}
 	}
 
 	#[test]
@@ -334,6 +344,11 @@ mod benchmarks {
 			assert_err!(Pallet::<Test>::test_benchmark_bad_verify(), "You forgot to sort!");
 			assert_ok!(Pallet::<Test>::test_benchmark_no_components());
 			assert_ok!(Pallet::<Test>::test_benchmark_variable_components());
+			assert!(matches!(
+				Pallet::<Test>::test_benchmark_override_benchmark(),
+				Err(BenchmarkError::Override(_)),
+			));
+			assert_eq!(Pallet::<Test>::test_benchmark_skip_benchmark(), Err(BenchmarkError::Skip),);
 		});
 	}
 }

substrate/frame/benchmarking/src/utils.rs

@@ -318,7 +318,7 @@ pub trait BenchmarkingSetup<T, I = ()> {
 		&self,
 		components: &[(BenchmarkParameter, u32)],
 		verify: bool,
-	) -> Result<Box<dyn FnOnce() -> Result<(), BenchmarkError>>, &'static str>;
+	) -> Result<Box<dyn FnOnce() -> Result<(), BenchmarkError>>, BenchmarkError>;
 }
 
 /// Grab an account, seeded by a name and index.

substrate/frame/bounties/src/benchmarking.rs

@@ -134,7 +134,8 @@ benchmarks! {
 		Bounties::<T>::on_initialize(T::BlockNumber::zero());
 
 		let bounty_id = BountyCount::get() - 1;
-		let curator = T::Lookup::lookup(curator_lookup)?;
+		let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?;
+
 		let beneficiary = T::Lookup::unlookup(account("beneficiary", 0, SEED));
 	}: _(RawOrigin::Signed(curator), bounty_id, beneficiary)
 
@@ -144,7 +145,8 @@ benchmarks! {
 		Bounties::<T>::on_initialize(T::BlockNumber::zero());
 
 		let bounty_id = BountyCount::get() - 1;
-		let curator = T::Lookup::lookup(curator_lookup)?;
+		let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?;
+
 
 		let beneficiary_account: T::AccountId = account("beneficiary", 0, SEED);
 		let beneficiary = T::Lookup::unlookup(beneficiary_account.clone());
@@ -181,7 +183,7 @@ benchmarks! {
 		Bounties::<T>::on_initialize(T::BlockNumber::zero());
 
 		let bounty_id = BountyCount::get() - 1;
-		let curator = T::Lookup::lookup(curator_lookup)?;
+		let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?;
 	}: _(RawOrigin::Signed(curator), bounty_id, Vec::new())
 	verify {
 		assert_last_event::<T>(RawEvent::BountyExtended(bounty_id).into())

substrate/frame/collective/Cargo.toml

@@ -39,6 +39,7 @@ std = [
 	"sp-runtime/std",
 	"frame-system/std",
 	"log/std",
+	"frame-benchmarking/std",
 ]
 runtime-benchmarks = [
 	"frame-benchmarking",

substrate/frame/contracts/src/benchmarking/mod.rs

@@ -1213,7 +1213,7 @@ benchmarks! {
 
 		for addr in &addresses {
 			if let Some(_) = ContractInfoOf::<T>::get(&addr) {
-				return Err("Expected that contract does not exist at this point.");
+				return Err("Expected that contract does not exist at this point.".into());
 			}
 		}
 	}: call(origin, callee, 0u32.into(), Weight::max_value(), vec![])
@@ -2241,7 +2241,7 @@ benchmarks! {
 			);
 		}
 		#[cfg(not(feature = "std"))]
-		return Err("Run this bench with a native runtime in order to see the schedule.");
+		return Err("Run this bench with a native runtime in order to see the schedule.".into());
 	}: {}
 
 	// Execute one erc20 transfer using the ink! erc20 example contract.

substrate/frame/election-provider-multi-phase/src/benchmarking.rs

@@ -278,7 +278,8 @@ frame_benchmarking::benchmarks! {
 		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)?;
+			<MultiPhase<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.
 		<QueuedSolution<T>>::put(ready_solution);
@@ -307,7 +308,7 @@ frame_benchmarking::benchmarks! {
 			..Default::default()
 		};
 
-		<MultiPhase<T>>::create_snapshot()?;
+		<MultiPhase<T>>::create_snapshot().map_err(<&str>::from)?;
 		MultiPhase::<T>::on_initialize_open_signed();
 		<Round<T>>::put(1);
 

substrate/frame/im-online/src/benchmarking.rs

@@ -83,7 +83,7 @@ benchmarks! {
 		let call = Call::heartbeat(input_heartbeat, signature);
 	}: {
 		ImOnline::<T>::validate_unsigned(TransactionSource::InBlock, &call)
-			.map_err(|e| -> &'static str { e.into() })?;
+			.map_err(<&str>::from)?;
 	}
 
 	validate_unsigned_and_then_heartbeat {
@@ -93,7 +93,7 @@ benchmarks! {
 		let call = Call::heartbeat(input_heartbeat, signature);
 	}: {
 		ImOnline::<T>::validate_unsigned(TransactionSource::InBlock, &call)
-			.map_err(|e| -> &'static str { e.into() })?;
+			.map_err(<&str>::from)?;
 		call.dispatch_bypass_filter(RawOrigin::None.into())?;
 	}
 }

substrate/frame/indices/Cargo.toml

@@ -42,5 +42,6 @@ std = [
 runtime-benchmarks = [
 	"frame-benchmarking",
 	"frame-support/runtime-benchmarks",
+	"frame-system/runtime-benchmarks",
 ]
 try-runtime = ["frame-support/try-runtime"]

substrate/frame/multisig/Cargo.toml

@@ -39,5 +39,6 @@ std = [
 runtime-benchmarks = [
 	"frame-benchmarking",
 	"frame-support/runtime-benchmarks",
+	"frame-system/runtime-benchmarks",
 ]
 try-runtime = ["frame-support/try-runtime"]