From 789893ea7559f76a461762a8d92c6dd55b8f9a39 Mon Sep 17 00:00:00 2001 From: marcio-diaz Date: Thu, 27 Feb 2020 12:24:29 +0100 Subject: [PATCH 1/4] Propagate DispatchError for benchmarks. --- bin/node/runtime/src/lib.rs | 10 ++--- frame/benchmarking/src/lib.rs | 6 +-- frame/benchmarking/src/utils.rs | 4 +- primitives/runtime/src/lib.rs | 7 +++ utils/frame/benchmarking-cli/src/lib.rs | 59 +++++++++++++------------ 5 files changed, 48 insertions(+), 38 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 730a983a43818..911a32bb724e0 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -821,14 +821,14 @@ impl_runtime_apis! { extrinsic: Vec, steps: Vec, repeat: u32, - ) -> Option> { + ) -> Result, Vec> { 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, + 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.".as_bytes().to_vec()), } } } diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index e6b6a4f3f59ec..4aadd91a24852 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -140,13 +140,13 @@ macro_rules! impl_benchmark { $( $name:ident ),* ) => { impl $crate::Benchmarking<$crate::BenchmarkResults> for Module { - fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, &'static str> { + fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, Vec> { // Map the input to the selected benchmark. let extrinsic = sp_std::str::from_utf8(extrinsic.as_slice()) - .map_err(|_| "Could not find extrinsic")?; + .map_err(|_| "Could not find extrinsic".as_bytes().to_vec())?; let selected_benchmark = match extrinsic { $( stringify!($name) => SelectedBenchmark::$name, )* - _ => return Err("Could not find extrinsic."), + _ => return Err("Could not find extrinsic.".as_bytes().to_vec()), }; // Warm up the DB diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index 9db981a61c841..7e4d571a24e35 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -42,7 +42,7 @@ sp_api::decl_runtime_apis! { extrinsic: Vec, steps: Vec, repeat: u32, - ) -> Option>; + ) -> Result, Vec>; } } @@ -78,7 +78,7 @@ pub trait Benchmarking { /// - `extrinsic`: The name of extrinsic function you want to benchmark encoded as bytes. /// - `steps`: The number of sample points you want to take across the range of parameters. /// - `repeat`: The number of times you want to repeat a benchmark. - fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, &'static str>; + fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, Vec>; } /// The required setup for creating a benchmark. diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 6501dafc0e54b..20400c73eba66 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -416,6 +416,13 @@ impl From for &'static str { } } +impl From for Vec { + fn from(err: DispatchError) -> Vec { + let s: &'static str = err.into(); + s.as_bytes().to_vec() + } +} + impl traits::Printable for DispatchError { fn print(&self) { "DispatchError".print(); diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index b2f360e584e87..cdc0e9c4c0f62 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -109,34 +109,37 @@ impl BenchmarkCmd { ) .execute(strategy.into()) .map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?; - let results = > 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 = , Vec> as Decode>::decode(&mut &result[..]) + .unwrap_or(Err(b"failed to decode results".to_vec())); + + 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(err) => { + eprintln!("Error: {:?}", std::str::from_utf8(&err).unwrap()); + } } Ok(()) From 8d98fea7a5f75193d8f0637d5f3364801b87c680 Mon Sep 17 00:00:00 2001 From: marcio-diaz Date: Thu, 27 Feb 2020 16:15:49 +0100 Subject: [PATCH 2/4] Apply review suggestions. --- bin/node/runtime/src/lib.rs | 9 ++++++--- frame/benchmarking/src/lib.rs | 6 +++--- frame/benchmarking/src/utils.rs | 2 +- primitives/runtime/src/lib.rs | 7 ------- utils/frame/benchmarking-cli/src/lib.rs | 9 ++++++--- 5 files changed, 16 insertions(+), 17 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 911a32bb724e0..37ff259ad52b5 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -822,14 +822,17 @@ impl_runtime_apis! { steps: Vec, repeat: u32, ) -> Result, Vec> { + use codec::Encode; use frame_benchmarking::Benchmarking; - match module.as_slice() { + let results = 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.".as_bytes().to_vec()), - } + _ => Err("Benchmark not found for this pallet."), + }; + + results.map_err(|e| e.encode()) } } } diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index 4aadd91a24852..e6b6a4f3f59ec 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -140,13 +140,13 @@ macro_rules! impl_benchmark { $( $name:ident ),* ) => { impl $crate::Benchmarking<$crate::BenchmarkResults> for Module { - fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, Vec> { + fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, &'static str> { // Map the input to the selected benchmark. let extrinsic = sp_std::str::from_utf8(extrinsic.as_slice()) - .map_err(|_| "Could not find extrinsic".as_bytes().to_vec())?; + .map_err(|_| "Could not find extrinsic")?; let selected_benchmark = match extrinsic { $( stringify!($name) => SelectedBenchmark::$name, )* - _ => return Err("Could not find extrinsic.".as_bytes().to_vec()), + _ => return Err("Could not find extrinsic."), }; // Warm up the DB diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index 7e4d571a24e35..83fbee1845ef9 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -78,7 +78,7 @@ pub trait Benchmarking { /// - `extrinsic`: The name of extrinsic function you want to benchmark encoded as bytes. /// - `steps`: The number of sample points you want to take across the range of parameters. /// - `repeat`: The number of times you want to repeat a benchmark. - fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, Vec>; + fn run_benchmark(extrinsic: Vec, steps: Vec, repeat: u32) -> Result, &'static str>; } /// The required setup for creating a benchmark. diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 20400c73eba66..6501dafc0e54b 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -416,13 +416,6 @@ impl From for &'static str { } } -impl From for Vec { - fn from(err: DispatchError) -> Vec { - let s: &'static str = err.into(); - s.as_bytes().to_vec() - } -} - impl traits::Printable for DispatchError { fn print(&self) { "DispatchError".print(); diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index cdc0e9c4c0f62..dd13f86d994f5 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -109,8 +109,9 @@ impl BenchmarkCmd { ) .execute(strategy.into()) .map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?; + let results = , Vec> as Decode>::decode(&mut &result[..]) - .unwrap_or(Err(b"failed to decode results".to_vec())); + .expect("failed to decode benchmark results"); match results { Ok(results) => { @@ -137,8 +138,10 @@ impl BenchmarkCmd { eprintln!("Done."); } - Err(err) => { - eprintln!("Error: {:?}", std::str::from_utf8(&err).unwrap()); + Err(error) => { + let error = ::decode(&mut &error[..]) + .expect("failed to decode benchmark error"); + eprintln!("Error: {:?}", error); } } From 7412561af526ddd66bf3288f0ca6e852d933ddfd Mon Sep 17 00:00:00 2001 From: marcio-diaz Date: Thu, 27 Feb 2020 19:25:25 +0100 Subject: [PATCH 3/4] Use RuntimeString. --- Cargo.lock | 1 + bin/node/runtime/src/lib.rs | 10 +++++----- frame/benchmarking/Cargo.toml | 3 ++- frame/benchmarking/src/utils.rs | 3 ++- utils/frame/benchmarking-cli/src/lib.rs | 8 ++------ 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 757b9ad080074..7ab098296e70a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1454,6 +1454,7 @@ dependencies = [ "parity-scale-codec", "sp-api", "sp-io", + "sp-runtime", "sp-runtime-interface", "sp-std", ] diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 37ff259ad52b5..9e4496f1e9d9e 100644 --- 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,18 +822,17 @@ impl_runtime_apis! { extrinsic: Vec, steps: Vec, repeat: u32, - ) -> Result, Vec> { - use codec::Encode; + ) -> Result, RuntimeString> { use frame_benchmarking::Benchmarking; - let results = match module.as_slice() { + 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."), }; - results.map_err(|e| e.encode()) + result.map_err(|e| e.into()) } } } diff --git a/frame/benchmarking/Cargo.toml b/frame/benchmarking/Cargo.toml index 810d6361ddcf9..3def3dd38ea11 100644 --- 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 index 83fbee1845ef9..f9d1ecf8cd322 100644 --- 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, steps: Vec, repeat: u32, - ) -> Result, Vec>; + ) -> Result, RuntimeString>; } } diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index dd13f86d994f5..c63f05c2b3b5c 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -110,7 +110,7 @@ impl BenchmarkCmd { .execute(strategy.into()) .map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?; - let results = , Vec> as Decode>::decode(&mut &result[..]) + let results = , String> as Decode>::decode(&mut &result[..]) .expect("failed to decode benchmark results"); match results { @@ -138,11 +138,7 @@ impl BenchmarkCmd { eprintln!("Done."); } - Err(error) => { - let error = ::decode(&mut &error[..]) - .expect("failed to decode benchmark error"); - eprintln!("Error: {:?}", error); - } + Err(error) => eprintln!("Error: {:?}", error), } Ok(()) From aac1cbae82200fc1fdd27c37f845e8c63f6eeb5b Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Fri, 28 Feb 2020 09:42:25 +0100 Subject: [PATCH 4/4] fix expect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Bastian Köcher --- utils/frame/benchmarking-cli/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs index c63f05c2b3b5c..b09b1bad62781 100644 --- a/utils/frame/benchmarking-cli/src/lib.rs +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -111,7 +111,7 @@ impl BenchmarkCmd { .map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?; let results = , String> as Decode>::decode(&mut &result[..]) - .expect("failed to decode benchmark results"); + .map_err(|e| format!("Failed to decode benchmark results: {:?}", e))?; match results { Ok(results) => {