From 3dad264d5aa75f79c568b17058a5c25cd81c73dd Mon Sep 17 00:00:00 2001 From: Mikers Date: Wed, 14 Aug 2024 12:47:32 -0700 Subject: [PATCH 01/12] mcopy initial commit wip --- actors/evm/src/interpreter/execution.rs | 1 + .../src/interpreter/instructions/memory.rs | 44 +++++++++++++++++++ .../evm/src/interpreter/instructions/mod.rs | 1 + 3 files changed, 46 insertions(+) diff --git a/actors/evm/src/interpreter/execution.rs b/actors/evm/src/interpreter/execution.rs index 5fcc105fc..22f013b8d 100644 --- a/actors/evm/src/interpreter/execution.rs +++ b/actors/evm/src/interpreter/execution.rs @@ -163,6 +163,7 @@ pub mod opcodes { 0x59: MSIZE, 0x5a: GAS, 0x5b: JUMPDEST, + 0x5e: MCOPY, 0x5F: PUSH0, 0x60: PUSH1, 0x61: PUSH2, diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index 8e410fe27..641fcd1f9 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -53,6 +53,25 @@ pub fn get_memory_region( })) } +#[inline] +pub fn mcopy( + state: &mut ExecutionState, + _: &System, + mem_index: U256, + input_index: U256, + size: U256, +) -> Result<(), ActorError> { + //TODO be careful because we'll be copying between two potentially overlapping slices in the same memory. + // MCOPY spec: Copying takes place as if an intermediate buffer was used, + // allowing the destination and source to overlap. + + let region = get_memory_region(&mut state.memory, input_index, size)?.expect("empty region"); + let memory_slice = state.memory[region.offset..region.offset + region.size.get()].to_vec(); + copy_to_memory(&mut state.memory, mem_index, size, U256::zero(), &memory_slice, true) + +} + + pub fn copy_to_memory( memory: &mut Memory, dest_offset: U256, @@ -200,6 +219,31 @@ mod tests { assert_eq!(&mem[0..4], result_data); } + + #[test] + fn test_mcopy() { + // happy path + evm_unit_test! { + (m) { + MCOPY; + } + m.state.memory.grow(32); + m.state.memory[..3].copy_from_slice(&[0x00, 0x01, 0x02]); + + m.state.stack.push(U256::from(2)).unwrap(); // length + m.state.stack.push(U256::from(1)).unwrap(); // offset + m.state.stack.push(U256::from(0)).unwrap(); // dest-offset + let result = m.step(); + assert!(result.is_ok(), "execution step failed"); + assert_eq!(m.state.stack.len(), 0); + let mut expected = [0u8; 32]; + expected[0] = 0x01; + expected[1] = 0x02; + expected[2] = 0x02; + assert_eq!(&*m.state.memory, &expected); + }; + } + #[test] fn test_mload_nothing() { evm_unit_test! { diff --git a/actors/evm/src/interpreter/instructions/mod.rs b/actors/evm/src/interpreter/instructions/mod.rs index 5c95e2fa5..b97b407e1 100644 --- a/actors/evm/src/interpreter/instructions/mod.rs +++ b/actors/evm/src/interpreter/instructions/mod.rs @@ -352,6 +352,7 @@ def_stdfun_code! { CODESIZE() => call::codesize } def_stdproc_code! { CODECOPY(a, b, c) => call::codecopy } def_stdfun! { CREATE(a, b, c) => lifecycle::create } def_stdfun! { CREATE2(a, b, c, d) => lifecycle::create2 } +def_stdproc! { MCOPY(a,b,c) => memory::mcopy } def_stdproc! { JUMPDEST() => control::nop } def_stdproc! { INVALID() => control::invalid } def_exit! { RETURN(a, b) => control::ret } From 69e27532606bb32c8c65fa55ecc02d2317866fe8 Mon Sep 17 00:00:00 2001 From: Mikers Date: Wed, 14 Aug 2024 12:52:35 -0700 Subject: [PATCH 02/12] improve readability of test --- .../src/interpreter/instructions/memory.rs | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index 641fcd1f9..53099bbb0 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -62,16 +62,14 @@ pub fn mcopy( size: U256, ) -> Result<(), ActorError> { //TODO be careful because we'll be copying between two potentially overlapping slices in the same memory. - // MCOPY spec: Copying takes place as if an intermediate buffer was used, + // MCOPY spec: Copying takes place as if an intermediate buffer was used, // allowing the destination and source to overlap. let region = get_memory_region(&mut state.memory, input_index, size)?.expect("empty region"); let memory_slice = state.memory[region.offset..region.offset + region.size.get()].to_vec(); - copy_to_memory(&mut state.memory, mem_index, size, U256::zero(), &memory_slice, true) - + copy_to_memory(&mut state.memory, mem_index, size, U256::zero(), &memory_slice, true) } - pub fn copy_to_memory( memory: &mut Memory, dest_offset: U256, @@ -219,27 +217,33 @@ mod tests { assert_eq!(&mem[0..4], result_data); } - #[test] fn test_mcopy() { - // happy path + const LENGTH: usize = 2; + const OFFSET: usize = 1; + const DEST_OFFSET: usize = 0; + evm_unit_test! { (m) { MCOPY; } + + // Grow memory and set initial values m.state.memory.grow(32); m.state.memory[..3].copy_from_slice(&[0x00, 0x01, 0x02]); - m.state.stack.push(U256::from(2)).unwrap(); // length - m.state.stack.push(U256::from(1)).unwrap(); // offset - m.state.stack.push(U256::from(0)).unwrap(); // dest-offset - let result = m.step(); - assert!(result.is_ok(), "execution step failed"); + // Set up stack + m.state.stack.push(U256::from(LENGTH)).unwrap(); + m.state.stack.push(U256::from(OFFSET)).unwrap(); + m.state.stack.push(U256::from(DEST_OFFSET)).unwrap(); + + // Execute and assert + assert!(m.step().is_ok(), "execution step failed"); assert_eq!(m.state.stack.len(), 0); + + // Setup expected memory and assert let mut expected = [0u8; 32]; - expected[0] = 0x01; - expected[1] = 0x02; - expected[2] = 0x02; + expected[..3].copy_from_slice(&[0x01, 0x02, 0x02]); assert_eq!(&*m.state.memory, &expected); }; } From dfa21227831bb88a732b2b98e67f9017437c5655 Mon Sep 17 00:00:00 2001 From: Mikers Date: Wed, 14 Aug 2024 12:59:44 -0700 Subject: [PATCH 03/12] add tests from SPEC --- .../src/interpreter/instructions/memory.rs | 121 +++++++++++++++++- 1 file changed, 120 insertions(+), 1 deletion(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index 53099bbb0..a29b718f6 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -61,7 +61,7 @@ pub fn mcopy( input_index: U256, size: U256, ) -> Result<(), ActorError> { - //TODO be careful because we'll be copying between two potentially overlapping slices in the same memory. + // We are copying between two potentially overlapping slices in the same memory. // MCOPY spec: Copying takes place as if an intermediate buffer was used, // allowing the destination and source to overlap. @@ -248,6 +248,125 @@ mod tests { }; } + #[test] + fn test_mcopy_0_32_32() { + evm_unit_test! { + (m) { + MCOPY; + } + + // Grow memory and set initial values + m.state.memory.grow(64); + m.state.memory[32..64].copy_from_slice(&[ + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f + ]); + + // Set up stack + m.state.stack.push(U256::from(32)).unwrap(); // length + m.state.stack.push(U256::from(32)).unwrap(); // source offset + m.state.stack.push(U256::from(0)).unwrap(); // destination offset + + // Execute and assert + assert!(m.step().is_ok(), "execution step failed"); + assert_eq!(m.state.stack.len(), 0); + + // Setup expected memory and assert + let mut expected = [0u8; 64]; + expected[0..64].copy_from_slice(&[ + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f + ]); + assert_eq!(&*m.state.memory, &expected); + }; + } + + #[test] + fn test_mcopy_0_0_32() { + evm_unit_test! { + (m) { + MCOPY; + } + + // Grow memory and set initial values + m.state.memory.grow(32); + m.state.memory[..32].copy_from_slice(&[0x01; 32]); + + // Set up stack + m.state.stack.push(U256::from(32)).unwrap(); // length + m.state.stack.push(U256::from(0)).unwrap(); // source offset + m.state.stack.push(U256::from(0)).unwrap(); // destination offset + + // Execute and assert + assert!(m.step().is_ok(), "execution step failed"); + assert_eq!(m.state.stack.len(), 0); + + // Setup expected memory and assert + let expected = [0x01; 32]; + assert_eq!(&m.state.memory[..32], &expected); + }; + } + + #[test] + fn test_mcopy_0_1_8() { + evm_unit_test! { + (m) { + MCOPY; + } + + // Grow memory and set initial values + m.state.memory.grow(32); + m.state.memory[..8].copy_from_slice(&[0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07]); + m.state.memory[8] = 0x08; + + // Set up stack + m.state.stack.push(U256::from(8)).unwrap(); // length + m.state.stack.push(U256::from(1)).unwrap(); // source offset + m.state.stack.push(U256::from(0)).unwrap(); // destination offset + + // Execute and assert + assert!(m.step().is_ok(), "execution step failed"); + assert_eq!(m.state.stack.len(), 0); + + // Setup expected memory and assert + let mut expected = [0u8; 32]; + expected[..8].copy_from_slice(&[0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08]); + expected[8] = 0x08; + assert_eq!(&m.state.memory[..9], &expected[..9]); + }; + } + + #[test] + fn test_mcopy_1_0_8() { + evm_unit_test! { + (m) { + MCOPY; + } + + // Grow memory and set initial values + m.state.memory.grow(32); + m.state.memory[..8].copy_from_slice(&[0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07]); + m.state.memory[8] = 0x08; + + // Set up stack + m.state.stack.push(U256::from(8)).unwrap(); // length + m.state.stack.push(U256::from(0)).unwrap(); // source offset + m.state.stack.push(U256::from(1)).unwrap(); // destination offset + + // Execute and assert + assert!(m.step().is_ok(), "execution step failed"); + assert_eq!(m.state.stack.len(), 0); + + // Setup expected memory and assert + let mut expected = [0u8; 32]; + expected[..8].copy_from_slice(&[0x00, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06]); + expected[8] = 0x07; + assert_eq!(&m.state.memory[..9], &expected[..9]); + }; + } + #[test] fn test_mload_nothing() { evm_unit_test! { From 09bfaece6f92b8b7af621c2f9ade3767de59b16e Mon Sep 17 00:00:00 2001 From: Mikers Date: Wed, 14 Aug 2024 13:43:37 -0700 Subject: [PATCH 04/12] failing tests --- .../src/interpreter/instructions/memory.rs | 100 +++++++++++++++++- 1 file changed, 96 insertions(+), 4 deletions(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index a29b718f6..331cbdecb 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -57,17 +57,23 @@ pub fn get_memory_region( pub fn mcopy( state: &mut ExecutionState, _: &System, - mem_index: U256, - input_index: U256, + dest_index: U256, + src_index: U256, size: U256, ) -> Result<(), ActorError> { // We are copying between two potentially overlapping slices in the same memory. // MCOPY spec: Copying takes place as if an intermediate buffer was used, // allowing the destination and source to overlap. - let region = get_memory_region(&mut state.memory, input_index, size)?.expect("empty region"); + // expand memory to accomodate requested src_index + size + let region = get_memory_region(&mut state.memory, src_index, size)?.expect("empty region"); let memory_slice = state.memory[region.offset..region.offset + region.size.get()].to_vec(); - copy_to_memory(&mut state.memory, mem_index, size, U256::zero(), &memory_slice, true) + + // expand memory to match dest_index + size + let _destination_region = get_memory_region(&mut state.memory, dest_index, size)?.expect("empty region"); + + //copy + copy_to_memory(&mut state.memory, dest_index, size, U256::zero(), &memory_slice, true) } pub fn copy_to_memory( @@ -367,6 +373,92 @@ mod tests { }; } + #[test] + fn test_mcopy_out_of_range_dest() { + evm_unit_test! { + (m) { + MCOPY; + } + + // Initial memory setup + m.state.memory.grow(32); + m.state.memory[..4].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); + + // Set up stack: Attempt to copy to a destination beyond the current memory range + m.state.stack.push(U256::from(4)).unwrap(); // length + m.state.stack.push(U256::from(0)).unwrap(); // source offset + m.state.stack.push(U256::from(64)).unwrap(); // out of range destination offset + + // Execute and expect memory expansion + assert!(m.step().is_ok(), "execution step failed"); + assert_eq!(m.state.stack.len(), 0); + + // Check that memory was expanded correctly + assert_eq!(m.state.memory.len(), 96); + + // Check the memory contents + let mut expected = vec![0u8; 68]; + expected[64..68].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); + assert_eq!(&*m.state.memory, &expected[..]); + }; + } + + #[test] + fn test_mcopy_partially_out_of_range_source() { + evm_unit_test! { + (m) { + MCOPY; + } + + // Initial memory setup + m.state.memory.grow(32); + m.state.memory[..28].copy_from_slice(&[0x01; 28]); + + // Set up stack: Source partially out of range + m.state.stack.push(U256::from(8)).unwrap(); // length + m.state.stack.push(U256::from(24)).unwrap(); // source offset (partially out of range) + m.state.stack.push(U256::from(0)).unwrap(); // destination offset + + // Execute and expect memory expansion + assert!(m.step().is_ok(), "execution step failed"); + assert_eq!(m.state.stack.len(), 0); + + // Check that memory was expanded correctly + let mut expected = vec![0u8; 32]; + expected[..8].copy_from_slice(&[0x01; 8]); + assert_eq!(&m.state.memory[..8], &expected[..8]); + }; + } + + #[test] + fn test_mcopy_fully_out_of_range_dest_fails() { + evm_unit_test! { + (m) { + MCOPY; + } + + // Initial memory setup + m.state.memory.grow(32); + m.state.memory[..4].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); + + // Set up stack: Attempt to copy to a destination fully out of range + m.state.stack.push(U256::from(4)).unwrap(); // length + m.state.stack.push(U256::from(0)).unwrap(); // source offset + m.state.stack.push(U256::from(128)).unwrap(); // fully out of range destination offset + + + // Execute and assert memory grows + assert!(m.step().is_ok(), "expected step to succeed and grow memory"); + assert_eq!(m.state.memory.len(), 160); // Expected memory to grow + + // Check the memory contents + let mut expected = vec![0u8; 132]; + expected[128..132].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); + assert_eq!(&m.state.memory[0..132], &expected[0..132]); + + }; + } + #[test] fn test_mload_nothing() { evm_unit_test! { From 8ff4c5fefb694a705f424ab98e926f403ccb7d40 Mon Sep 17 00:00:00 2001 From: Mikers Date: Wed, 14 Aug 2024 13:51:05 -0700 Subject: [PATCH 05/12] clean up tests for MCOPY --- .../evm/src/interpreter/instructions/memory.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index 331cbdecb..76ab82f1d 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -70,7 +70,8 @@ pub fn mcopy( let memory_slice = state.memory[region.offset..region.offset + region.size.get()].to_vec(); // expand memory to match dest_index + size - let _destination_region = get_memory_region(&mut state.memory, dest_index, size)?.expect("empty region"); + let _destination_region = + get_memory_region(&mut state.memory, dest_index, size)?.expect("empty region"); //copy copy_to_memory(&mut state.memory, dest_index, size, U256::zero(), &memory_slice, true) @@ -397,7 +398,8 @@ mod tests { assert_eq!(m.state.memory.len(), 96); // Check the memory contents - let mut expected = vec![0u8; 68]; + let mut expected = vec![0u8; 96]; + expected[..4].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); expected[64..68].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); assert_eq!(&*m.state.memory, &expected[..]); }; @@ -415,7 +417,7 @@ mod tests { m.state.memory[..28].copy_from_slice(&[0x01; 28]); // Set up stack: Source partially out of range - m.state.stack.push(U256::from(8)).unwrap(); // length + m.state.stack.push(U256::from(10)).unwrap(); // length m.state.stack.push(U256::from(24)).unwrap(); // source offset (partially out of range) m.state.stack.push(U256::from(0)).unwrap(); // destination offset @@ -423,9 +425,12 @@ mod tests { assert!(m.step().is_ok(), "execution step failed"); assert_eq!(m.state.stack.len(), 0); + // Check the length of the memory after the operation + assert_eq!(m.state.memory.len(), 32+EVM_WORD_SIZE); // Memory should remain at 32 bytes after the operation + // Check that memory was expanded correctly - let mut expected = vec![0u8; 32]; - expected[..8].copy_from_slice(&[0x01; 8]); + let mut expected = vec![0x01; 4]; // First 4 bytes copied + expected.extend_from_slice(&[0x00; 4]); // Remaining 4 bytes unchanged assert_eq!(&m.state.memory[..8], &expected[..8]); }; } @@ -453,6 +458,7 @@ mod tests { // Check the memory contents let mut expected = vec![0u8; 132]; + expected[..4].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); expected[128..132].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); assert_eq!(&m.state.memory[0..132], &expected[0..132]); From 50f466f9fc94d6eec305fb724119ea0aa1e9d676 Mon Sep 17 00:00:00 2001 From: Mikers Date: Wed, 14 Aug 2024 13:58:04 -0700 Subject: [PATCH 06/12] fix linter --- .../evm/src/interpreter/instructions/memory.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index 76ab82f1d..c825c3ac2 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -398,7 +398,7 @@ mod tests { assert_eq!(m.state.memory.len(), 96); // Check the memory contents - let mut expected = vec![0u8; 96]; + let mut expected = [0u8; 96]; expected[..4].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); expected[64..68].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); assert_eq!(&*m.state.memory, &expected[..]); @@ -452,15 +452,15 @@ mod tests { m.state.stack.push(U256::from(128)).unwrap(); // fully out of range destination offset - // Execute and assert memory grows - assert!(m.step().is_ok(), "expected step to succeed and grow memory"); - assert_eq!(m.state.memory.len(), 160); // Expected memory to grow + // Execute and assert memory grows + assert!(m.step().is_ok(), "expected step to succeed and grow memory"); + assert_eq!(m.state.memory.len(), 160); // Expected memory to grow - // Check the memory contents - let mut expected = vec![0u8; 132]; + // Check the memory contents + let mut expected = [0u8; 132]; expected[..4].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); - expected[128..132].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); - assert_eq!(&m.state.memory[0..132], &expected[0..132]); + expected[128..132].copy_from_slice(&[0x01, 0x02, 0x03, 0x04]); + assert_eq!(&m.state.memory[0..132], &expected[0..132]); }; } From c7df38d2e8fcad9a8a0f5366e6d64175f4b5ad2a Mon Sep 17 00:00:00 2001 From: Mikers Date: Wed, 14 Aug 2024 16:56:38 -0700 Subject: [PATCH 07/12] use copy_within for MCOPY --- .../src/interpreter/instructions/memory.rs | 41 ++++++++++++++----- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index c825c3ac2..68b0a21e0 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -61,20 +61,39 @@ pub fn mcopy( src_index: U256, size: U256, ) -> Result<(), ActorError> { - // We are copying between two potentially overlapping slices in the same memory. - // MCOPY spec: Copying takes place as if an intermediate buffer was used, - // allowing the destination and source to overlap. + // Copy memory from src_index to dest_index. + // Handles overlapping slices as if using an intermediate buffer, ensuring correct copying. + // Expands memory if src_index + size or dest_index + size exceeds current bounds. + // Returns an error if memory regions are invalid or cannot be allocated. - // expand memory to accomodate requested src_index + size - let region = get_memory_region(&mut state.memory, src_index, size)?.expect("empty region"); - let memory_slice = state.memory[region.offset..region.offset + region.size.get()].to_vec(); + copy_within_memory(&mut state.memory, dest_index, src_index, size) +} + +pub fn copy_within_memory( + memory: &mut Memory, + dest_index: U256, + src_index: U256, + size: U256, +) -> Result<(), ActorError> { + // Expand memory to accommodate requested src_index + size + let region = get_memory_region(memory, src_index, size)?.expect("empty region"); + + // Expand memory to match dest_index + size + let _destination_region = get_memory_region(memory, dest_index, size)?.expect("empty region"); + + if size > 0 { + let src_start = src_index.low_u64() as usize; + let src_end = src_start + size.low_u64() as usize; + let dest_start = dest_index.low_u64() as usize; - // expand memory to match dest_index + size - let _destination_region = - get_memory_region(&mut state.memory, dest_index, size)?.expect("empty region"); + // Named variables for clarity + let source_range = src_start..src_end; + let destination_index = dest_start; - //copy - copy_to_memory(&mut state.memory, dest_index, size, U256::zero(), &memory_slice, true) + memory.copy_within(source_range, destination_index); + } + + Ok(()) } pub fn copy_to_memory( From 17c13dca76ce68aff8bce57e84df42a3234123d9 Mon Sep 17 00:00:00 2001 From: Mikers Date: Wed, 14 Aug 2024 22:24:16 -0700 Subject: [PATCH 08/12] indicate unused --- actors/evm/src/interpreter/instructions/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index 68b0a21e0..6a6e4dce9 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -76,7 +76,7 @@ pub fn copy_within_memory( size: U256, ) -> Result<(), ActorError> { // Expand memory to accommodate requested src_index + size - let region = get_memory_region(memory, src_index, size)?.expect("empty region"); + let _region = get_memory_region(memory, src_index, size)?.expect("empty region"); // Expand memory to match dest_index + size let _destination_region = get_memory_region(memory, dest_index, size)?.expect("empty region"); From b283a30d74f10cf660c41abe27acf8535c6ada63 Mon Sep 17 00:00:00 2001 From: Mikers Date: Thu, 15 Aug 2024 07:00:32 -0700 Subject: [PATCH 09/12] move size check earlier --- .../src/interpreter/instructions/memory.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index 6a6e4dce9..b2619d4f5 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -66,7 +66,9 @@ pub fn mcopy( // Expands memory if src_index + size or dest_index + size exceeds current bounds. // Returns an error if memory regions are invalid or cannot be allocated. - copy_within_memory(&mut state.memory, dest_index, src_index, size) + if size > 0 { + copy_within_memory(&mut state.memory, dest_index, src_index, size) + } } pub fn copy_within_memory( @@ -81,17 +83,15 @@ pub fn copy_within_memory( // Expand memory to match dest_index + size let _destination_region = get_memory_region(memory, dest_index, size)?.expect("empty region"); - if size > 0 { - let src_start = src_index.low_u64() as usize; - let src_end = src_start + size.low_u64() as usize; - let dest_start = dest_index.low_u64() as usize; + let src_start = src_index.low_u64() as usize; + let src_end = src_start + size.low_u64() as usize; + let dest_start = dest_index.low_u64() as usize; - // Named variables for clarity - let source_range = src_start..src_end; - let destination_index = dest_start; + // Named variables for clarity + let source_range = src_start..src_end; + let destination_index = dest_start; - memory.copy_within(source_range, destination_index); - } + memory.copy_within(source_range, destination_index); Ok(()) } From 596d0820522f241074b498db3089e369d297b241 Mon Sep 17 00:00:00 2001 From: Mikers Date: Thu, 15 Aug 2024 08:13:04 -0700 Subject: [PATCH 10/12] rust lint add else --- actors/evm/src/interpreter/instructions/memory.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index b2619d4f5..d95326b33 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -68,6 +68,8 @@ pub fn mcopy( if size > 0 { copy_within_memory(&mut state.memory, dest_index, src_index, size) + } else { + Ok(()) } } From 101f6e2367cf366da451a1648d431205a28e60ac Mon Sep 17 00:00:00 2001 From: Mikers Date: Thu, 5 Sep 2024 12:51:54 -1000 Subject: [PATCH 11/12] use region variables and remove unneeded casting --- actors/evm/src/interpreter/instructions/memory.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index d95326b33..b8be8d963 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -80,19 +80,16 @@ pub fn copy_within_memory( size: U256, ) -> Result<(), ActorError> { // Expand memory to accommodate requested src_index + size - let _region = get_memory_region(memory, src_index, size)?.expect("empty region"); + let src_region = get_memory_region(memory, src_index, size)?.expect("empty region"); // Expand memory to match dest_index + size - let _destination_region = get_memory_region(memory, dest_index, size)?.expect("empty region"); - - let src_start = src_index.low_u64() as usize; - let src_end = src_start + size.low_u64() as usize; - let dest_start = dest_index.low_u64() as usize; + let destination_region = get_memory_region(memory, dest_index, size)?.expect("empty region"); // Named variables for clarity - let source_range = src_start..src_end; - let destination_index = dest_start; + let source_range = src_region.offset..(src_region.offset+src_region.size.get()); + let destination_index = destination_region.offset; + // Copy memory memory.copy_within(source_range, destination_index); Ok(()) From 4ddb665c807fff18702fda9577750ff09c497781 Mon Sep 17 00:00:00 2001 From: Mikers Date: Thu, 5 Sep 2024 12:58:48 -1000 Subject: [PATCH 12/12] fmt --- actors/evm/src/interpreter/instructions/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actors/evm/src/interpreter/instructions/memory.rs b/actors/evm/src/interpreter/instructions/memory.rs index b8be8d963..ba9e532ce 100644 --- a/actors/evm/src/interpreter/instructions/memory.rs +++ b/actors/evm/src/interpreter/instructions/memory.rs @@ -86,7 +86,7 @@ pub fn copy_within_memory( let destination_region = get_memory_region(memory, dest_index, size)?.expect("empty region"); // Named variables for clarity - let source_range = src_region.offset..(src_region.offset+src_region.size.get()); + let source_range = src_region.offset..(src_region.offset + src_region.size.get()); let destination_index = destination_region.offset; // Copy memory