Propagate DispatchError for benchmarks. (paritytech#5075)

* Propagate DispatchError for benchmarks. * Apply review suggestions. * Use RuntimeString. * fix expect Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com> Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
General-Beck · Mar 4, 2020 · edf3c5a · edf3c5a
1 parent f414b8e
commit edf3c5a
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 38 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs
@@ -31,7 +31,8 @@ pub use node_primitives::{AccountId, Signature};
 use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment};
 use sp_api::impl_runtime_apis;
 use sp_runtime::{
-	Permill, Perbill, Percent, ApplyExtrinsicResult, impl_opaque_keys, generic, create_runtime_str,
+	Permill, Perbill, Percent, ApplyExtrinsicResult, RuntimeString,
+	impl_opaque_keys, generic, create_runtime_str,
 };
 use sp_runtime::curve::PiecewiseLinear;
 use sp_runtime::transaction_validity::TransactionValidity;
@@ -821,15 +822,17 @@ impl_runtime_apis! {
 			extrinsic: Vec<u8>,
 			steps: Vec<u32>,
 			repeat: u32,
-		) -> Option<Vec<frame_benchmarking::BenchmarkResults>> {
+		) -> Result<Vec<frame_benchmarking::BenchmarkResults>, RuntimeString> {
 			use frame_benchmarking::Benchmarking;
 
-			match module.as_slice() {
-				b"pallet-balances" | b"balances" => Balances::run_benchmark(extrinsic, steps, repeat).ok(),
-				b"pallet-identity" | b"identity" => Identity::run_benchmark(extrinsic, steps, repeat).ok(),
-				b"pallet-timestamp" | b"timestamp" => Timestamp::run_benchmark(extrinsic, steps, repeat).ok(),
-				_ => None,
-			}
+			let result = match module.as_slice() {
+				b"pallet-balances" | b"balances" => Balances::run_benchmark(extrinsic, steps, repeat),
+				b"pallet-identity" | b"identity" => Identity::run_benchmark(extrinsic, steps, repeat),
+				b"pallet-timestamp" | b"timestamp" => Timestamp::run_benchmark(extrinsic, steps, repeat),
+				_ => Err("Benchmark not found for this pallet."),
+			};
+
+			result.map_err(|e| e.into())
 		}
 	}
 }

diff --git a/frame/benchmarking/Cargo.toml b/frame/benchmarking/Cargo.toml
@@ -12,9 +12,10 @@ description = "Macro for benchmarking a FRAME runtime."
 codec = { package = "parity-scale-codec", version = "1.1.2", default-features = false }
 sp-api = { version = "2.0.0-alpha.2", path = "../../primitives/api", default-features = false }
 sp-runtime-interface = { version = "2.0.0-alpha.2", path = "../../primitives/runtime-interface", default-features = false }
+sp-runtime = { version = "2.0.0-alpha.2", path = "../../primitives/runtime", default-features = false }
 sp-std = { version = "2.0.0-alpha.2", path = "../../primitives/std", default-features = false }
 sp-io = { path = "../../primitives/io", default-features = false, version = "2.0.0-alpha.2" }
 
 [features]
 default = [ "std" ]
-std = [ "sp-runtime-interface/std", "sp-api/std", "codec/std", "sp-std/std" ]
+std = [ "sp-runtime-interface/std", "sp-runtime/std", "sp-api/std", "codec/std", "sp-std/std" ]
diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs
@@ -19,6 +19,7 @@
 use codec::{Encode, Decode};
 use sp_std::vec::Vec;
 use sp_io::hashing::blake2_256;
+use sp_runtime::RuntimeString;
 
 /// An alphabet of possible parameters to use for benchmarking.
 #[derive(codec::Encode, codec::Decode, Clone, Copy, PartialEq, Debug)]
@@ -42,7 +43,7 @@ sp_api::decl_runtime_apis! {
 			extrinsic: Vec<u8>,
 			steps: Vec<u32>,
 			repeat: u32,
-		) -> Option<Vec<BenchmarkResults>>;
+		) -> Result<Vec<BenchmarkResults>, RuntimeString>;
 	}
 }
 

diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs
@@ -109,34 +109,36 @@ impl BenchmarkCmd {
 		)
 		.execute(strategy.into())
 		.map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?;
-		let results = <Option<Vec<BenchmarkResults>> as Decode>::decode(&mut &result[..])
-			.unwrap_or(None);
-
-		if let Some(results) = results {
-			// Print benchmark metadata
-			println!(
-				"Pallet: {:?}, Extrinsic: {:?}, Steps: {:?}, Repeat: {:?}",
-				self.pallet,
-				self.extrinsic,
-				self.steps,
-				self.repeat,
-			);
-
-			// Print the table header
-			results[0].0.iter().for_each(|param| print!("{:?},", param.0));
-
-			print!("extrinsic_time,storage_root_time\n");
-			// Print the values
-			results.iter().for_each(|result| {
-				let parameters = &result.0;
-				parameters.iter().for_each(|param| print!("{:?},", param.1));
-				// Print extrinsic time and storage root time
-				print!("{:?},{:?}\n", result.1, result.2);
-			});
-
-			eprintln!("Done.");
-		} else {
-			eprintln!("No Results.");
+
+		let results = <Result<Vec<BenchmarkResults>, String> as Decode>::decode(&mut &result[..])
+			.map_err(|e| format!("Failed to decode benchmark results: {:?}", e))?;
+
+		match results {
+			Ok(results) => {
+				// Print benchmark metadata
+				println!(
+					"Pallet: {:?}, Extrinsic: {:?}, Steps: {:?}, Repeat: {:?}",
+					self.pallet,
+					self.extrinsic,
+					self.steps,
+					self.repeat,
+				);
+
+				// Print the table header
+				results[0].0.iter().for_each(|param| print!("{:?},", param.0));
+
+				print!("extrinsic_time,storage_root_time\n");
+				// Print the values
+				results.iter().for_each(|result| {
+					let parameters = &result.0;
+					parameters.iter().for_each(|param| print!("{:?},", param.1));
+					// Print extrinsic time and storage root time
+					print!("{:?},{:?}\n", result.1, result.2);
+				});
+
+				eprintln!("Done.");
+			}
+			Err(error) => eprintln!("Error: {:?}", error),
 		}
 
 		Ok(())