Skip to content

Commit

Permalink
EVM: Implement CALLACTOR/METHODNUM and REVERT abort semantics (#633)
Browse files Browse the repository at this point in the history 
* implement CALLACTOR/METHODNUM

* signal EVM_CONTRACT_REVERTED error on reverts

* handle REVERT in CALL/CALLACTOR result

* update comments about REVERT semantics

* simplify call error handling

* fix tests

* fix empty input data handling for calls

* add mnemonic opcodes for CALLACTOR and METHODNUM to assembler

such a hack, but no other way.

* add methodnum and callactor tests

* rustfmt

* lift return

* fix comment
  • Loading branch information
@vyzo
vyzo authored Sep 12, 2022
1 parent bf3f20b commit f5be9a9
Show file tree
Hide file tree
Showing 14 changed files with 332 additions and 125 deletions.
15 changes: 14 additions & 1 deletion actors/evm/src/interpreter/execution.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use {
pub struct ExecutionState {
pub stack: Stack,
pub memory: Memory,
pub method: u64,
pub input_data: Bytes,
pub return_data: Bytes,
pub output_data: Bytes,
Expand All @@ -29,10 +30,11 @@ pub struct ExecutionState {
}

impl ExecutionState {
pub fn new(input_data: Bytes) -> Self {
pub fn new(method: u64, input_data: Bytes) -> Self {
Self {
stack: Stack::default(),
memory: Memory::default(),
method,
input_data,
return_data: Default::default(),
output_data: Bytes::new(),
Expand Down Expand Up @@ -849,6 +851,17 @@ impl<'r, BS: Blockstore + 'r, RT: Runtime<BS> + 'r> Machine<'r, BS, RT> {
lifecycle::selfdestruct(m.runtime, m.system)?;
Ok(ControlFlow::Exit) // selfdestruct halts the current context
}

// FEVM extensions opcodes
CALLACTOR(m) {
call::callactor(m.runtime, m.system)?;
Ok(ControlFlow::Continue)
}

METHODNUM(m) {
call::methodnum(m.runtime);
Ok(ControlFlow::Continue)
}
}

const JMPTABLE: [Instruction<Machine<'r, BS, RT>>; 256] = Machine::<'r, BS, RT>::jmptable();
Expand Down
128 changes: 91 additions & 37 deletions actors/evm/src/interpreter/instructions/call.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ use {
crate::interpreter::System,
crate::interpreter::U256,
crate::RawBytes,
crate::{InvokeParams, Method},
fil_actors_runtime::runtime::builtins::Type as ActorType,
crate::{Method, EVM_CONTRACT_REVERTED},
fil_actors_runtime::runtime::Runtime,
fvm_ipld_blockstore::Blockstore,
fvm_shared::econ::TokenAmount,
Expand Down Expand Up @@ -141,50 +140,30 @@ pub fn call<'r, BS: Blockstore, RT: Runtime<BS>>(
let input_region = get_memory_region(memory, input_offset, input_size)
.map_err(|_| StatusCode::InvalidMemoryAccess)?;

let mut result = {
let result = {
// ref to memory is dropped after calling so we can mutate it on output later
let input_data = input_region
.map(|MemoryRegion { offset, size }| &memory[offset..][..size.get()])
.ok_or(StatusCode::InvalidMemoryAccess)?;
let input_data = if let Some(MemoryRegion { offset, size }) = input_region {
&memory[offset..][..size.get()]
} else {
&[]
};

if precompiles::Precompiles::<BS, RT>::is_precompile(&dst) {
precompiles::Precompiles::call_precompile(rt, dst, input_data)
.map_err(|_| StatusCode::PrecompileFailure)?
let result = precompiles::Precompiles::call_precompile(rt, dst, input_data)
.map_err(|_| StatusCode::PrecompileFailure)?;
Ok(RawBytes::from(result))
} else {
// CALL and its brethren can only invoke other EVM contracts; see the (magic)
// CALLMETHOD/METHODNUM opcodes for calling fil actors with native call
// conventions.
let dst_addr = Address::try_from(dst)?
.as_id_address()
.ok_or_else(|| StatusCode::BadAddress("not an actor id address".to_string()))?;

let dst_code_cid = rt
.get_actor_code_cid(
&rt.resolve_address(&dst_addr).ok_or_else(|| {
StatusCode::BadAddress("cannot resolve address".to_string())
})?,
)
.ok_or_else(|| StatusCode::BadAddress("unknown actor".to_string()))?;
let evm_code_cid = rt.get_code_cid_for_type(ActorType::EVM);
if dst_code_cid != evm_code_cid {
return Err(StatusCode::BadAddress("cannot call non EVM actor".to_string()));
}

match kind {
CallKind::Call => {
let params = InvokeParams { input_data: RawBytes::from(input_data.to_vec()) };
let result = rt.send(
&dst_addr,
Method::InvokeContract as u64,
RawBytes::serialize(params).map_err(|_| {
StatusCode::InternalError(
"failed to marshall invocation data".to_string(),
)
})?,
TokenAmount::from(&value),
);
result.map_err(StatusCode::from)?.to_vec()
}
CallKind::Call => rt.send(
&dst_addr,
Method::InvokeContract as u64,
RawBytes::from(input_data.to_vec()),
TokenAmount::from(&value),
),
CallKind::DelegateCall => {
todo!()
}
Expand All @@ -198,6 +177,19 @@ pub fn call<'r, BS: Blockstore, RT: Runtime<BS>>(
}
};

if let Err(ae) = result {
return if ae.exit_code() == EVM_CONTRACT_REVERTED {
// reverted -- we don't have return data yet
// push failure
stack.push(U256::zero());
Ok(())
} else {
Err(StatusCode::from(ae))
};
}

let mut result = result.unwrap().to_vec();

// save return_data
state.return_data = result.clone().into();

Expand Down Expand Up @@ -233,3 +225,65 @@ pub fn call<'r, BS: Blockstore, RT: Runtime<BS>>(
stack.push(U256::from(1));
Ok(())
}

pub fn callactor<'r, BS: Blockstore, RT: Runtime<BS>>(
state: &mut ExecutionState,
platform: &'r System<'r, BS, RT>,
) -> Result<(), StatusCode> {
let ExecutionState { stack, memory, .. } = state;
let rt = &*platform.rt; // as immutable reference

// stack: GAS DEST VALUE METHODNUM INPUT-OFFSET INPUT-SIZE
// NOTE: we don't need output-offset/output-size (which the CALL instructions have)
// becase these are kinda useless; we can just use RETURNDATA anyway.
// NOTE: gas is currently ignored
let _gas = stack.pop();
let dst = stack.pop();
let value = stack.pop();
let method = stack.pop();
let input_offset = stack.pop();
let input_size = stack.pop();

let input_region = get_memory_region(memory, input_offset, input_size)
.map_err(|_| StatusCode::InvalidMemoryAccess)?;

let result = {
let dst_addr = Address::try_from(dst)?
.as_id_address()
.ok_or_else(|| StatusCode::BadAddress(format!("not an actor id address: {}", dst)))?;

if method.bits() > 64 {
return Err(StatusCode::ArgumentOutOfRange(format!("bad method number: {}", method)));
}
let methodnum = method.as_u64();

let input_data = if let Some(MemoryRegion { offset, size }) = input_region {
&memory[offset..][..size.get()]
} else {
&[]
}
.to_vec();
rt.send(&dst_addr, methodnum, RawBytes::from(input_data), TokenAmount::from(&value))
};

if let Err(ae) = result {
return if ae.exit_code() == EVM_CONTRACT_REVERTED {
// reverted -- we don't have return data yet
// push failure
stack.push(U256::zero());
Ok(())
} else {
Err(StatusCode::from(ae))
};
}

// save return_data
state.return_data = result.unwrap().to_vec().into();
// push success
stack.push(U256::from(1));
Ok(())
}

pub fn methodnum(state: &mut ExecutionState) {
state.stack.push(U256::from(state.method));
}
6 changes: 3 additions & 3 deletions actors/evm/src/interpreter/instructions/memory.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn get_memory_region_u64(
offset: U256,
size: NonZeroUsize,
) -> Result<MemoryRegion, ()> {
if offset > U256::from(u32::MAX) {
if offset.bits() >= 32 {
return Err(());
}

Expand All @@ -55,11 +55,11 @@ pub fn get_memory_region(
offset: U256,
size: U256,
) -> Result<Option<MemoryRegion>, ()> {
if size == U256::zero() {
if size.is_zero() {
return Ok(None);
}

if size > U256::from(u32::MAX) {
if size.bits() >= 32 {
return Err(());
}

Expand Down
6 changes: 6 additions & 0 deletions actors/evm/src/interpreter/opcode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ def_opcodes! {
0xa2: LOG2(4, -4),
0xa3: LOG3(5, -5),
0xa4: LOG4(6, -6),
//////////////////////////////////////////////////////////
// FEVM extension opcodes
// FIL call conventions
0xb0: CALLACTOR(6, -5),
0xb1: METHODNUM(0, 1),
//////////////////////////////////////////////////////////
0xf0: CREATE(3, -2),
0xf1: CALL(7, -6),
0xf2: CALLCODE(7, -6),
Expand Down
2 changes: 1 addition & 1 deletion actors/evm/src/interpreter/output.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub enum StatusCode {
/// An argument to a state accessing method has a value outside of the
/// accepted range of values.
#[strum(serialize = "argument out of range")]
ArgumentOutOfRange,
ArgumentOutOfRange(String),

/// The caller does not have enough funds for value transfer.
#[strum(serialize = "insufficient balance")]
Expand Down
63 changes: 35 additions & 28 deletions actors/evm/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use {
bytes::Bytes,
cid::Cid,
fil_actors_runtime::{
actor_error, cbor,
cbor,
runtime::{ActorCode, Runtime},
ActorDowncast, ActorError,
},
Expand All @@ -28,6 +28,8 @@ fil_actors_runtime::wasm_trampoline!(EvmContractActor);
/// The contract code size limit is 24kB.
const MAX_CODE_SIZE: usize = 24 << 10;

pub const EVM_CONTRACT_REVERTED: ExitCode = ExitCode::new(27);

#[derive(FromPrimitive)]
#[repr(u64)]
pub enum Method {
Expand Down Expand Up @@ -66,7 +68,10 @@ impl EvmContractActor {
})?;

// create a new execution context
let mut exec_state = ExecutionState::new(Bytes::copy_from_slice(&params.input_data));
let mut exec_state = ExecutionState::new(
Method::Constructor as u64,
Bytes::copy_from_slice(&params.input_data),
);

// identify bytecode valid jump destinations
let bytecode = Bytecode::new(&params.bytecode)
Expand All @@ -79,10 +84,15 @@ impl EvmContractActor {
_ => ActorError::unspecified(format!("EVM execution error: {e:?}")),
})?;

if !exec_status.reverted
&& exec_status.status_code == StatusCode::Success
&& !exec_status.output_data.is_empty()
{
// TODO this does not return revert data yet, but it has correct semantics.
if exec_status.reverted {
Err(ActorError::unchecked(EVM_CONTRACT_REVERTED, "constructor reverted".to_string()))
} else if exec_status.status_code == StatusCode::Success {
if exec_status.output_data.is_empty() {
return Err(ActorError::unspecified(
"EVM constructor returned empty contract".to_string(),
));
}
// constructor ran to completion successfully and returned
// the resulting bytecode.
let contract_bytecode = exec_status.output_data;
Expand All @@ -109,7 +119,8 @@ impl EvmContractActor {

pub fn invoke_contract<BS, RT>(
rt: &mut RT,
params: InvokeParams,
method: u64,
input_data: &RawBytes,
) -> Result<RawBytes, ActorError>
where
BS: Blockstore + Clone,
Expand Down Expand Up @@ -139,27 +150,28 @@ impl EvmContractActor {
ActorError::unspecified(format!("failed to create execution abstraction layer: {e:?}"))
})?;

let mut exec_state = ExecutionState::new(Bytes::copy_from_slice(&params.input_data));
let mut exec_state = ExecutionState::new(method, input_data.to_vec().into());

let exec_status =
execute(&bytecode, &mut exec_state, &mut system.reborrow()).map_err(|e| match e {
StatusCode::ActorError(e) => e,
_ => ActorError::unspecified(format!("EVM execution error: {e:?}")),
})?;

// TODO this is not the correct handling of reverts -- we need to abort (and return the
// output data), so that the entire transaction (including parent/sibling calls)
// can be reverted.
if exec_status.status_code == StatusCode::Success {
if !exec_status.reverted {
// this needs to be outside the transaction or else rustc has a fit about
// mutably borrowing the runtime twice.... sigh.
let contract_state = system.flush_state()?;
rt.transaction(|state: &mut State, _rt| {
state.contract_state = contract_state;
Ok(())
})?;
}
// TODO this does not return revert data yet, but it has correct semantics.
if exec_status.reverted {
return Err(ActorError::unchecked(
EVM_CONTRACT_REVERTED,
"contract reverted".to_string(),
));
} else if exec_status.status_code == StatusCode::Success {
// this needs to be outside the transaction or else rustc has a fit about
// mutably borrowing the runtime twice.... sigh.
let contract_state = system.flush_state()?;
rt.transaction(|state: &mut State, _rt| {
state.contract_state = contract_state;
Ok(())
})?;
} else if let StatusCode::ActorError(e) = exec_status.status_code {
return Err(e);
} else {
Expand Down Expand Up @@ -236,7 +248,7 @@ impl ActorCode for EvmContractActor {
Ok(RawBytes::default())
}
Some(Method::InvokeContract) => {
Self::invoke_contract(rt, cbor::deserialize_params(params)?)
Self::invoke_contract(rt, Method::InvokeContract as u64, params)
}
Some(Method::GetBytecode) => {
let cid = Self::bytecode(rt)?;
Expand All @@ -246,7 +258,7 @@ impl ActorCode for EvmContractActor {
let value = Self::storage_at(rt, cbor::deserialize_params(params)?)?;
Ok(RawBytes::serialize(value)?)
}
None => Err(actor_error!(unhandled_message; "Invalid method")),
None => Self::invoke_contract(rt, method, params),
}
}
}
Expand All @@ -257,11 +269,6 @@ pub struct ConstructorParams {
pub input_data: RawBytes,
}

#[derive(Serialize_tuple, Deserialize_tuple)]
pub struct InvokeParams {
pub input_data: RawBytes,
}

#[derive(Serialize_tuple, Deserialize_tuple)]
pub struct GetStorageAtParams {
pub storage_key: U256,
Expand Down
Loading

0 comments on commit f5be9a9

Please sign in to comment.