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] last call return data API #5779

Merged
merged 31 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6488586
store return data in frame
xermicus Sep 18, 2024
1a7569c
impl runtime apis
xermicus Sep 19, 2024
68fb924
Merge branch 'master' into cl/return-data-api
xermicus Sep 19, 2024
d53dcae
better name for output field
xermicus Sep 19, 2024
5073e45
uapi
xermicus Sep 19, 2024
81f77ef
wip tests
xermicus Sep 19, 2024
c981a75
test fixture
xermicus Sep 19, 2024
6f43354
store the output directly
xermicus Sep 20, 2024
0eb213c
executable output is owned by the frame
xermicus Sep 20, 2024
68e989b
some bikeshedding
xermicus Sep 20, 2024
039c6b3
fx
xermicus Sep 20, 2024
f198cf5
fix
xermicus Sep 20, 2024
8e8b2a2
test return data api on instantiate in the mock
xermicus Sep 20, 2024
4caa6a0
test return data api on nested calls in the mock
xermicus Sep 20, 2024
a04cfb7
prdoc
xermicus Sep 20, 2024
d366111
Merge branch 'master' into cl/return-data-api
xermicus Sep 20, 2024
fcacb89
".git/.scripts/commands/fmt/fmt.sh"
Sep 20, 2024
5878734
Merge branch 'master' into cl/return-data-api
xermicus Sep 23, 2024
f3ca36b
Merge branch 'master' into cl/return-data-api
xermicus Sep 24, 2024
b6c3705
work around borrow checker limitations
xermicus Sep 24, 2024
b212893
Merge branch 'master' into cl/return-data-api
xermicus Sep 24, 2024
7b69ee7
".git/.scripts/commands/fmt/fmt.sh"
Sep 24, 2024
1b83cf5
remove take_caller_output helper and provide a comment why we want to…
xermicus Sep 24, 2024
2e1c5fc
move sandbox write helpers into Memory trait
xermicus Sep 24, 2024
8ea20a1
write actual buf len
xermicus Sep 24, 2024
a7a9c63
Revert "work around borrow checker limitations"
xermicus Sep 25, 2024
1e3cddf
rm stale methods
xermicus Sep 25, 2024
eeffa2d
Merge branch 'master' into cl/return-data-api
xermicus Sep 25, 2024
06305fe
".git/.scripts/commands/fmt/fmt.sh"
Sep 25, 2024
456697c
Update substrate/frame/revive/src/wasm/runtime.rs
xermicus Sep 25, 2024
61f1f1e
fix return types
xermicus Sep 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions prdoc/pr_5779.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
title: "[pallet-revive] last call return data API"

doc:
- audience: Runtime Dev
description: |
This PR introduces 2 new syscall: `return_data_size` and `return_data_copy`,
resembling the semantics of the EVM `RETURNDATASIZE` and `RETURNDATACOPY` opcodes.

The ownership of `ExecReturnValue` (the return data) has moved to the `Frame`.
This allows implementing the new contract API surface functionality in ext with no additional copies.
Returned data is passed via contract memory, memory is (will be) metered,
hence the amount of returned data can not be statically known,
so we should avoid storing copies of the returned data if we can.
By moving the ownership of the exectuables return value into the `Frame` struct we achieve this.

A zero-copy implementation of those APIs would be technically possible without that internal change by making
the callsite in the runtime responsible for moving the returned data into the frame after any call.
However, resetting the stored output needs to be handled in ext, since plain transfers will _not_ affect the
stored return data (and we don't want to handle this special call case inside the `runtime` API).
This has drawbacks:
- It can not be tested easily in the mock.
- It introduces an inconsistency where resetting the stored output is handled in ext,
but the runtime API is responsible to store it back correctly after any calls made.
Instead, with ownership of the data in `Frame`, both can be handled in a single place.
Handling both in `fn run()` is more natural and leaves less room for runtime API bugs.

The returned output is reset each time _before_ running any executable in a nested stack.
This change should not incur any overhead to the overall memory usage as _only_ the returned data from the last
executed frame will be kept around at any time.

crates:
- name: pallet-revive
bump: major
- name: pallet-revive-fixtures
bump: minor
- name: pallet-revive-uapi
bump: minor

166 changes: 166 additions & 0 deletions substrate/frame/revive/fixtures/contracts/return_data_api.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
// This file is part of Substrate.

// Copyright (C) Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//! This tests that the `return_data_size` and `return_data_copy` APIs work.
//!
//! It does so by calling and instantiating the "return_with_data" fixture,
//! which always echoes back the input[4..] regardless of the call outcome.
//!
//! We also check that the saved return data is properly reset after a trap
//! and unaffected by plain transfers.

#![no_std]
#![no_main]

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

const INPUT_BUF_SIZE: usize = 128;
static INPUT_DATA: [u8; INPUT_BUF_SIZE] = [0xFF; INPUT_BUF_SIZE];
/// The "return_with_data" fixture echoes back 4 bytes less than the input
const OUTPUT_BUF_SIZE: usize = INPUT_BUF_SIZE - 4;
static OUTPUT_DATA: [u8; OUTPUT_BUF_SIZE] = [0xEE; OUTPUT_BUF_SIZE];

fn assert_return_data_after_call(input: &[u8]) {
assert_return_data_size_of(OUTPUT_BUF_SIZE as u64);
assert_plain_transfer_does_not_reset(OUTPUT_BUF_SIZE as u64);
assert_return_data_copy(&input[4..]);
reset_return_data();
}

/// Assert that what we get from [api::return_data_copy] matches `whole_return_data`,
/// either fully or partially with an offset and limited size.
fn assert_return_data_copy(whole_return_data: &[u8]) {
// The full return data should match
let mut buf = OUTPUT_DATA;
let mut full = &mut buf[..whole_return_data.len()];
api::return_data_copy(&mut full, 0);
assert_eq!(whole_return_data, full);

// Partial return data should match
let mut buf = OUTPUT_DATA;
let offset = 5; // we just pick some offset
let size = 32; // we just pick some size
let mut partial = &mut buf[offset..offset + size];
api::return_data_copy(&mut partial, offset as u32);
assert_eq!(*partial, whole_return_data[offset..offset + size]);
}

/// This function panics in a recursive contract call context.
fn recursion_guard() -> [u8; 20] {
let mut caller_address = [0u8; 20];
api::caller(&mut caller_address);

let mut own_address = [0u8; 20];
api::address(&mut own_address);

assert_ne!(caller_address, own_address);

own_address
}

/// Call ourselves recursively, which panics the callee and thus resets the return data.
fn reset_return_data() {
api::call(
uapi::CallFlags::ALLOW_REENTRY,
&recursion_guard(),
0u64,
0u64,
None,
&[0u8; 32],
&[0u8; 32],
None,
)
.unwrap_err();
assert_return_data_size_of(0);
}

/// 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 [api::return_data_size] to match the `expected` value after a plain transfer
/// (plain transfers don't issue a call and so should not reset the return data)
fn assert_plain_transfer_does_not_reset(expected: u64) {
api::transfer(&[0; 20], &u256_bytes(128)).unwrap();
assert_return_data_size_of(expected);
}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn deploy() {}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {
input!(code_hash: &[u8; 32],);

// We didn't do anything yet; return data size should be 0
assert_return_data_size_of(0);

recursion_guard();

let mut address_buf = [0; 20];
let construct_input = |exit_flag| {
let mut input = INPUT_DATA;
input[0] = exit_flag;
input[9] = 7;
input[17 / 2] = 127;
input[89 / 2] = 127;
input
};
let mut instantiate = |exit_flag| {
api::instantiate(
code_hash,
0u64,
0u64,
None,
&[0; 32],
&construct_input(exit_flag),
Some(&mut address_buf),
None,
None,
)
};
let call = |exit_flag, address_buf| {
api::call(
uapi::CallFlags::empty(),
address_buf,
0u64,
0u64,
None,
&[0; 32],
&construct_input(exit_flag),
None,
)
};

instantiate(0).unwrap();
assert_return_data_after_call(&construct_input(0)[..]);

instantiate(1).unwrap_err();
assert_return_data_after_call(&construct_input(1)[..]);

call(0, &address_buf).unwrap();
assert_return_data_after_call(&construct_input(0)[..]);

call(1, &address_buf).unwrap_err();
assert_return_data_after_call(&construct_input(1)[..]);
}
Loading