diff --git a/prdoc/pr_5779.prdoc b/prdoc/pr_5779.prdoc new file mode 100644 index 000000000000..659a3a19f695 --- /dev/null +++ b/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 + \ No newline at end of file diff --git a/substrate/frame/revive/fixtures/contracts/return_data_api.rs b/substrate/frame/revive/fixtures/contracts/return_data_api.rs new file mode 100644 index 000000000000..846396b0944d --- /dev/null +++ b/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)[..]); +} diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 233658696c8f..2e48bab29255 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -189,16 +189,12 @@ pub trait Ext: sealing::Sealed { input_data: Vec, allows_reentry: bool, read_only: bool, - ) -> Result; + ) -> Result<(), ExecError>; /// Execute code in the current frame. /// /// Returns the code size of the called contract. - fn delegate_call( - &mut self, - code: H256, - input_data: Vec, - ) -> Result; + fn delegate_call(&mut self, code: H256, input_data: Vec) -> Result<(), ExecError>; /// Instantiate a contract from the given code. /// @@ -213,7 +209,7 @@ pub trait Ext: sealing::Sealed { value: U256, input_data: Vec, salt: Option<&[u8; 32]>, - ) -> Result<(H160, ExecReturnValue), ExecError>; + ) -> Result; /// Transfer all funds to `beneficiary` and delete the contract. /// @@ -427,6 +423,12 @@ pub trait Ext: sealing::Sealed { /// Check if running in read-only context. fn is_read_only(&self) -> bool; + + /// Returns an immutable reference to the output of the last executed call frame. + fn last_frame_output(&self) -> &ExecReturnValue; + + /// Returns a mutable reference to the output of the last executed call frame. + fn last_frame_output_mut(&mut self) -> &mut ExecReturnValue; } /// Describes the different functions that can be exported by an [`Executable`]. @@ -547,6 +549,8 @@ struct Frame { read_only: bool, /// The caller of the currently executing frame which was spawned by `delegate_call`. delegate_caller: Option>, + /// The output of the last executed call frame. + last_frame_output: ExecReturnValue, } /// Used in a delegate call frame arguments in order to override the executable and caller. @@ -731,7 +735,7 @@ where value, debug_message, )? { - stack.run(executable, input_data) + stack.run(executable, input_data).map(|_| stack.first_frame.last_frame_output) } else { Self::transfer_no_contract(&origin, &dest, value) } @@ -772,7 +776,9 @@ where )? .expect(FRAME_ALWAYS_EXISTS_ON_INSTANTIATE); let address = T::AddressMapper::to_address(&stack.top_frame().account_id); - stack.run(executable, input_data).map(|ret| (address, ret)) + stack + .run(executable, input_data) + .map(|_| (address, stack.first_frame.last_frame_output)) } #[cfg(all(feature = "runtime-benchmarks", feature = "riscv"))] @@ -865,7 +871,7 @@ where { contract } else { - return Ok(None) + return Ok(None); } }; @@ -911,6 +917,7 @@ where nested_storage: storage_meter.nested(deposit_limit), allows_reentry: true, read_only, + last_frame_output: Default::default(), }; Ok(Some((frame, executable))) @@ -965,12 +972,24 @@ where /// Run the current (top) frame. /// /// This can be either a call or an instantiate. - fn run(&mut self, executable: E, input_data: Vec) -> ExecResult { + fn run(&mut self, executable: E, input_data: Vec) -> Result<(), ExecError> { let frame = self.top_frame(); let entry_point = frame.entry_point; let delegated_code_hash = if frame.delegate_caller.is_some() { Some(*executable.code_hash()) } else { None }; + // The output of the caller frame will be replaced by the output of this run. + // It is also not accessible from nested frames. + // Hence we drop it early to save the memory. + let frames_len = self.frames.len(); + if let Some(caller_frame) = match frames_len { + 0 => None, + 1 => Some(&mut self.first_frame.last_frame_output), + _ => self.frames.get_mut(frames_len - 2).map(|frame| &mut frame.last_frame_output), + } { + *caller_frame = Default::default(); + } + self.transient_storage.start_transaction(); let do_transaction = || { @@ -1109,7 +1128,9 @@ where } self.pop_frame(success); - output + output.map(|output| { + self.top_frame_mut().last_frame_output = output; + }) } /// Remove the current (top) frame from the stack. @@ -1285,7 +1306,7 @@ where input_data: Vec, allows_reentry: bool, read_only: bool, - ) -> ExecResult { + ) -> Result<(), ExecError> { // Before pushing the new frame: Protect the caller contract against reentrancy attacks. // It is important to do this before calling `allows_reentry` so that a direct recursion // is caught by it. @@ -1323,7 +1344,8 @@ where &Origin::from_account_id(self.account_id().clone()), &dest, value, - ) + )?; + Ok(()) } }; @@ -1336,11 +1358,7 @@ where result } - fn delegate_call( - &mut self, - code_hash: H256, - input_data: Vec, - ) -> Result { + fn delegate_call(&mut self, code_hash: H256, input_data: Vec) -> Result<(), ExecError> { let executable = E::from_storage(code_hash, self.gas_meter_mut())?; let top_frame = self.top_frame_mut(); let contract_info = top_frame.contract_info().clone(); @@ -1368,7 +1386,7 @@ where value: U256, input_data: Vec, salt: Option<&[u8; 32]>, - ) -> Result<(H160, ExecReturnValue), ExecError> { + ) -> Result { let executable = E::from_storage(code_hash, self.gas_meter_mut())?; let sender = &self.top_frame().account_id; let executable = self.push_frame( @@ -1385,7 +1403,7 @@ where )?; let address = T::AddressMapper::to_address(&self.top_frame().account_id); self.run(executable.expect(FRAME_ALWAYS_EXISTS_ON_INSTANTIATE), input_data) - .map(|ret| (address, ret)) + .map(|_| address) } fn terminate(&mut self, beneficiary: &H160) -> DispatchResult { @@ -1690,6 +1708,14 @@ where fn is_read_only(&self) -> bool { self.top_frame().read_only } + + fn last_frame_output(&self) -> &ExecReturnValue { + &self.top_frame().last_frame_output + } + + fn last_frame_output_mut(&mut self) -> &mut ExecReturnValue { + &mut self.top_frame_mut().last_frame_output + } } mod sealing { @@ -2353,15 +2379,17 @@ mod tests { // ALICE is the origin of the call stack assert!(ctx.ext.caller_is_origin()); // BOB calls CHARLIE - ctx.ext.call( - Weight::zero(), - U256::zero(), - &CHARLIE_ADDR, - U256::zero(), - vec![], - true, - false, - ) + ctx.ext + .call( + Weight::zero(), + U256::zero(), + &CHARLIE_ADDR, + U256::zero(), + vec![], + true, + false, + ) + .map(|_| ctx.ext.last_frame_output().clone()) }); ExtBuilder::default().build().execute_with(|| { @@ -2447,15 +2475,17 @@ mod tests { // root is the origin of the call stack. assert!(ctx.ext.caller_is_root()); // BOB calls CHARLIE. - ctx.ext.call( - Weight::zero(), - U256::zero(), - &CHARLIE_ADDR, - U256::zero(), - vec![], - true, - false, - ) + ctx.ext + .call( + Weight::zero(), + U256::zero(), + &CHARLIE_ADDR, + U256::zero(), + vec![], + true, + false, + ) + .map(|_| ctx.ext.last_frame_output().clone()) }); ExtBuilder::default().build().execute_with(|| { @@ -2666,6 +2696,7 @@ mod tests { vec![], Some(&[48; 32]), ) + .map(|address| (address, ctx.ext.last_frame_output().clone())) .unwrap(); *instantiated_contract_address.borrow_mut() = Some(address); @@ -2841,15 +2872,17 @@ mod tests { assert_eq!(info.storage_byte_deposit, 0); info.storage_byte_deposit = 42; assert_eq!( - ctx.ext.call( - Weight::zero(), - U256::zero(), - &CHARLIE_ADDR, - U256::zero(), - vec![], - true, - false - ), + ctx.ext + .call( + Weight::zero(), + U256::zero(), + &CHARLIE_ADDR, + U256::zero(), + vec![], + true, + false + ) + .map(|_| ctx.ext.last_frame_output().clone()), exec_trapped() ); assert_eq!(ctx.ext.contract_info().storage_byte_deposit, 42); @@ -3095,6 +3128,7 @@ mod tests { let dest = H160::from_slice(ctx.input_data.as_ref()); ctx.ext .call(Weight::zero(), U256::zero(), &dest, U256::zero(), vec![], false, false) + .map(|_| ctx.ext.last_frame_output().clone()) }); let code_charlie = MockLoader::insert(Call, |_, _| exec_success()); @@ -3137,15 +3171,17 @@ mod tests { fn call_deny_reentry() { let code_bob = MockLoader::insert(Call, |ctx, _| { if ctx.input_data[0] == 0 { - ctx.ext.call( - Weight::zero(), - U256::zero(), - &CHARLIE_ADDR, - U256::zero(), - vec![], - false, - false, - ) + ctx.ext + .call( + Weight::zero(), + U256::zero(), + &CHARLIE_ADDR, + U256::zero(), + vec![], + false, + false, + ) + .map(|_| ctx.ext.last_frame_output().clone()) } else { exec_success() } @@ -3153,15 +3189,9 @@ mod tests { // call BOB with input set to '1' let code_charlie = MockLoader::insert(Call, |ctx, _| { - ctx.ext.call( - Weight::zero(), - U256::zero(), - &BOB_ADDR, - U256::zero(), - vec![1], - true, - false, - ) + ctx.ext + .call(Weight::zero(), U256::zero(), &BOB_ADDR, U256::zero(), vec![1], true, false) + .map(|_| ctx.ext.last_frame_output().clone()) }); ExtBuilder::default().build().execute_with(|| { @@ -3360,7 +3390,7 @@ mod tests { let alice_nonce = System::account_nonce(&ALICE); assert_eq!(System::account_nonce(ctx.ext.account_id()), 0); assert_eq!(ctx.ext.caller().account_id().unwrap(), &ALICE); - let (addr, _) = ctx + let addr = ctx .ext .instantiate( Weight::zero(), @@ -3916,15 +3946,17 @@ mod tests { Ok(WriteOutcome::New) ); assert_eq!( - ctx.ext.call( - Weight::zero(), - U256::zero(), - &CHARLIE_ADDR, - U256::zero(), - vec![], - true, - false, - ), + ctx.ext + .call( + Weight::zero(), + U256::zero(), + &CHARLIE_ADDR, + U256::zero(), + vec![], + true, + false, + ) + .map(|_| ctx.ext.last_frame_output().clone()), exec_success() ); assert_eq!(ctx.ext.get_transient_storage(storage_key_1), Some(vec![3])); @@ -4020,15 +4052,17 @@ mod tests { Ok(WriteOutcome::New) ); assert_eq!( - ctx.ext.call( - Weight::zero(), - U256::zero(), - &CHARLIE_ADDR, - U256::zero(), - vec![], - true, - false - ), + ctx.ext + .call( + Weight::zero(), + U256::zero(), + &CHARLIE_ADDR, + U256::zero(), + vec![], + true, + false + ) + .map(|_| ctx.ext.last_frame_output().clone()), exec_trapped() ); assert_eq!(ctx.ext.get_transient_storage(storage_key), Some(vec![1, 2])); @@ -4102,4 +4136,148 @@ mod tests { assert_matches!(result, Ok(_)); }); } + + #[test] + fn last_frame_output_works_on_instantiate() { + let ok_ch = MockLoader::insert(Constructor, move |_, _| { + Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: vec![127] }) + }); + let revert_ch = MockLoader::insert(Constructor, move |_, _| { + Ok(ExecReturnValue { flags: ReturnFlags::REVERT, data: vec![70] }) + }); + let trap_ch = MockLoader::insert(Constructor, |_, _| Err("It's a trap!".into())); + let instantiator_ch = MockLoader::insert(Call, { + move |ctx, _| { + let value = ::Currency::minimum_balance().into(); + + // Successful instantiation should set the output + let address = ctx + .ext + .instantiate(Weight::zero(), U256::zero(), ok_ch, value, vec![], None) + .unwrap(); + assert_eq!( + ctx.ext.last_frame_output(), + &ExecReturnValue { flags: ReturnFlags::empty(), data: vec![127] } + ); + + // Plain transfers should not set the output + ctx.ext.transfer(&address, U256::from(1)).unwrap(); + assert_eq!( + ctx.ext.last_frame_output(), + &ExecReturnValue { flags: ReturnFlags::empty(), data: vec![127] } + ); + + // Reverted instantiation should set the output + ctx.ext + .instantiate(Weight::zero(), U256::zero(), revert_ch, value, vec![], None) + .unwrap(); + assert_eq!( + ctx.ext.last_frame_output(), + &ExecReturnValue { flags: ReturnFlags::REVERT, data: vec![70] } + ); + + // Trapped instantiation should clear the output + ctx.ext + .instantiate(Weight::zero(), U256::zero(), trap_ch, value, vec![], None) + .unwrap_err(); + assert_eq!( + ctx.ext.last_frame_output(), + &ExecReturnValue { flags: ReturnFlags::empty(), data: vec![] } + ); + + exec_success() + } + }); + + ExtBuilder::default() + .with_code_hashes(MockLoader::code_hashes()) + .existential_deposit(15) + .build() + .execute_with(|| { + set_balance(&ALICE, 1000); + set_balance(&BOB_CONTRACT_ID, 100); + place_contract(&BOB, instantiator_ch); + let origin = Origin::from_account_id(ALICE); + let mut storage_meter = storage::meter::Meter::new(&origin, 200, 0).unwrap(); + + MockStack::run_call( + origin, + BOB_ADDR, + &mut GasMeter::::new(GAS_LIMIT), + &mut storage_meter, + 0, + vec![], + None, + ) + .unwrap() + }); + } + + #[test] + fn last_frame_output_works_on_nested_call() { + // Call stack: BOB -> CHARLIE(revert) -> BOB' (success) + let code_bob = MockLoader::insert(Call, |ctx, _| { + if ctx.input_data.is_empty() { + // We didn't do anything yet + assert_eq!( + ctx.ext.last_frame_output(), + &ExecReturnValue { flags: ReturnFlags::empty(), data: vec![] } + ); + + ctx.ext + .call( + Weight::zero(), + U256::zero(), + &CHARLIE_ADDR, + U256::zero(), + vec![], + true, + false, + ) + .unwrap(); + assert_eq!( + ctx.ext.last_frame_output(), + &ExecReturnValue { flags: ReturnFlags::REVERT, data: vec![70] } + ); + } + + Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: vec![127] }) + }); + let code_charlie = MockLoader::insert(Call, |ctx, _| { + // We didn't do anything yet + assert_eq!( + ctx.ext.last_frame_output(), + &ExecReturnValue { flags: ReturnFlags::empty(), data: vec![] } + ); + + assert!(ctx + .ext + .call(Weight::zero(), U256::zero(), &BOB_ADDR, U256::zero(), vec![99], true, false) + .is_ok()); + assert_eq!( + ctx.ext.last_frame_output(), + &ExecReturnValue { flags: ReturnFlags::empty(), data: vec![127] } + ); + + Ok(ExecReturnValue { flags: ReturnFlags::REVERT, data: vec![70] }) + }); + + ExtBuilder::default().build().execute_with(|| { + place_contract(&BOB, code_bob); + place_contract(&CHARLIE, code_charlie); + let origin = Origin::from_account_id(ALICE); + let mut storage_meter = storage::meter::Meter::new(&origin, 0, 0).unwrap(); + + let result = MockStack::run_call( + origin, + BOB_ADDR, + &mut GasMeter::::new(GAS_LIMIT), + &mut storage_meter, + 0, + vec![0], + None, + ); + assert_matches!(result, Ok(_)); + }); + } } diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index d06cdcfd4654..5c5d144f24a2 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -4325,4 +4325,33 @@ mod run_tests { assert_eq!(received.result.data, chain_id.encode()); }); } + + #[test] + fn return_data_api_works() { + let (code_return_data_api, _) = compile_module("return_data_api").unwrap(); + let (code_return_with_data, hash_return_with_data) = + compile_module("return_with_data").unwrap(); + + ExtBuilder::default().existential_deposit(100).build().execute_with(|| { + let _ = ::Currency::set_balance(&ALICE, 1_000_000); + + // Upload the io echoing fixture for later use + assert_ok!(Contracts::upload_code( + RuntimeOrigin::signed(ALICE), + code_return_with_data, + deposit_limit::(), + )); + + // Create fixture: Constructor does nothing + let Contract { addr, .. } = + builder::bare_instantiate(Code::Upload(code_return_data_api)) + .build_and_unwrap_contract(); + + // Call the contract: It will issue calls and deploys, asserting on + assert_ok!(builder::call(addr) + .value(10 * 1024) + .data(hash_return_with_data.encode()) + .build()); + }); + } } diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index ebc407adacd2..4b5a9a04eb73 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -28,7 +28,7 @@ use crate::{ }; use alloc::{boxed::Box, vec, vec::Vec}; use codec::{Decode, DecodeLimit, Encode, MaxEncodedLen}; -use core::{fmt, marker::PhantomData}; +use core::{fmt, marker::PhantomData, mem}; use frame_support::{ dispatch::DispatchInfo, ensure, pallet_prelude::DispatchResultWithPostInfo, parameter_types, traits::Get, weights::Weight, @@ -237,8 +237,8 @@ parameter_types! { const XcmExecutionFailed: ReturnErrorCode = ReturnErrorCode::XcmExecutionFailed; } -impl From for ReturnErrorCode { - fn from(from: ExecReturnValue) -> Self { +impl From<&ExecReturnValue> for ReturnErrorCode { + fn from(from: &ExecReturnValue) -> Self { if from.flags.contains(ReturnFlags::REVERT) { Self::CalleeReverted } else { @@ -769,20 +769,16 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { } } - /// Fallible conversion of a `ExecResult` to `ReturnErrorCode`. - fn exec_into_return_code(from: ExecResult) -> Result { + /// Fallible conversion of a `ExecError` to `ReturnErrorCode`. + fn exec_error_into_return_code(from: ExecError) -> Result { use crate::exec::ErrorOrigin::Callee; - let ExecError { error, origin } = match from { - Ok(retval) => return Ok(retval.into()), - Err(err) => err, - }; - - match (error, origin) { + match (from.error, from.origin) { (_, Callee) => Ok(ReturnErrorCode::CalleeTrapped), (err, _) => Self::err_into_return_code(err), } } + fn decode_key(&self, memory: &M, key_ptr: u32, key_len: u32) -> Result { let res = match key_len { SENTINEL => { @@ -1036,28 +1032,32 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { }, }; - // `TAIL_CALL` only matters on an `OK` result. Otherwise the call stack comes to - // a halt anyways without anymore code being executed. - if flags.contains(CallFlags::TAIL_CALL) { - if let Ok(return_value) = call_outcome { + match call_outcome { + // `TAIL_CALL` only matters on an `OK` result. Otherwise the call stack comes to + // a halt anyways without anymore code being executed. + Ok(_) if flags.contains(CallFlags::TAIL_CALL) => { + let output = mem::take(self.ext.last_frame_output_mut()); return Err(TrapReason::Return(ReturnData { - flags: return_value.flags.bits(), - data: return_value.data, + flags: output.flags.bits(), + data: output.data, })); - } - } - - if let Ok(output) = &call_outcome { - self.write_sandbox_output( - memory, - output_ptr, - output_len_ptr, - &output.data, - true, - |len| Some(RuntimeCosts::CopyToContract(len)), - )?; + }, + Ok(_) => { + let output = mem::take(self.ext.last_frame_output_mut()); + let write_result = self.write_sandbox_output( + memory, + output_ptr, + output_len_ptr, + &output.data, + true, + |len| Some(RuntimeCosts::CopyToContract(len)), + ); + *self.ext.last_frame_output_mut() = output; + write_result?; + Ok(self.ext.last_frame_output().into()) + }, + Err(err) => Ok(Self::exec_error_into_return_code(err)?), } - Ok(Self::exec_into_return_code(call_outcome)?) } fn instantiate( @@ -1086,34 +1086,40 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { let salt: [u8; 32] = memory.read_array(salt_ptr)?; Some(salt) }; - let instantiate_outcome = self.ext.instantiate( + + match self.ext.instantiate( weight, deposit_limit, code_hash, value, input_data, salt.as_ref(), - ); - if let Ok((address, output)) = &instantiate_outcome { - if !output.flags.contains(ReturnFlags::REVERT) { - self.write_fixed_sandbox_output( + ) { + Ok(address) => { + if !self.ext.last_frame_output().flags.contains(ReturnFlags::REVERT) { + self.write_fixed_sandbox_output( + memory, + address_ptr, + &address.as_bytes(), + true, + already_charged, + )?; + } + let output = mem::take(self.ext.last_frame_output_mut()); + let write_result = self.write_sandbox_output( memory, - address_ptr, - &address.as_bytes(), + output_ptr, + output_len_ptr, + &output.data, true, - already_charged, - )?; - } - self.write_sandbox_output( - memory, - output_ptr, - output_len_ptr, - &output.data, - true, - |len| Some(RuntimeCosts::CopyToContract(len)), - )?; + |len| Some(RuntimeCosts::CopyToContract(len)), + ); + *self.ext.last_frame_output_mut() = output; + write_result?; + Ok(self.ext.last_frame_output().into()) + }, + Err(err) => Ok(Self::exec_error_into_return_code(err)?), } - Ok(Self::exec_into_return_code(instantiate_outcome.map(|(_, retval)| retval))?) } fn terminate(&mut self, memory: &M, beneficiary_ptr: u32) -> Result<(), TrapReason> { @@ -1993,4 +1999,44 @@ pub mod env { self.ext.unlock_delegate_dependency(&code_hash)?; Ok(()) } + + /// Stores the length of the data returned by the last call into the supplied buffer. + /// See [`pallet_revive_uapi::HostFn::return_data_size`]. + #[api_version(0)] + fn return_data_size(&mut self, memory: &mut M, out_ptr: u32) -> Result<(), TrapReason> { + Ok(self.write_fixed_sandbox_output( + memory, + out_ptr, + &as_bytes(U256::from(self.ext.last_frame_output().data.len())), + false, + |len| Some(RuntimeCosts::CopyToContract(len)), + )?) + } + + /// Stores data returned by the last call, starting from `offset`, into the supplied buffer. + /// See [`pallet_revive_uapi::HostFn::return_data`]. + #[api_version(0)] + fn return_data_copy( + &mut self, + memory: &mut M, + out_ptr: u32, + out_len_ptr: u32, + offset: u32, + ) -> Result<(), TrapReason> { + let output = mem::take(self.ext.last_frame_output_mut()); + let result = if offset as usize > output.data.len() { + Err(Error::::OutOfBounds.into()) + } else { + self.write_sandbox_output( + memory, + out_ptr, + out_len_ptr, + &output.data[offset as usize..], + false, + |len| Some(RuntimeCosts::CopyToContract(len)), + ) + }; + *self.ext.last_frame_output_mut() = output; + Ok(result?) + } } diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index 57a03332670f..816fdec3aaaf 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -620,6 +620,20 @@ pub trait HostFn: private::Sealed { /// Returns `ReturnCode::Success` when the message was successfully sent. When the XCM /// execution fails, `ReturnErrorCode::XcmSendFailed` is returned. fn xcm_send(dest: &[u8], msg: &[u8], output: &mut [u8; 32]) -> Result; + + /// Stores the size of the returned data of the last contract call or instantiation. + /// + /// # Parameters + /// + /// - `output`: A reference to the output buffer to write the size. + fn return_data_size(output: &mut [u8; 32]); + + /// Stores the returned data of the last contract call or contract instantiation. + /// + /// # Parameters + /// - `output`: A reference to the output buffer to write the data. + /// - `offset`: Byte offset into the returned data + fn return_data_copy(output: &mut &mut [u8], offset: u32); } mod private { diff --git a/substrate/frame/revive/uapi/src/host/riscv32.rs b/substrate/frame/revive/uapi/src/host/riscv32.rs index a60c338e8bd9..d5ea94c1a910 100644 --- a/substrate/frame/revive/uapi/src/host/riscv32.rs +++ b/substrate/frame/revive/uapi/src/host/riscv32.rs @@ -131,6 +131,8 @@ mod sys { msg_len: u32, out_ptr: *mut u8, ) -> ReturnCode; + pub fn return_data_size(out_ptr: *mut u8); + pub fn return_data_copy(out_ptr: *mut u8, out_len_ptr: *mut u32, offset: u32); } } @@ -548,4 +550,16 @@ impl HostFn for HostFnImpl { }; ret_code.into() } + + fn return_data_size(output: &mut [u8; 32]) { + unsafe { sys::return_data_size(output.as_mut_ptr()) }; + } + + fn return_data_copy(output: &mut &mut [u8], offset: u32) { + let mut output_len = output.len() as u32; + { + unsafe { sys::return_data_copy(output.as_mut_ptr(), &mut output_len, offset) }; + } + extract_from_slice(output, output_len as usize); + } }