Skip to content
This repository was archived by the owner on Jan 8, 2025. It is now read-only.

fix: conversion of stack values in opcode execution #1005

Merged
merged 2 commits into from
Oct 1, 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 @@ -40,7 +40,7 @@ pub impl CreateHelpersImpl of CreateHelpers {
fn prepare_create(ref self: VM, create_type: CreateType) -> Result<CreateArgs, EVMError> {
let value = self.stack.pop()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?;
self.memory.ensure_length(memory_expansion.new_size);
Expand Down
8 changes: 6 additions & 2 deletions crates/evm/src/instructions/block_information.cairo
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Block Information.

use core::num::traits::SaturatingAdd;
use core::starknet::SyscallResultTrait;
use core::starknet::syscalls::get_block_hash_syscall;

Expand All @@ -20,7 +21,9 @@ pub impl BlockInformation of BlockInformationTrait {
fn exec_blockhash(ref self: VM) -> Result<(), EVMError> {
self.charge_gas(gas::BLOCKHASH)?;

let block_number = self.stack.pop_u64()?;
// Saturate to MAX_U64 to avoid a revert when the hash requested is too big. It should just
// push 0.
let block_number = self.stack.pop_saturating_u64()?;
let current_block = self.env.block_number;

// If input block number is lower than current_block - 256, return 0
Expand All @@ -31,7 +34,8 @@ pub impl BlockInformation of BlockInformationTrait {
// TODO: monitor the changes in the `get_block_hash_syscall` syscall.
// source:
// https://docs.starknet.io/documentation/architecture_and_concepts/Smart_Contracts/system-calls-cairo1/#get_block_hash
if block_number + 10 > current_block || block_number + 256 < current_block {
if block_number.saturating_add(10) > current_block
|| block_number.saturating_add(256) < current_block {
return self.stack.push(0);
}

Expand Down
17 changes: 9 additions & 8 deletions crates/evm/src/instructions/environmental_information.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
fn exec_calldataload(ref self: VM) -> Result<(), EVMError> {
self.charge_gas(gas::VERYLOW)?;

let offset: usize = self.stack.pop_usize()?;
// Don't error out if the offset is too big. It should just push 0.
let offset: usize = self.stack.pop_saturating_usize()?;

let calldata = self.message().data;
let calldata_len = calldata.len();
Expand Down Expand Up @@ -113,7 +114,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
fn exec_calldatacopy(ref self: VM) -> Result<(), EVMError> {
let dest_offset = self.stack.pop_saturating_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

let words_size = bytes_32_words_size(size).into();
let copy_gas_cost = gas::COPY * words_size;
Expand Down Expand Up @@ -143,7 +144,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
fn exec_codecopy(ref self: VM) -> Result<(), EVMError> {
let dest_offset = self.stack.pop_saturating_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

let words_size = bytes_32_words_size(size).into();
let copy_gas_cost = gas::COPY * words_size;
Expand Down Expand Up @@ -192,7 +193,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
let evm_address = self.stack.pop_eth_address()?;
let dest_offset = self.stack.pop_saturating_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

// GAS
let words_size = bytes_32_words_size(size).into();
Expand Down Expand Up @@ -229,7 +230,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
fn exec_returndatacopy(ref self: VM) -> Result<(), EVMError> {
let dest_offset = self.stack.pop_saturating_usize()?;
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.
let return_data: Span<u8> = self.return_data();

let (last_returndata_index, overflow) = offset.overflowing_add(size);
Expand Down Expand Up @@ -549,7 +550,7 @@ mod tests {


#[test]
fn test_calldataload_with_offset_conversion_error() {
fn test_calldataload_with_offset_bigger_usize_succeeds() {
// Given
let calldata = u256_to_bytes_array(
0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
Expand All @@ -562,8 +563,8 @@ mod tests {
let result = vm.exec_calldataload();

// Then
assert!(result.is_err());
assert_eq!(result.unwrap_err(), EVMError::TypeConversionError(TYPE_CONVERSION_ERROR));
assert!(result.is_ok());
assert_eq!(vm.stack.pop().unwrap(), 0);
}

// *************************************************************************
Expand Down
4 changes: 3 additions & 1 deletion crates/evm/src/instructions/logging_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ fn exec_log_i(ref self: VM, topics_len: u8) -> Result<(), EVMError> {

// TODO(optimization): check benefits of n `pop` instead of `pop_n`
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.
let topics: Array<u256> = self.stack.pop_n(topics_len.into())?;

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

// TODO: avoid addition overflows here. We should use checked arithmetic.
self
.charge_gas(
gas::LOG
Expand Down
13 changes: 9 additions & 4 deletions crates/evm/src/instructions/memory_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ pub impl MemoryOperation of MemoryOperationTrait {
/// MLOAD operation.
/// Load word from memory and push to stack.
fn exec_mload(ref self: VM) -> Result<(), EVMError> {
let offset: usize = self.stack.pop_usize()?;
let offset: usize = self
.stack
.pop_usize()?; // Any offset bigger than a usize would MemoryOOG.

let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 32)].span())?;
self.memory.ensure_length(memory_expansion.new_size);
Expand All @@ -50,7 +52,9 @@ pub impl MemoryOperation of MemoryOperationTrait {
/// Save word to memory.
/// # Specification: https://www.evm.codes/#52?fork=shanghai
fn exec_mstore(ref self: VM) -> Result<(), EVMError> {
let offset: usize = self.stack.pop_usize()?;
let offset: usize = self
.stack
.pop_usize()?; // Any offset bigger than a usize would MemoryOOG.
let value: u256 = self.stack.pop()?;
let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, 32)].span())?;
self.memory.ensure_length(memory_expansion.new_size);
Expand All @@ -64,7 +68,7 @@ pub impl MemoryOperation of MemoryOperationTrait {
/// Save single byte to memory
/// # Specification: https://www.evm.codes/#53?fork=shanghai
fn exec_mstore8(ref self: VM) -> Result<(), EVMError> {
let offset = self.stack.pop_saturating_usize()?;
let offset = self.stack.pop_usize()?; // Any offset bigger than a usize would MemoryOOG.
let value = self.stack.pop()?;
let value: u8 = (value.low & 0xFF).try_into().unwrap();

Expand Down Expand Up @@ -282,14 +286,15 @@ pub impl MemoryOperation of MemoryOperationTrait {
fn exec_mcopy(ref self: VM) -> Result<(), EVMError> {
let dest_offset = self.stack.pop_saturating_usize()?;
let source_offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

let words_size = bytes_32_words_size(size).into();
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);
//TODO: handle add overflows
self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?;

if size == 0 {
Expand Down
6 changes: 4 additions & 2 deletions crates/evm/src/instructions/sha3.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ pub impl Sha3Impl of Sha3Trait {
///
/// # Specification: https://www.evm.codes/#20?fork=shanghai
fn exec_sha3(ref self: VM) -> Result<(), EVMError> {
let offset: usize = self.stack.pop_usize()?;
let mut size: usize = self.stack.pop_usize()?;
let offset: usize = self.stack.pop_saturating_usize()?;
let mut size: usize = self
.stack
.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

let words_size = bytes_32_words_size(size).into();
let word_gas_cost = gas::KECCAK256WORD * words_size;
Expand Down
18 changes: 9 additions & 9 deletions crates/evm/src/instructions/system_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ pub impl SystemOperations of SystemOperationsTrait {
let to = self.stack.pop_eth_address()?;
let value = self.stack.pop()?;
let args_offset = self.stack.pop_saturating_usize()?;
let args_size = self.stack.pop_usize()?;
let args_size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.
let ret_offset = self.stack.pop_saturating_usize()?;
let ret_size = self.stack.pop_usize()?;
let ret_size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

// GAS
let memory_expansion = gas::memory_expansion(
Expand Down Expand Up @@ -109,9 +109,9 @@ pub impl SystemOperations of SystemOperationsTrait {
let code_address = self.stack.pop_eth_address()?;
let value = self.stack.pop()?;
let args_offset = self.stack.pop_saturating_usize()?;
let args_size = self.stack.pop_usize()?;
let args_size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.
let ret_offset = self.stack.pop_saturating_usize()?;
let ret_size = self.stack.pop_usize()?;
let ret_size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

let to = self.message().target.evm;

Expand Down Expand Up @@ -193,9 +193,9 @@ pub impl SystemOperations of SystemOperationsTrait {
let gas = self.stack.pop_saturating_u64()?;
let code_address = self.stack.pop_eth_address()?;
let args_offset = self.stack.pop_saturating_usize()?;
let args_size = self.stack.pop_usize()?;
let args_size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.
let ret_offset = self.stack.pop_saturating_usize()?;
let ret_size = self.stack.pop_usize()?;
let ret_size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

// GAS
let memory_expansion = gas::memory_expansion(
Expand Down Expand Up @@ -246,9 +246,9 @@ pub impl SystemOperations of SystemOperationsTrait {
let gas = self.stack.pop_saturating_u64()?;
let to = self.stack.pop_eth_address()?;
let args_offset = self.stack.pop_saturating_usize()?;
let args_size = self.stack.pop_usize()?;
let args_size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.
let ret_offset = self.stack.pop_saturating_usize()?;
let ret_size = self.stack.pop_usize()?;
let ret_size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

// GAS
let memory_expansion = gas::memory_expansion(
Expand Down Expand Up @@ -293,7 +293,7 @@ pub impl SystemOperations of SystemOperationsTrait {
/// # Specification: https://www.evm.codes/#fd?fork=shanghai
fn exec_revert(ref self: VM) -> Result<(), EVMError> {
let offset = self.stack.pop_saturating_usize()?;
let size = self.stack.pop_usize()?;
let size = self.stack.pop_usize()?; // Any size bigger than a usize would MemoryOOG.

let memory_expansion = gas::memory_expansion(self.memory.size(), [(offset, size)].span())?;
self.memory.ensure_length(memory_expansion.new_size);
Expand Down
2 changes: 1 addition & 1 deletion crates/evm/src/interpreter.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::model::account::{Account, AccountTrait};
use crate::model::vm::{VM, VMTrait};
use crate::model::{
Message, Environment, Transfer, ExecutionSummary, ExecutionResult, ExecutionResultTrait,
ExecutionResultStatus, AddressTrait, TransactionResult, TransactionResultTrait, Address
ExecutionResultStatus, AddressTrait, TransactionResult, Address
};
use crate::precompiles::Precompiles;
use crate::precompiles::eth_precompile_addresses;
Expand Down
3 changes: 2 additions & 1 deletion crates/evm/src/stack.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ impl StackImpl of StackTrait {
Result::Ok(item.low)
}

/// Calls `Stack::pop` and converts it to usize
/// Calls `Stack::pop` and converts it to an EthAddress
/// If the value is bigger than an EthAddress, it will be truncated to keep the lower 160 bits.
///
/// # Errors
///
Expand Down
Loading