From 88479f320fa68aaf35c2b7dfad829c1ca1700f52 Mon Sep 17 00:00:00 2001 From: Yusuf Habib <109147010+manlikeHB@users.noreply.github.com> Date: Mon, 30 Sep 2024 10:59:27 +0100 Subject: [PATCH 01/22] validate eth tx test (#986) * add validate_eth_tx test * polish: fmt + docstrings --- crates/evm/src/backend/validation.cairo | 178 ++++++++++++++++++++---- 1 file changed, 153 insertions(+), 25 deletions(-) diff --git a/crates/evm/src/backend/validation.cairo b/crates/evm/src/backend/validation.cairo index 17b8da15c..91eff839e 100644 --- a/crates/evm/src/backend/validation.cairo +++ b/crates/evm/src/backend/validation.cairo @@ -20,8 +20,6 @@ pub fn validate_eth_tx(kakarot_state: @KakarotCore::ContractState, tx: Transacti let kakarot_storage = kakarot_state.snapshot_deref().storage(); // Validate transaction - //TODO: add case for eip155 transactions - // Validate chain_id for post eip155 let tx_chain_id = tx.chain_id(); let kakarot_chain_id: u64 = kakarot_state.eth_chain_id(); @@ -64,7 +62,9 @@ mod tests { use core::num::traits::Bounded; use core::ops::SnapshotDeref; + use core::starknet::ContractAddress; use core::starknet::storage::StorageTrait; + use evm::gas; use snforge_std::cheatcodes::storage::store_felt252; use snforge_std::{ start_mock_call, test_address, start_cheat_chain_id_global, store, @@ -74,9 +74,10 @@ mod tests { use utils::constants::BLOCK_GAS_LIMIT; use utils::eth_transaction::common::TxKind; use utils::eth_transaction::eip1559::TxEip1559; + use utils::eth_transaction::legacy::TxLegacy; use utils::eth_transaction::transaction::Transaction; - fn set_up() -> KakarotCore::ContractState { + fn set_up() -> (KakarotCore::ContractState, ContractAddress) { // Define the addresses used in the tests, whose calls will be mocked let kakarot_state = KakarotCore::unsafe_new_contract_state(); let kakarot_storage = kakarot_state.snapshot_deref().storage(); @@ -102,13 +103,13 @@ mod tests { native_token_address, selector!("balanceOf"), Bounded::::MAX ); // Min to pay for gas + value - kakarot_state + (kakarot_state, native_token_address) } #[test] fn test_validate_eth_tx_typical_case() { // Setup the environment - let kakarot_state = set_up(); + let (kakarot_state, _) = set_up(); // Create a transaction object for the test let tx = Transaction::Eip1559( @@ -132,51 +133,178 @@ mod tests { } #[test] - #[ignore] + #[should_panic(expected: ('Invalid chain id',))] fn test_validate_eth_tx_invalid_chain_id() { - panic!("unimplemented"); + // Setup the environment + let (kakarot_state, _) = set_up(); + + // Create a transaction object for the test + let tx = Transaction::Eip1559( + TxEip1559 { + chain_id: 600, // wrong chain_id + nonce: 0, + max_priority_fee_per_gas: 1_000_000_000, // 1 Gwei + max_fee_per_gas: 2_000_000_000, // 2 Gwei + gas_limit: 21000, // Standard gas limit for a simple transfer + to: TxKind::Call(0x1234567890123456789012345678901234567890.try_into().unwrap()), + value: 1000000000000000000_u256, // 1 ETH + input: array![].span(), + access_list: array![].span(), + } + ); + + // Test that the function performs validation and assert expected results + validate_eth_tx(@kakarot_state, tx); } #[test] - #[ignore] + #[should_panic(expected: ('Invalid nonce',))] fn test_validate_eth_tx_invalid_nonce() { - panic!("unimplemented"); + // Setup the environment + let (kakarot_state, _) = set_up(); + + // Create a transaction object for the test + let tx = Transaction::Eip1559( + TxEip1559 { + chain_id: 1, // Should match the chain_id in the environment + nonce: 600, //Invalid nonce + max_priority_fee_per_gas: 1_000_000_000, // 1 Gwei + max_fee_per_gas: 2_000_000_000, // 2 Gwei + gas_limit: 21000, // Standard gas limit for a simple transfer + to: TxKind::Call(0x1234567890123456789012345678901234567890.try_into().unwrap()), + value: 1000000000000000000_u256, // 1 ETH + input: array![].span(), + access_list: array![].span(), + } + ); + + // Test that the function performs validation and assert expected results + validate_eth_tx(@kakarot_state, tx); } #[test] - #[ignore] + #[should_panic(expected: ('Tx gas > Block gas',))] fn test_validate_eth_tx_gas_limit_exceeds_block_gas_limit() { - panic!("unimplemented"); + // Setup the environment + let (kakarot_state, _) = set_up(); + + // Create a transaction object for the test + let tx = Transaction::Eip1559( + TxEip1559 { + chain_id: 1, // Should match the chain_id in the environment + nonce: 0, + max_priority_fee_per_gas: 1_000_000_000, // 1 Gwei + max_fee_per_gas: 2_000_000_000, // 2 Gwei + gas_limit: BLOCK_GAS_LIMIT + 1, // Gas limit greater than block gas limit + to: TxKind::Call(0x1234567890123456789012345678901234567890.try_into().unwrap()), + value: 1000000000000000000_u256, // 1 ETH + input: array![].span(), + access_list: array![].span(), + } + ); + + // Test that the function performs validation and assert expected results + validate_eth_tx(@kakarot_state, tx); } #[test] - #[ignore] + #[should_panic(expected: ('Intrinsic gas > gas limit',))] fn test_validate_eth_tx_intrinsic_gas_exceeds_gas_limit() { - panic!("unimplemented"); + // Setup the environment + let (kakarot_state, _) = set_up(); + + // Create a transaction object for the test + let mut tx = Transaction::Eip1559( + TxEip1559 { + chain_id: 1, // Should match the chain_id in the environment + nonce: 0, + max_priority_fee_per_gas: 1_000_000_000, // 1 Gwei + max_fee_per_gas: 2_000_000_000, // 2 Gwei + gas_limit: 0, + to: TxKind::Call(0x1234567890123456789012345678901234567890.try_into().unwrap()), + value: 1000000000000000000_u256, // 1 ETH + input: array![].span(), + access_list: array![].span(), + } + ); + + // Test that the function performs validation and assert expected results + validate_eth_tx(@kakarot_state, tx); } #[test] - #[ignore] + #[should_panic(expected: ('Not enough ETH',))] fn test_validate_eth_tx_insufficient_balance() { - panic!("unimplemented"); - } + // Setup the environment + let (kakarot_state, native_token_address) = set_up(); - #[test] - #[ignore] - fn test_validate_eth_tx_effective_gas_price_errors() { - panic!("unimplemented"); + // Create a transaction object for the test + let tx = Transaction::Eip1559( + TxEip1559 { + chain_id: 1, // Should match the chain_id in the environment + nonce: 0, + max_priority_fee_per_gas: 1_000_000_000, // 1 Gwei + max_fee_per_gas: 2_000_000_000, // 2 Gwei + gas_limit: 21000, // Standard gas limit for a simple transfer + to: TxKind::Call(0x1234567890123456789012345678901234567890.try_into().unwrap()), + value: 1000000000000000000_u256, // 1 ETH + input: array![].span(), + access_list: array![].span(), + } + ); + + start_mock_call(native_token_address, selector!("balanceOf"), Bounded::::MIN); + + // Test that the function performs validation and assert expected results + validate_eth_tx(@kakarot_state, tx); } #[test] - #[ignore] fn test_validate_eth_tx_max_gas_limit() { - panic!("unimplemented"); + // Setup the environment + let (kakarot_state, _) = set_up(); + + // Create a transaction object for the test + let tx = Transaction::Eip1559( + TxEip1559 { + chain_id: 1, // Should match the chain_id in the environment + nonce: 0, + max_priority_fee_per_gas: 1_000_000_000, // 1 Gwei + max_fee_per_gas: 2_000_000_000, // 2 Gwei + gas_limit: BLOCK_GAS_LIMIT, // Gas limit = Max block gas limit + to: TxKind::Call(0x1234567890123456789012345678901234567890.try_into().unwrap()), + value: 1000000000000000000_u256, // 1 ETH + input: array![].span(), + access_list: array![].span(), + } + ); + + // Test that the function performs validation and assert expected results + let intrinsic_gas = validate_eth_tx(@kakarot_state, tx); + assert_eq!(intrinsic_gas, 21000); // Standard intrinsic gas for a simple transfer } #[test] - #[ignore] fn test_validate_eth_tx_pre_eip155() { - //TODO: implement pre-eip155 logic - panic!("unimplemented"); + // Setup the environment + let (kakarot_state, _) = set_up(); + + // Create a transaction object for the test + let tx = Transaction::Legacy( + TxLegacy { + chain_id: Option::None, + nonce: 0, + gas_price: 120000000000000000000000000, + gas_limit: 21000, + to: TxKind::Call(0x1234567890123456789012345678901234567890.try_into().unwrap()), + value: 1000000000000000000_u256, // Standard gas limit for a simple transfer + input: array![].span(), + } + ); + + // Test that the function performs validation and assert expected results + let intrinsic_gas = validate_eth_tx(@kakarot_state, tx); + + assert_eq!(intrinsic_gas, 21000); // Standard intrinsic gas for a simple transfer } } From c645fee949e769f820f39841e6c146b39f9ab62b Mon Sep 17 00:00:00 2001 From: fab Date: Mon, 30 Sep 2024 03:59:38 -0600 Subject: [PATCH 02/22] refactor: replace `while` loops with `for` (#987) * First batch of replacements * Second batch of replacements * Clean commented code * scarb fmt * Apply code review recommendations --- crates/contracts/src/storage.cairo | 20 ++-- crates/evm/src/backend/starknet_backend.cairo | 4 +- .../instructions/duplication_operations.cairo | 11 +- .../environmental_information.cairo | 55 +++++---- .../src/instructions/push_operations.cairo | 3 +- crates/evm/src/instructions/sha3.cairo | 22 ++-- crates/evm/src/memory.cairo | 18 ++- crates/evm/src/precompiles.cairo | 11 +- crates/evm/src/precompiles/blake2f.cairo | 17 +-- .../precompiles/ec_operations/ec_mul.cairo | 2 +- crates/evm/src/stack.cairo | 7 +- crates/evm/src/state.cairo | 2 +- crates/snforge_utils/src/lib.cairo | 103 +++++++++-------- crates/utils/src/crypto/blake2_compress.cairo | 4 +- .../utils/src/eth_transaction/eip2930.cairo | 2 +- crates/utils/src/felt_vec.cairo | 43 +++---- crates/utils/src/helpers.cairo | 52 ++++----- crates/utils/src/rlp.cairo | 2 +- crates/utils/src/serialization.cairo | 17 +-- crates/utils/src/traits.cairo | 11 +- crates/utils/src/traits/array.cairo | 4 +- crates/utils/src/traits/bytes.cairo | 108 ++++++++---------- crates/utils/src/traits/eth_address.cairo | 23 ++-- crates/utils/src/traits/integer.cairo | 2 +- 24 files changed, 234 insertions(+), 309 deletions(-) diff --git a/crates/contracts/src/storage.cairo b/crates/contracts/src/storage.cairo index c2501e69d..3f047b1a8 100644 --- a/crates/contracts/src/storage.cairo +++ b/crates/contracts/src/storage.cairo @@ -18,7 +18,7 @@ pub struct StorageBytecode { const BYTES_PER_FELT: NonZero = 31; -/// An implamentation of the `Store` trait for our specific `StorageBytecode` type. +/// An implementation of the `Store` trait for our specific `StorageBytecode` type. /// The packing-unpacking is done inside the `read` and `write` methods, thus transparent to the /// user. /// The bytecode is stored sequentially, starting from storage address 0, for compatibility purposes @@ -38,13 +38,13 @@ impl StoreBytecode of Store { // afterwards. let base: felt252 = 0; let mut packed_bytecode = array![]; - let mut i = 0; - while i != (chunks_count + 1) { - let storage_address: StorageAddress = (base + i.into()).try_into().unwrap(); - let chunk = storage_read_syscall(address_domain, storage_address).unwrap(); - packed_bytecode.append(chunk); - i += 1; - }; + for i in 0 + ..chunks_count + + 1 { + let storage_address: StorageAddress = (base + i.into()).try_into().unwrap(); + let chunk = storage_read_syscall(address_domain, storage_address).unwrap(); + packed_bytecode.append(chunk); + }; let bytecode = load_packed_bytes(packed_bytecode.span(), bytecode_len); SyscallResult::Ok(StorageBytecode { bytecode: bytecode.span() }) } @@ -131,10 +131,8 @@ mod tests { fn test_store_bytecode_multiple_chunks() { let mut state = account_contract_state(); let mut bytecode_array = array![]; - let mut i = 0; - while i != 100 { + for i in 0..100_u8 { bytecode_array.append(i); - i += 1; }; let bytecode = bytecode_array.span(); // Write the bytecode to the storage diff --git a/crates/evm/src/backend/starknet_backend.cairo b/crates/evm/src/backend/starknet_backend.cairo index c5cba92c7..dba29c576 100644 --- a/crates/evm/src/backend/starknet_backend.cairo +++ b/crates/evm/src/backend/starknet_backend.cairo @@ -138,7 +138,7 @@ pub fn fetch_balance(self: @Address) -> u256 { /// `Ok(())` if the commit was successful, otherwise an `EVMError`. fn commit_accounts(ref state: State) -> Result<(), EVMError> { let mut account_keys = state.accounts.keyset.to_span(); - while let Option::Some(evm_address) = account_keys.pop_front() { + for evm_address in account_keys { let account = state.accounts.changes.get(*evm_address).deref(); commit_account(@account, ref state); }; @@ -231,7 +231,7 @@ fn emit_events(ref self: State) -> Result<(), EVMError> { /// commit_storage MUST be called after commit_accounts. fn commit_storage(ref self: State) -> Result<(), EVMError> { let mut storage_keys = self.accounts_storage.keyset.to_span(); - while let Option::Some(state_key) = storage_keys.pop_front() { + for state_key in storage_keys { let (evm_address, key, value) = self.accounts_storage.changes.get(*state_key).deref(); let mut account = self.get_account(evm_address); // @dev: EIP-6780 - If selfdestruct on an account created, dont commit data diff --git a/crates/evm/src/instructions/duplication_operations.cairo b/crates/evm/src/instructions/duplication_operations.cairo index db322d326..5752279ba 100644 --- a/crates/evm/src/instructions/duplication_operations.cairo +++ b/crates/evm/src/instructions/duplication_operations.cairo @@ -123,25 +123,20 @@ mod tests { // ensures all values start from index `from` upto index `to` of stack are `0x0` fn ensures_zeros(ref stack: Stack, from: u32, to: u32) { - let mut idx: u32 = from; - if to > from { return; } - while idx != to { + for idx in from..to { assert(stack.peek_at(idx).unwrap() == 0x00, 'should be zero'); - idx += 1; }; } // push `n` number of `0x0` to the stack fn push_zeros(ref stack: Stack, n: u8) { - let mut i = 0; - while i != n { + for _ in 0..n { stack.push(0x0).unwrap(); - i += 1; - } + }; } #[test] diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index 2661198fa..bcb0729f0 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -90,10 +90,8 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { // Fill the rest of the data to load with zeros // TODO: optimize once we have dw-based exponentiation - let mut i = 32 - bytes_len; - while i != 0 { + for _ in 0..32 - bytes_len { data_to_load *= 256; - i -= 1; }; self.stack.push(data_to_load) } @@ -652,21 +650,22 @@ mod tests { // Memory initialization with a value to verify that if the offset + size is out of the // bound bytes, 0's have been copied. // Otherwise, the memory value would be 0, and we wouldn't be able to check it. - let mut i = 0; - while i != (size / 32) + 1 { - vm - .memory - .store( - 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF, - dest_offset + (i * 32) - ); - - let initial: u256 = vm.memory.load_internal(dest_offset + (i * 32)).into(); - - assert_eq!(initial, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF); - - i += 1; - }; + for i in 0 + ..(size / 32) + + 1 { + vm + .memory + .store( + 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF, + dest_offset + (i * 32) + ); + + let initial: u256 = vm.memory.load_internal(dest_offset + (i * 32)).into(); + + assert_eq!( + initial, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF + ); + }; // When vm.exec_calldatacopy().expect('exec_calldatacopy failed'); @@ -775,17 +774,15 @@ mod tests { let result: u256 = vm.memory.load_internal(dest_offset).into(); let mut results: Array = u256_to_bytes_array(result); - let mut i = 0; - while i != size { - // For out of bound bytes, 0s will be copied. - if (i + offset >= bytecode.len()) { - assert_eq!(*results[i], 0); - } else { - assert_eq!(*results[i], *bytecode[i + offset]); - } - - i += 1; - }; + for i in 0 + ..size { + // For out of bound bytes, 0s will be copied. + if (i + offset >= bytecode.len()) { + assert_eq!(*results[i], 0); + } else { + assert_eq!(*results[i], *bytecode[i + offset]); + } + }; } // ************************************************************************* diff --git a/crates/evm/src/instructions/push_operations.cairo b/crates/evm/src/instructions/push_operations.cairo index 31a53a698..d6bd057cb 100644 --- a/crates/evm/src/instructions/push_operations.cairo +++ b/crates/evm/src/instructions/push_operations.cairo @@ -333,9 +333,8 @@ mod tests { fn get_n_0xFF(mut n: u8) -> Span { let mut array: Array = ArrayTrait::new(); array.append(0x00); - while n != 0 { + for _ in 0..n { array.append(0xFF); - n -= 1; }; array.span() } diff --git a/crates/evm/src/instructions/sha3.cairo b/crates/evm/src/instructions/sha3.cairo index e04e36590..563a5fa12 100644 --- a/crates/evm/src/instructions/sha3.cairo +++ b/crates/evm/src/instructions/sha3.cairo @@ -102,17 +102,17 @@ fn compute_memory_words_amount(size: u32, offset: u32, mem_len: u32) -> (u32, u3 fn fill_array_with_memory_words( ref self: VM, ref to_hash: Array, mut offset: u32, mut amount: u32 ) -> u32 { - while amount != 0 { - let loaded = self.memory.load(offset); - let ((high_h, low_h), (high_l, low_l)) = loaded.split_into_u64_le(); - to_hash.append(low_h); - to_hash.append(high_h); - to_hash.append(low_l); - to_hash.append(high_l); - - offset += 32; - amount -= 1; - }; + for _ in 0 + ..amount { + let loaded = self.memory.load(offset); + let ((high_h, low_h), (high_l, low_l)) = loaded.split_into_u64_le(); + to_hash.append(low_h); + to_hash.append(high_h); + to_hash.append(low_l); + to_hash.append(high_l); + + offset += 32; + }; offset } diff --git a/crates/evm/src/memory.cairo b/crates/evm/src/memory.cairo index b24ec7ca7..601fc1bbc 100644 --- a/crates/evm/src/memory.cairo +++ b/crates/evm/src/memory.cairo @@ -369,12 +369,12 @@ pub(crate) impl InternalMemoryMethods of InternalMemoryTrait { fn load_aligned_words( ref self: Memory, mut chunk_index: usize, final_chunk: usize, ref elements: Array ) { - while chunk_index != final_chunk { - let value = self.items.get(chunk_index.into()); - // Pushes 16 items to `elements` - helpers::split_word_128(value.into(), ref elements); - chunk_index += 1; - } + for i in chunk_index + ..final_chunk { + let value = self.items.get(i.into()); + // Pushes 16 items to `elements` + helpers::split_word_128(value.into(), ref elements); + }; } /// Loads a `u256` element from the memory chunk at a specified offset. @@ -810,10 +810,8 @@ mod tests { memory.load_n_internal(16, ref results, 0); assert(results.len() == 16, 'error'); - let mut i = 0; - while i != results.len() { - assert(*results[i] == 0xFF, 'byte value loaded not correct'); - i += 1; + for result in results { + assert(result == 0xFF, 'byte value loaded not correct'); } } diff --git a/crates/evm/src/precompiles.cairo b/crates/evm/src/precompiles.cairo index a467a7b60..ff8dd9386 100644 --- a/crates/evm/src/precompiles.cairo +++ b/crates/evm/src/precompiles.cairo @@ -39,12 +39,11 @@ pub const FIRST_ROLLUP_PRECOMPILE_ADDRESS: u256 = 0x100; /// * `Set` - A set containing all Ethereum precompile addresses. pub fn eth_precompile_addresses() -> Set { let mut precompile_addresses: Array = array![]; - //TODO(2.8) use range operator - let mut i = FIRST_ETHEREUM_PRECOMPILE_ADDRESS; - while i <= LAST_ETHEREUM_PRECOMPILE_ADDRESS { - precompile_addresses.append(i.try_into().unwrap()); - i = i + 1; - }; + for i in FIRST_ETHEREUM_PRECOMPILE_ADDRESS + ..LAST_ETHEREUM_PRECOMPILE_ADDRESS + + 0x01 { + precompile_addresses.append(i.try_into().unwrap()); + }; SetTrait::from_array(precompile_addresses) } diff --git a/crates/evm/src/precompiles/blake2f.cairo b/crates/evm/src/precompiles/blake2f.cairo index 94ecdd935..ac16b81d2 100644 --- a/crates/evm/src/precompiles/blake2f.cairo +++ b/crates/evm/src/precompiles/blake2f.cairo @@ -39,22 +39,18 @@ pub impl Blake2f of Precompile { let mut h: Array = Default::default(); let mut m: Array = Default::default(); - let mut i = 0; let mut pos = 4; - while i != 8 { + for _ in 0..8_u8 { // safe unwrap, because we have made sure of the input length to be 213 h.append(input.slice(pos, 8).from_le_bytes().unwrap()); - i += 1; pos += 8; }; - let mut i = 0; let mut pos = 68; - while i != 16 { + for _ in 0..16_u8 { // safe unwrap, because we have made sure of the input length to be 213 m.append(input.slice(pos, 8).from_le_bytes().unwrap()); - i += 1; - pos += 8; + pos += 8 }; let mut t: Array = Default::default(); @@ -68,12 +64,9 @@ pub impl Blake2f of Precompile { let mut return_data: Array = Default::default(); - let mut i = 0; - while i != res.len() { - let bytes = (*res[i]).to_le_bytes_padded(); + for result in res { + let bytes = (*result).to_le_bytes_padded(); return_data.append_span(bytes); - - i += 1; }; Result::Ok((gas, return_data.span())) diff --git a/crates/evm/src/precompiles/ec_operations/ec_mul.cairo b/crates/evm/src/precompiles/ec_operations/ec_mul.cairo index 6c94d7d97..814d7e0e4 100644 --- a/crates/evm/src/precompiles/ec_operations/ec_mul.cairo +++ b/crates/evm/src/precompiles/ec_operations/ec_mul.cairo @@ -117,7 +117,7 @@ fn get_bits_little(s: u256) -> Array { fn ec_mul_inner(pt: (u384, u384), mut bits: Array) -> Option<(u384, u384)> { let (mut temp_x, mut temp_y) = pt; let mut result: Option<(u384, u384)> = Option::None; - while let Option::Some(bit) = bits.pop_front() { + for bit in bits { if bit != 0 { match result { Option::Some((xr, yr)) => result = ec_safe_add(temp_x, temp_y, xr, yr), diff --git a/crates/evm/src/stack.cairo b/crates/evm/src/stack.cairo index 06e7896f7..506e87d9f 100644 --- a/crates/evm/src/stack.cairo +++ b/crates/evm/src/stack.cairo @@ -208,9 +208,8 @@ impl StackImpl of StackTrait { fn pop_n(ref self: Stack, mut n: usize) -> Result, EVMError> { ensure(!(n > self.len()), EVMError::StackUnderflow)?; let mut popped_items = ArrayTrait::::new(); - while n != 0 { + for _ in 0..n { popped_items.append(self.pop().unwrap()); - n -= 1; }; Result::Ok(popped_items) } @@ -346,11 +345,9 @@ mod tests { fn test_should_fail_when_overflow() { // Given let mut stack = StackTrait::new(); - let mut i = 0; // When - while i != constants::STACK_MAX_DEPTH { - i += 1; + for _ in 0..constants::STACK_MAX_DEPTH { stack.push(1).unwrap(); }; diff --git a/crates/evm/src/state.cairo b/crates/evm/src/state.cairo index 1dd6fde36..0ea8ce3ca 100644 --- a/crates/evm/src/state.cairo +++ b/crates/evm/src/state.cairo @@ -93,7 +93,7 @@ impl StateChangeLogImpl, +Copy> of StateChangeLogTrait { fn clone(ref self: StateChangeLog) -> StateChangeLog { let mut cloned_changes = Default::default(); let mut keyset_span = self.keyset.to_span(); - while let Option::Some(key) = keyset_span.pop_front() { + for key in keyset_span { let value = self.changes.get(*key).deref(); cloned_changes.insert(*key, NullableTrait::new(value)); }; diff --git a/crates/snforge_utils/src/lib.cairo b/crates/snforge_utils/src/lib.cairo index 9809c2aec..0588fe66c 100644 --- a/crates/snforge_utils/src/lib.cairo +++ b/crates/snforge_utils/src/lib.cairo @@ -216,36 +216,35 @@ pub mod snforge_utils { fn build(self: @EventsFilter) -> ContractEvents { let events = (*self.events.events).span(); let mut filtered_events = array![]; - let mut i = 0; - while i < events.len() { - let (from, event) = events.at(i).clone(); - let mut include = true; + for i in 0 + ..events + .len() { + let (from, event) = events.at(i).clone(); + let mut include = true; - if let Option::Some(addr) = self.contract_address { - if from != *addr { - include = false; - } - } - - if include && self.key_filter.is_some() { - if !(event.keys.span() == (*self.key_filter).unwrap()) { - include = false; - } - } + if let Option::Some(addr) = self.contract_address { + if from != *addr { + include = false; + } + } - if include && self.data_filter.is_some() { - if !event.data.includes((*self.data_filter).unwrap()) { - include = false; - } - } + if include && self.key_filter.is_some() { + if !(event.keys.span() == (*self.key_filter).unwrap()) { + include = false; + } + } - if include { - filtered_events.append(event.clone()); - } + if include && self.data_filter.is_some() { + if !event.data.includes((*self.data_filter).unwrap()) { + include = false; + } + } - i += 1; - }; + if include { + filtered_events.append(event.clone()); + } + }; ContractEvents { events: filtered_events } } @@ -268,16 +267,17 @@ pub mod snforge_utils { let mut expected_data = array![]; event.append_keys_and_data(ref expected_keys, ref expected_data); - let mut i = 0; let mut found = false; - while i < self.events.len() { - let event = self.events.at(i); - if event.keys == @expected_keys && event.data == @expected_data { - found = true; - break; - } - i += 1; - }; + for i in 0 + ..self + .events + .len() { + let event = self.events.at(i); + if event.keys == @expected_keys && event.data == @expected_data { + found = true; + break; + } + }; assert(found, 'Expected event was not emitted'); } @@ -289,15 +289,16 @@ pub mod snforge_utils { let mut expected_data = array![]; event.append_keys_and_data(ref expected_keys, ref expected_data); - let mut i = 0; - while i < self.events.len() { - let event = self.events.at(i); - assert( - event.keys != @expected_keys || event.data != @expected_data, - 'Unexpected event was emitted' - ); - i += 1; - } + for i in 0 + ..self + .events + .len() { + let event = self.events.at(i); + assert( + event.keys != @expected_keys || event.data != @expected_data, + 'Unexpected event was emitted' + ); + } } } @@ -305,12 +306,14 @@ pub mod snforge_utils { pub fn store_evm(target: Address, evm_key: u256, evm_value: u256) { let storage_address = compute_storage_key(target.evm, evm_key); let serialized_value = [evm_value.low.into(), evm_value.high.into()].span(); - let mut offset: usize = 0; - while offset != serialized_value.len() { - store_felt252( - target.starknet, storage_address + offset.into(), *serialized_value.at(offset) - ); - offset += 1; - } + for offset in 0 + ..serialized_value + .len() { + store_felt252( + target.starknet, + storage_address + offset.into(), + *serialized_value.at(offset) + ); + }; } } diff --git a/crates/utils/src/crypto/blake2_compress.cairo b/crates/utils/src/crypto/blake2_compress.cairo index 4ce2840e4..e008e7271 100644 --- a/crates/utils/src/crypto/blake2_compress.cairo +++ b/crates/utils/src/crypto/blake2_compress.cairo @@ -70,10 +70,8 @@ fn rotate_right(value: u64, n: u32) -> u64 { /// updated state vector pub fn compress(rounds: usize, h: Span, m: Span, t: Span, f: bool) -> Span { let mut v = VecTrait::::new(); - let mut i = 0; - while i != 16 { + for _ in 0..16_u8 { v.push(0); - i += 1; }; let IV = IV(); diff --git a/crates/utils/src/eth_transaction/eip2930.cairo b/crates/utils/src/eth_transaction/eip2930.cairo index 2766720a9..d59ed6be3 100644 --- a/crates/utils/src/eth_transaction/eip2930.cairo +++ b/crates/utils/src/eth_transaction/eip2930.cairo @@ -17,7 +17,7 @@ pub impl AccessListItemImpl of AccessListItemTrait { let AccessListItem { ethereum_address, mut storage_keys } = *self; let mut storage_keys_arr = array![]; - while let Option::Some(storage_key) = storage_keys.pop_front() { + for storage_key in storage_keys { storage_keys_arr.append((ethereum_address, *storage_key)); }; diff --git a/crates/utils/src/felt_vec.cairo b/crates/utils/src/felt_vec.cairo index 82706e75d..cff02ceb0 100644 --- a/crates/utils/src/felt_vec.cairo +++ b/crates/utils/src/felt_vec.cairo @@ -56,17 +56,16 @@ pub impl Felt252VecTraitImpl< /// * A Span representing bytes conversion of `self` in little endian format fn to_le_bytes(ref self: Felt252Vec) -> Span { let mut res: Array = array![]; - let mut i = 0; - - while i != self.len() { - if self[i] == Zero::zero() { - res.append(Zero::zero()); - } else { - res.append_span(self[i].to_le_bytes()); - } - i += 1; - }; + for i in 0 + ..self + .len() { + if self[i] == Zero::zero() { + res.append(Zero::zero()); + } else { + res.append_span(self[i].to_le_bytes()); + } + }; res.span() } @@ -214,10 +213,8 @@ pub impl Felt252VecTraitImpl< } let stop = idx + vec.len(); - let mut i = idx; - while i != stop { + for i in idx..stop { self.set(i, vec[i - idx]); - i += 1; }; Result::Ok(()) @@ -266,11 +263,8 @@ pub impl Felt252VecTraitImpl< fn duplicate(ref self: Felt252Vec) -> Felt252Vec { let mut new_vec = Default::default(); - let mut i: u32 = 0; - - while i != self.len { + for i in 0..self.len { new_vec.push(self[i]); - i += 1; }; new_vec @@ -297,12 +291,8 @@ pub impl Felt252VecTraitImpl< fn clone_slice(ref self: Felt252Vec, idx: usize, len: usize) -> Felt252Vec { let mut new_vec = Default::default(); - let mut i: u32 = 0; - - while i != len { + for i in 0..len { new_vec.push(self[idx + i]); - - i += 1; }; new_vec @@ -328,14 +318,12 @@ pub impl Felt252VecTraitImpl< return false; }; - let mut i = 0; let mut result = true; - while i != lhs.len() { + for i in 0..lhs.len() { if lhs[i] != rhs[i] { result = false; break; } - i += 1; }; result } @@ -368,11 +356,8 @@ pub impl Felt252VecTraitImpl< return Result::Err(Felt252VecTraitErrors::Overflow); } - let mut i = start_idx; - while i != start_idx + len { + for i in start_idx..start_idx + len { self.set(i, value); - - i += 1; }; Result::Ok(()) diff --git a/crates/utils/src/helpers.cairo b/crates/utils/src/helpers.cairo index d10b7ad6d..d50eb3418 100644 --- a/crates/utils/src/helpers.cairo +++ b/crates/utils/src/helpers.cairo @@ -87,10 +87,9 @@ pub fn split_word(mut value: u256, mut len: usize, ref dst: Array) { /// * `value` - The u128 value to split into bytes /// * `len` - The number of bytes to split the value into pub fn split_u128_le(ref dest: Array, mut value: u128, mut len: usize) { - while len != 0 { + for _ in 0..len { dest.append((value % 256).try_into().unwrap()); value /= 256; - len -= 1; }; } @@ -137,13 +136,13 @@ pub fn load_word(mut len: usize, words: Span) -> u256 { let mut current: u256 = 0; let mut counter = 0; - while len != 0 { - let loaded: u8 = *words[counter]; - let tmp = current * 256; - current = tmp + loaded.into(); - len -= 1; - counter += 1; - }; + for _ in 0 + ..len { + let loaded: u8 = *words[counter]; + let tmp = current * 256; + current = tmp + loaded.into(); + counter += 1; + }; current } @@ -156,22 +155,20 @@ pub fn load_word(mut len: usize, words: Span) -> u256 { /// # Returns /// An `Array` representing the big-endian byte representation of the input value. pub fn u256_to_bytes_array(mut value: u256) -> Array { - let mut counter = 0; let mut bytes_arr: Array = ArrayTrait::new(); // low part - while counter != 16 { - bytes_arr.append((value.low & 0xFF).try_into().unwrap()); - value.low /= 256; - counter += 1; - }; + for _ in 0 + ..16_u8 { + bytes_arr.append((value.low & 0xFF).try_into().unwrap()); + value.low /= 256; + }; - let mut counter = 0; // high part - while counter != 16 { - bytes_arr.append((value.high & 0xFF).try_into().unwrap()); - value.high /= 256; - counter += 1; - }; + for _ in 0 + ..16_u8 { + bytes_arr.append((value.high & 0xFF).try_into().unwrap()); + value.high /= 256; + }; // Reverse the array as memory is arranged in big endian order. let mut counter = bytes_arr.len(); @@ -277,11 +274,8 @@ mod tests { // 16 bytes values let mut arr6 = ArrayTrait::new(); - arr6.append(0xff); - let mut counter: u128 = 0; - while counter < 15 { + for _ in 0..16_u8 { arr6.append(0xff); - counter += 1; }; let res6 = helpers::load_word(16, arr6.span()); assert(340282366920938463463374607431768211455 == res6, 'res6: wrong load'); @@ -319,10 +313,8 @@ mod tests { assert(res4.len() == 16, 'res4: wrong length'); assert(*res4[0] == 0xfe, 'res4: wrong MSB value'); - let mut counter: usize = 1; - while counter < 16 { + for counter in 1..16_u32 { assert(*res4[counter] == 0xff, 'res4: wrong value at index'); - counter += 1; }; } @@ -360,11 +352,9 @@ mod tests { let mut dst4: Array = ArrayTrait::new(); helpers::split_word(max_u128, 16, ref dst4); assert(dst4.len() == 16, 'dst4: wrong length'); - let mut counter: usize = 0; assert(*dst4[15] == 0xfe, 'dst4: wrong LSB value'); - while counter < 15 { + for counter in 0..dst4.len() - 1 { assert_eq!(*dst4[counter], 0xff); - counter += 1; }; } } diff --git a/crates/utils/src/rlp.cairo b/crates/utils/src/rlp.cairo index 513fd52a9..9c991671e 100644 --- a/crates/utils/src/rlp.cairo +++ b/crates/utils/src/rlp.cairo @@ -100,7 +100,7 @@ pub impl RLPImpl of RLPTrait { /// * If encoding a long sequence (should not happen in current implementation) fn encode_sequence(mut input: Span) -> Span { let mut joined_encodings: Array = Default::default(); - while let Option::Some(item) = input.pop_front() { + for item in input { match item { RLPItem::String(string) => { joined_encodings.append_span(Self::encode_string(*string)); diff --git a/crates/utils/src/serialization.cairo b/crates/utils/src/serialization.cairo index 34939f43b..371f9dd1c 100644 --- a/crates/utils/src/serialization.cairo +++ b/crates/utils/src/serialization.cairo @@ -95,22 +95,19 @@ pub fn serialize_transaction_signature( /// /// * `Option>` - The deserialized bytes if successful, or None if deserialization fails. pub fn deserialize_bytes(self: Span) -> Option> { - let mut i = 0; let mut bytes: Array = Default::default(); - while i != self.len() { - let v: Option = (*self[i]).try_into(); + for item in self { + let v: Option = (*item).try_into(); match v { Option::Some(v) => { bytes.append(v); }, Option::None => { break; } } - - i += 1; }; // it means there was an error in the above loop - if (i != self.len()) { + if (bytes.len() != self.len()) { Option::None } else { Option::Some(bytes) @@ -129,13 +126,9 @@ pub fn deserialize_bytes(self: Span) -> Option> { pub fn serialize_bytes(self: Span) -> Array { let mut array: Array = Default::default(); - let mut i = 0; - - while i != self.len() { - let value: felt252 = (*self[i]).into(); + for item in self { + let value: felt252 = (*item).into(); array.append(value); - - i += 1; }; array diff --git a/crates/utils/src/traits.cairo b/crates/utils/src/traits.cairo index 3cf2fbf2f..e5bfc3eb8 100644 --- a/crates/utils/src/traits.cairo +++ b/crates/utils/src/traits.cairo @@ -106,12 +106,11 @@ pub impl SpanU8TryIntoResultEthAddress of TryIntoResult, EthAddress> { ensure(!(len > 20), EVMError::TypeConversionError(TYPE_CONVERSION_ERROR))?; let offset: u32 = len.into() - 1; let mut result: u256 = 0; - let mut i: u32 = 0; - while i != len { - let byte: u256 = (*self.at(i)).into(); - result += byte.shl(8 * (offset - i).into()); - i += 1; - }; + for i in 0 + ..len { + let byte: u256 = (*self.at(i)).into(); + result += byte.shl(8 * (offset - i).into()); + }; let address: felt252 = result.try_into_result()?; Result::Ok(address.try_into().unwrap()) diff --git a/crates/utils/src/traits/array.cairo b/crates/utils/src/traits/array.cairo index 44a2bcb62..35b3669f7 100644 --- a/crates/utils/src/traits/array.cairo +++ b/crates/utils/src/traits/array.cairo @@ -39,10 +39,8 @@ pub impl ArrayExtension> of ArrayExtTrait { /// * `value` - The value to append. /// * `n` - The number of times to append the value. fn append_n<+Copy>(ref self: Array, value: T, mut n: usize) { - while n != 0 { + for _ in 0..n { self.append(value); - - n -= 1; }; } diff --git a/crates/utils/src/traits/bytes.cairo b/crates/utils/src/traits/bytes.cairo index c780098f2..6cec31aee 100644 --- a/crates/utils/src/traits/bytes.cairo +++ b/crates/utils/src/traits/bytes.cairo @@ -59,22 +59,21 @@ pub impl U8SpanExImpl of U8SpanExTrait { // Fill the last input word let mut last_input_word: u64 = 0; - let mut byte_counter: u8 = 0; // We enter a second loop for clarity. // O(2n) should be okay // We might want to regroup every computation into a single loop with appropriate `if` // branching For optimisation - while byte_counter.into() != last_input_num_bytes { - last_input_word += match self.get(full_u64_word_count * 8 + byte_counter.into()) { - Option::Some(byte) => { - let byte: u64 = (*byte.unbox()).into(); - byte.shl(8_u64 * byte_counter.into()) - }, - Option::None => { break; }, + for byte_counter in 0 + ..last_input_num_bytes { + last_input_word += match self.get(full_u64_word_count * 8 + byte_counter.into()) { + Option::Some(byte) => { + let byte: u64 = (*byte.unbox()).into(); + byte.shl(8_u64 * byte_counter.into()) + }, + Option::None => { break; }, + }; }; - byte_counter += 1; - }; (u64_words, last_input_word, last_input_num_bytes) } @@ -183,10 +182,8 @@ pub impl U8SpanExImpl of U8SpanExTrait { }; // append the data - let mut i = 0; - while i != self.len() { - arr.append(*self[i]); - i += 1; + for item in self { + arr.append(*item); }; arr.span() @@ -257,12 +254,11 @@ pub impl ToBytesImpl< let mask = Bounded::::MAX.into(); let mut bytes: Array = Default::default(); - let mut i: u8 = 0; - while i != bytes_used { - let val = Bitshift::::shr(self, eight * (bytes_used - i - 1).into()); - bytes.append((val & mask).try_into().unwrap()); - i += 1; - }; + for i in 0 + ..bytes_used { + let val = Bitshift::::shr(self, eight * (bytes_used - i - 1).into()); + bytes.append((val & mask).try_into().unwrap()); + }; bytes.span() } @@ -283,12 +279,11 @@ pub impl ToBytesImpl< let mut bytes: Array = Default::default(); - let mut i: u8 = 0; - while i != bytes_used { - let val = self.shr(eight * i.into()); - bytes.append((val & mask).try_into().unwrap()); - i += 1; - }; + for i in 0 + ..bytes_used { + let val = self.shr(eight * i.into()); + bytes.append((val & mask).try_into().unwrap()); + }; bytes.span() } @@ -450,29 +445,27 @@ pub impl ByteArrayExt of ByteArrayExTrait { let (nb_full_words, pending_word_len) = DivRem::div_rem( bytes.len(), 31_u32.try_into().unwrap() ); - let mut i = 0; - while i != nb_full_words { - let mut word: felt252 = 0; - let mut j = 0; - while j != 31 { - word = word * POW_256_1.into() + (*bytes.pop_front().unwrap()).into(); - j += 1; + for _ in 0 + ..nb_full_words { + let mut word: felt252 = 0; + for _ in 0 + ..31_u8 { + word = word * POW_256_1.into() + (*bytes.pop_front().unwrap()).into(); + }; + arr.append_word(word.try_into().unwrap(), 31); }; - arr.append_word(word.try_into().unwrap(), 31); - i += 1; - }; if pending_word_len == 0 { return arr; }; let mut pending_word: felt252 = 0; - let mut i = 0; - while i != pending_word_len { - pending_word = pending_word * POW_256_1.into() + (*bytes.pop_front().unwrap()).into(); - i += 1; - }; + for _ in 0 + ..pending_word_len { + pending_word = pending_word * POW_256_1.into() + + (*bytes.pop_front().unwrap()).into(); + }; arr.append_word(pending_word.try_into().unwrap(), pending_word_len); arr } @@ -493,11 +486,8 @@ pub impl ByteArrayExt of ByteArrayExTrait { /// * A Span containing the bytes from the ByteArray fn into_bytes(self: ByteArray) -> Span { let mut output: Array = Default::default(); - let len = self.len(); - let mut i = 0; - while i != len { + for i in 0..self.len() { output.append(self[i]); - i += 1; }; output.span() } @@ -545,22 +535,22 @@ pub impl ByteArrayExt of ByteArrayExTrait { // Fill the last input word let mut last_input_word: u64 = 0; - let mut byte_counter: u8 = 0; // We enter a second loop for clarity. // O(2n) should be okay // We might want to regroup every computation into a single loop with appropriate `if` // branching For optimisation - while byte_counter.into() != last_input_num_bytes { - last_input_word += match self.at(full_u64_word_count * 8 + byte_counter.into()) { - Option::Some(byte) => { - let byte: u64 = byte.into(); - byte.shl(8_u64 * byte_counter.into()) - }, - Option::None => { break; }, + + for byte_counter in 0 + ..last_input_num_bytes { + last_input_word += match self.at(full_u64_word_count * 8 + byte_counter.into()) { + Option::Some(byte) => { + let byte: u64 = byte.into(); + byte.shl(8_u64 * byte_counter.into()) + }, + Option::None => { break; }, + }; }; - byte_counter += 1; - }; (u64_words, last_input_word, last_input_num_bytes) } @@ -612,10 +602,8 @@ mod tests { let res = ByteArrayExTrait::from_bytes(arr.span()); // Ensure that the result is complete and keeps the same order - let mut i = 0; - while i != arr.len() { + for i in 0..arr.len() { assert(*arr[i] == res[i], 'byte mismatch'); - i += 1; }; } @@ -685,10 +673,8 @@ mod tests { let res = ByteArrayExTrait::from_bytes(arr.span()); // Ensure that the result is complete and keeps the same order - let mut i = 0; - while i != arr.len() { + for i in 0..arr.len() { assert(*arr[i] == res[i], 'byte mismatch'); - i += 1; }; } diff --git a/crates/utils/src/traits/eth_address.cairo b/crates/utils/src/traits/eth_address.cairo index c2f260716..9f0122b29 100644 --- a/crates/utils/src/traits/eth_address.cairo +++ b/crates/utils/src/traits/eth_address.cairo @@ -13,12 +13,11 @@ pub impl EthAddressExImpl of EthAddressExTrait { let bytes_used: u256 = 20; let value: u256 = self.into(); let mut bytes: Array = Default::default(); - let mut i = 0; - while i != bytes_used { - let val = value.shr(8 * (bytes_used - i - 1)); - bytes.append((val & 0xFF).try_into().unwrap()); - i += 1; - }; + for i in 0 + ..bytes_used { + let val = value.shr(8 * (bytes_used - i - 1)); + bytes.append((val & 0xFF).try_into().unwrap()); + }; bytes } @@ -40,13 +39,11 @@ pub impl EthAddressExImpl of EthAddressExTrait { } let offset: u32 = len - 1; let mut result: u256 = 0; - let mut i: u32 = 0; - while i != len { - let byte: u256 = (*input.at(i)).into(); - result += byte.shl((8 * (offset - i)).into()); - - i += 1; - }; + for i in 0 + ..len { + let byte: u256 = (*input.at(i)).into(); + result += byte.shl((8 * (offset - i)).into()); + }; result.try_into() } } diff --git a/crates/utils/src/traits/integer.cairo b/crates/utils/src/traits/integer.cairo index 9ee504f46..ebd00f99e 100644 --- a/crates/utils/src/traits/integer.cairo +++ b/crates/utils/src/traits/integer.cairo @@ -192,7 +192,7 @@ pub impl BitsUsedImpl< let bytes_used = self.bytes_used(); let last_byte = self.shr(eight * (bytes_used.into() - One::one())); - // safe unwrap since we know atmost 8 bits are used + // safe unwrap since we know at most 8 bits are used let bits_used: u8 = bits_used_internal::bits_used_in_byte(last_byte.try_into().unwrap()); bits_used.into() + 8 * (bytes_used - 1).into() From 7c289ed872597e1d3176d90ec2aeef87c53aef82 Mon Sep 17 00:00:00 2001 From: Shashank Trivedi <100513286+lordshashank@users.noreply.github.com> Date: Mon, 30 Sep 2024 15:38:07 +0530 Subject: [PATCH 03/22] Optimizes conversion of bytes to words (#985) * ceil to bytes32 * overflow and nits --- crates/evm/src/create_helpers.cairo | 4 +-- crates/evm/src/gas.cairo | 4 +-- .../environmental_information.cairo | 11 +++---- .../src/instructions/memory_operations.cairo | 4 +-- crates/evm/src/instructions/sha3.cairo | 4 +-- crates/utils/src/helpers.cairo | 31 +++++++++---------- 6 files changed, 28 insertions(+), 30 deletions(-) diff --git a/crates/evm/src/create_helpers.cairo b/crates/evm/src/create_helpers.cairo index e324e17b7..cdf91ed26 100644 --- a/crates/evm/src/create_helpers.cairo +++ b/crates/evm/src/create_helpers.cairo @@ -13,7 +13,7 @@ use crate::stack::StackTrait; use crate::state::StateTrait; use utils::address::{compute_contract_address, compute_create2_contract_address}; use utils::constants; -use utils::helpers::ceil32; +use utils::helpers::bytes_32_words_size; use utils::set::SetTrait; use utils::traits::{ BoolIntoNumeric, EthAddressIntoU256, U256TryIntoResult, SpanU8TryIntoResultEthAddress @@ -48,7 +48,7 @@ pub impl CreateHelpersImpl of CreateHelpers { let charged_gas = match create_type { CreateType::Create => gas::CREATE + memory_expansion.expansion_cost + init_code_gas, CreateType::Create2 => { - let calldata_words = ceil32(size) / 32; + let calldata_words = bytes_32_words_size(size); gas::CREATE + gas::KECCAK256WORD * calldata_words.into() + memory_expansion.expansion_cost diff --git a/crates/evm/src/gas.cairo b/crates/evm/src/gas.cairo index 0a479e47c..60011fd8e 100644 --- a/crates/evm/src/gas.cairo +++ b/crates/evm/src/gas.cairo @@ -194,7 +194,7 @@ pub fn memory_expansion( } }?; - let new_size = helpers::ceil32(max_size); + let new_size = helpers::bytes_32_words_size(max_size) * 32; if new_size <= current_size { return Result::Ok(MemoryExpansion { new_size: current_size, expansion_cost: 0 }); @@ -218,7 +218,7 @@ pub fn memory_expansion( /// * `init_code_gas` - The gas to be charged for the init code. #[inline(always)] pub fn init_code_cost(code_size: usize) -> u64 { - let code_size_in_words = helpers::ceil32(code_size) / 32; + let code_size_in_words = helpers::bytes_32_words_size(code_size); code_size_in_words.into() * INITCODE_WORD_COST } diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index bcb0729f0..cd91df08d 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -8,7 +8,7 @@ use crate::model::vm::{VM, VMTrait}; use crate::model::{AddressTrait}; use crate::stack::StackTrait; use crate::state::StateTrait; -use utils::helpers::{ceil32, load_word}; +use utils::helpers::{bytes_32_words_size, load_word}; use utils::set::SetTrait; use utils::traits::{EthAddressIntoU256}; @@ -113,7 +113,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { let offset = self.stack.pop_saturating_usize()?; let size = self.stack.pop_usize()?; - let words_size = (ceil32(size) / 32).into(); + 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(), [(dest_offset, size)].span() @@ -143,7 +143,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { let offset = self.stack.pop_saturating_usize()?; let size = self.stack.pop_usize()?; - let words_size = (ceil32(size) / 32).into(); + 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(), [(dest_offset, size)].span() @@ -193,7 +193,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { let size = self.stack.pop_usize()?; // GAS - let words_size = (ceil32(size) / 32).into(); + let words_size = bytes_32_words_size(size).into(); let memory_expansion = gas::memory_expansion( self.memory.size(), [(dest_offset, size)].span() )?; @@ -236,8 +236,7 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { } ensure(!(last_returndata_index > return_data.len()), EVMError::ReturnDataOutOfBounds)?; - //TODO: handle overflow in ceil32 function. - let words_size = (ceil32(size.into()) / 32).into(); + let words_size = bytes_32_words_size(size).into(); let copy_gas_cost = gas::COPY * words_size; let memory_expansion = gas::memory_expansion( diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index cecc865a6..fc56ebdc1 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -7,7 +7,7 @@ use crate::memory::MemoryTrait; use crate::model::vm::{VM, VMTrait}; use crate::stack::StackTrait; use crate::state::StateTrait; -use utils::helpers::ceil32; +use utils::helpers::bytes_32_words_size; use utils::set::SetTrait; #[inline(always)] @@ -282,7 +282,7 @@ pub impl MemoryOperation of MemoryOperationTrait { let source_offset = self.stack.pop_usize()?; let size = self.stack.pop_usize()?; - let words_size = (ceil32(size) / 32).into(); + 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() diff --git a/crates/evm/src/instructions/sha3.cairo b/crates/evm/src/instructions/sha3.cairo index 563a5fa12..28f740398 100644 --- a/crates/evm/src/instructions/sha3.cairo +++ b/crates/evm/src/instructions/sha3.cairo @@ -8,7 +8,7 @@ use crate::gas; use crate::memory::MemoryTrait; use crate::model::vm::{VM, VMTrait}; use crate::stack::StackTrait; -use utils::helpers::ceil32; +use utils::helpers::bytes_32_words_size; use utils::traits::array::ArrayExtTrait; use utils::traits::integer::U256Trait; @@ -26,7 +26,7 @@ pub impl Sha3Impl of Sha3Trait { let offset: usize = self.stack.pop_usize()?; let mut size: usize = self.stack.pop_usize()?; - let words_size = (ceil32(size) / 32).into(); + let words_size = bytes_32_words_size(size).into(); 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); diff --git a/crates/utils/src/helpers.cairo b/crates/utils/src/helpers.cairo index d50eb3418..97c0b100e 100644 --- a/crates/utils/src/helpers.cairo +++ b/crates/utils/src/helpers.cairo @@ -2,6 +2,7 @@ use core::array::ArrayTrait; use core::array::SpanTrait; use core::cmp::min; use core::hash::{HashStateExTrait, HashStateTrait}; +use core::num::traits::SaturatingAdd; use core::panic_with_felt252; use core::pedersen::PedersenTrait; @@ -31,28 +32,20 @@ pub fn u128_split(input: u128) -> (u64, u64) { (high.try_into().unwrap(), low.try_into().unwrap()) } - -/// Converts a value to the next closest multiple of 32 +/// Computes the number of 32-byte words required to represent `size` bytes /// /// # Arguments -/// * `value` - The value to ceil to the next multiple of 32 +/// * `size` - The size in bytes /// /// # Returns -/// The same value if it's a perfect multiple of 32 -/// else it returns the smallest multiple of 32 -/// that is greater than `value`. +/// The number of 32-byte words required to represent `size` bytes /// /// # Examples -/// ceil32(2) = 32 -/// ceil32(34) = 64 -pub fn ceil32(value: usize) -> usize { - let ceiling = 32_u32; - let (_q, r) = DivRem::div_rem(value, ceiling.try_into().unwrap()); - if r == 0_u8.into() { - return value; - } else { - return (value + ceiling - r).into(); - } +/// bytes_32_words_size(2) = 1 +/// bytes_32_words_size(34) = 2 +#[inline(always)] +pub fn bytes_32_words_size(size: usize) -> usize { + size.saturating_add(31) / 32 } /// Computes 256 ** (16 - i) for 0 <= i <= 16. @@ -357,4 +350,10 @@ mod tests { assert_eq!(*dst4[counter], 0xff); }; } + + #[test] + fn test_bytes_32_words_size_edge_case() { + let max_usize = core::num::traits::Bounded::::MAX; + assert_eq!(helpers::bytes_32_words_size(max_usize), (max_usize / 32)); + } } From 4ce7e2dbeb67c300d9354589e18a893f8418970b Mon Sep 17 00:00:00 2001 From: Shashank Trivedi <100513286+lordshashank@users.noreply.github.com> Date: Mon, 30 Sep 2024 15:42:04 +0530 Subject: [PATCH 04/22] refactor: remove `load_word` in favor of `FromBytes` (#988) * load word to bytes * unwrap and nits --- .../environmental_information.cairo | 8 +- .../src/instructions/system_operations.cairo | 26 ++++--- crates/evm/src/memory.cairo | 20 ++++- .../precompiles/ec_operations/ec_add.cairo | 15 ++-- .../precompiles/ec_operations/ec_mul.cairo | 12 +-- crates/utils/src/helpers.cairo | 77 ------------------- 6 files changed, 48 insertions(+), 110 deletions(-) diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index cd91df08d..d675193ff 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -8,8 +8,9 @@ use crate::model::vm::{VM, VMTrait}; use crate::model::{AddressTrait}; use crate::stack::StackTrait; use crate::state::StateTrait; -use utils::helpers::{bytes_32_words_size, load_word}; +use utils::helpers::bytes_32_words_size; use utils::set::SetTrait; +use utils::traits::bytes::FromBytes; use utils::traits::{EthAddressIntoU256}; @@ -85,8 +86,9 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait { let bytes_len = core::cmp::min(32, calldata_len - offset); let sliced = calldata.slice(offset, bytes_len); - // Fill data to load with bytes in calldata - let mut data_to_load: u256 = load_word(bytes_len, sliced); + let mut data_to_load: u256 = sliced + .from_be_bytes_partial() + .expect('Failed to parse calldata'); // Fill the rest of the data to load with zeros // TODO: optimize once we have dw-based exponentiation diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index c15f55bf3..8d252e182 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -387,8 +387,8 @@ mod tests { use snforge_std::{test_address, start_mock_call}; use utils::constants::EMPTY_KECCAK; use utils::helpers::compute_starknet_address; - use utils::helpers::load_word; - use utils::traits::bytes::U8SpanExTrait; + use utils::traits::bytes::{U8SpanExTrait, FromBytes}; + use utils::traits::{EthAddressIntoU256}; @@ -405,8 +405,11 @@ mod tests { vm.stack.push(0).expect('push failed'); assert(vm.exec_return().is_ok(), 'Exec return failed'); - // Then - assert(1000 == load_word(32, vm.return_data()), 'Wrong return_data'); + let return_data = vm.return_data(); + let parsed_return_data: u256 = return_data + .from_be_bytes() + .expect('Failed to parse return data'); + assert(1000 == parsed_return_data, 'Wrong return_data'); assert(!vm.is_running(), 'vm should be stopped'); assert_eq!(vm.error, false); } @@ -424,8 +427,11 @@ mod tests { vm.stack.push(0).expect('push failed'); assert(vm.exec_revert().is_ok(), 'Exec revert failed'); - // Then - assert(1000 == load_word(32, vm.return_data()), 'Wrong return_data'); + let return_data = vm.return_data(); + let parsed_return_data: u256 = return_data + .from_be_bytes() + .expect('Failed to parse return data'); + assert(1000 == parsed_return_data, 'Wrong return_data'); assert(!vm.is_running(), 'vm should be stopped'); assert_eq!(vm.error, true); } @@ -442,9 +448,11 @@ mod tests { vm.stack.push(32).expect('push failed'); vm.stack.push(1).expect('push failed'); assert(vm.exec_return().is_ok(), 'Exec return failed'); - - // Then - assert(256 == load_word(32, vm.return_data()), 'Wrong return_data'); + let return_data = vm.return_data(); + let parsed_return_data: u256 = return_data + .from_be_bytes_partial() + .expect('Failed to parse return data'); + assert(256 == parsed_return_data, 'Wrong return_data'); assert(!vm.is_running(), 'vm should be stopped'); assert_eq!(vm.error, false); } diff --git a/crates/evm/src/memory.cairo b/crates/evm/src/memory.cairo index 601fc1bbc..9659045c8 100644 --- a/crates/evm/src/memory.cairo +++ b/crates/evm/src/memory.cairo @@ -6,6 +6,7 @@ use utils::constants::{ POW_2_72, POW_2_80, POW_2_88, POW_2_96, POW_2_104, POW_2_112, POW_2_120, POW_256_16 }; use utils::traits::array::ArrayExtTrait; +use utils::traits::bytes::FromBytes; use utils::{helpers, math::Bitshift}; #[derive(Destruct, Default)] @@ -307,7 +308,10 @@ pub(crate) impl InternalMemoryMethods of InternalMemoryTrait { let nonzero_mask_f: NonZero = mask_f.try_into().unwrap(); let (word_high, word_low) = DivRem::div_rem(word.into(), nonzero_mask_i); let (_, word_low_l) = DivRem::div_rem(word_low, nonzero_mask_f); - let bytes_as_word = helpers::load_word(elements.len(), elements); + let bytes_as_word: u128 = elements + .slice(0, elements.len()) + .from_be_bytes_partial() + .expect('Failed to parse word_low'); let new_w: u128 = (word_high * mask_i + bytes_as_word.into() * mask_f + word_low_l) .try_into() .unwrap(); @@ -545,7 +549,14 @@ pub(crate) impl InternalMemoryMethods of InternalMemoryTrait { ) { let word = self.items.get(chunk_index.into()); let word_high = (word.into() / start_mask); - let word_low = helpers::load_word(16 - start_offset_in_chunk, elements); + + let bytes_to_read = 16 - start_offset_in_chunk; + + let word_low: u128 = elements + .slice(0, bytes_to_read) + .from_be_bytes_partial() + .expect('Failed to parse word_low'); + let new_word: u128 = (word_high * start_mask + word_low.into()).try_into().unwrap(); self.items.insert(chunk_index.into(), new_word); } @@ -579,7 +590,10 @@ pub(crate) impl InternalMemoryMethods of InternalMemoryTrait { let word = self.items.get(chunk_index.into()); let word_low = (word.into() % end_mask); - let low_bytes = helpers::load_word(end_offset_in_chunk, elements); + let low_bytes: u128 = elements + .slice(0, end_offset_in_chunk) + .from_be_bytes_partial() + .expect('Failed to parse low_bytes'); let new_word: u128 = (low_bytes.into() * end_mask + word_low).try_into().unwrap(); self.items.insert(chunk_index.into(), new_word); } diff --git a/crates/evm/src/precompiles/ec_operations/ec_add.cairo b/crates/evm/src/precompiles/ec_operations/ec_add.cairo index fe167792b..1d8b9422e 100644 --- a/crates/evm/src/precompiles/ec_operations/ec_add.cairo +++ b/crates/evm/src/precompiles/ec_operations/ec_add.cairo @@ -13,8 +13,7 @@ use crate::precompiles::ec_operations::{ eq_mod_p, eq_neg_mod_p, is_on_curve, double_ec_point_unchecked, BN254_PRIME_LIMBS, BN254_PRIME }; use garaga::core::circuit::AddInputResultTrait2; -use utils::helpers::{load_word}; -use utils::traits::bytes::{ToBytes, U8SpanExTrait}; +use utils::traits::bytes::{ToBytes, U8SpanExTrait, FromBytes}; const BASE_COST: u64 = 150; @@ -31,17 +30,13 @@ pub impl EcAdd of Precompile { // Pad the input to 128 bytes to avoid out-of-bounds accesses let mut input = input.pad_right_with_zeroes(128); - let x1_bytes = *(input.multi_pop_front::<32>().unwrap()); - let x1: u256 = load_word(U256_BYTES_LEN, x1_bytes.unbox().span()); + let x1: u256 = input.slice(0, 32).from_be_bytes().unwrap(); - let y1_bytes = *(input.multi_pop_front::<32>().unwrap()); - let y1: u256 = load_word(U256_BYTES_LEN, y1_bytes.unbox().span()); + let y1: u256 = input.slice(32, 32).from_be_bytes().unwrap(); - let x2_bytes = *(input.multi_pop_front::<32>().unwrap()); - let x2: u256 = load_word(U256_BYTES_LEN, x2_bytes.unbox().span()); + let x2: u256 = input.slice(64, 32).from_be_bytes().unwrap(); - let y2_bytes = *(input.multi_pop_front::<32>().unwrap()); - let y2: u256 = load_word(U256_BYTES_LEN, y2_bytes.unbox().span()); + let y2: u256 = input.slice(96, 32).from_be_bytes().unwrap(); let (x, y) = match ec_add(x1, y1, x2, y2) { Option::Some((x, y)) => { (x, y) }, diff --git a/crates/evm/src/precompiles/ec_operations/ec_mul.cairo b/crates/evm/src/precompiles/ec_operations/ec_mul.cairo index 814d7e0e4..bfb60a7e5 100644 --- a/crates/evm/src/precompiles/ec_operations/ec_mul.cairo +++ b/crates/evm/src/precompiles/ec_operations/ec_mul.cairo @@ -6,8 +6,7 @@ use crate::errors::EVMError; use crate::precompiles::Precompile; use crate::precompiles::ec_operations::ec_add::ec_safe_add; use crate::precompiles::ec_operations::{is_on_curve, double_ec_point_unchecked, BN254_PRIME}; -use utils::helpers::{load_word}; -use utils::traits::bytes::{ToBytes, U8SpanExTrait}; +use utils::traits::bytes::{ToBytes, U8SpanExTrait, FromBytes}; const BASE_COST: u64 = 6000; const U256_BYTES_LEN: usize = 32; @@ -23,14 +22,11 @@ pub impl EcMul of Precompile { // Pad the input to 128 bytes to avoid out-of-bounds accesses let mut input = input.pad_right_with_zeroes(96); - let x1_bytes = *(input.multi_pop_front::<32>().unwrap()); - let x1: u256 = load_word(U256_BYTES_LEN, x1_bytes.unbox().span()); + let x1: u256 = input.slice(0, 32).from_be_bytes().unwrap(); - let y1_bytes = *(input.multi_pop_front::<32>().unwrap()); - let y1: u256 = load_word(U256_BYTES_LEN, y1_bytes.unbox().span()); + let y1: u256 = input.slice(32, 32).from_be_bytes().unwrap(); - let s_bytes = *(input.multi_pop_front::<32>().unwrap()); - let s: u256 = load_word(U256_BYTES_LEN, s_bytes.unbox().span()); + let s: u256 = input.slice(64, 32).from_be_bytes().unwrap(); let (x, y) = match ec_mul(x1, y1, s) { Option::Some((x, y)) => { (x, y) }, diff --git a/crates/utils/src/helpers.cairo b/crates/utils/src/helpers.cairo index 97c0b100e..94e99eafd 100644 --- a/crates/utils/src/helpers.cairo +++ b/crates/utils/src/helpers.cairo @@ -112,34 +112,6 @@ pub fn split_word_128(value: u256, ref dst: Array) { split_word(value, 16, ref dst) } - -/// Loads a sequence of bytes into a single u256 in big-endian order. -/// -/// # Arguments -/// * `len` - The number of bytes to load. -/// * `words` - The span of bytes to load. -/// -/// # Returns -/// A `u256` value representing the loaded bytes in big-endian order. -pub fn load_word(mut len: usize, words: Span) -> u256 { - if len == 0 { - return 0; - } - - let mut current: u256 = 0; - let mut counter = 0; - - for _ in 0 - ..len { - let loaded: u8 = *words[counter]; - let tmp = current * 256; - current = tmp + loaded.into(); - counter += 1; - }; - - current -} - /// Converts a u256 to a bytes array represented by an array of u8 values in big-endian order. /// /// # Arguments @@ -226,55 +198,6 @@ mod tests { assert(1 == *bytes_array[30], 'wrong conversion'); } - #[test] - fn test_load_word() { - // No bytes to load - let res0 = helpers::load_word(0, ArrayTrait::new().span()); - assert(0 == res0, 'res0: wrong load'); - - // Single bytes value - let mut arr1 = ArrayTrait::new(); - arr1.append(0x01); - let res1 = helpers::load_word(1, arr1.span()); - assert(1 == res1, 'res1: wrong load'); - - let mut arr2 = ArrayTrait::new(); - arr2.append(0xff); - let res2 = helpers::load_word(1, arr2.span()); - assert(255 == res2, 'res2: wrong load'); - - // Two byte values - let mut arr3 = ArrayTrait::new(); - arr3.append(0x01); - arr3.append(0x00); - let res3 = helpers::load_word(2, arr3.span()); - assert(256 == res3, 'res3: wrong load'); - - let mut arr4 = ArrayTrait::new(); - arr4.append(0xff); - arr4.append(0xff); - let res4 = helpers::load_word(2, arr4.span()); - assert(65535 == res4, 'res4: wrong load'); - - // Four byte values - let mut arr5 = ArrayTrait::new(); - arr5.append(0xff); - arr5.append(0xff); - arr5.append(0xff); - arr5.append(0xff); - let res5 = helpers::load_word(4, arr5.span()); - assert(4294967295 == res5, 'res5: wrong load'); - - // 16 bytes values - let mut arr6 = ArrayTrait::new(); - for _ in 0..16_u8 { - arr6.append(0xff); - }; - let res6 = helpers::load_word(16, arr6.span()); - assert(340282366920938463463374607431768211455 == res6, 'res6: wrong load'); - } - - #[test] fn test_split_word_le() { // Test with 0 value and 0 len From 5759f305f6f3c32ef0bb3cce566b761bdaedde2a Mon Sep 17 00:00:00 2001 From: saimeunt Date: Mon, 30 Sep 2024 12:12:58 +0200 Subject: [PATCH 05/22] feat: implement eth_get_balance (#990) * implement eth_get_balance * add missing files * made requested changes * fmt --------- Co-authored-by: enitrat --- .../contracts/src/kakarot_core/eth_rpc.cairo | 20 +++++++++++++- .../src/kakarot_core/interface.cairo | 3 +++ .../contracts/src/kakarot_core/kakarot.cairo | 2 +- crates/evm/src/backend/starknet_backend.cairo | 4 +-- crates/evm/src/backend/validation.cairo | 27 ++++++++++++------- 5 files changed, 42 insertions(+), 14 deletions(-) diff --git a/crates/contracts/src/kakarot_core/eth_rpc.cairo b/crates/contracts/src/kakarot_core/eth_rpc.cairo index ac92d540e..e7180d44b 100644 --- a/crates/contracts/src/kakarot_core/eth_rpc.cairo +++ b/crates/contracts/src/kakarot_core/eth_rpc.cairo @@ -8,6 +8,7 @@ use evm::backend::starknet_backend; use evm::backend::validation::validate_eth_tx; use evm::model::{TransactionResult, Address}; use evm::{EVMTrait}; +use openzeppelin::token::erc20::interface::{IERC20CamelDispatcher, IERC20CamelDispatcherTrait}; use utils::constants::POW_2_53; use utils::eth_transaction::transaction::Transaction; @@ -123,7 +124,11 @@ pub impl EthRPC< TContractState, impl KakarotState: KakarotCoreState, +Drop > of IEthRPC { fn eth_get_balance(self: @TContractState, address: EthAddress) -> u256 { - panic!("unimplemented") + let kakarot_state = KakarotState::get_state(); + let starknet_address = kakarot_state.get_starknet_address(address); + let native_token_address = kakarot_state.get_native_token(); + let native_token = IERC20CamelDispatcher { contract_address: native_token_address }; + native_token.balanceOf(starknet_address) } fn eth_get_transaction_count(self: @TContractState, address: EthAddress) -> u64 { @@ -218,6 +223,9 @@ fn is_view(self: @KakarotCore::ContractState) -> bool { mod tests { use crate::kakarot_core::KakarotCore; use crate::kakarot_core::eth_rpc::IEthRPC; + use crate::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait; + use crate::test_utils::{setup_contracts_for_testing, fund_account_with_native_token}; + use evm::test_utils::{sequencer_evm_address, evm_address}; use snforge_std::{start_cheat_chain_id_global, stop_cheat_chain_id_global}; use utils::constants::POW_2_53; @@ -232,6 +240,16 @@ mod tests { stop_cheat_chain_id_global(); } + #[test] + fn test_eth_get_balance() { + let (native_token, kakarot_core) = setup_contracts_for_testing(); + // Uninitialized accounts should return a zero balance + assert_eq!(kakarot_core.eth_get_balance(evm_address()), 0); + let sequencer_starknet_address = kakarot_core.get_starknet_address(sequencer_evm_address()); + // Fund an initialized account and make sure the balance is correct + fund_account_with_native_token(sequencer_starknet_address, native_token, 0x1); + assert_eq!(kakarot_core.eth_get_balance(sequencer_evm_address()), 0x1); + } #[test] fn test_eth_chain_id_returns_input_when_less_than_pow_2_53() { diff --git a/crates/contracts/src/kakarot_core/interface.cairo b/crates/contracts/src/kakarot_core/interface.cairo index f5d8318bd..603a52f42 100644 --- a/crates/contracts/src/kakarot_core/interface.cairo +++ b/crates/contracts/src/kakarot_core/interface.cairo @@ -74,6 +74,9 @@ pub trait IExtendedKakarotCore { ref self: TContractState, evm_address: EthAddress ) -> ContractAddress; + /// Returns the balance of the specified address. + fn eth_get_balance(self: @TContractState, address: EthAddress) -> u256; + /// View entrypoint into the EVM /// Performs view calls into the blockchain /// It cannot modify the state of the chain diff --git a/crates/contracts/src/kakarot_core/kakarot.cairo b/crates/contracts/src/kakarot_core/kakarot.cairo index 30a297417..fec55b8a0 100644 --- a/crates/contracts/src/kakarot_core/kakarot.cairo +++ b/crates/contracts/src/kakarot_core/kakarot.cairo @@ -211,7 +211,7 @@ pub mod KakarotCore { // @return starknet_address The Starknet Account Contract address fn get_starknet_address(self: @ContractState, evm_address: EthAddress) -> ContractAddress { let registered_starknet_address = self.address_registry(evm_address); - if (registered_starknet_address.is_zero()) { + if (!registered_starknet_address.is_zero()) { return registered_starknet_address; } diff --git a/crates/evm/src/backend/starknet_backend.cairo b/crates/evm/src/backend/starknet_backend.cairo index dba29c576..79bb73247 100644 --- a/crates/evm/src/backend/starknet_backend.cairo +++ b/crates/evm/src/backend/starknet_backend.cairo @@ -121,9 +121,7 @@ pub fn fetch_original_storage(account: @Account, key: u256) -> u256 { /// The balance of the given address. pub fn fetch_balance(self: @Address) -> u256 { let kakarot_state = KakarotCore::unsafe_new_contract_state(); - let native_token_address = kakarot_state.get_native_token(); - let native_token = IERC20CamelDispatcher { contract_address: native_token_address }; - native_token.balanceOf(*self.starknet) + kakarot_state.eth_get_balance(*self.evm) } diff --git a/crates/evm/src/backend/validation.cairo b/crates/evm/src/backend/validation.cairo index 91eff839e..e2202e5d2 100644 --- a/crates/evm/src/backend/validation.cairo +++ b/crates/evm/src/backend/validation.cairo @@ -5,7 +5,6 @@ use core::ops::SnapshotDeref; use core::starknet::storage::{StoragePointerReadAccess}; use core::starknet::{get_caller_address}; use crate::gas; -use openzeppelin::token::erc20::interface::{IERC20CamelDispatcher, IERC20CamelDispatcherTrait}; use starknet::storage::StorageTrait; use utils::eth_transaction::check_gas_fee; use utils::eth_transaction::transaction::{Transaction, TransactionTrait}; @@ -46,10 +45,7 @@ pub fn validate_eth_tx(kakarot_state: @KakarotCore::ContractState, tx: Transacti assert(gas_limit >= intrinsic_gas, 'Intrinsic gas > gas limit'); // Validate balance - let balance = IERC20CamelDispatcher { - contract_address: kakarot_storage.Kakarot_native_token_address.read() - } - .balanceOf(starknet_caller_address); + let balance = kakarot_state.eth_get_balance(account.get_evm_address()); let max_gas_fee = tx.gas_limit().into() * tx.max_fee_per_gas(); let tx_cost = tx.value() + max_gas_fee.into(); assert(tx_cost <= balance, 'Not enough ETH'); @@ -61,11 +57,10 @@ mod tests { use contracts::kakarot_core::KakarotCore; use core::num::traits::Bounded; use core::ops::SnapshotDeref; - - use core::starknet::ContractAddress; - use core::starknet::storage::StorageTrait; + use core::starknet::storage::{StorageTrait, StoragePathEntry}; + use core::starknet::{EthAddress, ContractAddress}; use evm::gas; - use snforge_std::cheatcodes::storage::store_felt252; + use snforge_std::cheatcodes::storage::{store_felt252}; use snforge_std::{ start_mock_call, test_address, start_cheat_chain_id_global, store, start_cheat_caller_address, mock_call @@ -82,6 +77,7 @@ mod tests { let kakarot_state = KakarotCore::unsafe_new_contract_state(); let kakarot_storage = kakarot_state.snapshot_deref().storage(); let kakarot_address = test_address(); + let account_evm_address: EthAddress = 'account_evm_address'.try_into().unwrap(); let account_starknet_address = 'account_starknet_address'.try_into().unwrap(); let native_token_address = 'native_token_address'.try_into().unwrap(); @@ -95,10 +91,23 @@ mod tests { store_felt252(kakarot_address, base_fee_storage, 1_000_000_000); // 1 Gwei store_felt252(kakarot_address, block_gas_limit_storage, BLOCK_GAS_LIMIT.into()); store_felt252(kakarot_address, native_token_storage_address, native_token_address.into()); + let map_entry_address = kakarot_storage + .Kakarot_evm_to_starknet_address + .entry(account_evm_address) + .deref() + .__storage_pointer_address__; + store( + kakarot_address, + map_entry_address.into(), + array![account_starknet_address.into()].span() + ); // Mock the calls to the account contract and the native token contract start_cheat_caller_address(kakarot_address, account_starknet_address); start_mock_call(account_starknet_address, selector!("get_nonce"), 0); + start_mock_call( + account_starknet_address, selector!("get_evm_address"), account_evm_address + ); start_mock_call( native_token_address, selector!("balanceOf"), Bounded::::MAX ); // Min to pay for gas + value From 8b29b2feb9b5a76f09d046f79fb712cc1d980c6c Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 30 Sep 2024 13:24:22 +0200 Subject: [PATCH 06/22] dev: use native in ci (#992) * dev: use native in ci * remove outdated gas reports * fix fmt * fix runtime location * avoid clearing the ssj checkout * use correct working dir * update stack size when running rust ef-tests * update workflows * simplify workflows by avoiding re-building runtimes * rework workflow structure --- .github/workflows/build.yml | 32 +++++ .github/workflows/ci.yml | 116 ++---------------- .github/workflows/ef-tests.yml | 129 ++++++++++++++++++++ .github/workflows/gas_reports.yml | 43 ------- .github/workflows/gas_snapshot.yml | 34 ------ .github/workflows/test.yml | 25 ++-- .github/workflows/tests-unit.yml | 26 ++++ .gitignore | 2 + scripts/setup_cairo_native.sh | 185 +++++++++++++++++++++++++++++ 9 files changed, 400 insertions(+), 192 deletions(-) create mode 100644 .github/workflows/build.yml create mode 100644 .github/workflows/ef-tests.yml delete mode 100644 .github/workflows/gas_reports.yml delete mode 100644 .github/workflows/gas_snapshot.yml create mode 100644 .github/workflows/tests-unit.yml create mode 100755 scripts/setup_cairo_native.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 000000000..7cca4d20d --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,32 @@ +name: Reusable Build Workflow + +on: + workflow_call: + inputs: + artifact-name: + required: true + type: string + +permissions: read-all + +jobs: + build: + runs-on: ubuntu-latest + env: + CI_COMMIT_MESSAGE: CI Formatting Auto Commit + CI_COMMIT_AUTHOR: ${{ github.event.repository.name }} CI + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Scarb + uses: software-mansion/setup-scarb@v1 + + - name: Build contracts + run: scarb build -p contracts + + - name: Upload artifacts + uses: actions/upload-artifact@v3 + with: + name: ${{ inputs.artifact-name }} + path: target/dev diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 92fd8de84..b99ecd358 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,118 +16,24 @@ permissions: read-all jobs: build: - runs-on: ubuntu-latest - env: - CI_COMMIT_MESSAGE: CI Formatting Auto Commit - CI_COMMIT_AUTHOR: ${{ github.event.repository.name }} CI - steps: - - name: Checkout code - uses: actions/checkout@v4 - - - name: Set up Scarb - uses: software-mansion/setup-scarb@v1 - - - name: Build contracts - run: scarb build -p contracts - - - name: Upload artifacts - uses: actions/upload-artifact@v3 - with: - name: kakarot-ssj-build - path: target/dev + uses: ./.github/workflows/build.yml + with: + artifact-name: kakarot-ssj-build tests-unit: - # trunk-ignore(actionlint/runner-label) - runs-on: ubuntu-latest-16-cores - steps: - - uses: actions/checkout@v3 - - uses: foundry-rs/setup-snfoundry@v3 - - uses: software-mansion/setup-scarb@v1 - - run: scarb fmt --check - - run: scarb build - - run: scarb test + uses: ./.github/workflows/tests-unit.yml + with: + run-fmt-check: false ef-tests: - # trunk-ignore(actionlint/runner-label) - runs-on: ubuntu-latest-32-cores - needs: build - steps: - - name: Checkout ef-tests - uses: actions/checkout@v4 - with: - repository: kkrt-labs/ef-tests - - - name: Checkout local skip file - uses: actions/checkout@v4 - with: - sparse-checkout: | - blockchain-tests-skip.yml - sparse-checkout-cone-mode: false - path: skip-file - - - name: Replace the skip files - run: mv skip-file/blockchain-tests-skip.yml blockchain-tests-skip.yml - - - name: Rust cache - uses: Swatinem/rust-cache@v2 - with: - cache-on-failure: "true" - - - name: Setup - run: | - mkdir -p build/common - make setup setup-kakarot-v0 - - - name: Install nextest - uses: taiki-e/install-action@nextest - - - name: Download Kakarot-SSJ build artifacts in v1 - uses: actions/download-artifact@v3 - with: - name: kakarot-ssj-build - path: ./build/v1 - - - name: Move Cairo1Helpers - run: - mv build/v1/contracts_Cairo1Helpers.compiled_contract_class.json - build/common/cairo1_helpers.json - - - name: Set up Python - uses: actions/setup-python@v4 - with: - python-version: 3.10.14 - - - name: Run tests - run: | - set -o pipefail - make ef-test-v1 | tee data.txt - set +o pipefail - - - name: Retrieve ef-tests execution resources - run: python scripts/compute_resources.py - env: - KAKAROT_VERSION: v1 - - - name: Upload resources - uses: actions/upload-artifact@v3 - with: - path: resources - name: resources - - - name: Generate blockchain-tests-skip.yml file - if: github.event_name == 'workflow_dispatch' - run: make generate-skip-file - - - name: Upload skip file - if: github.event_name == 'workflow_dispatch' - uses: actions/upload-artifact@v3 - with: - path: blockchain-tests-skip.yml - name: blockchain-tests-skip + uses: ./.github/workflows/ef-tests.yml + needs: [build] + with: + artifact-name: kakarot-ssj-build resources: runs-on: ubuntu-latest - needs: ef-tests + needs: [ef-tests] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/ef-tests.yml b/.github/workflows/ef-tests.yml new file mode 100644 index 000000000..5957d20fc --- /dev/null +++ b/.github/workflows/ef-tests.yml @@ -0,0 +1,129 @@ +name: EF-Tests + +on: + workflow_call: + inputs: + artifact-name: + required: true + type: string + +permissions: read-all + +jobs: + ef-tests: + # trunk-ignore(actionlint/runner-label) + runs-on: ubuntu-latest-16-cores + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Cache cairo-native setup + id: cache-cairo-native + uses: actions/cache@v3 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + target/ + ./libcairo_native_runtime.a + key: + ${{ runner.os }}-cairo-native-${{ hashFiles('**/Cargo.lock', + 'scripts/setup_cairo_native.sh') }} + + - name: Make setup script executable + run: chmod +x ./scripts/setup_cairo_native.sh + + - name: Setup Cairo Native + run: | + if [[ "${{ steps.cache-cairo-native.outputs.cache-hit }}" == 'true' ]]; then + sudo ./scripts/setup_cairo_native.sh -s + else + sudo ./scripts/setup_cairo_native.sh + fi + + - name: Set Environment Variables + run: | + echo "MLIR_SYS_190_PREFIX=/usr/lib/llvm-19/" >> $GITHUB_ENV + echo "LLVM_SYS_191_PREFIX=/usr/lib/llvm-19/" >> $GITHUB_ENV + echo "TABLEGEN_190_PREFIX=/usr/lib/llvm-19/" >> $GITHUB_ENV + echo "CAIRO_NATIVE_RUNTIME_LIBRARY=$(pwd)/libcairo_native_runtime.a" >> $GITHUB_ENV + + - name: Checkout ef-tests + uses: actions/checkout@v4 + with: + repository: kkrt-labs/ef-tests + ref: feat/cairo-native + path: ef-tests # Check out to a subdirectory to avoid cleaning the kakarot-ssj directory + + - name: Checkout local skip file + uses: actions/checkout@v4 + with: + sparse-checkout: | + blockchain-tests-skip.yml + sparse-checkout-cone-mode: false + path: skip-file + + - name: Setup ef-tests + run: | + mv skip-file/blockchain-tests-skip.yml ef-tests/blockchain-tests-skip.yml + cd ef-tests + mkdir -p build/common + make setup setup-kakarot-v0 + + - name: Install nextest + uses: taiki-e/install-action@nextest + + - name: Download Kakarot-SSJ build artifacts in v1 + uses: actions/download-artifact@v3 + with: + name: ${{ inputs.artifact-name }} + path: ef-tests/build/v1 + + - name: Move Cairo1Helpers + run: | + mv ef-tests/build/v1/contracts_Cairo1Helpers.compiled_contract_class.json \ + ef-tests/build/common/cairo1_helpers.json + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: 3.10.14 + + # Add this step to verify the file exists + - name: Verify libcairo_native_runtime.a + run: | + echo $CAIRO_NATIVE_RUNTIME_LIBRARY + ls -l $CAIRO_NATIVE_RUNTIME_LIBRARY + + - name: Run tests + working-directory: ef-tests + run: | + set -o pipefail + RUST_MIN_STACK=1342177280 make ef-test-v1-native | tee data.txt + set +o pipefail + + - name: Retrieve ef-tests execution resources + working-directory: ef-tests + run: python scripts/compute_resources.py + env: + KAKAROT_VERSION: v1 + + - name: Upload resources + uses: actions/upload-artifact@v3 + with: + path: ef-tests/resources + name: resources + + - name: Generate blockchain-tests-skip.yml file + if: github.event_name == 'workflow_dispatch' + working-directory: ef-tests + run: make generate-skip-file + + - name: Upload skip file + if: github.event_name == 'workflow_dispatch' + uses: actions/upload-artifact@v3 + with: + path: ef-tests/blockchain-tests-skip.yml + name: blockchain-tests-skip diff --git a/.github/workflows/gas_reports.yml b/.github/workflows/gas_reports.yml deleted file mode 100644 index 6200d6329..000000000 --- a/.github/workflows/gas_reports.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: Compare Snapshot - -on: pull_request - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -permissions: - pull-requests: write - -jobs: - compare-snapshot: - permissions: write-all - # trunk-ignore(actionlint/runner-label) - runs-on: ubuntu-latest-16-cores - - steps: - - name: Checkout code - uses: actions/checkout@v3 - - - name: Set up Python - uses: actions/setup-python@v4 - with: - python-version: 3.9 - - - name: Set up Scarb - uses: software-mansion/setup-scarb@v1 - - - name: Run compare_snapshot script - id: run-script - env: - GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} - continue-on-error: true - run: | - result=$(python scripts/compare_snapshot.py 2>&1) - status=$(echo "$result" | tail -n 1 | awk -F ':' '{print $1}') - echo "$result" >> "$GITHUB_STEP_SUMMARY" - echo "status=$status" >> "$GITHUB_OUTPUT" - - - name: Exit step based on status - if: steps.run-script.outputs.status == 'ERROR' - run: exit 1 diff --git a/.github/workflows/gas_snapshot.yml b/.github/workflows/gas_snapshot.yml deleted file mode 100644 index 3e8ffa6cf..000000000 --- a/.github/workflows/gas_snapshot.yml +++ /dev/null @@ -1,34 +0,0 @@ -name: Generate and Upload Gas Snapshot - -permissions: read-all - -on: - push: - branches: - - main - workflow_dispatch: {} - -jobs: - build: - # trunk-ignore(actionlint/runner-label) - runs-on: ubuntu-latest-16-cores - - steps: - - name: Checkout code - uses: actions/checkout@v3 - - - name: Set up Python - uses: actions/setup-python@v4 - with: - python-version: 3.x - - name: Set up Scarb - uses: software-mansion/setup-scarb@v1 - - - name: Generate gas snapshot - run: python scripts/gen_snapshot.py - - - name: Upload gas snapshot to GitHub Artifacts - uses: actions/upload-artifact@v2 - with: - name: gas-snapshot - path: gas_snapshot.json diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 32c1a450a..cc095668a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -9,13 +9,18 @@ concurrency: cancel-in-progress: true jobs: - tests: - # trunk-ignore(actionlint/runner-label) - runs-on: ubuntu-latest-16-cores - steps: - - uses: actions/checkout@v3 - - uses: foundry-rs/setup-snfoundry@v3 - - uses: software-mansion/setup-scarb@v1 - - run: scarb fmt --check - - run: scarb build - - run: scarb test + build: + uses: ./.github/workflows/build.yml + with: + artifact-name: kakarot-ssj-build + + tests-unit: + uses: ./.github/workflows/tests-unit.yml + with: + run-fmt-check: true + + ef-tests: + uses: ./.github/workflows/ef-tests.yml + needs: [build] + with: + artifact-name: kakarot-ssj-build diff --git a/.github/workflows/tests-unit.yml b/.github/workflows/tests-unit.yml new file mode 100644 index 000000000..2ff0ef28d --- /dev/null +++ b/.github/workflows/tests-unit.yml @@ -0,0 +1,26 @@ +name: Reusable Unit Tests Workflow + +on: + workflow_call: + inputs: + run-fmt-check: + type: boolean + default: false + required: false + +permissions: read-all + +jobs: + unit-tests: + runs-on: ubuntu-latest-16-cores # trunk-ignore(actionlint/runner-label) + steps: + - uses: actions/checkout@v4 + - uses: foundry-rs/setup-snfoundry@v3 + - uses: software-mansion/setup-scarb@v1 + + - name: Run format check + if: inputs.run-fmt-check + run: scarb fmt --check + + - name: Run tests + run: scarb test diff --git a/.gitignore b/.gitignore index 0469dd3ad..bcdcc22c4 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ target node_modules/ *.snfoundry_cache cache/ + +scripts/libcairo_native_runtime.a diff --git a/scripts/setup_cairo_native.sh b/scripts/setup_cairo_native.sh new file mode 100755 index 000000000..8b74c612c --- /dev/null +++ b/scripts/setup_cairo_native.sh @@ -0,0 +1,185 @@ +#!/bin/bash + +install_essential_deps_linux() { + apt-get update -y + apt-get install -y \ + curl \ + jq \ + ripgrep \ + wget \ + ca-certificates \ + gnupg \ + git +} + +setup_llvm_deps() { + case "$(uname)" in + Darwin) + brew update + brew install llvm@19 + + LIBRARY_PATH=/opt/homebrew/lib + MLIR_SYS_190_PREFIX="$(brew --prefix llvm@19)" + LLVM_SYS_191_PREFIX="${MLIR_SYS_190_PREFIX}" + TABLEGEN_190_PREFIX="${MLIR_SYS_190_PREFIX}" + + export LIBRARY_PATH + export MLIR_SYS_190_PREFIX + export LLVM_SYS_191_PREFIX + export TABLEGEN_190_PREFIX + ;; + Linux) + export DEBIAN_FRONTEND=noninteractive + export TZ=America/New_York + + # shellcheck disable=SC2312 + CODENAME=$(grep VERSION_CODENAME /etc/os-release | cut -d= -f2) + if [[ -z ${CODENAME} ]]; then + echo "Error: Unable to determine OS codename" + exit 1 + fi + + # shellcheck disable=SC2312 + echo "deb http://apt.llvm.org/${CODENAME}/ llvm-toolchain-${CODENAME}-19 main" >/etc/apt/sources.list.d/llvm-19.list + echo "deb-src http://apt.llvm.org/${CODENAME}/ llvm-toolchain-${CODENAME}-19 main" >>/etc/apt/sources.list.d/llvm-19.list + # shellcheck disable=SC2312 + if ! wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -; then + echo "Error: Failed to add LLVM GPG key" + exit 1 + fi + + if ! apt-get update && apt-get upgrade -y; then + echo "Error: Failed to update and upgrade packages" + exit 1 + fi + if ! apt-get install -y llvm-19 llvm-19-dev llvm-19-runtime clang-19 clang-tools-19 lld-19 libpolly-19-dev libmlir-19-dev mlir-19-tools; then + echo "Error: Failed to install LLVM packages" + exit 1 + fi + + MLIR_SYS_190_PREFIX=/usr/lib/llvm-19/ + LLVM_SYS_191_PREFIX=/usr/lib/llvm-19/ + TABLEGEN_190_PREFIX=/usr/lib/llvm-19/ + + export MLIR_SYS_190_PREFIX + export LLVM_SYS_191_PREFIX + export TABLEGEN_190_PREFIX + + { + echo "MLIR_SYS_190_PREFIX=${MLIR_SYS_190_PREFIX}" + echo "LLVM_SYS_191_PREFIX=${LLVM_SYS_191_PREFIX}" + echo "TABLEGEN_190_PREFIX=${TABLEGEN_190_PREFIX}" + } >>"${GITHUB_ENV}" + ;; + *) + echo "Error: Unsupported operating system" + exit 1 + ;; + esac + + # GitHub Actions specific + [[ -n ${GITHUB_ACTIONS} ]] && { + { + echo "MLIR_SYS_190_PREFIX=${MLIR_SYS_190_PREFIX}" + echo "LLVM_SYS_191_PREFIX=${LLVM_SYS_191_PREFIX}" + echo "TABLEGEN_190_PREFIX=${TABLEGEN_190_PREFIX}" + } >>"${GITHUB_ENV}" + } +} + +install_rust() { + if command -v cargo >/dev/null 2>&1; then + echo "Rust is already installed with cargo available in PATH." + return 0 + fi + + echo "cargo not found. Installing Rust..." + # shellcheck disable=SC2312 + if ! curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain 1.81.0 --no-modify-path; then + echo >&2 "Failed to install Rust. Aborting." + return 1 + fi + + # shellcheck disable=SC1091 + if [[ -f "${HOME}/.cargo/env" ]]; then + . "${HOME}/.cargo/env" + else + echo >&2 "Failed to find Rust environment file. Aborting." + return 1 + fi + + echo "Rust installed successfully." +} + +install_cairo_native_runtime() { + install_rust || { + echo "Error: Failed to install Rust" + exit 1 + } + + git clone https://github.com/lambdaclass/cairo_native.git + pushd ./cairo_native || exit 1 + make deps + make runtime + cp libcairo_native_runtime.a ../libcairo_native_runtime.a + popd || exit 1 + + rm -rf ./cairo_native + + CAIRO_NATIVE_RUNTIME_LIBRARY=$(pwd)/libcairo_native_runtime.a + if [[ -z ${CAIRO_NATIVE_RUNTIME_LIBRARY} ]]; then + echo "Error: Failed to set CAIRO_NATIVE_RUNTIME_LIBRARY" + exit 1 + fi + export CAIRO_NATIVE_RUNTIME_LIBRARY + + echo "CAIRO_NATIVE_RUNTIME_LIBRARY=${CAIRO_NATIVE_RUNTIME_LIBRARY}" + + [[ -n ${GITHUB_ACTIONS} ]] && echo "CAIRO_NATIVE_RUNTIME_LIBRARY=${CAIRO_NATIVE_RUNTIME_LIBRARY}" >>"${GITHUB_ENV}" +} + +main() { + # New argument parsing + SKIP_RUNTIME=false + while getopts ":s" opt; do + case ${opt} in + s) + SKIP_RUNTIME=true + ;; + \?) + echo "Invalid option: ${OPTARG}" 1>&2 + exit 1 + ;; + *) + echo "Error: Unhandled option" 1>&2 + exit 1 + ;; + esac + done + shift $((OPTIND - 1)) + + # shellcheck disable=SC2312 + [[ "$(uname)" == "Linux" ]] && install_essential_deps_linux + + setup_llvm_deps + + if [[ ${SKIP_RUNTIME} == false ]]; then + install_cairo_native_runtime + else + echo "Skipping Cairo native runtime installation" + # Set the environment variable if the library file exists + # shellcheck disable=SC2312 + if [[ -f "$(pwd)/libcairo_native_runtime.a" ]]; then + CAIRO_NATIVE_RUNTIME_LIBRARY=$(pwd)/libcairo_native_runtime.a + export CAIRO_NATIVE_RUNTIME_LIBRARY + echo "CAIRO_NATIVE_RUNTIME_LIBRARY=${CAIRO_NATIVE_RUNTIME_LIBRARY}" + [[ -n ${GITHUB_ACTIONS} ]] && echo "CAIRO_NATIVE_RUNTIME_LIBRARY=${CAIRO_NATIVE_RUNTIME_LIBRARY}" >>"${GITHUB_ENV}" + else + echo "Warning: libcairo_native_runtime.a not found. CAIRO_NATIVE_RUNTIME_LIBRARY not set." + fi + fi + + echo "LLVM and Cairo native runtime dependencies setup completed." +} + +main "$@" From 77bfa665629b3509557f11707cbab6bd632b9d2d Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 30 Sep 2024 16:20:15 +0200 Subject: [PATCH 07/22] fix: sar (#994) --- crates/evm/src/instructions/comparison_operations.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/src/instructions/comparison_operations.cairo b/crates/evm/src/instructions/comparison_operations.cairo index e99f9e079..e6f6e84b4 100644 --- a/crates/evm/src/instructions/comparison_operations.cairo +++ b/crates/evm/src/instructions/comparison_operations.cairo @@ -184,7 +184,7 @@ pub impl ComparisonAndBitwiseOperations of ComparisonAndBitwiseOperationsTrait { Bounded::::MAX }; - if (shift > 256) { + if (shift >= 256) { self.stack.push(sign) } else { // XORing with sign before and after the shift propagates the sign bit of the operation From 3d92e66a4c1371053e24c3e964b050e2e237d065 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 30 Sep 2024 16:20:35 +0200 Subject: [PATCH 08/22] fix: create tx nonce overflow (#995) --- crates/evm/src/backend/validation.cairo | 5 ++++- crates/evm/src/create_helpers.cairo | 11 ++++++----- crates/evm/src/instructions/system_operations.cairo | 7 ++++++- crates/evm/src/interpreter.cairo | 9 +++------ 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/crates/evm/src/backend/validation.cairo b/crates/evm/src/backend/validation.cairo index e2202e5d2..30414f5ed 100644 --- a/crates/evm/src/backend/validation.cairo +++ b/crates/evm/src/backend/validation.cairo @@ -1,6 +1,7 @@ use contracts::account_contract::{IAccountDispatcher, IAccountDispatcherTrait}; use contracts::kakarot_core::KakarotCore; use contracts::kakarot_core::eth_rpc::IEthRPC; +use core::num::traits::Bounded; use core::ops::SnapshotDeref; use core::starknet::storage::{StoragePointerReadAccess}; use core::starknet::{get_caller_address}; @@ -29,7 +30,9 @@ pub fn validate_eth_tx(kakarot_state: @KakarotCore::ContractState, tx: Transacti // Validate nonce let starknet_caller_address = get_caller_address(); let account = IAccountDispatcher { contract_address: starknet_caller_address }; - assert(account.get_nonce() == tx.nonce(), 'Invalid nonce'); + let account_nonce = account.get_nonce(); + assert(account_nonce == tx.nonce(), 'Invalid nonce'); + assert(account_nonce != Bounded::::MAX, 'Nonce overflow'); // Validate gas let gas_limit = tx.gas_limit(); diff --git a/crates/evm/src/create_helpers.cairo b/crates/evm/src/create_helpers.cairo index cdf91ed26..28e8dc40b 100644 --- a/crates/evm/src/create_helpers.cairo +++ b/crates/evm/src/create_helpers.cairo @@ -98,22 +98,23 @@ pub impl CreateHelpersImpl of CreateHelpers { return self.stack.push(0); } + sender + .set_nonce( + sender_current_nonce + 1 + ); // Will not overflow because of the previous check. + self.env.state.set_account(sender); + let mut target_account = self.env.state.get_account(create_args.to); let target_address = target_account.address(); // Collision happens if the target account loaded in state has code or nonce set, meaning // - it's deployed on SN and is an active EVM contract // - it's not deployed on SN and is an active EVM contract in the Kakarot cache if target_account.has_code_or_nonce() { - sender.set_nonce(sender_current_nonce + 1); - self.env.state.set_account(sender); return self.stack.push(0); }; ensure(create_args.bytecode.len() <= constants::MAX_INITCODE_SIZE, EVMError::OutOfGas)?; - sender.set_nonce(sender_current_nonce + 1); - self.env.state.set_account(sender); - let child_message = Message { caller: sender_address, target: target_address, diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index 8d252e182..c55a5d071 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -1,3 +1,4 @@ +use core::num::traits::CheckedAdd; use crate::call_helpers::CallHelpers; use crate::create_helpers::{CreateHelpers, CreateType}; use crate::errors::{ensure, EVMError}; @@ -265,7 +266,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 gas_to_charge = message_call_gas + .cost + .checked_add(memory_expansion.expansion_cost) + .ok_or(EVMError::OutOfGas)?; + self.charge_gas(gas_to_charge)?; self .generic_call( diff --git a/crates/evm/src/interpreter.cairo b/crates/evm/src/interpreter.cairo index 38c2fd9c8..a17d06317 100644 --- a/crates/evm/src/interpreter.cairo +++ b/crates/evm/src/interpreter.cairo @@ -106,6 +106,7 @@ pub impl EVMImpl of EVMTrait { let (message, is_deploy_tx) = { let mut sender_account = env.state.get_account(origin.evm); + // Charge the intrinsic gas to the sender so that it's not available for the execution // of the transaction but don't trigger any actual transfer, as only the actual consumde // gas is charged at the end of the transaction @@ -114,12 +115,8 @@ pub impl EVMImpl of EVMTrait { let (message, is_deploy_tx) = self .prepare_message(@tx, @sender_account, ref env, gas_left); - // Increment nonce of sender AFTER computing eventual created address - if sender_account.nonce() == Bounded::::MAX { - return TransactionResultTrait::exceptional_failure( - EVMError::NonceOverflow.to_bytes(), tx.gas_limit() - ); - } + // Increment nonce of sender AFTER computing the created address + // to use the correct nonce when computing the address. sender_account.set_nonce(sender_account.nonce() + 1); env.state.set_account(sender_account); From a3fa5bcd9cc3b01490c098493a5ec4a937ba0339 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 30 Sep 2024 17:25:36 +0200 Subject: [PATCH 09/22] dev: implement compile-time filtering for tests (#997) * dev: implement compile-time filtering * fmt --- .gitignore | 1 + Makefile | 19 ++++- crates/evm/src/backend/starknet_backend.cairo | 68 ++++++++--------- scripts/filter_tests.py | 50 +++++++++++++ scripts/run_filtered_tests.py | 73 +++++++++++++++++++ 5 files changed, 177 insertions(+), 34 deletions(-) create mode 100644 scripts/filter_tests.py create mode 100644 scripts/run_filtered_tests.py diff --git a/.gitignore b/.gitignore index bcdcc22c4..1fa41de69 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ node_modules/ cache/ scripts/libcairo_native_runtime.a +scripts/__pycache__ diff --git a/Makefile b/Makefile index df6124e9f..c69fc0ec1 100644 --- a/Makefile +++ b/Makefile @@ -1,2 +1,19 @@ -install: +install: bash scripts/install_hook.sh + +test-unit: + @PACKAGE="$(word 2,$(MAKECMDGOALS))" && \ + FILTER="$(word 3,$(MAKECMDGOALS))" && \ + if [ -z "$$PACKAGE" ] && [ -z "$$FILTER" ]; then \ + scarb test; \ + elif [ -n "$$PACKAGE" ] && [ -z "$$FILTER" ]; then \ + scarb test -p $$PACKAGE; \ + elif [ -n "$$PACKAGE" ] && [ -n "$$FILTER" ]; then \ + uv run scripts/run_filtered_tests.py $$PACKAGE $$FILTER; \ + else \ + echo "Usage: make test-unit [PACKAGE] [FILTER]"; \ + exit 1; \ + fi + +%: + @: diff --git a/crates/evm/src/backend/starknet_backend.cairo b/crates/evm/src/backend/starknet_backend.cairo index 79bb73247..52a0de60c 100644 --- a/crates/evm/src/backend/starknet_backend.cairo +++ b/crates/evm/src/backend/starknet_backend.cairo @@ -479,37 +479,39 @@ mod tests { assert_called(starknet_address, selector!("set_code_hash")); assert_called(starknet_address, selector!("set_nonce")); } - - #[test] - #[ignore] - //TODO(starknet-foundry): it's impossible to deploy an un-declared class, nor is it possible to - //mock_deploy. - fn test_exec_sstore_finalized() { // // Given - // setup_test_environment(); - // let mut vm = VMBuilderTrait::new_with_presets().build(); - // let evm_address = vm.message().target.evm; - // let starknet_address = compute_starknet_address( - // test_address(), evm_address, uninitialized_account() - // ); - // let account = Account { - // address: Address { evm: evm_address, starknet: starknet_address }, - // code: [].span(), - // nonce: 1, - // balance: 0, - // selfdestruct: false, - // is_created: false, - // }; - // let key: u256 = 0x100000000000000000000000000000001; - // let value: u256 = 0xABDE1E11A5; - // vm.stack.push(value).expect('push failed'); - // vm.stack.push(key).expect('push failed'); - - // // When - - // vm.exec_sstore().expect('exec_sstore failed'); - // starknet_backend::commit(ref vm.env.state).expect('commit storage failed'); - - // // Then - // assert(fetch_original_storage(@account, key) == value, 'wrong committed value') - } } +// #[test] +// #[ignore] +//TODO(starknet-foundry): it's impossible to deploy an un-declared class, nor is it possible to +//mock_deploy. +// fn test_exec_sstore_finalized() { // // Given +// setup_test_environment(); +// let mut vm = VMBuilderTrait::new_with_presets().build(); +// let evm_address = vm.message().target.evm; +// let starknet_address = compute_starknet_address( +// test_address(), evm_address, uninitialized_account() +// ); +// let account = Account { +// address: Address { evm: evm_address, starknet: starknet_address }, +// code: [].span(), +// nonce: 1, +// balance: 0, +// selfdestruct: false, +// is_created: false, +// }; +// let key: u256 = 0x100000000000000000000000000000001; +// let value: u256 = 0xABDE1E11A5; +// vm.stack.push(value).expect('push failed'); +// vm.stack.push(key).expect('push failed'); + +// // When + +// vm.exec_sstore().expect('exec_sstore failed'); +// starknet_backend::commit(ref vm.env.state).expect('commit storage failed'); + +// // Then +// assert(fetch_original_storage(@account, key) == value, 'wrong committed value') +// } +// } + + diff --git a/scripts/filter_tests.py b/scripts/filter_tests.py new file mode 100644 index 000000000..7d514b79a --- /dev/null +++ b/scripts/filter_tests.py @@ -0,0 +1,50 @@ +import os +import re +import sys + + +def filter_tests(directory, filter_string): + for root, _, files in os.walk(directory): + for file in files: + if file.endswith(".cairo"): + file_path = os.path.join(root, file) + filter_file(file_path, filter_string) + + print(f"Filtered tests for {filter_string}") + + +def filter_file(file_path, filter_string): + with open(file_path, "r") as f: + content = f.read() + + # Regular expression to match test functions, including nested braces + test_pattern = re.compile( + r"#\[test\]\s*(?:#\[available_gas\([^\)]+\)\]\s*)?fn\s+(\w+)\s*\([^)]*\)\s*(\{(?:[^{}]|\{(?:[^{}]|\{[^{}]*\})*\})*\})", + re.DOTALL, + ) + + def replace_func(match): + full_match = match.group(0) + func_name = match.group(1) + if filter_string.lower() in func_name.lower(): + return full_match + else: + return "" + + new_content = test_pattern.sub(replace_func, content) + + if new_content != content: + with open(file_path, "w") as f: + f.write(new_content) + + +if __name__ == "__main__": + if len(sys.argv) != 2: + print("Usage: python filter_tests.py ") + sys.exit(1) + + filter_string = sys.argv[1] + crates_dir = os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "crates" + ) + filter_tests(crates_dir, filter_string) diff --git a/scripts/run_filtered_tests.py b/scripts/run_filtered_tests.py new file mode 100644 index 000000000..9c755c572 --- /dev/null +++ b/scripts/run_filtered_tests.py @@ -0,0 +1,73 @@ +import os +import pty +import select +import shutil +import subprocess +import sys +import tempfile +from contextlib import contextmanager +from pathlib import Path + +from filter_tests import filter_tests + +PROJECT_FILES = ["Scarb.toml", "Scarb.lock", ".tool-versions"] + + +@contextmanager +def temporary_project_copy(src_dir): + with tempfile.TemporaryDirectory() as temp_dir: + temp_path = Path(temp_dir) + src_path = Path(src_dir) + + for file in PROJECT_FILES: + if (src_file := src_path / file).exists(): + shutil.copy2(src_file, temp_path / file) + + if (src_crates := src_path / "crates").exists(): + shutil.copytree(src_crates, temp_path / "crates", symlinks=True) + + yield temp_path + + +def stream_output(fd): + while True: + try: + r, _, _ = select.select([fd], [], [], 0.1) + if r: + data = os.read(fd, 1024) + if not data: + break + sys.stdout.buffer.write(data) + sys.stdout.buffer.flush() + except OSError: + break + + +def run_scarb_command(command, cwd): + master, slave = pty.openpty() + with subprocess.Popen( + command, shell=True, stdout=slave, stderr=slave, close_fds=True, cwd=cwd + ) as process: + os.close(slave) + stream_output(master) + return_code = process.wait() + + if return_code != 0: + print(f"Error: Scarb command failed with return code {return_code}") + sys.exit(return_code) + + +def run_filtered_tests(package, filter_name): + project_root = Path(__file__).parent.parent + + with temporary_project_copy(project_root) as temp_project_dir: + filter_tests(temp_project_dir / "crates", filter_name) + run_scarb_command(f"scarb test -p {package} {filter_name}", temp_project_dir) + + +if __name__ == "__main__": + if len(sys.argv) != 3: + print("Usage: python run_filtered_tests.py ") + sys.exit(1) + + run_filtered_tests(sys.argv[1], sys.argv[2]) From 81dc3c7936343c3dc9fee17b591a285566b15ce9 Mon Sep 17 00:00:00 2001 From: Gerson <71728860+Gerson2102@users.noreply.github.com> Date: Mon, 30 Sep 2024 09:52:09 -0600 Subject: [PATCH 10/22] Implementation of eth_get_transaction_count function (#983) * Implementation of eth_get_transaction_count function * Refactoring validate nonce in validation.cairo * Changing the deprecated function * Validating nonce without new function * Adding method to interface and test it * fix tests * fmt --------- Co-authored-by: enitrat --- .../contracts/src/kakarot_core/eth_rpc.cairo | 27 ++++++++++++++++--- .../src/kakarot_core/interface.cairo | 3 +++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/contracts/src/kakarot_core/eth_rpc.cairo b/crates/contracts/src/kakarot_core/eth_rpc.cairo index e7180d44b..c824fb4f6 100644 --- a/crates/contracts/src/kakarot_core/eth_rpc.cairo +++ b/crates/contracts/src/kakarot_core/eth_rpc.cairo @@ -132,7 +132,12 @@ pub impl EthRPC< } fn eth_get_transaction_count(self: @TContractState, address: EthAddress) -> u64 { - panic!("unimplemented") + let kakarot_state = KakarotState::get_state(); + let starknet_address = kakarot_state.get_starknet_address(address); + println!("starknet_address: {:?}", starknet_address); + let account = IAccountDispatcher { contract_address: starknet_address }; + let nonce = account.get_nonce(); + nonce } fn eth_chain_id(self: @TContractState) -> u64 { @@ -225,9 +230,12 @@ mod tests { use crate::kakarot_core::eth_rpc::IEthRPC; use crate::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait; use crate::test_utils::{setup_contracts_for_testing, fund_account_with_native_token}; - use evm::test_utils::{sequencer_evm_address, evm_address}; - use snforge_std::{start_cheat_chain_id_global, stop_cheat_chain_id_global}; + use evm::test_utils::{sequencer_evm_address, evm_address, uninitialized_account}; + use snforge_std::{ + start_mock_call, start_cheat_chain_id_global, stop_cheat_chain_id_global, test_address + }; use utils::constants::POW_2_53; + use utils::helpers::compute_starknet_address; fn set_up() -> KakarotCore::ContractState { // Define the kakarot state to access contract functions @@ -240,6 +248,19 @@ mod tests { stop_cheat_chain_id_global(); } + #[test] + fn test_eth_get_transaction_count() { + let kakarot_state = set_up(); + // Deployed eoa should return a zero nonce + let starknet_address = compute_starknet_address( + test_address(), + evm_address(), + 0.try_into().unwrap() // Using 0 as the kakarot storage is empty + ); + start_mock_call::(starknet_address, selector!("get_nonce"), 1); + assert_eq!(kakarot_state.eth_get_transaction_count(evm_address()), 1); + } + #[test] fn test_eth_get_balance() { let (native_token, kakarot_core) = setup_contracts_for_testing(); diff --git a/crates/contracts/src/kakarot_core/interface.cairo b/crates/contracts/src/kakarot_core/interface.cairo index 603a52f42..aa5e8e9fa 100644 --- a/crates/contracts/src/kakarot_core/interface.cairo +++ b/crates/contracts/src/kakarot_core/interface.cairo @@ -88,6 +88,9 @@ pub trait IExtendedKakarotCore { /// Executes an EVM transaction and possibly modifies the state fn eth_send_transaction(ref self: TContractState, tx: Transaction) -> (bool, Span, u64); + // Returns the transaction count (nonce) of the specified address + fn eth_get_transaction_count(self: @TContractState, address: EthAddress) -> u64; + /// Upgrade the KakarotCore smart contract /// Using replace_class_syscall fn upgrade(ref self: TContractState, new_class_hash: ClassHash); From 109ab1e161dc913cac30b4dc255f4d52a1876433 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 30 Sep 2024 17:52:36 +0200 Subject: [PATCH 11/22] fix: mulmod (#998) * fix: mulmod * fmt --- .../src/instructions/stop_and_arithmetic_operations.cairo | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/evm/src/instructions/stop_and_arithmetic_operations.cairo b/crates/evm/src/instructions/stop_and_arithmetic_operations.cairo index 70c288c03..9f302aee2 100644 --- a/crates/evm/src/instructions/stop_and_arithmetic_operations.cairo +++ b/crates/evm/src/instructions/stop_and_arithmetic_operations.cairo @@ -1,5 +1,6 @@ //! Stop and Arithmetic Operations. use core::integer::{u512_safe_div_rem_by_u256}; +use core::math::u256_mul_mod_n; use core::num::traits::{OverflowingAdd, OverflowingMul, OverflowingSub}; use crate::errors::EVMError; use crate::gas; @@ -186,12 +187,7 @@ pub impl StopAndArithmeticOperations of StopAndArithmeticOperationsTrait { let n = self.stack.pop()?; let result: u256 = match TryInto::>::try_into(n) { - Option::Some(_) => { - // (x * y) mod N <=> (x mod N) * (y mod N) mod N - // It is more gas-efficient than to use u256_wide_mul - // Won't panic because n is not zero - (a % n) * (b % n) % n - }, + Option::Some(n_nz) => { u256_mul_mod_n(a, b, n_nz) }, Option::None => 0, }; From 9baf1a937598fac6d9bd6c64e68064a933abfb15 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 30 Sep 2024 17:52:51 +0200 Subject: [PATCH 12/22] fix: overflow in message_call_gas (#996) * fix: overflow in message_call_gas * fmt --- crates/evm/src/gas.cairo | 20 ++++++++++++------- .../src/instructions/system_operations.cairo | 8 ++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/crates/evm/src/gas.cairo b/crates/evm/src/gas.cairo index 60011fd8e..fa359bd2e 100644 --- a/crates/evm/src/gas.cairo +++ b/crates/evm/src/gas.cairo @@ -103,27 +103,33 @@ pub fn max_message_call_gas(gas: u64) -> u64 { /// * `memory_cost`: The amount needed to extend the memory in the current frame. /// * `extra_gas`: The amount of gas needed for transferring value + creating a new account inside a /// message call. -/// * `call_stipend`: The amount of stipend provided to a message call to execute code while -/// transferring value(native token). /// /// # Returns /// -/// * `message_call_gas`: `MessageCallGas` +/// * `Result`: The calculated MessageCallGas or an error if overflow +/// occurs. pub fn calculate_message_call_gas( value: u256, gas: u64, gas_left: u64, memory_cost: u64, extra_gas: u64 -) -> MessageCallGas { +) -> Result { let call_stipend = if value == 0 { 0 } else { CALL_STIPEND }; - let gas = if gas_left < extra_gas + memory_cost { + + // Check for overflow when adding extra_gas and memory_cost + let total_extra_cost = extra_gas.checked_add(memory_cost).ok_or(EVMError::OutOfGas)?; + let gas = if gas_left < total_extra_cost { gas } else { - min(gas, max_message_call_gas(gas_left - memory_cost - extra_gas)) + let remaining_gas = gas_left - total_extra_cost; // Safe because of the check above + min(gas, max_message_call_gas(remaining_gas)) }; - return MessageCallGas { cost: gas + extra_gas, stipend: gas + call_stipend }; + let cost = gas.checked_add(extra_gas).ok_or(EVMError::OutOfGas)?; + let stipend = gas.checked_add(call_stipend).ok_or(EVMError::OutOfGas)?; + + Result::Ok(MessageCallGas { cost, stipend }) } diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index c55a5d071..fcfb1a633 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -65,7 +65,7 @@ pub impl SystemOperations of SystemOperationsTrait { self.gas_left(), memory_expansion.expansion_cost, access_gas_cost + transfer_gas_cost + create_gas_cost - ); + )?; self.charge_gas(message_call_gas.cost + memory_expansion.expansion_cost)?; // Only the transfer gas is left to charge. @@ -140,7 +140,7 @@ pub impl SystemOperations of SystemOperationsTrait { self.gas_left(), memory_expansion.expansion_cost, access_gas_cost + transfer_gas_cost - ); + )?; self.charge_gas(message_call_gas.cost + memory_expansion.expansion_cost)?; // If sender_balance < value, return early, pushing @@ -212,7 +212,7 @@ 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)?; self @@ -265,7 +265,7 @@ 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 - ); + )?; let gas_to_charge = message_call_gas .cost .checked_add(memory_expansion.expansion_cost) From 1a80ed7a63bfc8c4725f54da4c703dc4f1947409 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 30 Sep 2024 18:38:40 +0200 Subject: [PATCH 13/22] fix: mcopy offset oob (#999) --- crates/evm/src/instructions/memory_operations.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index fc56ebdc1..0248a7b35 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -279,7 +279,7 @@ pub impl MemoryOperation of MemoryOperationTrait { /// # Specification: https://www.evm.codes/#5e?fork=cancun fn exec_mcopy(ref self: VM) -> Result<(), EVMError> { let dest_offset = self.stack.pop_saturating_usize()?; - let source_offset = self.stack.pop_usize()?; + let source_offset = self.stack.pop_saturating_usize()?; let size = self.stack.pop_usize()?; let words_size = bytes_32_words_size(size).into(); From e2bd71dce49ec2dac026c67eab54a695d050efcc Mon Sep 17 00:00:00 2001 From: omahs <73983677+omahs@users.noreply.github.com> Date: Tue, 1 Oct 2024 10:25:56 +0200 Subject: [PATCH 14/22] fix: typos (#1000) * fix typos * fix typo * fix typos * fix typo * fix typo --- crates/contracts/src/cairo1_helpers.cairo | 2 +- crates/contracts/src/kakarot_core/interface.cairo | 4 ++-- crates/evm/src/call_helpers.cairo | 2 +- crates/evm/src/interpreter.cairo | 2 +- docs/general/contract_storage.md | 6 +++--- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/contracts/src/cairo1_helpers.cairo b/crates/contracts/src/cairo1_helpers.cairo index 4bd3b89d2..1a22b9721 100644 --- a/crates/contracts/src/cairo1_helpers.cairo +++ b/crates/contracts/src/cairo1_helpers.cairo @@ -88,7 +88,7 @@ pub trait IHelpers { /// * The recovered Ethereum address. fn recover_eth_address(self: @T, msg_hash: u256, signature: Signature) -> (bool, EthAddress); - /// Performs signature verification in the secp256r1 ellipitic curve. + /// Performs signature verification in the secp256r1 elliptic curve. /// /// # Arguments /// diff --git a/crates/contracts/src/kakarot_core/interface.cairo b/crates/contracts/src/kakarot_core/interface.cairo index aa5e8e9fa..4bce029d5 100644 --- a/crates/contracts/src/kakarot_core/interface.cairo +++ b/crates/contracts/src/kakarot_core/interface.cairo @@ -9,7 +9,7 @@ pub trait IKakarotCore { /// Gets the native token used by the Kakarot smart contract fn get_native_token(self: @TContractState) -> ContractAddress; - /// Deterministically computes a Starknet address for an given EVM address + /// Deterministically computes a Starknet address for a given EVM address /// The address is computed as the Starknet address corresponding to the deployment of an EOA, /// Using its EVM address as salt, and KakarotCore as deployer. fn compute_starknet_address(self: @TContractState, evm_address: EthAddress) -> ContractAddress; @@ -59,7 +59,7 @@ pub trait IExtendedKakarotCore { /// Gets the native token used by the Kakarot smart contract fn get_native_token(self: @TContractState) -> ContractAddress; - /// Deterministically computes a Starknet address for an given EVM address + /// Deterministically computes a Starknet address for a given EVM address /// The address is computed as the Starknet address corresponding to the deployment of an EOA, /// Using its EVM address as salt, and KakarotCore as deployer. fn compute_starknet_address(self: @TContractState, evm_address: EthAddress) -> ContractAddress; diff --git a/crates/evm/src/call_helpers.cairo b/crates/evm/src/call_helpers.cairo index 61e67031d..26088d43c 100644 --- a/crates/evm/src/call_helpers.cairo +++ b/crates/evm/src/call_helpers.cairo @@ -107,7 +107,7 @@ pub impl CallHelpersImpl of CallHelpers { self.stack.push(0)?; }, ExecutionResultStatus::Exception => { - // If the call has halted exceptionnaly, + // If the call has halted exceptionally, // the return_data is emptied, and nothing is stored in memory self.return_data = [].span(); self.stack.push(0)?; diff --git a/crates/evm/src/interpreter.cairo b/crates/evm/src/interpreter.cairo index a17d06317..d17883c1e 100644 --- a/crates/evm/src/interpreter.cairo +++ b/crates/evm/src/interpreter.cairo @@ -108,7 +108,7 @@ pub impl EVMImpl of EVMTrait { let mut sender_account = env.state.get_account(origin.evm); // Charge the intrinsic gas to the sender so that it's not available for the execution - // of the transaction but don't trigger any actual transfer, as only the actual consumde + // of the transaction but don't trigger any actual transfer, as only the actual consumed // gas is charged at the end of the transaction sender_account.set_balance(sender_account.balance() - max_fee.into()); diff --git a/docs/general/contract_storage.md b/docs/general/contract_storage.md index 042907767..b77599022 100644 --- a/docs/general/contract_storage.md +++ b/docs/general/contract_storage.md @@ -75,7 +75,7 @@ As Kakarot is a contract that is deployed on Starknet and is not a client that can directly manipulate a storage database, our approach differs from one of a traditional client. We do not directly manipulate tries. Instead, we have access to contracts storage on the Starknet blockchain, that we can query using -syscalls to read and update the value of a of a storage slot. +syscalls to read and update the value of a storage slot. There are two different ways of handling Storage in Kakarot. @@ -105,7 +105,7 @@ This design has some limitations: - We perform a `call_contract_syscall` for each SLOAD/SSTORE operation that is committed to Starknet, which has an extra overhead compared to directly - modifying the current contract's storage . Given that only KakarotCore can + modifying the current contract's storage. Given that only KakarotCore can modify the storage of a Kakarot contract, we could directly store the whole world state in the main Kakarot contract storage. - It adds external entrypoints with admin rights to read and write from storage @@ -171,7 +171,7 @@ compatibility with Starknet. ### Tracking and reverting storage changes The storage mechanism presented in the [Local State](./local_state.md) section -enable us to revert storage changes by using a concept similar to Geth's +enables us to revert storage changes by using a concept similar to Geth's journal. Each storage change will be stored in a `StateChangeLog` implemented using a `Felt252Dict` data structure, that will associate each modified storage address to its new value. This allows us to perform three things: From 79a5e8d93e027c942e4fb2970cc7f00a0ee4aaaf Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Tue, 1 Oct 2024 15:20:54 +0200 Subject: [PATCH 15/22] fix: saturate jumpi index taken on stack (#1002) * fix: saturate jumpi index taken on stack * scout: remove print --- crates/contracts/src/kakarot_core/eth_rpc.cairo | 1 - crates/evm/src/instructions/memory_operations.cairo | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/contracts/src/kakarot_core/eth_rpc.cairo b/crates/contracts/src/kakarot_core/eth_rpc.cairo index c824fb4f6..8e98b20e4 100644 --- a/crates/contracts/src/kakarot_core/eth_rpc.cairo +++ b/crates/contracts/src/kakarot_core/eth_rpc.cairo @@ -134,7 +134,6 @@ pub impl EthRPC< fn eth_get_transaction_count(self: @TContractState, address: EthAddress) -> u64 { let kakarot_state = KakarotState::get_state(); let starknet_address = kakarot_state.get_starknet_address(address); - println!("starknet_address: {:?}", starknet_address); let account = IAccountDispatcher { contract_address: starknet_address }; let nonce = account.get_nonce(); nonce diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 0248a7b35..3c60ee4b4 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -188,7 +188,9 @@ pub impl MemoryOperation of MemoryOperationTrait { /// The new pc target has to be a JUMPDEST opcode. /// # Specification: https://www.evm.codes/#57?fork=shanghai fn exec_jumpi(ref self: VM) -> Result<(), EVMError> { - let index = self.stack.pop_usize()?; + let index = self + .stack + .pop_saturating_usize()?; // Saturate because if b is 0, we skip the jump but don't want to fail here. let b = self.stack.pop()?; self.charge_gas(gas::HIGH)?; From 6a23cedebd2899e14882af9cb4930ac664846ad8 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Tue, 1 Oct 2024 15:28:42 +0200 Subject: [PATCH 16/22] refactor: Bitshift takes usize as arg (#1001) dev: optimized bitshifts by using a lookup table for powers of two tmp --- .../instructions/comparison_operations.cairo | 9 +- crates/evm/src/memory.cairo | 2 +- crates/utils/src/constants.cairo | 261 ++++++++++++++++++ crates/utils/src/crypto/blake2_compress.cairo | 2 +- crates/utils/src/crypto/modexp/arith.cairo | 24 +- crates/utils/src/crypto/modexp/mpnat.cairo | 11 +- crates/utils/src/math.cairo | 38 +-- crates/utils/src/traits/bytes.cairo | 19 +- crates/utils/src/traits/eth_address.cairo | 8 +- crates/utils/src/traits/integer.cairo | 4 +- 10 files changed, 321 insertions(+), 57 deletions(-) diff --git a/crates/evm/src/instructions/comparison_operations.cairo b/crates/evm/src/instructions/comparison_operations.cairo index e6f6e84b4..b040f21b0 100644 --- a/crates/evm/src/instructions/comparison_operations.cairo +++ b/crates/evm/src/instructions/comparison_operations.cairo @@ -132,6 +132,7 @@ pub impl ComparisonAndBitwiseOperations of ComparisonAndBitwiseOperationsTrait { if i > 31 { return self.stack.push(0); } + let i: usize = i.try_into().unwrap(); // Safe because i <= 31 // Right shift value by offset bits and then take the least significant byte. let result = x.shr((31 - i) * 8) & 0xFF; @@ -150,7 +151,7 @@ pub impl ComparisonAndBitwiseOperations of ComparisonAndBitwiseOperationsTrait { if shift > 255 { return self.stack.push(0); } - + let shift: usize = shift.try_into().unwrap(); // Safe because shift <= 255 let result = val.wrapping_shl(shift); self.stack.push(result) } @@ -163,6 +164,11 @@ pub impl ComparisonAndBitwiseOperations of ComparisonAndBitwiseOperationsTrait { let shift = *popped[0]; let value = *popped[1]; + // if shift is bigger than 255 return 0 + if shift > 255 { + return self.stack.push(0); + } + let shift: usize = shift.try_into().unwrap(); // Safe because shift <= 255 let result = value.wrapping_shr(shift); self.stack.push(result) } @@ -187,6 +193,7 @@ pub impl ComparisonAndBitwiseOperations of ComparisonAndBitwiseOperationsTrait { if (shift >= 256) { self.stack.push(sign) } else { + let shift: usize = shift.try_into().unwrap(); // Safe because shift <= 256 // XORing with sign before and after the shift propagates the sign bit of the operation let result = (sign ^ value.value).shr(shift) ^ sign; self.stack.push(result) diff --git a/crates/evm/src/memory.cairo b/crates/evm/src/memory.cairo index 9659045c8..fb3fc11ac 100644 --- a/crates/evm/src/memory.cairo +++ b/crates/evm/src/memory.cairo @@ -89,7 +89,7 @@ impl MemoryImpl of MemoryTrait { // First erase byte value at offset, then set the new value using bitwise ops let word: u128 = self.items.get(chunk_index.into()); - let new_word = (word & ~mask) | (value.into().shl(right_offset.into() * 8)); + let new_word = (word & ~mask) | (value.into().shl(right_offset * 8)); self.items.insert(chunk_index.into(), new_word); } diff --git a/crates/utils/src/constants.cairo b/crates/utils/src/constants.cairo index 78ce66bb3..0b4b1c676 100644 --- a/crates/utils/src/constants.cairo +++ b/crates/utils/src/constants.cairo @@ -210,6 +210,267 @@ pub const POW_2: [ 0x80000000000000000000000000000000 ]; +pub const POW_2_256: [ + u256 + ; 256] = [ + 0x1, + 0x2, + 0x4, + 0x8, + 0x10, + 0x20, + 0x40, + 0x80, + 0x100, + 0x200, + 0x400, + 0x800, + 0x1000, + 0x2000, + 0x4000, + 0x8000, + 0x10000, + 0x20000, + 0x40000, + 0x80000, + 0x100000, + 0x200000, + 0x400000, + 0x800000, + 0x1000000, + 0x2000000, + 0x4000000, + 0x8000000, + 0x10000000, + 0x20000000, + 0x40000000, + 0x80000000, + 0x100000000, + 0x200000000, + 0x400000000, + 0x800000000, + 0x1000000000, + 0x2000000000, + 0x4000000000, + 0x8000000000, + 0x10000000000, + 0x20000000000, + 0x40000000000, + 0x80000000000, + 0x100000000000, + 0x200000000000, + 0x400000000000, + 0x800000000000, + 0x1000000000000, + 0x2000000000000, + 0x4000000000000, + 0x8000000000000, + 0x10000000000000, + 0x20000000000000, + 0x40000000000000, + 0x80000000000000, + 0x100000000000000, + 0x200000000000000, + 0x400000000000000, + 0x800000000000000, + 0x1000000000000000, + 0x2000000000000000, + 0x4000000000000000, + 0x8000000000000000, + 0x10000000000000000, + 0x20000000000000000, + 0x40000000000000000, + 0x80000000000000000, + 0x100000000000000000, + 0x200000000000000000, + 0x400000000000000000, + 0x800000000000000000, + 0x1000000000000000000, + 0x2000000000000000000, + 0x4000000000000000000, + 0x8000000000000000000, + 0x10000000000000000000, + 0x20000000000000000000, + 0x40000000000000000000, + 0x80000000000000000000, + 0x100000000000000000000, + 0x200000000000000000000, + 0x400000000000000000000, + 0x800000000000000000000, + 0x1000000000000000000000, + 0x2000000000000000000000, + 0x4000000000000000000000, + 0x8000000000000000000000, + 0x10000000000000000000000, + 0x20000000000000000000000, + 0x40000000000000000000000, + 0x80000000000000000000000, + 0x100000000000000000000000, + 0x200000000000000000000000, + 0x400000000000000000000000, + 0x800000000000000000000000, + 0x1000000000000000000000000, + 0x2000000000000000000000000, + 0x4000000000000000000000000, + 0x8000000000000000000000000, + 0x10000000000000000000000000, + 0x20000000000000000000000000, + 0x40000000000000000000000000, + 0x80000000000000000000000000, + 0x100000000000000000000000000, + 0x200000000000000000000000000, + 0x400000000000000000000000000, + 0x800000000000000000000000000, + 0x1000000000000000000000000000, + 0x2000000000000000000000000000, + 0x4000000000000000000000000000, + 0x8000000000000000000000000000, + 0x10000000000000000000000000000, + 0x20000000000000000000000000000, + 0x40000000000000000000000000000, + 0x80000000000000000000000000000, + 0x100000000000000000000000000000, + 0x200000000000000000000000000000, + 0x400000000000000000000000000000, + 0x800000000000000000000000000000, + 0x1000000000000000000000000000000, + 0x2000000000000000000000000000000, + 0x4000000000000000000000000000000, + 0x8000000000000000000000000000000, + 0x10000000000000000000000000000000, + 0x20000000000000000000000000000000, + 0x40000000000000000000000000000000, + 0x80000000000000000000000000000000, + 0x100000000000000000000000000000000, + 0x200000000000000000000000000000000, + 0x400000000000000000000000000000000, + 0x800000000000000000000000000000000, + 0x1000000000000000000000000000000000, + 0x2000000000000000000000000000000000, + 0x4000000000000000000000000000000000, + 0x8000000000000000000000000000000000, + 0x10000000000000000000000000000000000, + 0x20000000000000000000000000000000000, + 0x40000000000000000000000000000000000, + 0x80000000000000000000000000000000000, + 0x100000000000000000000000000000000000, + 0x200000000000000000000000000000000000, + 0x400000000000000000000000000000000000, + 0x800000000000000000000000000000000000, + 0x1000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000, + 0x10000000000000000000000000000000000000, + 0x20000000000000000000000000000000000000, + 0x40000000000000000000000000000000000000, + 0x80000000000000000000000000000000000000, + 0x100000000000000000000000000000000000000, + 0x200000000000000000000000000000000000000, + 0x400000000000000000000000000000000000000, + 0x800000000000000000000000000000000000000, + 0x1000000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000000, + 0x10000000000000000000000000000000000000000, + 0x20000000000000000000000000000000000000000, + 0x40000000000000000000000000000000000000000, + 0x80000000000000000000000000000000000000000, + 0x100000000000000000000000000000000000000000, + 0x200000000000000000000000000000000000000000, + 0x400000000000000000000000000000000000000000, + 0x800000000000000000000000000000000000000000, + 0x1000000000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000000000, + 0x10000000000000000000000000000000000000000000, + 0x20000000000000000000000000000000000000000000, + 0x40000000000000000000000000000000000000000000, + 0x80000000000000000000000000000000000000000000, + 0x100000000000000000000000000000000000000000000, + 0x200000000000000000000000000000000000000000000, + 0x400000000000000000000000000000000000000000000, + 0x800000000000000000000000000000000000000000000, + 0x1000000000000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000000000000, + 0x10000000000000000000000000000000000000000000000, + 0x20000000000000000000000000000000000000000000000, + 0x40000000000000000000000000000000000000000000000, + 0x80000000000000000000000000000000000000000000000, + 0x100000000000000000000000000000000000000000000000, + 0x200000000000000000000000000000000000000000000000, + 0x400000000000000000000000000000000000000000000000, + 0x800000000000000000000000000000000000000000000000, + 0x1000000000000000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000000000000000, + 0x10000000000000000000000000000000000000000000000000, + 0x20000000000000000000000000000000000000000000000000, + 0x40000000000000000000000000000000000000000000000000, + 0x80000000000000000000000000000000000000000000000000, + 0x100000000000000000000000000000000000000000000000000, + 0x200000000000000000000000000000000000000000000000000, + 0x400000000000000000000000000000000000000000000000000, + 0x800000000000000000000000000000000000000000000000000, + 0x1000000000000000000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000000000000000000, + 0x10000000000000000000000000000000000000000000000000000, + 0x20000000000000000000000000000000000000000000000000000, + 0x40000000000000000000000000000000000000000000000000000, + 0x80000000000000000000000000000000000000000000000000000, + 0x100000000000000000000000000000000000000000000000000000, + 0x200000000000000000000000000000000000000000000000000000, + 0x400000000000000000000000000000000000000000000000000000, + 0x800000000000000000000000000000000000000000000000000000, + 0x1000000000000000000000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000000000000000000000, + 0x10000000000000000000000000000000000000000000000000000000, + 0x20000000000000000000000000000000000000000000000000000000, + 0x40000000000000000000000000000000000000000000000000000000, + 0x80000000000000000000000000000000000000000000000000000000, + 0x100000000000000000000000000000000000000000000000000000000, + 0x200000000000000000000000000000000000000000000000000000000, + 0x400000000000000000000000000000000000000000000000000000000, + 0x800000000000000000000000000000000000000000000000000000000, + 0x1000000000000000000000000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000000000000000000000000, + 0x10000000000000000000000000000000000000000000000000000000000, + 0x20000000000000000000000000000000000000000000000000000000000, + 0x40000000000000000000000000000000000000000000000000000000000, + 0x80000000000000000000000000000000000000000000000000000000000, + 0x100000000000000000000000000000000000000000000000000000000000, + 0x200000000000000000000000000000000000000000000000000000000000, + 0x400000000000000000000000000000000000000000000000000000000000, + 0x800000000000000000000000000000000000000000000000000000000000, + 0x1000000000000000000000000000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000000000000000000000000000, + 0x10000000000000000000000000000000000000000000000000000000000000, + 0x20000000000000000000000000000000000000000000000000000000000000, + 0x40000000000000000000000000000000000000000000000000000000000000, + 0x80000000000000000000000000000000000000000000000000000000000000, + 0x100000000000000000000000000000000000000000000000000000000000000, + 0x200000000000000000000000000000000000000000000000000000000000000, + 0x400000000000000000000000000000000000000000000000000000000000000, + 0x800000000000000000000000000000000000000000000000000000000000000, + 0x1000000000000000000000000000000000000000000000000000000000000000, + 0x2000000000000000000000000000000000000000000000000000000000000000, + 0x4000000000000000000000000000000000000000000000000000000000000000, + 0x8000000000000000000000000000000000000000000000000000000000000000, +]; + pub const POW_2_0: u128 = 0x1; pub const POW_2_8: u128 = 0x100; pub const POW_2_16: u128 = 0x10000; diff --git a/crates/utils/src/crypto/blake2_compress.cairo b/crates/utils/src/crypto/blake2_compress.cairo index e008e7271..fe20466ef 100644 --- a/crates/utils/src/crypto/blake2_compress.cairo +++ b/crates/utils/src/crypto/blake2_compress.cairo @@ -54,7 +54,7 @@ fn rotate_right(value: u64, n: u32) -> u64 { let bits = BitSize::::bits(); // The number of bits in a u64 let n = n % bits; // Ensure n is less than 64 - let res = value.wrapping_shr(n.into()) | value.wrapping_shl((bits - n).into()); + let res = value.wrapping_shr(n) | value.wrapping_shl((bits - n)); res } } diff --git a/crates/utils/src/crypto/modexp/arith.cairo b/crates/utils/src/crypto/modexp/arith.cairo index d82bf2f61..d302048a8 100644 --- a/crates/utils/src/crypto/modexp/arith.cairo +++ b/crates/utils/src/crypto/modexp/arith.cairo @@ -300,7 +300,7 @@ pub fn mod_inv(x: Word) -> Word { break; } - let mask: u64 = 1_u64.shl(i.into()) - 1; + let mask: u64 = 1_u64.shl(i) - 1; let xy = x.wrapping_mul(y) & mask; let q = (mask + 1) / 2; if xy >= q { @@ -310,7 +310,7 @@ pub fn mod_inv(x: Word) -> Word { }; let xy = x.wrapping_mul(y); - let q = 1_u64.wrapping_shl((WORD_BITS - 1).into()); + let q = 1_u64.wrapping_shl((WORD_BITS - 1)); if xy >= q { y += q; } @@ -415,7 +415,7 @@ pub fn borrowing_sub(x: Word, y: Word, borrow: bool) -> (Word, bool) { /// The double word obtained by joining `hi` and `lo` pub fn join_as_double(hi: Word, lo: Word) -> DoubleWord { let hi: DoubleWord = hi.into(); - (hi.shl(WORD_BITS.into())).into() + lo.into() + hi.shl(WORD_BITS).into() + lo.into() } /// Computes `x^2`, storing the result in `out`. @@ -457,14 +457,14 @@ fn big_sq(ref x: MPNat, ref out: Felt252Vec) { } out.set(i + j, res.as_u64()); - c = new_c + res.shr(WORD_BITS.into()); + c = new_c + res.shr(WORD_BITS); j += 1; }; let (sum, carry) = carrying_add(out[i + s], c.as_u64(), false); out.set(i + s, sum); - out.set(i + s + 1, (c.shr(WORD_BITS.into()) + (carry.into())).as_u64()); + out.set(i + s + 1, (c.shr(WORD_BITS) + (carry.into())).as_u64()); i += 1; } @@ -482,8 +482,8 @@ pub fn in_place_shl(ref a: Felt252Vec, shift: u32) -> Word { } let mut a_digit = a[i]; - let carry = a_digit.wrapping_shr(carry_shift.into()); - a_digit = a_digit.wrapping_shl(shift.into()) | c; + let carry = a_digit.wrapping_shr(carry_shift); + a_digit = a_digit.wrapping_shl(shift) | c; a.set(i, a_digit); c = carry; @@ -508,8 +508,8 @@ pub fn in_place_shr(ref a: Felt252Vec, shift: u32) -> Word { let j = i - 1; let mut a_digit = a[j]; - let borrow = a_digit.wrapping_shl(borrow_shift.into()); - a_digit = a_digit.wrapping_shr(shift.into()) | b; + let borrow = a_digit.wrapping_shl(borrow_shift); + a_digit = a_digit.wrapping_shr(shift) | b; a.set(j, a_digit); b = borrow; @@ -574,7 +574,7 @@ pub fn in_place_mul_sub(ref a: Felt252Vec, ref x: Felt252Vec, y: Wor + offset_carry.into() - ((x_digit.into()) * (y.into())); - let new_offset_carry = (offset_sum.shr(WORD_BITS.into())).as_u64(); + let new_offset_carry = (offset_sum.shr(WORD_BITS)).as_u64(); let new_x = offset_sum.as_u64(); offset_carry = new_offset_carry; a.set(i, new_x); @@ -661,7 +661,7 @@ mod tests { let mut result = mp_nat_to_u128(ref x); let mask = BASE.wrapping_pow(x.digits.len().into()).wrapping_sub(1); - assert_eq!(result, n.wrapping_shl(shift.into()) & mask); + assert_eq!(result, n.wrapping_shl(shift) & mask); } fn check_in_place_shr(n: u128, shift: u32) { @@ -669,7 +669,7 @@ mod tests { in_place_shr(ref x.digits, shift); let mut result = mp_nat_to_u128(ref x); - assert_eq!(result, n.wrapping_shr(shift.into())); + assert_eq!(result, n.wrapping_shr(shift)); } fn check_mod_inv(n: Word) { diff --git a/crates/utils/src/crypto/modexp/mpnat.cairo b/crates/utils/src/crypto/modexp/mpnat.cairo index 5f461837f..6cd2d7fff 100644 --- a/crates/utils/src/crypto/modexp/mpnat.cairo +++ b/crates/utils/src/crypto/modexp/mpnat.cairo @@ -323,7 +323,7 @@ pub impl MPNatTraitImpl of MPNatTrait { in_place_shr(ref b.digits, 1); - res.digits.set(wordpos, res.digits[wordpos] | (x.shl(bitpos.into()))); + res.digits.set(wordpos, res.digits[wordpos] | (x.shl(bitpos))); bitpos += 1; if bitpos == WORD_BITS { @@ -404,7 +404,7 @@ pub impl MPNatTraitImpl of MPNatTrait { let mut digits = Felt252VecImpl::new(); digits.expand(trailing_zeros + 1).unwrap(); let mut tmp = MPNat { digits }; - tmp.digits.set(trailing_zeros, 1_u64.shl(additional_zero_bits.into())); + tmp.digits.set(trailing_zeros, 1_u64.shl(additional_zero_bits)); tmp }; @@ -415,7 +415,7 @@ pub impl MPNatTraitImpl of MPNatTrait { digits.expand(num_digits).unwrap(); let mut tmp = MPNat { digits }; if additional_zero_bits > 0 { - tmp.digits.set(0, modulus.digits[trailing_zeros].shr(additional_zero_bits.into())); + tmp.digits.set(0, modulus.digits[trailing_zeros].shr(additional_zero_bits)); let mut i = 1; loop { if i == num_digits { @@ -429,10 +429,9 @@ pub impl MPNatTraitImpl of MPNatTrait { i - 1, tmp.digits[i - 1] - + (d & power_of_two_mask) - .shl((WORD_BITS - additional_zero_bits).into()) + + (d & power_of_two_mask).shl(WORD_BITS - additional_zero_bits) ); - tmp.digits.set(i, d.shr(additional_zero_bits.into())); + tmp.digits.set(i, d.shr(additional_zero_bits)); i += 1; }; diff --git a/crates/utils/src/math.cairo b/crates/utils/src/math.cairo index 6e77fdbc9..ec1344b2d 100644 --- a/crates/utils/src/math.cairo +++ b/crates/utils/src/math.cairo @@ -1,5 +1,5 @@ use core::integer::{u512}; -use core::num::traits::{Zero, One, BitSize, OverflowingAdd, OverflowingMul}; +use core::num::traits::{Zero, One, BitSize, OverflowingAdd, OverflowingMul, Bounded}; use core::panic_with_felt252; use core::traits::{BitAnd}; @@ -203,7 +203,7 @@ pub trait Bitshift { /// /// Panics if the shift is greater than 255. /// Panics if the result overflows the type T. - fn shl(self: T, shift: T) -> T; + fn shl(self: T, shift: usize) -> T; /// Shift a number right by a given number of bits. /// @@ -219,7 +219,7 @@ pub trait Bitshift { /// # Panics /// /// Panics if the shift is greater than 255. - fn shr(self: T, shift: T) -> T; + fn shr(self: T, shift: usize) -> T; } impl BitshiftImpl< @@ -236,24 +236,26 @@ impl BitshiftImpl< +PartialOrd, +BitSize, +TryInto, + +TryInto, + +TryInto, > of Bitshift { - fn shl(self: T, shift: T) -> T { + fn shl(self: T, shift: usize) -> T { // if we shift by more than nb_bits of T, the result is 0 // we early return to save gas and prevent unexpected behavior - if shift > BitSize::::bits().try_into().unwrap() - One::one() { + if shift > BitSize::::bits() - One::one() { panic_with_felt252('mul Overflow'); } let two = One::one() + One::one(); - self * two.pow(shift) + self * two.pow(shift.try_into().expect('mul Overflow')) } - fn shr(self: T, shift: T) -> T { + fn shr(self: T, shift: usize) -> T { // early return to save gas if shift > nb_bits of T - if shift > BitSize::::bits().try_into().unwrap() - One::one() { + if shift > BitSize::::bits() - One::one() { panic_with_felt252('mul Overflow'); } let two = One::one() + One::one(); - self / two.pow(shift) + self / two.pow(shift.try_into().expect('mul Overflow')) } } @@ -270,7 +272,7 @@ pub trait WrappingBitshift { /// # Returns /// /// The result of shifting `self` left by `shift` bits, wrapped if necessary - fn wrapping_shl(self: T, shift: T) -> T; + fn wrapping_shl(self: T, shift: usize) -> T; /// Shift a number right by a given number of bits. /// If the shift is greater than 255, the result is 0. @@ -283,7 +285,7 @@ pub trait WrappingBitshift { /// # Returns /// /// The result of shifting `self` right by `shift` bits, or 0 if shift > 255 - fn wrapping_shr(self: T, shift: T) -> T; + fn wrapping_shr(self: T, shift: usize) -> T; } pub impl WrappingBitshiftImpl< @@ -300,21 +302,25 @@ pub impl WrappingBitshiftImpl< +OverflowingMul, +WrappingExponentiation, +BitSize, + +Bounded, +TryInto, + +TryInto, + +TryInto, + +Into > of WrappingBitshift { - fn wrapping_shl(self: T, shift: T) -> T { + fn wrapping_shl(self: T, shift: usize) -> T { let two = One::::one() + One::::one(); - let (result, _) = self.overflowing_mul(two.wrapping_pow(shift)); + let (result, _) = self.overflowing_mul(two.wrapping_pow(shift.try_into().unwrap())); result } - fn wrapping_shr(self: T, shift: T) -> T { + fn wrapping_shr(self: T, shift: usize) -> T { let two = One::::one() + One::::one(); - if shift > BitSize::::bits().try_into().unwrap() - One::one() { + if shift > BitSize::::bits() - One::one() { return Zero::zero(); } - self / two.pow(shift) + self / two.pow(shift.try_into().unwrap()) } } diff --git a/crates/utils/src/traits/bytes.cairo b/crates/utils/src/traits/bytes.cairo index 6cec31aee..1499597d5 100644 --- a/crates/utils/src/traits/bytes.cairo +++ b/crates/utils/src/traits/bytes.cairo @@ -50,7 +50,7 @@ pub impl U8SpanExImpl of U8SpanExTrait { Option::Some(byte) => { let byte: u64 = (*byte.unbox()).into(); // Accumulate pending_word in a little endian manner - byte.shl(8_u64 * byte_counter.into()) + byte.shl(8_u32 * byte_counter.into()) }, Option::None => { break; }, }; @@ -69,7 +69,7 @@ pub impl U8SpanExImpl of U8SpanExTrait { last_input_word += match self.get(full_u64_word_count * 8 + byte_counter.into()) { Option::Some(byte) => { let byte: u64 = (*byte.unbox()).into(); - byte.shl(8_u64 * byte_counter.into()) + byte.shl(8_u32 * byte_counter.into()) }, Option::None => { break; }, }; @@ -246,17 +246,13 @@ pub impl ToBytesImpl< fn to_be_bytes(self: T) -> Span { let bytes_used = self.bytes_used(); - let one = One::::one(); - let two = one + one; - let eight = two * two * two; - // 0xFF let mask = Bounded::::MAX.into(); let mut bytes: Array = Default::default(); for i in 0 ..bytes_used { - let val = Bitshift::::shr(self, eight * (bytes_used - i - 1).into()); + let val = Bitshift::::shr(self, 8_u32 * (bytes_used.into() - i.into() - 1)); bytes.append((val & mask).try_into().unwrap()); }; @@ -270,9 +266,6 @@ pub impl ToBytesImpl< fn to_le_bytes(mut self: T) -> Span { let bytes_used = self.bytes_used(); - let one = One::::one(); - let two = one + one; - let eight = two * two * two; // 0xFF let mask = Bounded::::MAX.into(); @@ -281,7 +274,7 @@ pub impl ToBytesImpl< for i in 0 ..bytes_used { - let val = self.shr(eight * i.into()); + let val = self.shr(8_u32 * i.into()); bytes.append((val & mask).try_into().unwrap()); }; @@ -526,7 +519,7 @@ pub impl ByteArrayExt of ByteArrayExTrait { Option::Some(byte) => { let byte: u64 = byte.into(); // Accumulate pending_word in a little endian manner - byte.shl(8_u64 * byte_counter.into()) + byte.shl(8_u32 * byte_counter.into()) }, Option::None => { break; }, }; @@ -546,7 +539,7 @@ pub impl ByteArrayExt of ByteArrayExTrait { last_input_word += match self.at(full_u64_word_count * 8 + byte_counter.into()) { Option::Some(byte) => { let byte: u64 = byte.into(); - byte.shl(8_u64 * byte_counter.into()) + byte.shl(8_u32 * byte_counter.into()) }, Option::None => { break; }, }; diff --git a/crates/utils/src/traits/eth_address.cairo b/crates/utils/src/traits/eth_address.cairo index 9f0122b29..d039f1de4 100644 --- a/crates/utils/src/traits/eth_address.cairo +++ b/crates/utils/src/traits/eth_address.cairo @@ -4,18 +4,18 @@ use crate::traits::EthAddressIntoU256; #[generate_trait] pub impl EthAddressExImpl of EthAddressExTrait { + const BYTES_USED: u8 = 20; /// Converts an EthAddress to an array of bytes. /// /// # Returns /// /// * `Array` - A 20-byte array representation of the EthAddress. fn to_bytes(self: EthAddress) -> Array { - let bytes_used: u256 = 20; let value: u256 = self.into(); let mut bytes: Array = Default::default(); for i in 0 - ..bytes_used { - let val = value.shr(8 * (bytes_used - i - 1)); + ..Self::BYTES_USED { + let val = value.shr(8_u32 * (Self::BYTES_USED.into() - i.into() - 1)); bytes.append((val & 0xFF).try_into().unwrap()); }; @@ -42,7 +42,7 @@ pub impl EthAddressExImpl of EthAddressExTrait { for i in 0 ..len { let byte: u256 = (*input.at(i)).into(); - result += byte.shl((8 * (offset - i)).into()); + result += byte.shl((8 * (offset - i))); }; result.try_into() } diff --git a/crates/utils/src/traits/integer.cairo b/crates/utils/src/traits/integer.cairo index ebd00f99e..d038336b8 100644 --- a/crates/utils/src/traits/integer.cairo +++ b/crates/utils/src/traits/integer.cairo @@ -186,11 +186,9 @@ pub impl BitsUsedImpl< if self == Zero::zero() { return 0; } - let two: T = One::one() + One::one(); - let eight: T = two * two * two; let bytes_used = self.bytes_used(); - let last_byte = self.shr(eight * (bytes_used.into() - One::one())); + let last_byte = self.shr(8_u32 * (bytes_used.into() - One::one())); // safe unwrap since we know at most 8 bits are used let bits_used: u8 = bits_used_internal::bits_used_in_byte(last_byte.try_into().unwrap()); From 4afdbfe67c5e701990f05d14d509bb6472658eff Mon Sep 17 00:00:00 2001 From: enitrat Date: Tue, 1 Oct 2024 18:53:54 +0200 Subject: [PATCH 17/22] bump: cairo native --- scripts/setup_cairo_native.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/setup_cairo_native.sh b/scripts/setup_cairo_native.sh index 8b74c612c..459dea491 100755 --- a/scripts/setup_cairo_native.sh +++ b/scripts/setup_cairo_native.sh @@ -119,6 +119,8 @@ install_cairo_native_runtime() { git clone https://github.com/lambdaclass/cairo_native.git pushd ./cairo_native || exit 1 + git fetch + git checkout fix-clone-drop-for-enum-structs make deps make runtime cp libcairo_native_runtime.a ../libcairo_native_runtime.a From 0a76b4f1b442061a67565f8a8531986fe854c654 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Tue, 1 Oct 2024 18:59:01 +0200 Subject: [PATCH 18/22] fix: conversion of stack values in opcode execution (#1005) * fix: conversion of stack values in opcode execution * fix test --- crates/evm/src/create_helpers.cairo | 2 +- .../src/instructions/block_information.cairo | 8 ++++++-- .../environmental_information.cairo | 17 +++++++++-------- .../src/instructions/logging_operations.cairo | 4 +++- .../src/instructions/memory_operations.cairo | 13 +++++++++---- crates/evm/src/instructions/sha3.cairo | 6 ++++-- .../src/instructions/system_operations.cairo | 18 +++++++++--------- crates/evm/src/interpreter.cairo | 2 +- crates/evm/src/stack.cairo | 3 ++- 9 files changed, 44 insertions(+), 29 deletions(-) diff --git a/crates/evm/src/create_helpers.cairo b/crates/evm/src/create_helpers.cairo index 28e8dc40b..3b8c39eca 100644 --- a/crates/evm/src/create_helpers.cairo +++ b/crates/evm/src/create_helpers.cairo @@ -40,7 +40,7 @@ pub impl CreateHelpersImpl of CreateHelpers { fn prepare_create(ref self: VM, create_type: CreateType) -> Result { 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); diff --git a/crates/evm/src/instructions/block_information.cairo b/crates/evm/src/instructions/block_information.cairo index f5e61db79..3f834a917 100644 --- a/crates/evm/src/instructions/block_information.cairo +++ b/crates/evm/src/instructions/block_information.cairo @@ -1,5 +1,6 @@ //! Block Information. +use core::num::traits::SaturatingAdd; use core::starknet::SyscallResultTrait; use core::starknet::syscalls::get_block_hash_syscall; @@ -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 @@ -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); } diff --git a/crates/evm/src/instructions/environmental_information.cairo b/crates/evm/src/instructions/environmental_information.cairo index d675193ff..9da876792 100644 --- a/crates/evm/src/instructions/environmental_information.cairo +++ b/crates/evm/src/instructions/environmental_information.cairo @@ -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(); @@ -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; @@ -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; @@ -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(); @@ -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 = self.return_data(); let (last_returndata_index, overflow) = offset.overflowing_add(size); @@ -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 @@ -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); } // ************************************************************************* diff --git a/crates/evm/src/instructions/logging_operations.cairo b/crates/evm/src/instructions/logging_operations.cairo index 6e8b1cd4a..12c86a30d 100644 --- a/crates/evm/src/instructions/logging_operations.cairo +++ b/crates/evm/src/instructions/logging_operations.cairo @@ -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 = 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 diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 3c60ee4b4..fc2d81b61 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -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); @@ -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); @@ -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(); @@ -282,7 +286,7 @@ 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; @@ -290,6 +294,7 @@ 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)?; if size == 0 { diff --git a/crates/evm/src/instructions/sha3.cairo b/crates/evm/src/instructions/sha3.cairo index 28f740398..622abf23f 100644 --- a/crates/evm/src/instructions/sha3.cairo +++ b/crates/evm/src/instructions/sha3.cairo @@ -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; diff --git a/crates/evm/src/instructions/system_operations.cairo b/crates/evm/src/instructions/system_operations.cairo index fcfb1a633..5511b6017 100644 --- a/crates/evm/src/instructions/system_operations.cairo +++ b/crates/evm/src/instructions/system_operations.cairo @@ -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( @@ -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; @@ -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( @@ -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( @@ -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); diff --git a/crates/evm/src/interpreter.cairo b/crates/evm/src/interpreter.cairo index d17883c1e..ccf14cda7 100644 --- a/crates/evm/src/interpreter.cairo +++ b/crates/evm/src/interpreter.cairo @@ -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; diff --git a/crates/evm/src/stack.cairo b/crates/evm/src/stack.cairo index 506e87d9f..74ddcba68 100644 --- a/crates/evm/src/stack.cairo +++ b/crates/evm/src/stack.cairo @@ -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 /// From 6138ec1fd6b3a6db55a29e1ded8833c735574e48 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Wed, 2 Oct 2024 11:39:18 +0200 Subject: [PATCH 19/22] ci: downgrade cairo native (#1008) --- scripts/setup_cairo_native.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/setup_cairo_native.sh b/scripts/setup_cairo_native.sh index 459dea491..1cf59c891 100755 --- a/scripts/setup_cairo_native.sh +++ b/scripts/setup_cairo_native.sh @@ -120,7 +120,6 @@ install_cairo_native_runtime() { git clone https://github.com/lambdaclass/cairo_native.git pushd ./cairo_native || exit 1 git fetch - git checkout fix-clone-drop-for-enum-structs make deps make runtime cp libcairo_native_runtime.a ../libcairo_native_runtime.a From dcbb421a608e50320b3aaa179ac1cce4cfc79fe5 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Wed, 2 Oct 2024 14:30:14 +0200 Subject: [PATCH 20/22] dev: use checked math (#1009) --- 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}; From 7048c0d39a6237b23e81e047460d2740b36819ab Mon Sep 17 00:00:00 2001 From: saimeunt Date: Wed, 2 Oct 2024 17:00:16 +0200 Subject: [PATCH 21/22] test: get_starknet_address (#1006) * test_kakarot_core_get_starknet_address * ci: downgrade cairo native (#1008) * dev: use checked math (#1009) * fix get_starknet_address test --------- Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> --- .../contracts/tests/test_kakarot_core.cairo | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/crates/contracts/tests/test_kakarot_core.cairo b/crates/contracts/tests/test_kakarot_core.cairo index 89bcde981..45fde481f 100644 --- a/crates/contracts/tests/test_kakarot_core.cairo +++ b/crates/contracts/tests/test_kakarot_core.cairo @@ -18,7 +18,7 @@ use evm::test_utils::chain_id; use evm::test_utils; use snforge_std::{ declare, DeclareResultTrait, start_cheat_caller_address, spy_events, EventSpyTrait, - cheat_caller_address, CheatSpan, store + cheat_caller_address, CheatSpan, store, load }; use snforge_utils::snforge_utils::{EventsFilterBuilderTrait, ContractEventsTrait}; use starknet::storage::StorageTrait; @@ -159,6 +159,49 @@ fn test_kakarot_core_upgrade_contract() { assert(version == 1, 'version is not 1'); } +#[test] +fn test_kakarot_core_get_starknet_address() { + let (_, kakarot_core) = contract_utils::setup_contracts_for_testing(); + let mut kakarot_state = KakarotCore::unsafe_new_contract_state(); + + let registered_evm_address = test_utils::evm_address(); + let registered_starknet_address = test_utils::starknet_address(); + let registered_map_entry_address = kakarot_state + .snapshot_deref() + .storage() + .Kakarot_evm_to_starknet_address + .entry(registered_evm_address) + .deref() + .__storage_pointer_address__; + // store the registered address in the mapping + store( + kakarot_core.contract_address, + registered_map_entry_address.into(), + [registered_starknet_address.into()].span() + ); + // when the address is registered in the mapping, it's returned + assert_eq!( + kakarot_core.get_starknet_address(registered_evm_address), registered_starknet_address + ); + + let unregistered_evm_address = test_utils::other_evm_address(); + let unregistered_map_entry_address = kakarot_state + .snapshot_deref() + .storage() + .Kakarot_evm_to_starknet_address + .entry(unregistered_evm_address) + .deref() + .__storage_pointer_address__; + let map_data = load(kakarot_core.contract_address, unregistered_map_entry_address.into(), 1); + // when the map entry is empty + assert_eq!(*map_data[0], 0); + // then an unregistered address should return the same as compute_starknet_address + assert_eq!( + kakarot_core.get_starknet_address(unregistered_evm_address), + kakarot_core.compute_starknet_address(unregistered_evm_address) + ); +} + #[test] fn test_eth_send_transaction_non_deploy_tx() { // Given From 96dea687d312610d88f5cd7634598b5cae9c07ce Mon Sep 17 00:00:00 2001 From: enitrat Date: Sat, 28 Sep 2024 16:44:53 +0200 Subject: [PATCH 22/22] dev: optimized bitshifts by using a lookup table for powers of two --- crates/utils/src/math.cairo | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/utils/src/math.cairo b/crates/utils/src/math.cairo index ec1344b2d..fab2f7ddf 100644 --- a/crates/utils/src/math.cairo +++ b/crates/utils/src/math.cairo @@ -2,6 +2,7 @@ use core::integer::{u512}; use core::num::traits::{Zero, One, BitSize, OverflowingAdd, OverflowingMul, Bounded}; use core::panic_with_felt252; use core::traits::{BitAnd}; +use utils::constants::POW_2_256; // === Exponentiation === @@ -245,6 +246,14 @@ impl BitshiftImpl< if shift > BitSize::::bits() - One::one() { panic_with_felt252('mul Overflow'); } + // if the shift is within the bit size of u256 (<= 255 bits), + // use the POW_2 lookup table to get 2^shift for efficient multiplication + if shift <= BitSize::::bits() - One::::one() { + // In case the pow2 is greater than the max value of T, we have an overflow + // so we can panic + return self * (*POW_2_256.span().at(shift)).try_into().expect('mul Overflow'); + } + // for shifts greater than 255 bits, perform the shift manually let two = One::one() + One::one(); self * two.pow(shift.try_into().expect('mul Overflow')) } @@ -254,6 +263,10 @@ impl BitshiftImpl< if shift > BitSize::::bits() - One::one() { panic_with_felt252('mul Overflow'); } + // use the POW_2 lookup table when the bit size + if shift <= BitSize::::bits() - One::::one() { + return self / (*POW_2_256.span().at(shift)).try_into().expect('mul Overflow'); + } let two = One::one() + One::one(); self / two.pow(shift.try_into().expect('mul Overflow')) } @@ -309,12 +322,23 @@ pub impl WrappingBitshiftImpl< +Into > of WrappingBitshift { fn wrapping_shl(self: T, shift: usize) -> T { + if shift <= BitSize::::bits() - One::::one() { + let pow_2: u256 = (*POW_2_256.span().at(shift)); + let pow2_mod_t: u256 = pow_2 % Bounded::::MAX.into(); + let (result, _) = self.overflowing_mul(pow2_mod_t.try_into().unwrap()); + return result; + } let two = One::::one() + One::::one(); let (result, _) = self.overflowing_mul(two.wrapping_pow(shift.try_into().unwrap())); result } fn wrapping_shr(self: T, shift: usize) -> T { + if shift <= BitSize::::bits() - One::::one() { + let pow_2: u256 = (*POW_2_256.span().at(shift)); + let pow2_mod_t: u256 = pow_2 % Bounded::::MAX.into(); + return self / pow2_mod_t.try_into().unwrap(); + } let two = One::::one() + One::::one(); if shift > BitSize::::bits() - One::one() {