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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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 fe6735da2c9bb41d0b971274a70653816e0603e1 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Thu, 3 Oct 2024 21:02:20 +0200 Subject: [PATCH 22/26] fix working native in setup script (#1013) --- scripts/setup_cairo_native.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/setup_cairo_native.sh b/scripts/setup_cairo_native.sh index 1cf59c891..4f3101749 100755 --- a/scripts/setup_cairo_native.sh +++ b/scripts/setup_cairo_native.sh @@ -120,6 +120,7 @@ install_cairo_native_runtime() { git clone https://github.com/lambdaclass/cairo_native.git pushd ./cairo_native || exit 1 git fetch + git checkout ae17dd370a7bbf6affeefb9fa6954965e8b52239 make deps make runtime cp libcairo_native_runtime.a ../libcairo_native_runtime.a From f0013f78ed67cb7b3109dbd68afedef295a04ae0 Mon Sep 17 00:00:00 2001 From: Diego Barquero Quesada Date: Thu, 3 Oct 2024 13:03:20 -0600 Subject: [PATCH 23/26] dev: test emit_events (#1003) * creating test case for emit events * fixing format * side stuff * Update crates/evm/src/model.cairo Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> * Update crates/evm/src/backend/starknet_backend.cairo Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> * changes for implementation of into trait --------- Co-authored-by: enitrat Co-authored-by: Mathieu <60658558+enitrat@users.noreply.github.com> --- crates/evm/src/backend/starknet_backend.cairo | 68 +++++++++++++++++-- crates/snforge_utils/src/lib.cairo | 24 +++---- 2 files changed, 75 insertions(+), 17 deletions(-) diff --git a/crates/evm/src/backend/starknet_backend.cairo b/crates/evm/src/backend/starknet_backend.cairo index 52a0de60c..c5db6215f 100644 --- a/crates/evm/src/backend/starknet_backend.cairo +++ b/crates/evm/src/backend/starknet_backend.cairo @@ -244,18 +244,23 @@ fn commit_storage(ref self: State) -> Result<(), EVMError> { #[cfg(test)] mod tests { - use core::starknet::ClassHash; + use core::starknet::{ClassHash}; use crate::backend::starknet_backend; - use crate::model::Address; use crate::model::account::Account; + use crate::model::{Address, Event}; use crate::state::{State, StateTrait}; use crate::test_utils::{ setup_test_environment, uninitialized_account, account_contract, register_account }; use crate::test_utils::{evm_address}; - use snforge_std::{test_address, start_mock_call, get_class_hash}; - use snforge_utils::snforge_utils::{assert_not_called, assert_called}; - use super::commit_storage; + use snforge_std::{ + test_address, start_mock_call, get_class_hash, spy_events, EventSpyTrait, + Event as StarknetEvent + }; + use snforge_utils::snforge_utils::{ + assert_not_called, assert_called, EventsFilterBuilderTrait, ContractEventsTrait + }; + use super::{commit_storage, emit_events}; use utils::helpers::compute_starknet_address; use utils::traits::bytes::U8SpanExTrait; @@ -274,6 +279,18 @@ mod tests { } } + // Implementation to convert an `Event` into a serialized `StarknetEvent` + impl EventIntoStarknetEvent of Into { + fn into(self: Event) -> StarknetEvent { + let mut serialized_keys = array![]; + let mut serialized_data = array![]; + Serde::>::serialize(@self.keys, ref serialized_keys); + Serde::>::serialize(@self.data, ref serialized_data); + StarknetEvent { keys: serialized_keys, data: serialized_data } + } + } + + mod test_commit_storage { use snforge_std::start_mock_call; use snforge_utils::snforge_utils::{assert_called_with, assert_not_called}; @@ -479,6 +496,47 @@ mod tests { assert_called(starknet_address, selector!("set_code_hash")); assert_called(starknet_address, selector!("set_nonce")); } + + #[test] + fn test_emit_events() { + // Initialize the state + let mut state: State = Default::default(); + + // Prepare a list of events with different combinations of keys and data + let evm_events = array![ + Event { keys: array![], data: array![] }, // Empty event + Event { keys: array![1.into()], data: array![2, 3] }, // Single key, multiple data + Event { + keys: array![4.into(), 5.into()], data: array![6] + }, // Multiple keys, single data + Event { + keys: array![7.into(), 8.into(), 9.into()], data: array![10, 11, 12, 13] + } // Multiple keys and data + ]; + + // Add each event to the state + for event in evm_events.clone() { + state.add_event(event); + }; + + // Emit the events and assert that no events are left in the state + let mut spy = spy_events(); + emit_events(ref state).expect('emit events failed'); + assert!(state.events.is_empty()); + + // Capture emitted events + let contract_events = EventsFilterBuilderTrait::from_events(@spy.get_events()) + .with_contract_address(test_address()) + .build(); + + // Assert that each original event was emitted as expected + for event in evm_events { + let starknet_event = EventIntoStarknetEvent::into( + event + ); // Convert to StarkNet event format + contract_events.assert_emitted(@starknet_event); + }; + } } // #[test] // #[ignore] diff --git a/crates/snforge_utils/src/lib.cairo b/crates/snforge_utils/src/lib.cairo index 0588fe66c..eba213ef3 100644 --- a/crates/snforge_utils/src/lib.cairo +++ b/crates/snforge_utils/src/lib.cairo @@ -150,7 +150,7 @@ pub mod snforge_utils { /// A wrapper structure on an array of events emitted by a given contract. #[derive(Drop, Clone)] pub struct ContractEvents { - pub events: Array + pub events: Span } pub trait EventsFilterTrait { @@ -246,33 +246,33 @@ pub mod snforge_utils { } }; - ContractEvents { events: filtered_events } + ContractEvents { events: filtered_events.span() } } } pub trait ContractEventsTrait { fn assert_emitted, impl TDrop: Drop>( - self: ContractEvents, event: @T + self: @ContractEvents, event: @T ); fn assert_not_emitted, impl TDrop: Drop>( - self: ContractEvents, event: @T + self: @ContractEvents, event: @T ); } impl ContractEventsTraitImpl of ContractEventsTrait { fn assert_emitted, impl TDrop: Drop>( - self: ContractEvents, event: @T + self: @ContractEvents, event: @T ) { let mut expected_keys = array![]; let mut expected_data = array![]; event.append_keys_and_data(ref expected_keys, ref expected_data); + let contract_events = (*self.events); let mut found = false; for i in 0 - ..self - .events + ..contract_events .len() { - let event = self.events.at(i); + let event = contract_events.at(i); if event.keys == @expected_keys && event.data == @expected_data { found = true; break; @@ -283,17 +283,17 @@ pub mod snforge_utils { } fn assert_not_emitted, impl TDrop: Drop>( - self: ContractEvents, event: @T + self: @ContractEvents, event: @T ) { let mut expected_keys = array![]; let mut expected_data = array![]; event.append_keys_and_data(ref expected_keys, ref expected_data); + let contract_events = (*self.events); for i in 0 - ..self - .events + ..contract_events .len() { - let event = self.events.at(i); + let event = contract_events.at(i); assert( event.keys != @expected_keys || event.data != @expected_data, 'Unexpected event was emitted' From 4514d5f5f679898cc5bafcac5b2f014f5a61c5af Mon Sep 17 00:00:00 2001 From: Elias Rad <146735585+nnsW3@users.noreply.github.com> Date: Fri, 4 Oct 2024 10:56:59 +0300 Subject: [PATCH 24/26] Update contract_storage.md --- docs/general/contract_storage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/general/contract_storage.md b/docs/general/contract_storage.md index b77599022..d730ec0bc 100644 --- a/docs/general/contract_storage.md +++ b/docs/general/contract_storage.md @@ -15,7 +15,7 @@ _Account state associated to an Ethereum address. Source: [EVM Illustrated](https://takenobu-hs.github.io/downloads/ethereum_evm_illustrated.pdf)_ In traditional EVM clients, like Geth, the _world state_ is stored as a _trie_, -and information about account are stored in the world state trie and can be +and information about account is stored in the world state trie and can be retrieved through queries. Each account in the world state trie is associated with an account storage trie, which stores all of the information related to the account. When Geth updates the storage of a contract by executing the SSTORE From 4867f8072515c3564bb6d224facb85c916f3f042 Mon Sep 17 00:00:00 2001 From: Elias Rad <146735585+nnsW3@users.noreply.github.com> Date: Fri, 4 Oct 2024 10:58:11 +0300 Subject: [PATCH 25/26] Update machine.md --- docs/general/machine.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/general/machine.md b/docs/general/machine.md index efc8f6d07..bd4049954 100644 --- a/docs/general/machine.md +++ b/docs/general/machine.md @@ -4,7 +4,7 @@ The EVM is a stack-based computer responsible for the execution of EVM bytecode. It has two context-bound data structures: the stack and the memory. The stack is a 256bit-words based data structure used to store and retrieve intermediate values during the execution of opcodes. The memory is a byte-addressable data -structure organized into 32-byte words used a volatile space to store data +structure organized into 32-byte words used in a volatile space to store data during execution. Both the stack and the memory are initialized empty at the start of a call context, and destroyed when a call context ends. From f60e9ce12ad7784959d310ba41593323633db6f8 Mon Sep 17 00:00:00 2001 From: Elias Rad <146735585+nnsW3@users.noreply.github.com> Date: Fri, 4 Oct 2024 11:03:59 +0300 Subject: [PATCH 26/26] Update uninitialized_account.cairo --- crates/contracts/src/uninitialized_account.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/contracts/src/uninitialized_account.cairo b/crates/contracts/src/uninitialized_account.cairo index 46c4ea9e0..cb46a53e8 100644 --- a/crates/contracts/src/uninitialized_account.cairo +++ b/crates/contracts/src/uninitialized_account.cairo @@ -50,7 +50,7 @@ pub mod UninitializedAccount { self.ownable.initializer(owner_address); let implementation_class = IKakarotCoreDispatcher { contract_address: owner_address } .get_account_contract_class_hash(); - //TODO: Difference from KakarotZero in that the account contract takes the class + //TODO: Difference from KakarotZero is that the account contract takes the class //implementation to write it in storage, // as it is not a transparent proxy in Cairo1 calldata.append(implementation_class.into());