[pallet-revive] pack exceeding syscall arguments into registers (#7319)

This PR changes how we call runtime API methods with more than 6
arguments: They are no longer spilled to the stack but packed into
registers instead. Pointers are 32 bit wide so we can pack two of them
into a single 64 bit register. Since we mostly pass pointers, this
technique effectively increases the number of arguments we can pass
using the available registers.

To make this work for `instantiate` too we now pass the code hash and
the call data in the same buffer, akin to how the `create` family
opcodes work in the EVM. The code hash is fixed in size, implying the
start of the constructor call data.

---------

Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
Co-authored-by: command-bot <>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>

Author: 3 people

prdoc/pr_7319.prdoc

@@ -0,0 +1,16 @@
+title: '[pallet-revive] pack exceeding syscall arguments into registers'
+doc:
+- audience: Runtime Dev
+  description: |-
+    This PR changes how we call runtime API methods with more than 6 arguments: They are no longer spilled to the stack but packed into registers instead. Pointers are 32 bit wide so we can pack two of them into a single 64 bit register. Since we mostly pass pointers, this technique effectively increases the number of arguments we can pass using the available registers.
+
+    To make this work for `instantiate` too we now pass the code hash and the call data in the same buffer, akin to how the `create` family opcodes work in the EVM. The code hash is fixed in size, implying the start of the constructor call data.
+crates:
+- name: pallet-revive-fixtures
+  bump: major
+- name: pallet-revive-proc-macro
+  bump: major
+- name: pallet-revive
+  bump: major
+- name: pallet-revive-uapi
+  bump: major

substrate/frame/revive/fixtures/contracts/call_diverging_out_len.rs

@@ -66,12 +66,11 @@ fn assert_instantiate<const N: usize>(expected_output: [u8; BUF_SIZE]) {
 	let output_buf_capped = &mut &mut output_buf[..N];
 
 	api::instantiate(
-		&code_hash,
 		u64::MAX,
 		u64::MAX,
 		&[u8::MAX; 32],
 		&[0; 32],
-		&[0; 32],
+		&code_hash,
 		None,
 		Some(output_buf_capped),
 		None,

substrate/frame/revive/fixtures/contracts/caller_contract.rs

@@ -21,6 +21,9 @@
 use common::{input, u256_bytes};
 use uapi::{HostFn, HostFnImpl as api, ReturnErrorCode};
 
+const INPUT: [u8; 8] = [0u8, 1, 34, 51, 68, 85, 102, 119];
+const REVERTED_INPUT: [u8; 7] = [1u8, 34, 51, 68, 85, 102, 119];
+
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn deploy() {}
@@ -36,17 +39,21 @@ pub extern "C" fn call() {
 	let salt = [0u8; 32];
 
 	// Callee will use the first 4 bytes of the input to return an exit status.
-	let input = [0u8, 1, 34, 51, 68, 85, 102, 119];
-	let reverted_input = [1u8, 34, 51, 68, 85, 102, 119];
+	let mut input_deploy = [0; 32 + INPUT.len()];
+	input_deploy[..32].copy_from_slice(code_hash);
+	input_deploy[32..].copy_from_slice(&INPUT);
+
+	let mut reverted_input_deploy = [0; 32 + REVERTED_INPUT.len()];
+	reverted_input_deploy[..32].copy_from_slice(code_hash);
+	reverted_input_deploy[32..].copy_from_slice(&REVERTED_INPUT);
 
 	// Fail to deploy the contract since it returns a non-zero exit status.
 	let res = api::instantiate(
-		code_hash,
 		u64::MAX,       // How much ref_time weight to devote for the execution. u64::MAX = use all.
 		u64::MAX,       // How much proof_size weight to devote for the execution. u64::MAX = use all.
 		&[u8::MAX; 32], // No deposit limit.
 		&value,
-		&reverted_input,
+		&reverted_input_deploy,
 		None,
 		None,
 		Some(&salt),
@@ -55,12 +62,11 @@ pub extern "C" fn call() {
 
 	// Fail to deploy the contract due to insufficient ref_time weight.
 	let res = api::instantiate(
-		code_hash,
 		1u64,           // too little ref_time weight
 		u64::MAX,       // How much proof_size weight to devote for the execution. u64::MAX = use all.
 		&[u8::MAX; 32], // No deposit limit.
 		&value,
-		&input,
+		&input_deploy,
 		None,
 		None,
 		Some(&salt),
@@ -69,12 +75,11 @@ pub extern "C" fn call() {
 
 	// Fail to deploy the contract due to insufficient proof_size weight.
 	let res = api::instantiate(
-		code_hash,
 		u64::MAX,       // How much ref_time weight to devote for the execution. u64::MAX = use all.
 		1u64,           // Too little proof_size weight
 		&[u8::MAX; 32], // No deposit limit.
 		&value,
-		&input,
+		&input_deploy,
 		None,
 		None,
 		Some(&salt),
@@ -85,12 +90,11 @@ pub extern "C" fn call() {
 	let mut callee = [0u8; 20];
 
 	api::instantiate(
-		code_hash,
 		u64::MAX,       // How much ref_time weight to devote for the execution. u64::MAX = use all.
 		u64::MAX,       // How much proof_size weight to devote for the execution. u64::MAX = use all.
 		&[u8::MAX; 32], // No deposit limit.
 		&value,
-		&input,
+		&input_deploy,
 		Some(&mut callee),
 		None,
 		Some(&salt),
@@ -101,11 +105,11 @@ pub extern "C" fn call() {
 	let res = api::call(
 		uapi::CallFlags::empty(),
 		&callee,
-		u64::MAX,       // How much ref_time weight to devote for the execution. u64::MAX = use all.
-		u64::MAX,       // How much proof_size weight to devote for the execution. u64::MAX = use all.
+		u64::MAX, // How much ref_time weight to devote for the execution. u64::MAX = use all.
+		u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all.
 		&[u8::MAX; 32], // No deposit limit.
 		&value,
-		&reverted_input,
+		&REVERTED_INPUT,
 		None,
 	);
 	assert!(matches!(res, Err(ReturnErrorCode::CalleeReverted)));
@@ -118,7 +122,7 @@ pub extern "C" fn call() {
 		u64::MAX,       // How much proof_size weight to devote for the execution. u64::MAX = use all.
 		&[u8::MAX; 32], // No deposit limit.
 		&value,
-		&input,
+		&INPUT,
 		None,
 	);
 	assert!(matches!(res, Err(ReturnErrorCode::OutOfResources)));
@@ -127,11 +131,11 @@ pub extern "C" fn call() {
 	let res = api::call(
 		uapi::CallFlags::empty(),
 		&callee,
-		u64::MAX,       // How much ref_time weight to devote for the execution. u64::MAX = use all.
-		1u64,           // too little proof_size weight
+		u64::MAX, // How much ref_time weight to devote for the execution. u64::MAX = use all.
+		1u64,     // too little proof_size weight
 		&[u8::MAX; 32], // No deposit limit.
 		&value,
-		&input,
+		&INPUT,
 		None,
 	);
 	assert!(matches!(res, Err(ReturnErrorCode::OutOfResources)));
@@ -141,13 +145,13 @@ pub extern "C" fn call() {
 	api::call(
 		uapi::CallFlags::empty(),
 		&callee,
-		u64::MAX,       // How much ref_time weight to devote for the execution. u64::MAX = use all.
-		u64::MAX,       // How much proof_size weight to devote for the execution. u64::MAX = use all.
+		u64::MAX, // How much ref_time weight to devote for the execution. u64::MAX = use all.
+		u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all.
 		&[u8::MAX; 32], // No deposit limit.
 		&value,
-		&input,
+		&INPUT,
 		Some(&mut &mut output[..]),
 	)
 	.unwrap();
-	assert_eq!(&output, &input[4..])
+	assert_eq!(&output, &INPUT[4..])
 }

substrate/frame/revive/fixtures/contracts/create1_with_value.rs

@@ -34,16 +34,6 @@ pub extern "C" fn call() {
 	api::value_transferred(&mut value);
 
 	// Deploy the contract with no salt (equivalent to create1).
-	let ret = api::instantiate(
-		code_hash,
-		u64::MAX,
-		u64::MAX,
-		&[u8::MAX; 32],
-		&value,
-		&[],
-		None,
-		None,
-		None
-	);
-	assert!(ret.is_ok());
+	api::instantiate(u64::MAX, u64::MAX, &[u8::MAX; 32], &value, code_hash, None, None, None)
+		.unwrap();
 }

substrate/frame/revive/fixtures/contracts/create_storage_and_instantiate.rs

@@ -30,22 +30,24 @@ pub extern "C" fn deploy() {}
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
 	input!(
-		input: [u8; 4],
 		code_hash: &[u8; 32],
+		input: [u8; 4],
 		deposit_limit: &[u8; 32],
 	);
 
 	let value = u256_bytes(10_000u64);
 	let salt = [0u8; 32];
 	let mut address = [0u8; 20];
+	let mut deploy_input = [0; 32 + 4];
+	deploy_input[..32].copy_from_slice(code_hash);
+	deploy_input[32..].copy_from_slice(&input);
 
 	let ret = api::instantiate(
-		code_hash,
 		u64::MAX, // How much ref_time weight to devote for the execution. u64::MAX = use all.
 		u64::MAX, // How much proof_size weight to devote for the execution. u64::MAX = use all.
 		deposit_limit,
 		&value,
-		input,
+		&deploy_input,
 		Some(&mut address),
 		None,
 		Some(&salt),

substrate/frame/revive/fixtures/contracts/destroy_and_transfer.rs

@@ -29,17 +29,15 @@ const VALUE: [u8; 32] = u256_bytes(65536);
 pub extern "C" fn deploy() {
 	input!(code_hash: &[u8; 32],);
 
-	let input = [0u8; 0];
 	let mut address = [0u8; 20];
 	let salt = [47u8; 32];
 
 	api::instantiate(
-		code_hash,
 		u64::MAX,       // How much ref_time weight to devote for the execution. u64::MAX = use all.
 		u64::MAX,       // How much proof_size weight to devote for the execution. u64::MAX = use all.
 		&[u8::MAX; 32], // No deposit limit.
 		&VALUE,
-		&input,
+		code_hash,
 		Some(&mut address),
 		None,
 		Some(&salt),

substrate/frame/revive/fixtures/contracts/instantiate_return_code.rs

@@ -28,16 +28,14 @@ pub extern "C" fn deploy() {}
 #[no_mangle]
 #[polkavm_derive::polkavm_export]
 pub extern "C" fn call() {
-	input!(buffer, 36, code_hash: &[u8; 32],);
-	let input = &buffer[32..];
+	input!(buffer: &[u8; 36],);
 
 	let err_code = match api::instantiate(
-		code_hash,
-		u64::MAX,               // How much ref_time weight to devote for the execution. u64::MAX = use all.
-		u64::MAX,               // How much proof_size weight to devote for the execution. u64::MAX = use all.
-		&[u8::MAX; 32],         // No deposit limit.
+		u64::MAX,       // How much ref_time weight to devote for the execution. u64::MAX = use all.
+		u64::MAX,       // How much proof_size weight to devote for the execution. u64::MAX = use all.
+		&[u8::MAX; 32], // No deposit limit.
 		&u256_bytes(10_000u64), // Value to transfer.
-		input,
+		buffer,
 		None,
 		None,
 		Some(&[0u8; 32]), // Salt.

substrate/frame/revive/fixtures/contracts/return_data_api.rs

@@ -88,8 +88,9 @@ fn assert_balance_transfer_does_reset() {
 		&[u8::MAX; 32],
 		&u256_bytes(128),
 		&[],
-		None
-	).unwrap();
+		None,
+	)
+	.unwrap();
 	assert_return_data_size_of(0);
 }
 
@@ -117,13 +118,16 @@ pub extern "C" fn call() {
 		input
 	};
 	let mut instantiate = |exit_flag| {
+		let input = construct_input(exit_flag);
+		let mut deploy_input = [0; 32 + INPUT_BUF_SIZE];
+		deploy_input[..32].copy_from_slice(code_hash);
+		deploy_input[32..].copy_from_slice(&input);
 		api::instantiate(
-			code_hash,
 			u64::MAX,
 			u64::MAX,
 			&[u8::MAX; 32],
 			&[0; 32],
-			&construct_input(exit_flag),
+			&deploy_input,
 			Some(&mut address_buf),
 			None,
 			None,

substrate/frame/revive/proc-macro/src/lib.rs

@@ -355,6 +355,11 @@ where
 {
 	const ALLOWED_REGISTERS: usize = 6;
 
+	// too many arguments
+	if param_names.clone().count() > ALLOWED_REGISTERS {
+		panic!("Syscalls take a maximum of {ALLOWED_REGISTERS} arguments");
+	}
+
 	// all of them take one register but we truncate them before passing into the function
 	// it is important to not allow any type which has illegal bit patterns like 'bool'
 	if !param_types.clone().all(|ty| {
@@ -369,39 +374,7 @@ where
 		panic!("Only primitive unsigned integers are allowed as arguments to syscalls");
 	}
 
-	// too many arguments: pass as pointer to a struct in memory
-	if param_names.clone().count() > ALLOWED_REGISTERS {
-		let fields = param_names.clone().zip(param_types.clone()).map(|(name, ty)| {
-			quote! {
-				#name: #ty,
-			}
-		});
-		return quote! {
-			#[derive(Default)]
-			#[repr(C)]
-			struct Args {
-				#(#fields)*
-			}
-			let Args { #(#param_names,)* } = {
-				let len = ::core::mem::size_of::<Args>();
-				let mut args = Args::default();
-				let ptr = &mut args as *mut Args as *mut u8;
-				// Safety
-				// 1. The struct is initialized at all times.
-				// 2. We only allow primitive integers (no bools) as arguments so every bit pattern is safe.
-				// 3. The reference doesn't outlive the args field.
-				// 4. There is only the single reference to the args field.
-				// 5. The length of the generated slice is the same as the struct.
-				let reference = unsafe {
-					::core::slice::from_raw_parts_mut(ptr, len)
-				};
-				memory.read_into_buf(__a0__ as _, reference)?;
-				args
-			};
-		}
-	}
-
-	// otherwise: one argument per register
+	// one argument per register
 	let bindings = param_names.zip(param_types).enumerate().map(|(idx, (name, ty))| {
 		let reg = quote::format_ident!("__a{}__", idx);
 		quote! {