deduplication for COPY operations

[harsh-ps-2003]

Ref #1637

It reduces code duplication by creating a generic copy_cost function. It maintains the special logic for extcodecopy_cost while still using the generic function.

It will make adding new COPY operations easier in the future, as they can all use the same copy_cost function.

Also adds copy_to_memory method for the COPY opcodes to remove code duplication.

[DaniPopes]

This is fine but doesnt fix the issue, there is also the opcode implementation to dedup

[harsh-ps-2003]

@DaniPopes what's up with this clippy thing, I didn't even touch pairings or utils! when I run cargo clippy only thing I get is inline-const is experimental! sorry, first time contributor :( Any dedup that I missed? Thanks for the patience

[DaniPopes]

It's due to Rust 1.80 just being released, it should be fixed on main with #1661

[DaniPopes]

These to_vec clones can be removed by using raw pointers. Unfortunately for performance we have to use them to get around the aliasing issue with &mut Interpreter

You can change source: &[u8] to source: *const [u8]

[DaniPopes]

source's length is not len, this should be *const [u8]. This is why tests are failing

[DaniPopes]

Thanks, looks fine, will leave it up to @rakita

[rakita]

I would return a Option<usize> here that represents memory_offset.
https://github.com/bluealloy/revm/pull/1655/files#r1694998333

Without return function silently fails, and can work only if it is in the last line of instruction.

[rakita]

I would remove this, and change behaviour of function to do gas, len check, memory resize.

As memory_offset is reused, we can return that from this function as Option<usize>

[harsh-ps-2003]

The problem with changing the functions behaviour as suggested by you is that we encountering is a classic case of Rust's borrow checker preventing simultaneous mutable and immutable borrows of the same data.
When we do :

let source = interpreter.contract.input.as_ref();
    let memory_offset = copy_to_memory(interpreter, memory_offset, data_offset, len);
    if let Some(memory_offset) = memory_offset {
        // Note: this can't panic because we resized memory to fit.
        interpreter.shared_memory.set_data(memory_offset, data_offset, len, source);
    };

We create an immutable borrow of interpreter.contract.input and then we try to mutably borrow interpreter in the copy_to_memory function call. Finally, we try to use the immutable borrow source again in the set_data call.
The Rust borrow checker doesn't allow this because it can't guarantee that the mutable borrow in copy_to_memory won't invalidate the source reference.

How do you suggest I tackle this! @rakita

[rakita]

can this line: let source = interpreter.contract.input.as_ref(); be moved inside if let Some(memory_.. so you don't have a borrow?

[harsh-ps-2003]

Now when I return Option<usize> I get expected enum std::option::Option<usize>
found unit type () from the gas! macro. Thats why the tests are failing. How do you suggest I tackle this! @rakita

[rakita]

• There is a gas! version that takes Return. gas!(10,None) something similar should be added for gas_or_fail!
• Change as_usize_or_fail to as_usize_or_fail_ret!(interpreter, memory_offset,None);
• Change resize_memory! to resize_memory!(interpreter, memory_offset, len, None);