diff --git a/prdoc/pr_5779.prdoc b/prdoc/pr_5779.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..659a3a19f695492c0dc2f9a03b6c64a0f528cd93
--- /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 0000000000000000000000000000000000000000..846396b0944d303479d98b6ff227c938b086ec62
--- /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 233658696c8fa968aab32972e25b1b91ce8642c4..2e48bab292555ef04a38640121a7ee48c20b9c95 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<u8>,
allows_reentry: bool,
read_only: bool,
- ) -> Result<ExecReturnValue, ExecError>;
+ ) -> 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<u8>,
- ) -> Result<ExecReturnValue, ExecError>;
+ fn delegate_call(&mut self, code: H256, input_data: Vec<u8>) -> Result<(), ExecError>;
/// Instantiate a contract from the given code.
///
@@ -213,7 +209,7 @@ pub trait Ext: sealing::Sealed {
value: U256,
input_data: Vec<u8>,
salt: Option<&[u8; 32]>,
- ) -> Result<(H160, ExecReturnValue), ExecError>;
+ ) -> Result<H160, ExecError>;
/// 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<T: Config> {
read_only: bool,
/// The caller of the currently executing frame which was spawned by `delegate_call`.
delegate_caller: Option<Origin<T>>,
+ /// 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<u8>) -> ExecResult {
+ fn run(&mut self, executable: E, input_data: Vec<u8>) -> 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<u8>,
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<u8>,
- ) -> Result<ExecReturnValue, ExecError> {
+ fn delegate_call(&mut self, code_hash: H256, input_data: Vec<u8>) -> 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<u8>,
salt: Option<&[u8; 32]>,
- ) -> Result<(H160, ExecReturnValue), ExecError> {
+ ) -> Result<H160, ExecError> {
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 = <Test as Config>::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::<Test>::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::<Test>::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 d06cdcfd4654ce79fcbb2f18af448742fa932c2d..5c5d144f24a27bc3405448d150abe016fac1c905 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 _ = <Test as Config>::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::<Test>(),
+ ));
+
+ // 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 ebc407adacd2b3e9b2110996ecc80d92fd89448c..4b5a9a04eb737892a7c261c0bda06711b5f72e26 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<ExecReturnValue> 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<E::T>> Runtime<'a, E, M> {
}
}
- /// Fallible conversion of a `ExecResult` to `ReturnErrorCode`.
- fn exec_into_return_code(from: ExecResult) -> Result<ReturnErrorCode, DispatchError> {
+ /// Fallible conversion of a `ExecError` to `ReturnErrorCode`.
+ fn exec_error_into_return_code(from: ExecError) -> Result<ReturnErrorCode, DispatchError> {
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<Key, TrapReason> {
let res = match key_len {
SENTINEL => {
@@ -1036,28 +1032,32 @@ impl<'a, E: Ext, M: ?Sized + Memory<E::T>> 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<E::T>> 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::<E::T>::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 57a03332670f6b8ad0866e60cb4b330073119cb5..816fdec3aaaf3a8dcc660065cde4ae97a26e39bb 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 a60c338e8bd9c72f8089c4bd7b8bf5417f9bd9be..d5ea94c1a9103e7347c8d26cfb619b4536cfeae3 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);
+ }
}