Skip to content

Commit

Permalink
fix: memory expansions (#913)
Browse files Browse the repository at this point in the history 
* fix: memory extensions and gas costs

* fix: memory expansion timing
  • Loading branch information
@enitrat
enitrat committed Sep 6, 2024
1 parent 717a757 commit 7cbfdae
Show file tree
Hide file tree
Showing 13 changed files with 286 additions and 258 deletions.
3 changes: 2 additions & 1 deletion crates/evm/src/create_helpers.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ impl CreateHelpersImpl of CreateHelpers {
let offset = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?;

let memory_expansion = gas::memory_expansion(self.memory.size(), offset + size);
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span());
self.memory.ensure_length(memory_expansion.new_size);
let init_code_gas = gas::init_code_cost(size);
let charged_gas = match create_type {
CreateType::Create => gas::CREATE + memory_expansion.expansion_cost + init_code_gas,
Expand Down
35 changes: 28 additions & 7 deletions crates/evm/src/gas.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,21 +155,42 @@ fn calculate_memory_gas_cost(size_in_bytes: usize) -> u128 {
}


fn memory_expansion(memory_size: usize, max_offset: usize) -> MemoryExpansion {
let new_size = helpers::ceil32(max_offset);
/// Calculates memory expansion based on multiple memory operations.
///
/// # Arguments
///
/// * `current_size`: Current size of the memory.
/// * `operations`: A span of tuples (offset, size) representing memory operations.
///
/// # Returns
///
/// * `MemoryExpansion`: New size and expansion cost.
fn memory_expansion(current_size: usize, operations: Span<(usize, usize)>) -> MemoryExpansion {
let mut max_size = current_size;

for (
offset, size
) in operations {
if *size != 0 {
let end = *offset + *size;
if end > max_size {
max_size = end;
}
}
};

let new_size = helpers::ceil32(max_size);

if new_size <= memory_size {
return MemoryExpansion { new_size: memory_size, expansion_cost: 0 };
if new_size <= current_size {
return MemoryExpansion { new_size: current_size, expansion_cost: 0 };
}

let prev_cost = calculate_memory_gas_cost(memory_size);
let prev_cost = calculate_memory_gas_cost(current_size);
let new_cost = calculate_memory_gas_cost(new_size);
let expansion_cost = new_cost - prev_cost;

MemoryExpansion { new_size, expansion_cost }
}


/// Calculates the gas to be charged for the init code in CREATE/CREATE2
/// opcodes as well as create transactions.
///
Expand Down
25 changes: 16 additions & 9 deletions crates/evm/src/instructions/environmental_information.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ impl EnvironmentInformationImpl of EnvironmentInformationTrait {

let words_size: u128 = (ceil32(size) / 32).into();
let copy_gas_cost = gas::COPY * words_size;
let memory_expansion = gas::memory_expansion(self.memory.size(), dest_offset + size);
let memory_expansion = gas::memory_expansion(
self.memory.size(), [(dest_offset, size)].span()
);
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?;

let calldata: Span<u8> = self.message().data;
Expand All @@ -156,7 +159,10 @@ impl EnvironmentInformationImpl of EnvironmentInformationTrait {

let words_size: u128 = (ceil32(size) / 32).into();
let copy_gas_cost = gas::COPY * words_size;
let memory_expansion = gas::memory_expansion(self.memory.size(), dest_offset + size);
let memory_expansion = gas::memory_expansion(
self.memory.size(), [(dest_offset, size)].span()
);
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?;

let bytecode: Span<u8> = self.message().code;
Expand Down Expand Up @@ -202,7 +208,10 @@ impl EnvironmentInformationImpl of EnvironmentInformationTrait {

// GAS
let words_size: u128 = (ceil32(size) / 32).into();
let memory_expansion = gas::memory_expansion(self.memory.size(), dest_offset + size);
let memory_expansion = gas::memory_expansion(
self.memory.size(), [(dest_offset, size)].span()
);
self.memory.ensure_length(memory_expansion.new_size);
let copy_gas_cost = gas::COPY * words_size;
let access_gas_cost = if self.accessed_addresses.contains(evm_address) {
gas::WARM_ACCESS_COST
Expand Down Expand Up @@ -245,12 +254,10 @@ impl EnvironmentInformationImpl of EnvironmentInformationTrait {
let words_size: u128 = (ceil32(size.into()) / 32).into();
let copy_gas_cost = gas::COPY * words_size;

let (max_memory_size, overflow) = dest_offset.overflowing_add(size);
if overflow {
return Result::Err(EVMError::OutOfGas);
}

let memory_expansion = gas::memory_expansion(self.memory.size(), max_memory_size);
let memory_expansion = gas::memory_expansion(
self.memory.size(), [(dest_offset, size)].span()
);
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?;

let data_to_copy: Span<u8> = return_data.slice(offset, size);
Expand Down
43 changes: 28 additions & 15 deletions crates/evm/src/instructions/logging_operations.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ mod internal {
let size = self.stack.pop_usize()?;
let topics: Array<u256> = self.stack.pop_n(topics_len.into())?;

let memory_expansion = gas::memory_expansion(self.memory.size(), offset + size);
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span());
self.memory.ensure_length(memory_expansion.new_size);
self
.charge_gas(
gas::LOG
Expand Down Expand Up @@ -97,15 +98,15 @@ mod tests {
use evm::memory::MemoryTrait;
use evm::stack::StackTrait;
use evm::state::StateTrait;
use evm::test_utils::{VMBuilderTrait};
use evm::test_utils::{VMBuilderTrait, MemoryTestUtilsTrait};
use utils::helpers::u256_to_bytes_array;

#[test]
fn test_exec_log0() {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);

vm.stack.push(0x1F).expect('push failed');
vm.stack.push(0x00).expect('push failed');
Expand Down Expand Up @@ -133,7 +134,7 @@ mod tests {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);

vm.stack.push(0x0123456789ABCDEF).expect('push failed');
vm.stack.push(0x20).expect('push failed');
Expand Down Expand Up @@ -163,7 +164,7 @@ mod tests {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);

vm.stack.push(Bounded::<u256>::MAX).expect('push failed');
vm.stack.push(0x0123456789ABCDEF).expect('push failed');
Expand Down Expand Up @@ -195,8 +196,12 @@ mod tests {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);
vm
.memory
.store_with_expansion(
0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20
);

vm.stack.push(0x00).expect('push failed');
vm.stack.push(Bounded::<u256>::MAX).expect('push failed');
Expand Down Expand Up @@ -232,8 +237,12 @@ mod tests {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);
vm
.memory
.store_with_expansion(
0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20
);

vm.stack.push(Bounded::<u256>::MAX).expect('push failed');
vm.stack.push(0x00).expect('push failed');
Expand Down Expand Up @@ -269,7 +278,7 @@ mod tests {
// Given
let mut vm = VMBuilderTrait::new().with_read_only().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);

vm.stack.push(0x0123456789ABCDEF).expect('push failed');
vm.stack.push(0x20).expect('push failed');
Expand All @@ -290,7 +299,7 @@ mod tests {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);

vm.stack.push(0x0123456789ABCDEF).expect('push failed');
vm.stack.push(0x00).expect('push failed');
Expand Down Expand Up @@ -318,7 +327,7 @@ mod tests {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);

vm.stack.push(0x0123456789ABCDEF).expect('push failed');
vm.stack.push(Bounded::<u256>::MAX).expect('push failed');
Expand All @@ -340,7 +349,7 @@ mod tests {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);

vm.stack.push(0x0123456789ABCDEF).expect('push failed');
vm.stack.push(0x20).expect('push failed');
Expand All @@ -362,8 +371,12 @@ mod tests {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();

vm.memory.store(Bounded::<u256>::MAX, 0);
vm.memory.store(0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0);
vm
.memory
.store_with_expansion(
0x0123456789ABCDEF000000000000000000000000000000000000000000000000, 0x20
);

vm.stack.push(Bounded::<u256>::MAX).expect('push failed');
vm.stack.push(0x00).expect('push failed');
Expand Down
45 changes: 18 additions & 27 deletions crates/evm/src/instructions/memory_operations.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ impl MemoryOperation of MemoryOperationTrait {
fn exec_mload(ref self: VM) -> Result<(), EVMError> {
let offset: usize = self.stack.pop_usize()?;

let memory_expansion = gas::memory_expansion(self.memory.size(), offset + 32);
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 32)].span());
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::VERYLOW + memory_expansion.expansion_cost)?;

let result = self.memory.load(offset);
Expand All @@ -56,7 +57,8 @@ impl MemoryOperation of MemoryOperationTrait {
fn exec_mstore(ref self: VM) -> Result<(), EVMError> {
let offset: usize = self.stack.pop_usize()?;
let value: u256 = self.stack.pop()?;
let memory_expansion = gas::memory_expansion(self.memory.size(), offset + 32);
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 32)].span());
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::VERYLOW + memory_expansion.expansion_cost)?;

self.memory.store(value, offset);
Expand All @@ -71,7 +73,8 @@ impl MemoryOperation of MemoryOperationTrait {
let value = self.stack.pop()?;
let value: u8 = (value.low & 0xFF).try_into().unwrap();

let memory_expansion = gas::memory_expansion(self.memory.size(), offset + 1);
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 1)].span());
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::VERYLOW + memory_expansion.expansion_cost)?;

self.memory.store_byte(value, offset);
Expand Down Expand Up @@ -289,8 +292,9 @@ impl MemoryOperation of MemoryOperationTrait {
let dest_offset = self.stack.pop_usize()?;

let memory_expansion = gas::memory_expansion(
self.memory.size(), max(dest_offset, source_offset) + size
self.memory.size(), [(max(dest_offset, source_offset), size)].span()
);
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::VERYLOW + memory_expansion.expansion_cost)?;

if size == 0 {
Expand Down Expand Up @@ -322,8 +326,8 @@ mod tests {
use evm::stack::StackTrait;
use evm::state::{StateTrait, compute_storage_address};
use evm::test_utils::{
evm_address, VMBuilderTrait, setup_test_storages, register_account, uninitialized_account,
native_token
evm_address, VMBuilderTrait, MemoryTestUtilsTrait, setup_test_storages, register_account,
uninitialized_account, native_token
};
use snforge_std::{test_address, start_mock_call, store};
use snforge_utils::snforge_utils::store_evm;
Expand Down Expand Up @@ -377,7 +381,7 @@ mod tests {
fn assert_mload(value: u256, offset: u256, expected_value: u256, expected_memory_size: u32) {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();
vm.memory.store(value, 0);
vm.memory.store_with_expansion(value, 0);

vm.stack.push(offset).expect('push failed');

Expand Down Expand Up @@ -565,33 +569,18 @@ mod tests {
}

#[test]
fn test_exec_msize_store_max_offset_0() {
fn test_exec_msize_should_return_size_of_memory() {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();
vm.memory.store(Bounded::<u256>::MAX, 0x00);
vm.memory.store_with_expansion(Bounded::<u256>::MAX, 0x00);

// When
let result = vm.exec_msize();

// Then
assert(result.is_ok(), 'should have succeeded');
assert(vm.stack.len() == 1, 'stack should have one element');
assert(vm.stack.pop().unwrap() == 32, 'should 32 bytes after MSTORE');
}

#[test]
fn test_exec_msize_store_max_offset_1() {
// Given
let mut vm = VMBuilderTrait::new_with_presets().build();
vm.memory.store(Bounded::<u256>::MAX, 0x01);

// When
let result = vm.exec_msize();

// Then
assert(result.is_ok(), 'should have succeeded');
assert(vm.stack.len() == 1, 'stack should have one element');
assert(vm.stack.pop().unwrap() == 64, 'should 64 bytes after MSTORE');
assert(vm.stack.pop().unwrap() == 32, 'should 32 bytes after MSIZE');
}

#[test]
Expand Down Expand Up @@ -1045,7 +1034,7 @@ mod tests {
let mut i = 0;
for element in values
.span() {
vm.memory.store((*element).into(), source_offset + 0x20 * i);
vm.memory.store_with_expansion((*element).into(), source_offset + 0x20 * i);
i += 1;
};
vm.stack.push(dest_offset.into()).expect('push failed');
Expand All @@ -1054,7 +1043,9 @@ mod tests {

// When
let expected_gas = gas::VERYLOW
+ gas::memory_expansion(vm.memory.size(), max(dest_offset, source_offset) + size)
+ gas::memory_expansion(
vm.memory.size(), [(max(dest_offset, source_offset), size)].span()
)
.expansion_cost;
let gas_before = vm.gas_left();
let result = vm.exec_mcopy();
Expand Down
Loading

0 comments on commit 7cbfdae

Please sign in to comment.