[pallet-revive] last call return data API

prdoc/pr_5779.prdoc

@@ -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
+

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

@@ -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)[..]);
+}

substrate/frame/revive/src/chain_extension.rs

@@ -344,15 +344,16 @@ impl<'a, 'b, E: Ext, M: ?Sized + Memory<E::T>> Environment<'a, 'b, E, M> {
 		allow_skip: bool,
 		weight_per_byte: Option<Weight>,
 	) -> Result<()> {
-		self.runtime.write_sandbox_output(
+		if let Some(weight_per_byte) = weight_per_byte {
+			let weight = weight_per_byte.saturating_mul(buffer.len() as u64);
+			self.runtime.charge_gas(RuntimeCosts::ChainExtension(weight))?;
+		}
+		super::wasm::write_sandbox_output::<E, M>(
 			self.memory,
 			self.output_ptr,
 			self.output_len_ptr,
 			buffer,
 			allow_skip,
-			|len| {
-				weight_per_byte.map(|w| RuntimeCosts::ChainExtension(w.saturating_mul(len.into())))
-			},
 		)
 	}
 }