Skip to content

Commit

Permalink
fix: memory expansion cost could overflow (#949)
Browse files Browse the repository at this point in the history
  • Loading branch information
@enitrat
enitrat committed Sep 17, 2024
1 parent cf6cfd4 commit 08311ef
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 31 deletions.
2 changes: 1 addition & 1 deletion crates/evm/src/create_helpers.cairo
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 08311ef

Please sign in to comment.