Skip to content

Commit

Permalink
[pallet-revive] change some getter APIs to return value in register (#…
Browse files Browse the repository at this point in the history
…6920)

Call data, return data and code sizes can never exceed `u32::MAX`; they
are also not generic. Hence we know that they are guaranteed to always
fit into a 64bit register and `revive` can just zero extend them into a
256bit integer value. Which is slightly more efficient than passing them
on the stack.

---------

Signed-off-by: Cyrill Leutwiler <[email protected]>
Signed-off-by: xermicus <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: Alexander Theißen <[email protected]>
  • Loading branch information
xermicus and athei authored Dec 18, 2024
1 parent fd0fb76 commit 53f6473
Show file tree
Hide file tree
Showing 18 changed files with 508 additions and 488 deletions.
14 changes: 14 additions & 0 deletions prdoc/pr_6920.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
title: '[pallet-revive] change some getter APIs to return value in register'
doc:
- audience: Runtime Dev
description: Call data, return data and code sizes can never exceed `u32::MAX`;
they are also not generic. Hence we know that they are guaranteed to always fit
into a 64bit register and `revive` can just zero extend them into a 256bit integer
value. Which is slightly more efficient than passing them on the stack.
crates:
- name: pallet-revive-fixtures
bump: major
- name: pallet-revive
bump: major
- name: pallet-revive-uapi
bump: major
5 changes: 1 addition & 4 deletions substrate/frame/revive/fixtures/contracts/call_data_size.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,5 @@ pub extern "C" fn deploy() {}
#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {
let mut buf = [0; 32];
api::call_data_size(&mut buf);

api::return_value(ReturnFlags::empty(), &buf);
api::return_value(ReturnFlags::empty(), &api::call_data_size().to_le_bytes());
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ macro_rules! input {
// e.g input!(buffer, 512, var1: u32, var2: [u8], );
($buffer:ident, $size:expr, $($rest:tt)*) => {
let mut $buffer = [0u8; $size];
let input_size = $crate::u64_output!($crate::api::call_data_size,);
let input_size = $crate::api::call_data_size();
let $buffer = &mut &mut $buffer[..$size.min(input_size as usize)];
$crate::api::call_data_copy($buffer, 0);
input!(@inner $buffer, 0, $($rest)*);
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/revive/fixtures/contracts/extcodesize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#![no_std]
#![no_main]

use common::{input, u64_output};
use common::input;
use uapi::{HostFn, HostFnImpl as api};

#[no_mangle]
Expand All @@ -30,7 +30,7 @@ pub extern "C" fn deploy() {}
pub extern "C" fn call() {
input!(address: &[u8; 20], expected: u64,);

let received = u64_output!(api::code_size, address);
let received = api::code_size(address);

assert_eq!(expected, received);
}
4 changes: 1 addition & 3 deletions substrate/frame/revive/fixtures/contracts/return_data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,7 @@ fn recursion_guard() -> [u8; 20] {

/// Assert [api::return_data_size] to match the `expected` value.
fn assert_return_data_size_of(expected: u64) {
let mut return_data_size = [0xff; 32];
api::return_data_size(&mut return_data_size);
assert_eq!(return_data_size, u256_bytes(expected));
assert_eq!(api::return_data_size(), expected);
}

/// Assert the return data to be reset after a balance transfer.
Expand Down
Binary file modified substrate/frame/revive/rpc/examples/js/pvm/ErrorTester.polkavm
Binary file not shown.
Binary file modified substrate/frame/revive/rpc/examples/js/pvm/EventExample.polkavm
Binary file not shown.
Binary file modified substrate/frame/revive/rpc/examples/js/pvm/Flipper.polkavm
Binary file not shown.
Binary file modified substrate/frame/revive/rpc/examples/js/pvm/FlipperCaller.polkavm
Binary file not shown.
Binary file modified substrate/frame/revive/rpc/examples/js/pvm/PiggyBank.polkavm
Binary file not shown.
33 changes: 22 additions & 11 deletions substrate/frame/revive/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -596,19 +596,15 @@ mod benchmarks {
#[benchmark(pov_mode = Measured)]
fn seal_code_size() {
let contract = Contract::<T>::with_index(1, WasmModule::dummy(), vec![]).unwrap();
build_runtime!(runtime, memory: [contract.address.encode(), vec![0u8; 32], ]);
build_runtime!(runtime, memory: [contract.address.encode(),]);

let result;
#[block]
{
result = runtime.bench_code_size(memory.as_mut_slice(), 0, 20);
result = runtime.bench_code_size(memory.as_mut_slice(), 0);
}

assert_ok!(result);
assert_eq!(
U256::from_little_endian(&memory[20..]),
U256::from(WasmModule::dummy().code.len())
);
assert_eq!(result.unwrap(), WasmModule::dummy().code.len() as u64);
}

#[benchmark(pov_mode = Measured)]
Expand Down Expand Up @@ -783,19 +779,34 @@ mod benchmarks {
assert_eq!(U256::from_little_endian(&memory[..]), runtime.ext().minimum_balance());
}

#[benchmark(pov_mode = Measured)]
fn seal_return_data_size() {
let mut setup = CallSetup::<T>::default();
let (mut ext, _) = setup.ext();
let mut runtime = crate::wasm::Runtime::new(&mut ext, vec![]);
let mut memory = memory!(vec![],);
*runtime.ext().last_frame_output_mut() =
ExecReturnValue { data: vec![42; 256], ..Default::default() };
let result;
#[block]
{
result = runtime.bench_return_data_size(memory.as_mut_slice());
}
assert_eq!(result.unwrap(), 256);
}

#[benchmark(pov_mode = Measured)]
fn seal_call_data_size() {
let mut setup = CallSetup::<T>::default();
let (mut ext, _) = setup.ext();
let mut runtime = crate::wasm::Runtime::new(&mut ext, vec![42u8; 128 as usize]);
let mut memory = memory!(vec![0u8; 32 as usize],);
let mut memory = memory!(vec![0u8; 4],);
let result;
#[block]
{
result = runtime.bench_call_data_size(memory.as_mut_slice(), 0);
result = runtime.bench_call_data_size(memory.as_mut_slice());
}
assert_ok!(result);
assert_eq!(U256::from_little_endian(&memory[..]), U256::from(128));
assert_eq!(result.unwrap(), 128);
}

#[benchmark(pov_mode = Measured)]
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/revive/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ pub trait Ext: sealing::Sealed {
fn code_hash(&self, address: &H160) -> H256;

/// Returns the code size of the contract at the given `address` or zero.
fn code_size(&self, address: &H160) -> U256;
fn code_size(&self, address: &H160) -> u64;

/// Returns the code hash of the contract being executed.
fn own_code_hash(&mut self) -> &H256;
Expand Down Expand Up @@ -1663,7 +1663,7 @@ where
})
}

fn code_size(&self, address: &H160) -> U256 {
fn code_size(&self, address: &H160) -> u64 {
<ContractInfoOf<T>>::get(&address)
.and_then(|contract| CodeInfoOf::<T>::get(contract.code_hash))
.map(|info| info.code_len())
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/revive/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4377,11 +4377,11 @@ fn call_data_size_api_works() {
// Call the contract: It echoes back the value returned by the call data size API.
let received = builder::bare_call(addr).build_and_unwrap_result();
assert_eq!(received.flags, ReturnFlags::empty());
assert_eq!(U256::from_little_endian(&received.data), U256::zero());
assert_eq!(u64::from_le_bytes(received.data.try_into().unwrap()), 0);

let received = builder::bare_call(addr).data(vec![1; 256]).build_and_unwrap_result();
assert_eq!(received.flags, ReturnFlags::empty());
assert_eq!(U256::from_little_endian(&received.data), U256::from(256));
assert_eq!(u64::from_le_bytes(received.data.try_into().unwrap()), 256);
});
}

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/revive/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ impl<T: Config> CodeInfo<T> {
}

/// Returns the code length.
pub fn code_len(&self) -> U256 {
pub fn code_len(&self) -> u64 {
self.code_len.into()
}
}
Expand Down
46 changes: 20 additions & 26 deletions substrate/frame/revive/src/wasm/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,8 @@ pub enum RuntimeCosts {
Caller,
/// Weight of calling `seal_call_data_size`.
CallDataSize,
/// Weight of calling `seal_return_data_size`.
ReturnDataSize,
/// Weight of calling `seal_origin`.
Origin,
/// Weight of calling `seal_is_contract`.
Expand Down Expand Up @@ -453,6 +455,7 @@ impl<T: Config> Token<T> for RuntimeCosts {
CopyToContract(len) => T::WeightInfo::seal_copy_to_contract(len),
CopyFromContract(len) => T::WeightInfo::seal_return(len),
CallDataSize => T::WeightInfo::seal_call_data_size(),
ReturnDataSize => T::WeightInfo::seal_return_data_size(),
CallDataLoad => T::WeightInfo::seal_call_data_load(),
CallDataCopy(len) => T::WeightInfo::seal_call_data_copy(len),
Caller => T::WeightInfo::seal_caller(),
Expand Down Expand Up @@ -1283,17 +1286,13 @@ pub mod env {
/// Returns the total size of the contract call input data.
/// See [`pallet_revive_uapi::HostFn::call_data_size `].
#[stable]
fn call_data_size(&mut self, memory: &mut M, out_ptr: u32) -> Result<(), TrapReason> {
fn call_data_size(&mut self, memory: &mut M) -> Result<u64, TrapReason> {
self.charge_gas(RuntimeCosts::CallDataSize)?;
let value =
U256::from(self.input_data.as_ref().map(|input| input.len()).unwrap_or_default());
Ok(self.write_fixed_sandbox_output(
memory,
out_ptr,
&value.to_little_endian(),
false,
already_charged,
)?)
Ok(self
.input_data
.as_ref()
.map(|input| input.len().try_into().expect("usize fits into u64; qed"))
.unwrap_or_default())
}

/// Stores the input passed by the caller into the supplied buffer.
Expand Down Expand Up @@ -1420,16 +1419,10 @@ pub mod env {
/// Retrieve the code size for a given contract address.
/// See [`pallet_revive_uapi::HostFn::code_size`].
#[stable]
fn code_size(&mut self, memory: &mut M, addr_ptr: u32, out_ptr: u32) -> Result<(), TrapReason> {
fn code_size(&mut self, memory: &mut M, addr_ptr: u32) -> Result<u64, TrapReason> {
self.charge_gas(RuntimeCosts::CodeSize)?;
let address = memory.read_h160(addr_ptr)?;
Ok(self.write_fixed_sandbox_output(
memory,
out_ptr,
&self.ext.code_size(&address).to_little_endian(),
false,
already_charged,
)?)
Ok(self.ext.code_size(&address))
}

/// Stores the address of the current contract into the supplied buffer.
Expand Down Expand Up @@ -1667,14 +1660,15 @@ pub mod env {
/// Stores the length of the data returned by the last call into the supplied buffer.
/// See [`pallet_revive_uapi::HostFn::return_data_size`].
#[stable]
fn return_data_size(&mut self, memory: &mut M, out_ptr: u32) -> Result<(), TrapReason> {
Ok(self.write_fixed_sandbox_output(
memory,
out_ptr,
&U256::from(self.ext.last_frame_output().data.len()).to_little_endian(),
false,
|len| Some(RuntimeCosts::CopyToContract(len)),
)?)
fn return_data_size(&mut self, memory: &mut M) -> Result<u64, TrapReason> {
self.charge_gas(RuntimeCosts::ReturnDataSize)?;
Ok(self
.ext
.last_frame_output()
.data
.len()
.try_into()
.expect("usize fits into u64; qed"))
}

/// Stores data returned by the last call, starting from `offset`, into the supplied buffer.
Expand Down
Loading

0 comments on commit 53f6473

Please sign in to comment.