Fix sp-api handling of multiple arguments (#6484)

With the switch to `decode_all_with_depth_limit` we silently broken
support for functions with multiple arguments. The old generated code
tried to decode each parameter separately, which does not play well with
`decode_all`.

This pr adds a test to ensure that this does not happen again and fixes
the bug by decoding everything at once by wrapping it into tuples.

substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs

@@ -34,7 +34,7 @@ use syn::{
 	fold::{self, Fold}, parse_quote,
 };
 
-use std::{collections::HashSet, iter};
+use std::collections::HashSet;
 
 /// Unique identifier used to make the hidden includes unique for this macro.
 const HIDDEN_INCLUDES_ID: &str = "IMPL_RUNTIME_APIS";
@@ -71,10 +71,8 @@ fn generate_impl_call(
 	let params = extract_parameter_names_types_and_borrows(signature, AllowSelfRefInParameters::No)?;
 
 	let c = generate_crate_access(HIDDEN_INCLUDES_ID);
-	let c_iter = iter::repeat(&c);
 	let fn_name = &signature.ident;
-	let fn_name_str = iter::repeat(fn_name.to_string());
-	let input = iter::repeat(input);
+	let fn_name_str = fn_name.to_string();
 	let pnames = params.iter().map(|v| &v.0);
 	let pnames2 = params.iter().map(|v| &v.0);
 	let ptypes = params.iter().map(|v| &v.1);
@@ -82,15 +80,14 @@ fn generate_impl_call(
 
 	Ok(
 		quote!(
-			#(
-				let #pnames : #ptypes = match #c_iter::DecodeLimit::decode_all_with_depth_limit(
-					#c_iter::MAX_EXTRINSIC_DEPTH,
-					&mut #input
+			let (#( #pnames ),*) : ( #( #ptypes ),* ) =
+				match #c::DecodeLimit::decode_all_with_depth_limit(
+					#c::MAX_EXTRINSIC_DEPTH,
+					&mut #input,
 				) {
-					Ok(input) => input,
+					Ok(res) => res,
 					Err(e) => panic!("Bad input data provided to {}: {}", #fn_name_str, e.what()),
 				};
-			)*
 
 			#[allow(deprecated)]
 			<#runtime as #impl_trait>::#fn_name(#( #pborrow #pnames2 ),*)
@@ -138,7 +135,7 @@ fn generate_impl_calls(
 
 /// Generate the dispatch function that is used in native to call into the runtime.
 fn generate_dispatch_function(impls: &[ItemImpl]) -> Result<TokenStream> {
-	let data = Ident::new("data", Span::call_site());
+	let data = Ident::new("__sp_api__input_data", Span::call_site());
 	let c = generate_crate_access(HIDDEN_INCLUDES_ID);
 	let impl_calls = generate_impl_calls(impls, &data)?
 		.into_iter()

substrate/primitives/api/test/tests/runtime_calls.rs

@@ -207,3 +207,14 @@ fn record_proof_works() {
 		&runtime_code,
 	).expect("Executes block while using the proof backend");
 }
+
+#[test]
+fn call_runtime_api_with_multiple_arguments() {
+	let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::Both).build();
+
+	let data = vec![1, 2, 4, 5, 6, 7, 8, 8, 10, 12];
+	let block_id = BlockId::Number(client.chain_info().best_number);
+	client.runtime_api()
+		.test_multiple_arguments(&block_id, data.clone(), data.clone(), data.len() as u32)
+		.unwrap();
+}

substrate/test-utils/runtime/src/lib.rs

@@ -313,6 +313,9 @@ cfg_if! {
 				fn test_ecdsa_crypto() -> (ecdsa::AppSignature, ecdsa::AppPublic);
 				/// Run various tests against storage.
 				fn test_storage();
+				/// Test that ensures that we can call a function that takes multiple
+				/// arguments.
+				fn test_multiple_arguments(data: Vec<u8>, other: Vec<u8>, num: u32);
 			}
 		}
 	} else {
@@ -359,6 +362,9 @@ cfg_if! {
 				fn test_ecdsa_crypto() -> (ecdsa::AppSignature, ecdsa::AppPublic);
 				/// Run various tests against storage.
 				fn test_storage();
+				/// Test that ensures that we can call a function that takes multiple
+				/// arguments.
+				fn test_multiple_arguments(data: Vec<u8>, other: Vec<u8>, num: u32);
 			}
 		}
 	}
@@ -641,6 +647,11 @@ cfg_if! {
 					test_read_storage();
 					test_read_child_storage();
 				}
+
+				fn test_multiple_arguments(data: Vec<u8>, other: Vec<u8>, num: u32) {
+					assert_eq!(&data[..], &other[..]);
+					assert_eq!(data.len(), num as usize);
+				}
 			}
 
 			impl sp_consensus_aura::AuraApi<Block, AuraId> for Runtime {
@@ -862,6 +873,11 @@ cfg_if! {
 					test_read_storage();
 					test_read_child_storage();
 				}
+
+				fn test_multiple_arguments(data: Vec<u8>, other: Vec<u8>, num: u32) {
+					assert_eq!(&data[..], &other[..]);
+					assert_eq!(data.len(), num as usize);
+				}
 			}
 
 			impl sp_consensus_aura::AuraApi<Block, AuraId> for Runtime {