Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
16 changes: 16 additions & 0 deletions prdoc/pr_7319.prdoc
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
46 changes: 25 additions & 21 deletions substrate/frame/revive/fixtures/contracts/caller_contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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)));
Expand All @@ -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)));
Expand All @@ -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)));
Expand All @@ -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..])
}
14 changes: 2 additions & 12 deletions substrate/frame/revive/fixtures/contracts/create1_with_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 8 additions & 4 deletions substrate/frame/revive/fixtures/contracts/return_data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down
39 changes: 6 additions & 33 deletions substrate/frame/revive/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand All @@ -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! {
Expand Down
Binary file modified substrate/frame/revive/rpc/examples/js/pvm/FlipperCaller.polkavm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping it reduces code size not increase it :(

But we mainly do that to reduce complexity of having two different ways of passing arguments.

Binary file not shown.
Binary file modified substrate/frame/revive/rpc/examples/js/pvm/PiggyBank.polkavm
Binary file not shown.
Loading
Loading