From 2a76f9e32bec0f414b8de0699b703bb2e1c0ada7 Mon Sep 17 00:00:00 2001 From: Zhang Zhuo Date: Tue, 28 Feb 2023 00:32:38 +0800 Subject: [PATCH] refactor duplicated memory copy codes (#1248) ### Description the memory copy codes in calldatacopy/returndatacopy/(ext)codecopy are almost identical. So use a unified function to reduce repeated codes. ### Issue Link [_link issue here_] ### Type of change - [x] Refactor ### Contents - [_item_] ### Rationale [_design decisions and extended information_] ### How Has This Been Tested? [_explanation_]
## How to fill a PR description Please give a concise description of your PR. The target readers could be future developers, reviewers, and auditors. By reading your description, they should easily understand the changes proposed in this pull request. MUST: Reference the issue to resolve ### Single responsability Is RECOMMENDED to create single responsibility commits, but not mandatory. Anyway, you MUST enumerate the changes in a unitary way, e.g. ``` This PR contains: - Cleanup of xxxx, yyyy - Changed xxxx to yyyy in order to bla bla - Added xxxx function to ... - Refactored .... ``` ### Design choices RECOMMENDED to: - What types of design choices did you face? - What decisions you have made? - Any valuable information that could help reviewers to think critically --- bus-mapping/src/evm/opcodes/calldatacopy.rs | 20 ++-------------- bus-mapping/src/evm/opcodes/codecopy.rs | 17 +------------- bus-mapping/src/evm/opcodes/extcodecopy.rs | 20 +++------------- bus-mapping/src/evm/opcodes/returndatacopy.rs | 16 +------------ eth-types/src/evm_types/memory.rs | 23 +++++++++++++++++++ 5 files changed, 30 insertions(+), 66 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 0a2242a8bc..69f8ac6c18 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -22,24 +22,8 @@ impl Opcode for Calldatacopy { let length = geth_step.stack.nth_last(2)?.as_usize(); let call_ctx = state.call_ctx_mut()?; let memory = &mut call_ctx.memory; - if length != 0 { - let minimal_length = memory_offset as usize + length; - memory.extend_at_least(minimal_length); - - let mem_starts = memory_offset as usize; - let mem_ends = mem_starts + length as usize; - let data_starts = data_offset as usize; - let data_ends = data_starts + length as usize; - let call_data = &call_ctx.call_data; - if data_ends <= call_data.len() { - memory.0[mem_starts..mem_ends].copy_from_slice(&call_data[data_starts..data_ends]); - } else if let Some(actual_length) = call_data.len().checked_sub(data_starts) { - let mem_code_ends = mem_starts + actual_length; - memory.0[mem_starts..mem_code_ends].copy_from_slice(&call_data[data_starts..]); - // since we already resize the memory, no need to copy 0s for - // out of bound bytes - } - } + + memory.copy_from(memory_offset, &call_ctx.call_data, data_offset, length); let copy_event = gen_copy_event(state, geth_step)?; state.push_copy(copy_event); diff --git a/bus-mapping/src/evm/opcodes/codecopy.rs b/bus-mapping/src/evm/opcodes/codecopy.rs index b6aa63436f..e1cdf2eaad 100644 --- a/bus-mapping/src/evm/opcodes/codecopy.rs +++ b/bus-mapping/src/evm/opcodes/codecopy.rs @@ -30,23 +30,8 @@ impl Opcode for Codecopy { let call_ctx = state.call_ctx_mut()?; let memory = &mut call_ctx.memory; - if length != 0 { - let minimal_length = (dest_offset + length) as usize; - memory.extend_at_least(minimal_length); - let mem_starts = dest_offset as usize; - let mem_ends = mem_starts + length as usize; - let code_starts = code_offset as usize; - let code_ends = code_starts + length as usize; - if code_ends <= code.len() { - memory[mem_starts..mem_ends].copy_from_slice(&code[code_starts..code_ends]); - } else if let Some(actual_length) = code.len().checked_sub(code_starts) { - let mem_code_ends = mem_starts + actual_length; - memory[mem_starts..mem_code_ends].copy_from_slice(&code[code_starts..]); - // since we already resize the memory, no need to copy 0s for - // out of bound bytes - } - } + memory.copy_from(dest_offset, &code, code_offset, length as usize); let copy_event = gen_copy_event(state, geth_step)?; state.push_copy(copy_event); diff --git a/bus-mapping/src/evm/opcodes/extcodecopy.rs b/bus-mapping/src/evm/opcodes/extcodecopy.rs index 6f92c32ed7..217cbade61 100644 --- a/bus-mapping/src/evm/opcodes/extcodecopy.rs +++ b/bus-mapping/src/evm/opcodes/extcodecopy.rs @@ -32,23 +32,8 @@ impl Opcode for Extcodecopy { let call_ctx = state.call_ctx_mut()?; let memory = &mut call_ctx.memory; - if length != 0 { - let minimal_length = (dest_offset + length) as usize; - memory.extend_at_least(minimal_length); - - let mem_starts = dest_offset as usize; - let mem_ends = mem_starts + length as usize; - let code_starts = code_offset as usize; - let code_ends = code_starts + length as usize; - if code_ends <= code.len() { - memory[mem_starts..mem_ends].copy_from_slice(&code[code_starts..code_ends]); - } else if let Some(actual_length) = code.len().checked_sub(code_starts) { - let mem_code_ends = mem_starts + actual_length; - memory[mem_starts..mem_code_ends].copy_from_slice(&code[code_starts..]); - // since we already resize the memory, no need to copy 0s for - // out of bound bytes - } - } + + memory.copy_from(dest_offset, &code, code_offset, length as usize); let copy_event = gen_copy_event(state, geth_step)?; state.push_copy(copy_event); @@ -115,6 +100,7 @@ fn gen_extcodecopy_step( } else { H256::zero() }; + state.account_read( &mut exec_step, external_address, diff --git a/bus-mapping/src/evm/opcodes/returndatacopy.rs b/bus-mapping/src/evm/opcodes/returndatacopy.rs index 7333c44d85..d1456cb8d2 100644 --- a/bus-mapping/src/evm/opcodes/returndatacopy.rs +++ b/bus-mapping/src/evm/opcodes/returndatacopy.rs @@ -29,21 +29,7 @@ impl Opcode for Returndatacopy { let call_ctx = state.call_ctx_mut()?; let memory = &mut call_ctx.memory; let length = size.as_usize(); - if length != 0 { - let mem_starts = dest_offset.as_usize(); - let mem_ends = mem_starts + length; - let data_starts = offset.as_usize(); - let data_ends = data_starts + length; - let minimal_length = dest_offset.as_usize() + length; - if data_ends <= return_data.len() { - memory.extend_at_least(minimal_length); - memory[mem_starts..mem_ends].copy_from_slice(&return_data[data_starts..data_ends]); - } else { - assert_eq!(geth_steps.len(), 1); - // if overflows this opcode would fails current context, so - // there is no more steps. - } - } + memory.copy_from(dest_offset.as_u64(), &return_data, offset.as_u64(), length); let copy_event = gen_copy_event(state, geth_step)?; state.push_copy(copy_event); diff --git a/eth-types/src/evm_types/memory.rs b/eth-types/src/evm_types/memory.rs index 7935e4185e..a5efc09acf 100644 --- a/eth-types/src/evm_types/memory.rs +++ b/eth-types/src/evm_types/memory.rs @@ -326,6 +326,29 @@ impl Memory { self.0.resize(memory_size, 0); } } + + /// Copy source data to memory. Used in (ext)codecopy/calldatacopy. + pub fn copy_from(&mut self, dst_offset: u64, data: &[u8], data_offset: u64, length: usize) { + // https://github.com/ethereum/go-ethereum/blob/df52967ff6080a27243569020ff64cd956fb8362/core/vm/instructions.go#L312 + if length != 0 { + let minimal_length = dst_offset as usize + length; + self.extend_at_least(minimal_length); + + let mem_starts = dst_offset as usize; + let mem_ends = mem_starts + length as usize; + let dst_slice = &mut self.0[mem_starts..mem_ends]; + dst_slice.fill(0); + let data_starts = data_offset as usize; + let actual_length = std::cmp::min( + length, + data.len().checked_sub(data_starts).unwrap_or_default(), + ); + if actual_length != 0 { + let src_slice = &data[data_starts..data_starts + actual_length]; + dst_slice[..actual_length].copy_from_slice(src_slice); + } + } + } } #[cfg(test)]