Don't panic if parameter can not be converted between node and native runtime (#1659)

* Don't panic if parameter can not be converted between node and native runtime

* FIxes after merge

* Use correct copyright year

substrate/core/sr-api-macros/Cargo.toml

@@ -15,6 +15,7 @@ blake2-rfc = "0.2"
 [dev-dependencies]
 substrate-client = { path = "../client" }
 substrate-test-client = { path = "../test-client" }
+substrate-state-machine = { path = "../state-machine" }
 sr-primitives = { path = "../sr-primitives" }
 sr-version = { path = "../sr-version" }
 substrate-primitives = { path = "../primitives" }

substrate/core/sr-api-macros/benches/bench.rs

@@ -1,4 +1,4 @@
-// Copyright 2018 Parity Technologies (UK) Ltd.
+// Copyright 2018 - 2019 Parity Technologies (UK) Ltd.
 // This file is part of Substrate.
 
 // Substrate is free software: you can redistribute it and/or modify

substrate/core/sr-api-macros/src/decl_runtime_apis.rs

@@ -17,7 +17,7 @@
 use utils::{
 	generate_crate_access, generate_hidden_includes, generate_runtime_mod_name_for_trait,
 	fold_fn_decl_for_client_side, unwrap_or_error, extract_parameter_names_types_and_borrows,
-	generate_native_call_generator_fn_name
+	generate_native_call_generator_fn_name, return_type_extract_type,
 };
 
 use proc_macro;
@@ -157,12 +157,15 @@ fn generate_native_call_generators(decl: &ItemTrait) -> Result<TokenStream> {
 	// Auxilariy function that is used to convert between types that use different block types.
 	// The function expects that both a convertable by encoding the one and decoding the other.
 	result.push(quote!(
+		#[cfg(any(feature = "std", test))]
 		fn convert_between_block_types
-			<I: #crate_::runtime_api::Encode, R: #crate_::runtime_api::Decode>(input: &I) -> R
+			<I: #crate_::runtime_api::Encode, R: #crate_::runtime_api::Decode>(
+				input: &I, error_desc: &'static str,
+			) -> ::std::result::Result<R, &'static str>
 		{
 			<R as #crate_::runtime_api::Decode>::decode(
 				&mut &#crate_::runtime_api::Encode::encode(input)[..]
-			).unwrap()
+			).ok_or_else(|| error_desc)
 		}
 	));
 
@@ -172,20 +175,30 @@ fn generate_native_call_generators(decl: &ItemTrait) -> Result<TokenStream> {
 		let trait_fn_name = &fn_.ident;
 		let fn_name = generate_native_call_generator_fn_name(&fn_.ident);
 		let output = return_type_replace_block_with_node_block(fn_.decl.output.clone());
+		let output_ty = return_type_extract_type(&output);
+		let output = quote!( ::std::result::Result<#output_ty, &'static str> );
 
 		// Every type that is using the `Block` generic parameter, we need to encode/decode,
 		// to make it compatible between the runtime/node.
 		let conversions = params.iter().filter(|v| type_is_using_block(&v.1)).map(|(n, t, _)| {
+			let name_str = format!(
+				"Could not convert parameter `{}` between node and runtime!", quote!(#n)
+			);
 			quote!(
-				let #n: #t = convert_between_block_types(&#n);
+				let #n: #t = convert_between_block_types(&#n, #name_str)?;
 			)
 		});
 		// Same as for the input types, we need to check if we also need to convert the output,
 		// before returning it.
 		let output_conversion = if return_type_is_using_block(&fn_.decl.output) {
-			quote!( convert_between_block_types(&res) )
+			quote!(
+				convert_between_block_types(
+					&res,
+					"Could not convert return value from runtime to node!"
+				)
+			)
 		} else {
-			quote!( res )
+			quote!( Ok(res) )
 		};
 
 		let input_names = params.iter().map(|v| &v.0);
@@ -240,7 +253,7 @@ fn generate_native_call_generators(decl: &ItemTrait) -> Result<TokenStream> {
 				#(, #impl_generics_params)*
 			>(
 				#( #fn_inputs ),*
-			) -> impl FnOnce() #output + 'a #where_clause {
+			) -> impl FnOnce() -> #output + 'a #where_clause {
 				move || {
 					#( #conversions )*
 					let res = ApiImpl::#trait_fn_name(#( #input_borrows #input_names ),*);

substrate/core/sr-api-macros/src/impl_runtime_apis.rs

@@ -330,7 +330,7 @@ fn generate_runtime_api_base_structures(impls: &[ItemImpl]) -> Result<TokenStrea
 		impl<C: #crate_::runtime_api::CallRuntimeAt<#block>> RuntimeApiImpl<C> {
 			fn call_api_at<
 				R: #crate_::runtime_api::Encode + #crate_::runtime_api::Decode + PartialEq,
-				NC: FnOnce() -> R + ::std::panic::UnwindSafe,
+				NC: FnOnce() -> ::std::result::Result<R, &'static str> + ::std::panic::UnwindSafe,
 			>(
 				&self,
 				at: &#block_id,

substrate/core/sr-api-macros/src/lib.rs

@@ -16,7 +16,7 @@
 
 //! Macros for declaring and implementing runtime apis.
 
-#![recursion_limit = "256"]
+#![recursion_limit = "512"]
 extern crate proc_macro;
 extern crate proc_macro2;
 extern crate quote;

substrate/core/sr-api-macros/src/utils.rs

@@ -58,6 +58,14 @@ pub fn generate_runtime_mod_name_for_trait(trait_: &Ident) -> Ident {
 	Ident::new(&format!("runtime_decl_for_{}", trait_.to_string()), Span::call_site())
 }
 
+/// Get the type of a `syn::ReturnType`.
+pub fn return_type_extract_type(rt: &syn::ReturnType) -> Type {
+	match rt {
+		syn::ReturnType::Default => parse_quote!( () ),
+		syn::ReturnType::Type(_, ref ty) => *ty.clone(),
+	}
+}
+
 /// Fold the given `FnDecl` to make it usable on the client side.
 pub fn fold_fn_decl_for_client_side(
 	mut input: FnDecl,
@@ -70,14 +78,8 @@ pub fn fold_fn_decl_for_client_side(
 
 	// Wrap the output in a `Result`
 	input.output = {
-		let generate_result = |ty: &Type| {
-			parse_quote!( -> ::std::result::Result<#ty, #crate_::error::Error> )
-		};
-
-		match &input.output {
-			syn::ReturnType::Default => generate_result(&parse_quote!( () )),
-			syn::ReturnType::Type(_, ref ty) => generate_result(&ty),
-		}
+		let ty = return_type_extract_type(&input.output);
+		parse_quote!( -> ::std::result::Result<#ty, #crate_::error::Error> )
 	};
 
 	input

substrate/core/sr-api-macros/tests/runtime_calls.rs

@@ -0,0 +1,60 @@
+// Copyright 2019 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Substrate is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+extern crate substrate_client;
+extern crate substrate_test_client as test_client;
+extern crate sr_primitives as runtime_primitives;
+extern crate substrate_state_machine as state_machine;
+
+use test_client::runtime::{TestAPI, DecodeFails};
+use runtime_primitives::{generic::BlockId, traits::ProvideRuntimeApi};
+use state_machine::ExecutionStrategy;
+
+fn calling_function_with_strat(strat: ExecutionStrategy) {
+	let client = test_client::new_with_api_execution_strat(strat);
+	let runtime_api = client.runtime_api();
+	let block_id = BlockId::Number(client.info().unwrap().chain.best_number);
+
+	assert_eq!(runtime_api.benchmark_add_one(&block_id, &1).unwrap(), 2);
+}
+
+#[test]
+fn calling_native_runtime_function() {
+	calling_function_with_strat(ExecutionStrategy::NativeWhenPossible);
+}
+
+#[test]
+fn calling_wasm_runtime_function() {
+	calling_function_with_strat(ExecutionStrategy::AlwaysWasm);
+}
+
+#[test]
+#[should_panic(expected = "Could not convert parameter `param` between node and runtime!")]
+fn calling_native_runtime_function_with_non_decodable_parameter() {
+	let client = test_client::new_with_api_execution_strat(ExecutionStrategy::NativeWhenPossible);
+	let runtime_api = client.runtime_api();
+	let block_id = BlockId::Number(client.info().unwrap().chain.best_number);
+	runtime_api.fail_convert_parameter(&block_id, DecodeFails::new()).unwrap();
+}
+
+#[test]
+#[should_panic(expected = "Could not convert return value from runtime to node!")]
+fn calling_native_runtime_function_with_non_decodable_return_value() {
+	let client = test_client::new_with_api_execution_strat(ExecutionStrategy::NativeWhenPossible);
+	let runtime_api = client.runtime_api();
+	let block_id = BlockId::Number(client.info().unwrap().chain.best_number);
+	runtime_api.fail_convert_return_value(&block_id).unwrap();
+}