From 76e22baff8b54b5969e1424b99a23a906e9ab3b5 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sun, 21 Apr 2024 09:32:06 +0200 Subject: [PATCH] feat: add helper methods to CallInputs (#1345) --- .../interpreter/src/instructions/contract.rs | 18 +-- crates/interpreter/src/interpreter_action.rs | 2 +- .../src/interpreter_action/call_inputs.rs | 147 ++++++++++++++---- crates/interpreter/src/lib.rs | 4 +- crates/revm/src/context/evm_context.rs | 10 +- 5 files changed, 137 insertions(+), 44 deletions(-) diff --git a/crates/interpreter/src/instructions/contract.rs b/crates/interpreter/src/instructions/contract.rs index f5e040a27d..acb896f4a0 100644 --- a/crates/interpreter/src/instructions/contract.rs +++ b/crates/interpreter/src/instructions/contract.rs @@ -11,8 +11,8 @@ use crate::{ instructions::utility::read_u16, interpreter::Interpreter, primitives::{Address, Bytes, Eof, Spec, SpecId::*, B256, U256}, - CallInputs, CallScheme, CreateInputs, CreateScheme, EOFCreateInput, Host, InstructionResult, - InterpreterAction, InterpreterResult, LoadAccountResult, TransferValue, MAX_INITCODE_SIZE, + CallInputs, CallScheme, CallValue, CreateInputs, CreateScheme, EOFCreateInput, Host, + InstructionResult, InterpreterAction, InterpreterResult, LoadAccountResult, MAX_INITCODE_SIZE, }; use core::{cmp::max, ops::Range}; use std::boxed::Box; @@ -304,7 +304,7 @@ pub fn extcall(interpreter: &mut Interpreter, host target_address, caller: interpreter.contract.target_address, bytecode_address: target_address, - value: TransferValue::Value(value), + value: CallValue::Transfer(value), scheme: CallScheme::Call, is_static: interpreter.is_static, is_eof: true, @@ -336,7 +336,7 @@ pub fn extdcall(interpreter: &mut Interpreter, hos target_address, caller: interpreter.contract.target_address, bytecode_address: target_address, - value: TransferValue::ApparentValue(interpreter.contract.call_value), + value: CallValue::Apparent(interpreter.contract.call_value), // TODO(EOF) should be EofDelegateCall? scheme: CallScheme::DelegateCall, is_static: interpreter.is_static, @@ -368,7 +368,7 @@ pub fn extscall(interpreter: &mut Interpreter, host: &mut H) { target_address, caller: interpreter.contract.target_address, bytecode_address: target_address, - value: TransferValue::Value(U256::ZERO), + value: CallValue::Transfer(U256::ZERO), scheme: CallScheme::Call, is_static: interpreter.is_static, is_eof: true, @@ -494,7 +494,7 @@ pub fn call(interpreter: &mut Interpreter, host: & target_address: to, caller: interpreter.contract.target_address, bytecode_address: to, - value: TransferValue::Value(value), + value: CallValue::Transfer(value), scheme: CallScheme::Call, is_static: interpreter.is_static, is_eof: false, @@ -545,7 +545,7 @@ pub fn call_code(interpreter: &mut Interpreter, ho target_address: interpreter.contract.target_address, caller: interpreter.contract.target_address, bytecode_address: to, - value: TransferValue::Value(value), + value: CallValue::Transfer(value), scheme: CallScheme::CallCode, is_static: interpreter.is_static, is_eof: false, @@ -586,7 +586,7 @@ pub fn delegate_call(interpreter: &mut Interpreter target_address: interpreter.contract.target_address, caller: interpreter.contract.caller, bytecode_address: to, - value: TransferValue::ApparentValue(interpreter.contract.call_value), + value: CallValue::Apparent(interpreter.contract.call_value), scheme: CallScheme::DelegateCall, is_static: interpreter.is_static, is_eof: false, @@ -627,7 +627,7 @@ pub fn static_call(interpreter: &mut Interpreter, target_address: to, caller: interpreter.contract.target_address, bytecode_address: to, - value: TransferValue::Value(U256::ZERO), + value: CallValue::Transfer(U256::ZERO), scheme: CallScheme::StaticCall, is_static: true, is_eof: false, diff --git a/crates/interpreter/src/interpreter_action.rs b/crates/interpreter/src/interpreter_action.rs index 25dc1448dd..0581e220e4 100644 --- a/crates/interpreter/src/interpreter_action.rs +++ b/crates/interpreter/src/interpreter_action.rs @@ -5,7 +5,7 @@ mod create_outcome; mod eof_create_inputs; mod eof_create_outcome; -pub use call_inputs::{CallInputs, CallScheme, TransferValue}; +pub use call_inputs::{CallInputs, CallScheme, CallValue}; pub use call_outcome::CallOutcome; pub use create_inputs::{CreateInputs, CreateScheme}; pub use create_outcome::CreateOutcome; diff --git a/crates/interpreter/src/interpreter_action/call_inputs.rs b/crates/interpreter/src/interpreter_action/call_inputs.rs index 5f9cb32abf..196f7b1a6d 100644 --- a/crates/interpreter/src/interpreter_action/call_inputs.rs +++ b/crates/interpreter/src/interpreter_action/call_inputs.rs @@ -9,45 +9,54 @@ pub struct CallInputs { /// The call data of the call. pub input: Bytes, /// The return memory offset where the output of the call is written. - /// For EOF this range is invalid as EOF does write output to memory. + /// + /// In EOF, this range is invalid as EOF calls do not write output to memory. pub return_memory_offset: Range, /// The gas limit of the call. pub gas_limit: u64, - /// The account address of bytecode that is going to be executed. + /// The account address of bytecode that is going to be executed. + /// + /// Previously `context.code_address`. pub bytecode_address: Address, /// Target address, this account storage is going to be modified. + /// + /// Previously `context.address`. pub target_address: Address, /// This caller is invoking the call. + /// + /// Previously `context.caller`. pub caller: Address, - /// Value that is transferred in Ether. + /// Call value. + /// + /// NOTE: This value may not necessarily be transferred from caller to callee, see [`CallValue`]. /// - /// If enum is [`TransferValue::Value`] balance is transferer from `caller` to the `target_address`. + /// Previously `transfer.value` or `context.apparent_value`. + pub value: CallValue, + /// The call scheme. /// - /// If enum is [`TransferValue::ApparentValue`] balance transfer is **not** - /// done and apparent value is used by CALLVALUE opcode. Used by delegate call. - pub value: TransferValue, - /// The scheme used for the call. Call, callcode, delegatecall or staticcall. + /// Previously `context.scheme`. pub scheme: CallScheme, - /// Whether this is a static call. + /// Whether the call is initiated inside a static call. pub is_static: bool, - /// Call is initiated from EOF bytecode. + /// Whether the call is initiated from EOF bytecode. pub is_eof: bool, } impl CallInputs { /// Creates new call inputs. + /// + /// Returns `None` if the transaction is not a call. pub fn new(tx_env: &TxEnv, gas_limit: u64) -> Option { let TransactTo::Call(target_address) = tx_env.transact_to else { return None; }; - Some(CallInputs { input: tx_env.data.clone(), gas_limit, target_address, bytecode_address: target_address, caller: tx_env.caller, - value: TransferValue::Value(tx_env.value), + value: CallValue::Transfer(tx_env.value), scheme: CallScheme::Call, is_static: false, is_eof: false, @@ -55,19 +64,61 @@ impl CallInputs { }) } - /// Returns boxed call inputs. + /// Creates new boxed call inputs. + /// + /// Returns `None` if the transaction is not a call. pub fn new_boxed(tx_env: &TxEnv, gas_limit: u64) -> Option> { Self::new(tx_env, gas_limit).map(Box::new) } - /// Return call value - pub fn call_value(&self) -> U256 { - let (TransferValue::Value(value) | TransferValue::ApparentValue(value)) = self.value; - value + /// Returns `true` if the call will transfer a non-zero value. + #[inline] + pub fn transfers_value(&self) -> bool { + self.value.transfer().is_some_and(|x| x > U256::ZERO) + } + + /// Returns the transfer value. + /// + /// This is the value that is transferred from caller to callee, see [`CallValue`]. + #[inline] + pub const fn transfer_value(&self) -> Option { + self.value.transfer() + } + + /// Returns the **apparent** call value. + /// + /// This value is not actually transferred, see [`CallValue`]. + #[inline] + pub const fn apparent_value(&self) -> Option { + self.value.apparent() + } + + /// Returns the address of the transfer source account. + /// + /// This is only meaningful if `transfers_value` is `true`. + #[inline] + pub const fn transfer_from(&self) -> Address { + self.caller + } + + /// Returns the address of the transfer target account. + /// + /// This is only meaningful if `transfers_value` is `true`. + #[inline] + pub const fn transfer_to(&self) -> Address { + self.target_address + } + + /// Returns the call value, regardless of the transfer value type. + /// + /// NOTE: this value may not necessarily be transferred from caller to callee, see [`CallValue`]. + #[inline] + pub const fn call_value(&self) -> U256 { + self.value.get() } } -/// Call schemes. +/// Call scheme. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum CallScheme { @@ -81,19 +132,61 @@ pub enum CallScheme { StaticCall, } -/// Transfered value from caller to callee. +/// Call value. #[derive(Clone, Debug, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub enum TransferValue { - /// Transfer value from caller to callee. - Value(U256), - /// For delegate call, the value is not transferred but - /// apparent value is used for CALLVALUE opcode - ApparentValue(U256), +pub enum CallValue { + /// Concrete value, transferred from caller to callee at the end of the transaction. + Transfer(U256), + /// Apparent value, that is **not** actually transferred. + /// + /// Set when in a `DELEGATECALL` call type, and used by the `CALLVALUE` opcode. + Apparent(U256), } -impl Default for TransferValue { +impl Default for CallValue { + #[inline] fn default() -> Self { - TransferValue::Value(U256::ZERO) + CallValue::Transfer(U256::ZERO) + } +} + +impl CallValue { + /// Returns the call value, regardless of the type. + #[inline] + pub const fn get(&self) -> U256 { + match *self { + Self::Transfer(value) | Self::Apparent(value) => value, + } + } + + /// Returns the transferred value, if any. + #[inline] + pub const fn transfer(&self) -> Option { + match *self { + Self::Transfer(transfer) => Some(transfer), + Self::Apparent(_) => None, + } + } + + /// Returns whether the call value will be transferred. + #[inline] + pub const fn is_transfer(&self) -> bool { + matches!(self, Self::Transfer(_)) + } + + /// Returns the apparent value, if any. + #[inline] + pub const fn apparent(&self) -> Option { + match *self { + Self::Transfer(_) => None, + Self::Apparent(apparent) => Some(apparent), + } + } + + /// Returns whether the call value is apparent, and not actually transferred. + #[inline] + pub const fn is_apparent(&self) -> bool { + matches!(self, Self::Apparent(_)) } } diff --git a/crates/interpreter/src/lib.rs b/crates/interpreter/src/lib.rs index 7d749fb9b1..505074098a 100644 --- a/crates/interpreter/src/lib.rs +++ b/crates/interpreter/src/lib.rs @@ -38,8 +38,8 @@ pub use interpreter::{ EMPTY_SHARED_MEMORY, STACK_LIMIT, }; pub use interpreter_action::{ - CallInputs, CallOutcome, CallScheme, CreateInputs, CreateOutcome, CreateScheme, EOFCreateInput, - EOFCreateOutcome, InterpreterAction, TransferValue, + CallInputs, CallOutcome, CallScheme, CallValue, CreateInputs, CreateOutcome, CreateScheme, + EOFCreateInput, EOFCreateOutcome, InterpreterAction, }; pub use opcode::{Instruction, OpCode, OPCODE_INFO_JUMPTABLE}; pub use primitives::{MAX_CODE_SIZE, MAX_INITCODE_SIZE}; diff --git a/crates/revm/src/context/evm_context.rs b/crates/revm/src/context/evm_context.rs index a313226bcf..b92d711da8 100644 --- a/crates/revm/src/context/evm_context.rs +++ b/crates/revm/src/context/evm_context.rs @@ -1,4 +1,4 @@ -use revm_interpreter::TransferValue; +use revm_interpreter::CallValue; use super::inner_evm_context::InnerEvmContext; use crate::{ @@ -175,11 +175,11 @@ impl EvmContext { // Touch address. For "EIP-158 State Clear", this will erase empty accounts. match inputs.value { // if transfer value is zero, do the touch. - TransferValue::Value(value) if value == U256::ZERO => { + CallValue::Transfer(value) if value == U256::ZERO => { self.load_account(inputs.target_address)?; self.journaled_state.touch(&inputs.target_address); } - TransferValue::Value(value) => { + CallValue::Transfer(value) => { // Transfer value from caller to called account if let Some(result) = self.inner.journaled_state.transfer( &inputs.caller, @@ -241,7 +241,7 @@ pub(crate) mod test_utils { bytecode_address: to, target_address: to, caller: MOCK_CALLER, - value: TransferValue::Value(U256::ZERO), + value: CallValue::Transfer(U256::ZERO), scheme: revm_interpreter::CallScheme::Call, is_eof: false, is_static: false, @@ -344,7 +344,7 @@ mod tests { let mut evm_context = test_utils::create_empty_evm_context(Box::new(env), db); let contract = address!("dead10000000000000000000000000000001dead"); let mut call_inputs = test_utils::create_mock_call_inputs(contract); - call_inputs.value = TransferValue::Value(U256::from(1)); + call_inputs.value = CallValue::Transfer(U256::from(1)); let res = evm_context.make_call_frame(&call_inputs); let Ok(FrameOrResult::Result(result)) = res else { panic!("Expected FrameOrResult::Result");