From e8f1ff8201063dfbefa4bf71f8dad1dec9e13e4e Mon Sep 17 00:00:00 2001 From: enitrat Date: Wed, 2 Oct 2024 13:28:47 +0200 Subject: [PATCH] dev: use checked math --- crates/evm/src/create_helpers.cairo | 17 +++++-- crates/evm/src/gas.cairo | 3 +- .../instructions/duplication_operations.cairo | 3 +- .../environmental_information.cairo | 51 ++++++++++++++----- .../src/instructions/logging_operations.cairo | 16 +++--- .../src/instructions/memory_operations.cairo | 25 ++++++--- crates/evm/src/instructions/sha3.cairo | 9 +++- .../stop_and_arithmetic_operations.cairo | 8 ++- .../src/instructions/system_operations.cairo | 12 ++++- crates/evm/src/interpreter.cairo | 2 +- crates/evm/src/precompiles/identity.cairo | 5 +- crates/evm/src/precompiles/p256verify.cairo | 34 ++----------- crates/evm/src/precompiles/sha256.cairo | 5 +- crates/utils/src/helpers.cairo | 3 -- 14 files changed, 118 insertions(+), 75 deletions(-) diff --git a/crates/evm/src/create_helpers.cairo b/crates/evm/src/create_helpers.cairo index 3b8c39eca..14c3d435c 100644 --- a/crates/evm/src/create_helpers.cairo +++ b/crates/evm/src/create_helpers.cairo @@ -1,4 +1,5 @@ use core::num::traits::Bounded; +use core::num::traits::CheckedAdd; use core::num::traits::Zero; use core::starknet::EthAddress; use crate::errors::{ensure, EVMError}; @@ -18,7 +19,6 @@ use utils::set::SetTrait; use utils::traits::{ BoolIntoNumeric, EthAddressIntoU256, U256TryIntoResult, SpanU8TryIntoResultEthAddress }; - /// Helper struct to prepare CREATE and CREATE2 opcodes #[derive(Drop)] pub struct CreateArgs { @@ -46,13 +46,20 @@ pub impl CreateHelpersImpl of CreateHelpers { 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, + CreateType::Create => gas::CREATE + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)? + .checked_add(init_code_gas) + .ok_or(EVMError::OutOfGas)?, CreateType::Create2 => { let calldata_words = bytes_32_words_size(size); gas::CREATE - + gas::KECCAK256WORD * calldata_words.into() - + memory_expansion.expansion_cost - + init_code_gas + .checked_add(gas::KECCAK256WORD * calldata_words.into()) + .ok_or(EVMError::OutOfGas)? + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)? + .checked_add(init_code_gas) + .ok_or(EVMError::OutOfGas)? }, }; self.charge_gas(charged_gas)?; diff --git a/crates/evm/src/gas.cairo b/crates/evm/src/gas.cairo index fa359bd2e..9454410ae 100644 --- a/crates/evm/src/gas.cairo +++ b/crates/evm/src/gas.cairo @@ -4,6 +4,7 @@ use crate::errors::EVMError; use utils::eth_transaction::common::TxKindTrait; use utils::eth_transaction::eip2930::{AccessListItem}; use utils::eth_transaction::transaction::{Transaction, TransactionTrait}; +use utils::helpers::bytes_32_words_size; use utils::helpers; //! Gas costs for EVM operations @@ -153,7 +154,7 @@ pub fn calculate_message_call_gas( /// * `total_gas_cost` - The gas cost for storing data in memory. pub fn calculate_memory_gas_cost(size_in_bytes: usize) -> u64 { let _512: NonZero = 512_u64.try_into().unwrap(); - let size_in_words = (size_in_bytes + 31) / 32; + let size_in_words = bytes_32_words_size(size_in_bytes); let linear_cost = size_in_words.into() * MEMORY; let (q0, r0) = DivRem::div_rem(size_in_words.into(), _512); diff --git a/crates/evm/src/instructions/duplication_operations.cairo b/crates/evm/src/instructions/duplication_operations.cairo index 5752279ba..a69796598 100644 --- a/crates/evm/src/instructions/duplication_operations.cairo +++ b/crates/evm/src/instructions/duplication_operations.cairo @@ -8,8 +8,9 @@ use crate::stack::StackTrait; /// Generic DUP operation #[inline(always)] -fn exec_dup_i(ref self: VM, i: u8) -> Result<(), EVMError> { +fn exec_dup_i(ref self: VM, i: NonZero) -> Result<(), EVMError> { self.charge_gas(gas::VERYLOW)?; + let i: u8 = i.into(); let item = self.stack.peek_at((i - 1).into())?; self.stack.push(item) } diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index 9da876792..22d47d8a7 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -1,5 +1,6 @@ use core::num::traits::OverflowingAdd; use core::num::traits::Zero; +use core::num::traits::{CheckedAdd, CheckedSub}; use crate::errors::{ensure, EVMError}; use crate::gas; use crate::memory::MemoryTrait; @@ -79,12 +80,17 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { let calldata_len = calldata.len(); // All bytes after the end of the calldata are set to 0. - if offset >= calldata_len { - return self.stack.push(0); - } + let bytes_len = match calldata_len.checked_sub(offset) { + Option::None => { return self.stack.push(0); }, + Option::Some(remaining_len) => { + if remaining_len == 0 { + return self.stack.push(0); + } + core::cmp::min(32, remaining_len) + } + }; // Slice the calldata - let bytes_len = core::cmp::min(32, calldata_len - offset); let sliced = calldata.slice(offset, bytes_len); let mut data_to_load: u256 = sliced @@ -122,7 +128,13 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { 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 total_cost = gas::VERYLOW + .checked_add(copy_gas_cost) + .ok_or(EVMError::OutOfGas)? + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; let calldata: Span = self.message().data; copy_bytes_to_memory(ref self, calldata, dest_offset, offset, size); @@ -152,7 +164,13 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { 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 total_cost = gas::VERYLOW + .checked_add(copy_gas_cost) + .ok_or(EVMError::OutOfGas)? + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; let bytecode: Span = self.message().code; @@ -208,7 +226,12 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { self.accessed_addresses.add(evm_address); gas::COLD_ACCOUNT_ACCESS_COST }; - self.charge_gas(access_gas_cost + copy_gas_cost + memory_expansion.expansion_cost)?; + let total_cost = access_gas_cost + .checked_add(copy_gas_cost) + .ok_or(EVMError::OutOfGas)? + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; let bytecode = self.env.state.get_account(evm_address).code; copy_bytes_to_memory(ref self, bytecode, dest_offset, offset, size); @@ -246,7 +269,12 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { 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 total_cost = gas::VERYLOW + .checked_add(copy_gas_cost) + .ok_or(EVMError::OutOfGas)? + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; let data_to_copy: Span = return_data.slice(offset, size); self.memory.store_n(data_to_copy, dest_offset); @@ -287,10 +315,9 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { fn copy_bytes_to_memory( ref self: VM, bytes: Span, dest_offset: usize, offset: usize, size: usize ) { - let bytes_slice = if offset < bytes.len() { - bytes.slice(offset, core::cmp::min(size, bytes.len() - offset)) - } else { - [].span() + let bytes_slice = match bytes.len().checked_sub(offset) { + Option::Some(remaining) => bytes.slice(offset, core::cmp::min(size, remaining)), + Option::None => [].span() }; self.memory.store_padded_segment(dest_offset, size, bytes_slice); diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index 12c86a30d..4be7153b5 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -1,5 +1,6 @@ //! Logging Operations. +use core::num::traits::CheckedAdd; use crate::errors::{EVMError, ensure}; use crate::gas; use crate::memory::MemoryTrait; @@ -68,13 +69,14 @@ fn exec_log_i(ref self: VM, topics_len: u8) -> Result<(), EVMError> { self.memory.ensure_length(memory_expansion.new_size); // TODO: avoid addition overflows here. We should use checked arithmetic. - self - .charge_gas( - gas::LOG - + topics_len.into() * gas::LOGTOPIC - + size.into() * gas::LOGDATA - + memory_expansion.expansion_cost - )?; + let total_cost = gas::LOG + .checked_add(topics_len.into() * gas::LOGTOPIC) + .ok_or(EVMError::OutOfGas)? + .checked_add(size.into() * gas::LOGDATA) + .ok_or(EVMError::OutOfGas)? + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; let mut data: Array = Default::default(); self.memory.load_n(size, ref data, offset); diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index fc2d81b61..4386d275b 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -1,4 +1,5 @@ use core::cmp::max; +use core::num::traits::CheckedAdd; use crate::backend::starknet_backend::fetch_original_storage; //! Stack Memory Storage and Flow Operations. use crate::errors::{EVMError, ensure}; @@ -9,7 +10,6 @@ use crate::stack::StackTrait; use crate::state::StateTrait; use utils::helpers::bytes_32_words_size; use utils::set::SetTrait; - #[inline(always)] fn jump(ref self: VM, index: usize) -> Result<(), EVMError> { match self.message().code.get(index) { @@ -42,7 +42,10 @@ pub impl MemoryOperation of MemoryOperationTrait { 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 total_cost = gas::VERYLOW + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; let result = self.memory.load(offset); self.stack.push(result) @@ -58,7 +61,10 @@ pub impl MemoryOperation of MemoryOperationTrait { 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); - self.charge_gas(gas::VERYLOW + memory_expansion.expansion_cost)?; + let total_cost = gas::VERYLOW + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; self.memory.store(value, offset); Result::Ok(()) @@ -74,7 +80,10 @@ pub impl MemoryOperation of MemoryOperationTrait { 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)?; + let total_cost = gas::VERYLOW + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; self.memory.store_byte(value, offset); @@ -294,8 +303,12 @@ pub impl MemoryOperation of MemoryOperationTrait { 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)?; + let total_cost = gas::VERYLOW + .checked_add(copy_gas_cost) + .ok_or(EVMError::OutOfGas)? + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; if size == 0 { return Result::Ok(()); diff --git a/crates/evm/src/instructions/sha3.cairo b/crates/evm/src/instructions/sha3.cairo index 622abf23f..73b3d99b9 100644 --- a/crates/evm/src/instructions/sha3.cairo +++ b/crates/evm/src/instructions/sha3.cairo @@ -1,6 +1,7 @@ use core::cmp::min; //! SHA3. use core::keccak::{cairo_keccak}; +use core::num::traits::CheckedAdd; // Internal imports use crate::errors::EVMError; @@ -11,7 +12,6 @@ use crate::stack::StackTrait; use utils::helpers::bytes_32_words_size; use utils::traits::array::ArrayExtTrait; use utils::traits::integer::U256Trait; - #[generate_trait] pub impl Sha3Impl of Sha3Trait { /// SHA3 operation : Hashes n bytes in memory at a given offset in memory @@ -32,7 +32,12 @@ pub impl Sha3Impl of Sha3Trait { let word_gas_cost = gas::KECCAK256WORD * words_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::KECCAK256 + word_gas_cost + memory_expansion.expansion_cost)?; + let total_cost = gas::KECCAK256 + .checked_add(word_gas_cost) + .ok_or(EVMError::OutOfGas)? + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; let mut to_hash: Array = Default::default(); diff --git a/crates/evm/src/instructions/stop_and_arithmetic_operations.cairo b/crates/evm/src/instructions/stop_and_arithmetic_operations.cairo index 9f302aee2..54b9da66b 100644 --- a/crates/evm/src/instructions/stop_and_arithmetic_operations.cairo +++ b/crates/evm/src/instructions/stop_and_arithmetic_operations.cairo @@ -1,6 +1,7 @@ //! Stop and Arithmetic Operations. use core::integer::{u512_safe_div_rem_by_u256}; use core::math::u256_mul_mod_n; +use core::num::traits::CheckedAdd; use core::num::traits::{OverflowingAdd, OverflowingMul, OverflowingSub}; use crate::errors::EVMError; use crate::gas; @@ -164,7 +165,6 @@ pub impl StopAndArithmeticOperations of StopAndArithmeticOperationsTrait { let result: u256 = match TryInto::>::try_into(n) { Option::Some(nonzero_n) => { - // This is more gas efficient than computing (a mod N) + (b mod N) mod N let sum = u256_wide_add(a, b); let (_, r) = u512_safe_div_rem_by_u256(sum, nonzero_n); r @@ -204,7 +204,10 @@ pub impl StopAndArithmeticOperations of StopAndArithmeticOperationsTrait { // Gas let bytes_used = exponent.bytes_used(); - self.charge_gas(gas::EXP + gas::EXP_GAS_PER_BYTE * bytes_used.into())?; + let total_cost = gas::EXP + .checked_add(gas::EXP_GAS_PER_BYTE * bytes_used.into()) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; let result = base.wrapping_pow(exponent); @@ -238,6 +241,7 @@ pub impl StopAndArithmeticOperations of StopAndArithmeticOperationsTrait { let result = if b < 32 { let s = 8 * b + 7; + //TODO: use POW_2 table for optimization let two_pow_s = 2.pow(s); // Get v, the t-th bit of x. To do this we bitshift x by s bits to the right and apply a // mask to get the last bit. diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index 5511b6017..65b95d75d 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -66,7 +66,11 @@ pub impl SystemOperations of SystemOperationsTrait { memory_expansion.expansion_cost, access_gas_cost + transfer_gas_cost + create_gas_cost )?; - self.charge_gas(message_call_gas.cost + memory_expansion.expansion_cost)?; + let total_cost = message_call_gas + .cost + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; // Only the transfer gas is left to charge. let read_only = self.message().read_only; @@ -213,7 +217,11 @@ pub impl SystemOperations of SystemOperationsTrait { let message_call_gas = gas::calculate_message_call_gas( 0, gas, self.gas_left(), memory_expansion.expansion_cost, access_gas_cost )?; - self.charge_gas(message_call_gas.cost + memory_expansion.expansion_cost)?; + let total_cost = message_call_gas + .cost + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(total_cost)?; self .generic_call( diff --git a/crates/evm/src/interpreter.cairo b/crates/evm/src/interpreter.cairo index ccf14cda7..793f0eef9 100644 --- a/crates/evm/src/interpreter.cairo +++ b/crates/evm/src/interpreter.cairo @@ -1,6 +1,6 @@ use contracts::kakarot_core::KakarotCore; use contracts::kakarot_core::interface::IKakarotCore; -use core::num::traits::{Bounded, Zero}; +use core::num::traits::Zero; use core::ops::SnapshotDeref; use core::starknet::EthAddress; use core::starknet::storage::{StoragePointerReadAccess}; diff --git a/crates/evm/src/precompiles/identity.cairo b/crates/evm/src/precompiles/identity.cairo index 7bd38806e..e84ceaf80 100644 --- a/crates/evm/src/precompiles/identity.cairo +++ b/crates/evm/src/precompiles/identity.cairo @@ -1,6 +1,7 @@ use core::starknet::EthAddress; use crate::errors::EVMError; use crate::precompiles::Precompile; +use utils::helpers::bytes_32_words_size; const BASE_COST: u64 = 15; const COST_PER_WORD: u64 = 3; @@ -12,8 +13,8 @@ pub impl Identity of Precompile { } fn exec(input: Span) -> Result<(u64, Span), EVMError> { - let data_word_size = ((input.len() + 31) / 32).into(); - let gas = BASE_COST + data_word_size * COST_PER_WORD; + let data_word_size = bytes_32_words_size(input.len()); + let gas = BASE_COST + data_word_size.into() * COST_PER_WORD; return Result::Ok((gas, input)); } diff --git a/crates/evm/src/precompiles/p256verify.cairo b/crates/evm/src/precompiles/p256verify.cairo index fd5efe49f..f0bbc4948 100644 --- a/crates/evm/src/precompiles/p256verify.cairo +++ b/crates/evm/src/precompiles/p256verify.cairo @@ -57,35 +57,11 @@ pub impl P256Verify of Precompile { return Result::Ok((gas, [].span())); } - let message_hash = input.slice(0, 32); - let message_hash = match message_hash.from_be_bytes() { - Option::Some(message_hash) => message_hash, - Option::None => { return Result::Ok((gas, [].span())); } - }; - - let r: Option = input.slice(32, 32).from_be_bytes(); - let r = match r { - Option::Some(r) => r, - Option::None => { return Result::Ok((gas, [].span())); } - }; - - let s: Option = input.slice(64, 32).from_be_bytes(); - let s = match s { - Option::Some(s) => s, - Option::None => { return Result::Ok((gas, [].span())); } - }; - - let x: Option = input.slice(96, 32).from_be_bytes(); - let x = match x { - Option::Some(x) => x, - Option::None => { return Result::Ok((gas, [].span())); } - }; - - let y: Option = input.slice(128, 32).from_be_bytes(); - let y = match y { - Option::Some(y) => y, - Option::None => { return Result::Ok((gas, [].span())); } - }; + let message_hash: u256 = input.slice(0, 32).from_be_bytes().unwrap(); + let r: u256 = input.slice(32, 32).from_be_bytes().unwrap(); + let s: u256 = input.slice(64, 32).from_be_bytes().unwrap(); + let x: u256 = input.slice(96, 32).from_be_bytes().unwrap(); + let y: u256 = input.slice(128, 32).from_be_bytes().unwrap(); let public_key: Option = Secp256Trait::secp256_ec_new_syscall(x, y) .unwrap_syscall(); diff --git a/crates/evm/src/precompiles/sha256.cairo b/crates/evm/src/precompiles/sha256.cairo index af9174531..60be11a69 100644 --- a/crates/evm/src/precompiles/sha256.cairo +++ b/crates/evm/src/precompiles/sha256.cairo @@ -2,6 +2,7 @@ use core::sha256::compute_sha256_u32_array; use core::starknet::EthAddress; use crate::errors::EVMError; use crate::precompiles::Precompile; +use utils::helpers::bytes_32_words_size; use utils::math::Bitshift; use utils::traits::bytes::{FromBytes, ToBytes}; @@ -15,8 +16,8 @@ pub impl Sha256 of Precompile { } fn exec(mut input: Span) -> Result<(u64, Span), EVMError> { - let data_word_size = ((input.len() + 31) / 32).into(); - let gas = BASE_COST + data_word_size * COST_PER_WORD; + let data_word_size = bytes_32_words_size(input.len()); + let gas = BASE_COST + data_word_size.into() * COST_PER_WORD; let mut sha256_input: Array = array![]; while let Option::Some(bytes4) = input.multi_pop_front::<4>() { diff --git a/crates/utils/src/helpers.cairo b/crates/utils/src/helpers.cairo index 94e99eafd..0c84cb933 100644 --- a/crates/utils/src/helpers.cairo +++ b/crates/utils/src/helpers.cairo @@ -8,11 +8,8 @@ use core::panic_with_felt252; use core::pedersen::PedersenTrait; use core::starknet::{EthAddress, ContractAddress, ClassHash}; use core::traits::TryInto; -use core::traits::{DivRem}; use crate::constants::{CONTRACT_ADDRESS_PREFIX, MAX_ADDRESS}; use crate::constants::{POW_2, POW_256_1, POW_256_REV}; -use crate::math::{Bitshift, WrappingBitshift}; - use crate::traits::array::{ArrayExtTrait}; use crate::traits::{U256TryIntoContractAddress, EthAddressIntoU256, BoolIntoNumeric};