From 47169b3392f0747358d65670c982dba180346686 Mon Sep 17 00:00:00 2001 From: rakita Date: Tue, 24 Sep 2024 14:31:46 +0200 Subject: [PATCH] chore: refactor -copy common code (#1799) * chore: refactor *copy common code * doc link fix --- crates/interpreter/src/gas/calc.rs | 11 +++-- crates/interpreter/src/instructions/macros.rs | 7 ++- crates/interpreter/src/instructions/memory.rs | 2 +- crates/interpreter/src/instructions/system.rs | 48 ++++++++++--------- documentation/src/crates/revm/handler.md | 2 +- 5 files changed, 41 insertions(+), 29 deletions(-) diff --git a/crates/interpreter/src/gas/calc.rs b/crates/interpreter/src/gas/calc.rs index 5163013ea1..74376ee4f2 100644 --- a/crates/interpreter/src/gas/calc.rs +++ b/crates/interpreter/src/gas/calc.rs @@ -115,8 +115,8 @@ pub fn exp_cost(spec_id: SpecId, power: U256) -> Option { /// `*COPY` opcodes cost calculation. #[inline] -pub const fn verylowcopy_cost(len: u64) -> Option { - VERYLOW.checked_add(tri!(cost_per_word(len, COPY))) +pub const fn copy_cost_verylow(len: u64) -> Option { + copy_cost(VERYLOW, len) } /// `EXTCODECOPY` opcode cost calculation. @@ -129,7 +129,12 @@ pub const fn extcodecopy_cost(spec_id: SpecId, len: u64, load: Eip7702CodeLoad<( } else { 20 }; - base_gas.checked_add(tri!(cost_per_word(len, COPY))) + copy_cost(base_gas, len) +} + +#[inline] +pub const fn copy_cost(base_cost: u64, len: u64) -> Option { + base_cost.checked_add(tri!(cost_per_word(len, COPY))) } /// `LOG` opcode cost calculation. diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index 22c38e123d..75a42a2f6a 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -73,11 +73,14 @@ macro_rules! refund { #[macro_export] macro_rules! gas_or_fail { ($interp:expr, $gas:expr) => { + $crate::gas_or_fail!($interp, $gas, ()) + }; + ($interp:expr, $gas:expr, $ret:expr) => { match $gas { - Some(gas_used) => $crate::gas!($interp, gas_used), + Some(gas_used) => $crate::gas!($interp, gas_used, $ret), None => { $interp.instruction_result = $crate::InstructionResult::OutOfGas; - return; + return $ret; } } }; diff --git a/crates/interpreter/src/instructions/memory.rs b/crates/interpreter/src/instructions/memory.rs index 1e02b75b33..0ba63903d5 100644 --- a/crates/interpreter/src/instructions/memory.rs +++ b/crates/interpreter/src/instructions/memory.rs @@ -40,7 +40,7 @@ pub fn mcopy(interpreter: &mut Interpreter, _host: // into usize or fail let len = as_usize_or_fail!(interpreter, len); // deduce gas - gas_or_fail!(interpreter, gas::verylowcopy_cost(len as u64)); + gas_or_fail!(interpreter, gas::copy_cost_verylow(len as u64)); if len == 0 { return; } diff --git a/crates/interpreter/src/instructions/system.rs b/crates/interpreter/src/instructions/system.rs index a4729af04c..a07a4f083d 100644 --- a/crates/interpreter/src/instructions/system.rs +++ b/crates/interpreter/src/instructions/system.rs @@ -37,13 +37,10 @@ pub fn codesize(interpreter: &mut Interpreter, _host: &mut H) pub fn codecopy(interpreter: &mut Interpreter, _host: &mut H) { pop!(interpreter, memory_offset, code_offset, len); let len = as_usize_or_fail!(interpreter, len); - gas_or_fail!(interpreter, gas::verylowcopy_cost(len as u64)); - if len == 0 { + let Some(memory_offset) = memory_resize(interpreter, memory_offset, len) else { return; - } - let memory_offset = as_usize_or_fail!(interpreter, memory_offset); + }; let code_offset = as_usize_saturated!(code_offset); - resize_memory!(interpreter, memory_offset, len); // Inform the optimizer that the bytecode cannot be EOF to remove a bounds check. assume!(!interpreter.contract.bytecode.is_eof()); @@ -92,14 +89,11 @@ pub fn callvalue(interpreter: &mut Interpreter, _host: &mut H) pub fn calldatacopy(interpreter: &mut Interpreter, _host: &mut H) { pop!(interpreter, memory_offset, data_offset, len); let len = as_usize_or_fail!(interpreter, len); - gas_or_fail!(interpreter, gas::verylowcopy_cost(len as u64)); - if len == 0 { + let Some(memory_offset) = memory_resize(interpreter, memory_offset, len) else { return; - } - let memory_offset = as_usize_or_fail!(interpreter, memory_offset); - let data_offset = as_usize_saturated!(data_offset); - resize_memory!(interpreter, memory_offset, len); + }; + let data_offset = as_usize_saturated!(data_offset); // Note: this can't panic because we resized memory to fit. interpreter.shared_memory.set_data( memory_offset, @@ -125,33 +119,26 @@ pub fn returndatacopy(interpreter: &mut Interprete pop!(interpreter, memory_offset, offset, len); let len = as_usize_or_fail!(interpreter, len); - gas_or_fail!(interpreter, gas::verylowcopy_cost(len as u64)); - let data_offset = as_usize_saturated!(offset); - let data_end = data_offset.saturating_add(len); // Old legacy behavior is to panic if data_end is out of scope of return buffer. // This behavior is changed in EOF. + let data_end = data_offset.saturating_add(len); if data_end > interpreter.return_data_buffer.len() && !interpreter.is_eof { interpreter.instruction_result = InstructionResult::OutOfOffset; return; } - // if len is zero memory is not resized. - if len == 0 { + let Some(memory_offset) = memory_resize(interpreter, memory_offset, len) else { return; - } - - // resize memory - let memory_offset = as_usize_or_fail!(interpreter, memory_offset); - resize_memory!(interpreter, memory_offset, len); + }; // Note: this can't panic because we resized memory to fit. interpreter.shared_memory.set_data( memory_offset, data_offset, len, - &interpreter.return_data_buffer, + interpreter.return_data_buffer.as_ref(), ); } @@ -182,6 +169,23 @@ pub fn gas(interpreter: &mut Interpreter, _host: &mut H) { push!(interpreter, U256::from(interpreter.gas.remaining())); } +// common logic for copying data from a source buffer to the EVM's memory +pub fn memory_resize( + interpreter: &mut Interpreter, + memory_offset: U256, + len: usize, +) -> Option { + // safe to cast usize to u64 + gas_or_fail!(interpreter, gas::copy_cost_verylow(len as u64), None); + if len == 0 { + return None; + } + let memory_offset = as_usize_or_fail_ret!(interpreter, memory_offset, None); + resize_memory!(interpreter, memory_offset, len, None); + + Some(memory_offset) +} + #[cfg(test)] mod test { use super::*; diff --git a/documentation/src/crates/revm/handler.md b/documentation/src/crates/revm/handler.md index 5cabe97df2..3ab714696b 100644 --- a/documentation/src/crates/revm/handler.md +++ b/documentation/src/crates/revm/handler.md @@ -8,7 +8,7 @@ Functions can be grouped in five categories and are marked in that way in the co * Pre-execution functions: [`PreExecutionHandler`](https://github.com/bluealloy/revm/blob/main/crates/revm/src/handler/handle_types/pre_execution.rs) * Execution functions: [`ExecutionHandler`](https://github.com/bluealloy/revm/blob/main/crates/revm/src/handler/handle_types/execution.rs) * Post-execution functions: [`PostExecutionHandler`](https://github.com/bluealloy/revm/blob/main/crates/revm/src/handler/handle_types/post_execution.rs) -* Instruction table: [`InstructionTable`](https://github.com/bluealloy/revm/blob/main/crates/interpreter/src/opcode.rs) +* Instruction table: [`InstructionTable`](https://github.com/bluealloy/revm/blob/main/crates/interpreter/src/table.rs) ### Handle Registers