Assessment reports>Scroll zkEVM>Discussion>Comment explaining gas-cost calculations for MCOPY is incorrect

Comment explaining gas-cost calculations for MCOPY is incorrect

The MCopyGadget::configure function in zkevm-circuits/src/evm_circuit/execution/mcopy.rs contains code to calculate the gas cost of an MCOPY operation. This includes gas costs due to possible memory expansion. Memory is expanded both for reads and writes but only for bytes actually read or written. Thus, an MCOPY operation will not cause memory expansion when no bytes are copied (due to the length being zero), even for large addresses for source and destination. The relevant part of the source code in MCopyGadget::configure is the following:

let memory_src_address =
    MemoryAddressGadget::construct(cb, src_offset.clone(), length.clone());
let memory_dest_address =
    MemoryAddressGadget::construct(cb, dest_offset.clone(), length.clone());

// if no acutal copy happens, memory_word_size doesn't change. MemoryExpansionGadget handle
// it internally
let memory_expansion = MemoryExpansionGadget::construct(
    cb,
    [
        memory_src_address.end_offset(),
        memory_dest_address.end_offset(),   
    ],
);

The comment here suggests that MemoryExpansionGadget handles the case of length zero. However, it does not actually contain any special logic for this case. In fact, from the above snippet it seems that it would be impossible for MemoryExpansionGadget to do so, at it is only passed the end offsets of the two addresses (so the start address plus the length).

The memory-expansion calculation is still correct, but it is due to a special behavior of the MemoryAddressGadget instead. The end_offset() function is implemented as follows:

fn end_offset(&self) -> Expression<F> {
    self.offset() + self.length()
}

fn length(&self) -> Expression<F> {
    from_bytes::expr(&self.memory_length.cells)
}

pub(crate) fn offset(&self) -> Expression<F> {
    self.has_length() * from_bytes::expr(&self.memory_offset_bytes.cells)
}

pub(crate) fn has_length(&self) -> Expression<F> {
    1.expr() - self.memory_length_is_zero.expr()
}

The upshot is that MemoryAddressGadget will report an offset of zero whenever length is zero, independent of the original offset, and thus the reported end offset will in this case also be zero. This is what ensures that the memory-expansion calculation is correct.

We recommend to update the comment to clarify the reason the memory-expansion calculation is correct.

The comments were corrected in commit .

Zellic © 2024Back to top ↑