Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: memory expansion cost could overflow #949

Merged
merged 1 commit into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/evm/src/create_helpers.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub 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)].span());
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 {
Expand Down
2 changes: 2 additions & 0 deletions crates/evm/src/errors.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub enum EVMError {
OutOfGas,
Assertion,
DepthLimit,
MemoryLimitOOG
}

#[generate_trait]
Expand All @@ -81,6 +82,7 @@ pub impl EVMErrorImpl of EVMErrorTrait {
EVMError::OutOfGas => 'out of gas'.into(),
EVMError::Assertion => 'assertion failed'.into(),
EVMError::DepthLimit => 'max call depth exceeded'.into(),
EVMError::MemoryLimitOOG => 'memory limit out of gas'.into(),
}
}

Expand Down
42 changes: 28 additions & 14 deletions crates/evm/src/gas.cairo
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use core::cmp::min;
use core::num::traits::CheckedAdd;
use evm::errors::EVMError;
use utils::eth_transaction::common::TxKindTrait;
use utils::eth_transaction::eip2930::{AccessListItem};
use utils::eth_transaction::transaction::{Transaction, TransactionTrait};
Expand Down Expand Up @@ -166,30 +168,42 @@ pub fn calculate_memory_gas_cost(size_in_bytes: usize) -> u64 {
/// # Returns
///
/// * `MemoryExpansion`: New size and expansion cost.
pub 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;
}
pub fn memory_expansion(
current_size: usize, mut operations: Span<(usize, usize)>
) -> Result<MemoryExpansion, EVMError> {
let mut current_max_size = current_size;

// Using a high-level loop because Cairo doesn't support the `for` loop syntax with breaks
let max_size = loop {
match operations.pop_front() {
Option::Some((
offset, size
)) => {
if *size != 0 {
match (*offset).checked_add(*size) {
Option::Some(end) => {
if end > current_max_size {
current_max_size = end;
}
},
Option::None => { break Result::Err(EVMError::MemoryLimitOOG); },
}
}
},
Option::None => { break Result::Ok((current_max_size)); },
}
};
}?;

let new_size = helpers::ceil32(max_size);

if new_size <= current_size {
return MemoryExpansion { new_size: current_size, expansion_cost: 0 };
return Result::Ok(MemoryExpansion { new_size: current_size, expansion_cost: 0 });
}

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 }
Result::Ok(MemoryExpansion { new_size, expansion_cost })
}

/// Calculates the gas to be charged for the init code in CREATE/CREATE2
Expand Down
8 changes: 4 additions & 4 deletions crates/evm/src/instructions/environmental_information.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
let copy_gas_cost = gas::COPY * words_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)?;

Expand Down Expand Up @@ -149,7 +149,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
let copy_gas_cost = gas::COPY * words_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)?;

Expand Down Expand Up @@ -198,7 +198,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
let words_size = (ceil32(size) / 32).into();
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) {
Expand Down Expand Up @@ -244,7 +244,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {

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)?;

Expand Down
2 changes: 1 addition & 1 deletion crates/evm/src/instructions/logging_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn exec_log_i(ref self: VM, topics_len: u8) -> Result<(), EVMError> {
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)].span());
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?;
self.memory.ensure_length(memory_expansion.new_size);
self
.charge_gas(
Expand Down
9 changes: 5 additions & 4 deletions crates/evm/src/instructions/memory_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub 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)].span());
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)?;

Expand All @@ -52,7 +52,7 @@ pub 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)].span());
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)?;

Expand All @@ -68,7 +68,7 @@ pub 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)].span());
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)?;

Expand Down Expand Up @@ -290,7 +290,7 @@ pub impl MemoryOperation of MemoryOperationTrait {
let copy_gas_cost = gas::COPY * words_size;
let memory_expansion = gas::memory_expansion(
self.memory.size(), [(max(dest_offset, source_offset), size)].span()
);
)?;
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?;

Expand Down Expand Up @@ -1054,6 +1054,7 @@ mod tests {
+ gas::memory_expansion(
vm.memory.size(), [(max(dest_offset, source_offset), size)].span()
)
.unwrap()
.expansion_cost
+ copy_gas_cost;
let gas_before = vm.gas_left();
Expand Down
2 changes: 1 addition & 1 deletion crates/evm/src/instructions/sha3.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub impl Sha3Impl of Sha3Trait {

let words_size = (ceil32(size) / 32).into();
let word_gas_cost = gas::KECCAK256WORD * words_size;
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span());
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?;
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::KECCAK256 + word_gas_cost + memory_expansion.expansion_cost)?;

Expand Down
12 changes: 6 additions & 6 deletions crates/evm/src/instructions/system_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub impl SystemOperations of SystemOperationsTrait {
// GAS
let memory_expansion = gas::memory_expansion(
self.memory.size(), [(args_offset, args_size), (ret_offset, ret_size)].span()
);
)?;
self.memory.ensure_length(memory_expansion.new_size);

let access_gas_cost = if self.accessed_addresses.contains(to) {
Expand Down Expand Up @@ -117,7 +117,7 @@ pub impl SystemOperations of SystemOperationsTrait {
// GAS
let memory_expansion = gas::memory_expansion(
self.memory.size(), [(args_offset, args_size), (ret_offset, ret_size)].span()
);
)?;
self.memory.ensure_length(memory_expansion.new_size);

let access_gas_cost = if self.accessed_addresses.contains(code_address) {
Expand Down Expand Up @@ -172,7 +172,7 @@ pub impl SystemOperations of SystemOperationsTrait {
fn exec_return(ref self: VM) -> Result<(), EVMError> {
let offset = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?;
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span());
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?;
self.memory.ensure_length(memory_expansion.new_size);
self.charge_gas(gas::ZERO + memory_expansion.expansion_cost)?;

Expand All @@ -199,7 +199,7 @@ pub impl SystemOperations of SystemOperationsTrait {
// GAS
let memory_expansion = gas::memory_expansion(
self.memory.size(), [(args_offset, args_size), (ret_offset, ret_size)].span()
);
)?;
self.memory.ensure_length(memory_expansion.new_size);

let access_gas_cost = if self.accessed_addresses.contains(code_address) {
Expand Down Expand Up @@ -252,7 +252,7 @@ pub impl SystemOperations of SystemOperationsTrait {
// GAS
let memory_expansion = gas::memory_expansion(
self.memory.size(), [(args_offset, args_size), (ret_offset, ret_size)].span()
);
)?;
self.memory.ensure_length(memory_expansion.new_size);

let access_gas_cost = if self.accessed_addresses.contains(to) {
Expand Down Expand Up @@ -290,7 +290,7 @@ pub impl SystemOperations of SystemOperationsTrait {
let offset = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?;

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

Expand Down
Loading