From f11b720479b9a35c88b753ebe8cc9e7675641739 Mon Sep 17 00:00:00 2001 From: enitrat Date: Fri, 13 Sep 2024 16:23:02 +0200 Subject: [PATCH 1/3] fix: mcopy opcode --- crates/evm/src/instructions/memory_operations.cairo | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 659297291..09badfbcd 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -7,6 +7,7 @@ use evm::memory::MemoryTrait; use evm::model::vm::{VM, VMTrait}; use evm::stack::StackTrait; use evm::state::StateTrait; +use utils::helpers::ceil32; use utils::set::SetTrait; #[inline(always)] @@ -281,15 +282,17 @@ pub impl MemoryOperation of MemoryOperationTrait { /// Copy memory from one location to another. /// # Specification: https://www.evm.codes/#5e?fork=cancun fn exec_mcopy(ref self: VM) -> Result<(), EVMError> { - let size = self.stack.pop_usize()?; - let source_offset = self.stack.pop_usize()?; let dest_offset = self.stack.pop_usize()?; + let source_offset = self.stack.pop_usize()?; + let size = self.stack.pop_usize()?; + let words_size = (ceil32(size) / 32).into(); + let copy_gas_cost = gas::COPY * words_size; let memory_expansion = gas::memory_expansion( self.memory.size(), [(max(dest_offset, source_offset), size)].span() ); self.memory.ensure_length(memory_expansion.new_size); - self.charge_gas(gas::VERYLOW + memory_expansion.expansion_cost)?; + self.charge_gas(gas::VERYLOW + copy_gas_cost + memory_expansion.expansion_cost)?; if size == 0 { return Result::Ok(()); @@ -1039,9 +1042,9 @@ mod tests { vm.memory.store_with_expansion((*element).into(), source_offset + 0x20 * i); i += 1; }; - vm.stack.push(dest_offset.into()).expect('push failed'); - vm.stack.push(source_offset.into()).expect('push failed'); vm.stack.push(size.into()).expect('push failed'); + vm.stack.push(source_offset.into()).expect('push failed'); + vm.stack.push(dest_offset.into()).expect('push failed'); // When let expected_gas = gas::VERYLOW From a2a90861b4ec34676f1e1ab80b501b178fa15c8e Mon Sep 17 00:00:00 2001 From: enitrat Date: Fri, 13 Sep 2024 16:48:24 +0200 Subject: [PATCH 2/3] adapt tests --- .../src/instructions/memory_operations.cairo | 5 +- crates/evm/src/interpreter.cairo | 209 +----------------- 2 files changed, 5 insertions(+), 209 deletions(-) diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 09badfbcd..1440e2959 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -1046,12 +1046,15 @@ mod tests { vm.stack.push(source_offset.into()).expect('push failed'); vm.stack.push(dest_offset.into()).expect('push failed'); + let words_size = ((size + 31) / 32).into(); + let copy_gas_cost = gas::COPY * words_size; + // When let expected_gas = gas::VERYLOW + gas::memory_expansion( vm.memory.size(), [(max(dest_offset, source_offset), size)].span() ) - .expansion_cost; + .expansion_cost + copy_gas_cost; let gas_before = vm.gas_left(); let result = vm.exec_mcopy(); let gas_after = vm.gas_left(); diff --git a/crates/evm/src/interpreter.cairo b/crates/evm/src/interpreter.cairo index 69018b95f..e42c38e95 100644 --- a/crates/evm/src/interpreter.cairo +++ b/crates/evm/src/interpreter.cairo @@ -99,7 +99,7 @@ pub impl EVMImpl of EVMTrait { accessed_addresses: accessed_addresses.spanset(), accessed_storage_keys: accessed_storage_keys.spanset(), }; - let mut summary = EVMTrait::process_message_call(message, env, is_deploy_tx); + let mut summary = Self::process_message_call(message, env, is_deploy_tx); // Gas refunds let gas_used = tx.gas_limit() - summary.gas_left; @@ -970,210 +970,3 @@ pub impl EVMImpl of EVMTrait { return Result::Err(EVMError::InvalidOpcode(opcode)); } } - -pub(crate) fn process_transaction( - ref self: KakarotCore::ContractState, - origin: Address, - tx: Transaction, - gas_price: u128, - intrinsic_gas: u64 -) -> TransactionResult { - // Charge the cost of intrinsic gas - which has been verified to be <= gas_limit. - let gas_left = tx.gas_limit() - intrinsic_gas; - let mut env = starknet_backend::get_env(origin.evm, gas_price); - - let mut sender_account = env.state.get_account(origin.evm); - sender_account.set_nonce(sender_account.nonce() + 1); - env.state.set_account(sender_account); - - // Handle deploy/non-deploy transaction cases - let (to, is_deploy_tx, code, code_address, calldata) = match tx.kind() { - TxKind::Create => { - // Deploy tx case. - let mut origin_nonce: u64 = get_tx_info().unbox().nonce.try_into().unwrap(); - let to_evm_address = compute_contract_address(origin.evm, origin_nonce); - let to_starknet_address = self.compute_starknet_address(to_evm_address); - let to = Address { evm: to_evm_address, starknet: to_starknet_address }; - let code = tx.input(); - let calldata = [].span(); - (to, true, code, Zero::zero(), calldata) - }, - TxKind::Call(to) => { - let target_starknet_address = self.compute_starknet_address(to); - let to = Address { evm: to, starknet: target_starknet_address }; - let code = env.state.get_account(to.evm).code; - (to, false, code, to, tx.input()) - } - }; - - let mut accessed_addresses: Set = Default::default(); - accessed_addresses.add(env.coinbase); - accessed_addresses.add(to.evm); - accessed_addresses.add(origin.evm); - accessed_addresses.extend(eth_precompile_addresses().spanset()); - - let mut accessed_storage_keys: Set<(EthAddress, u256)> = Default::default(); - - if let Option::Some(mut access_list) = tx.access_list() { - for access_list_item in access_list { - let AccessListItem { ethereum_address, storage_keys: _ } = *access_list_item; - let storage_keys = access_list_item.to_storage_keys(); - accessed_addresses.add(ethereum_address); - accessed_storage_keys.extend_from_span(storage_keys); - } - }; - - let message = Message { - caller: origin, - target: to, - gas_limit: gas_left, - data: calldata, - code, - code_address: code_address, - value: tx.value(), - should_transfer_value: true, - depth: 0, - read_only: false, - accessed_addresses: accessed_addresses.spanset(), - accessed_storage_keys: accessed_storage_keys.spanset(), - }; - let mut summary = EVMTrait::process_message_call(message, env, is_deploy_tx); - - // Gas refunds - let gas_used = tx.gas_limit() - summary.gas_left; - let gas_refund = core::cmp::min(gas_used / 5, summary.gas_refund); - - // Charging gas fees to the sender - // At the end of the tx, the sender must have paid - // (gas_used - gas_refund) * gas_price to the miner - // Because tx.gas_price == env.gas_price, and we checked the sender has enough balance - // to cover the gas fees + the value transfer, this transfer should never fail. - // We can thus directly charge the sender for the effective gas fees, - // without pre-emtively charging for the tx gas fee and then refund. - // This is not true for EIP-1559 transactions - not supported yet. - let total_gas_used = gas_used - gas_refund; - let _transaction_fee = total_gas_used.into() * gas_price; - - //TODO(gas): EF-tests doesn't yet support in-EVM gas charging, they assume that the gas - //charged is always correct for now. - // As correct gas accounting is not an immediate priority, we can just ignore the gas - // charging for now. - // match summary - // .state - // .add_transfer( - // Transfer { - // sender: origin, - // recipient: Address { - // evm: coinbase, starknet: block_info.sequencer_address, - // }, - // amount: transaction_fee.into() - // } - // ) { - // Result::Ok(_) => {}, - // Result::Err(err) => { - // - // return TransactionResultTrait::exceptional_failure( - // err.to_bytes(), tx.gas_limit() - // ); - // } - // }; - - TransactionResult { - success: summary.is_success(), - return_data: summary.return_data, - gas_used: total_gas_used, - state: summary.state, - } -} - - -#[cfg(test)] -mod tests { - use contracts::kakarot_core::KakarotCore; - use contracts::test_data::counter_evm_bytecode; - use evm::model::Address; - use evm::test_utils::{ - setup_test_environment, native_token, chain_id, tx_gas_limit, gas_price, register_account, - other_evm_address, uninitialized_account, evm_address - }; - use snforge_std::{start_mock_call, test_address}; - - use super::process_transaction; - use utils::constants::EMPTY_KECCAK; - use utils::eth_transaction::legacy::{TxLegacy}; - use utils::eth_transaction::transaction::{Transaction}; - use utils::helpers::{U8SpanExTrait, u256_to_bytes_array}; - - - #[test] - fn test_process_transaction() { - // Pre - setup_test_environment(); - let chain_id = chain_id(); - - // Given - let eoa_evm_address = evm_address(); - let eoa_starknet_address = utils::helpers::compute_starknet_address( - test_address(), eoa_evm_address, uninitialized_account() - ); - register_account(eoa_evm_address, eoa_starknet_address); - start_mock_call::(eoa_starknet_address, selector!("get_nonce"), 0); - start_mock_call::>(eoa_starknet_address, selector!("bytecode"), [].span()); - start_mock_call::(eoa_starknet_address, selector!("get_code_hash"), EMPTY_KECCAK); - - let contract_evm_address = other_evm_address(); - let contract_starknet_address = utils::helpers::compute_starknet_address( - test_address(), contract_evm_address, uninitialized_account() - ); - register_account(contract_evm_address, contract_starknet_address); - start_mock_call::(contract_starknet_address, selector!("get_nonce"), 1); - let counter_evm_bytecode = counter_evm_bytecode(); - start_mock_call::< - Span - >(contract_starknet_address, selector!("bytecode"), counter_evm_bytecode); - - start_mock_call::< - u256 - >( - contract_starknet_address, - selector!("get_code_hash"), - counter_evm_bytecode.compute_keccak256_hash() - ); - start_mock_call::(contract_starknet_address, selector!("storage"), 0); - - let nonce = 0; - let gas_limit = tx_gas_limit(); - let gas_price = gas_price(); - let value = 0; - // selector: function get() - let input = [0x6d, 0x4c, 0xe6, 0x3c].span(); - - let tx = TxLegacy { - chain_id: Option::Some(chain_id), - nonce, - to: contract_evm_address.into(), - value, - gas_price, - gas_limit, - input - }; - - // When - let mut kakarot_core = KakarotCore::unsafe_new_contract_state(); - start_mock_call::< - u256 - >(native_token(), selector!("balanceOf"), 0xfffffffffffffffffffffffffff); - let result = process_transaction( - ref kakarot_core, - origin: Address { evm: eoa_evm_address, starknet: eoa_starknet_address }, - tx: Transaction::Legacy(tx), - :gas_price, - intrinsic_gas: 0 - ); - let return_data = result.return_data; - - // Then - assert!(result.success); - assert_eq!(return_data, u256_to_bytes_array(0).span()); - } -} From 36196b95abe87a4644fd36d8308a35024b23b350 Mon Sep 17 00:00:00 2001 From: enitrat Date: Fri, 13 Sep 2024 16:51:55 +0200 Subject: [PATCH 3/3] fmt --- crates/evm/src/instructions/memory_operations.cairo | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/evm/src/instructions/memory_operations.cairo b/crates/evm/src/instructions/memory_operations.cairo index 1440e2959..ab6222651 100644 --- a/crates/evm/src/instructions/memory_operations.cairo +++ b/crates/evm/src/instructions/memory_operations.cairo @@ -1054,7 +1054,8 @@ mod tests { + gas::memory_expansion( vm.memory.size(), [(max(dest_offset, source_offset), size)].span() ) - .expansion_cost + copy_gas_cost; + .expansion_cost + + copy_gas_cost; let gas_before = vm.gas_left(); let result = vm.exec_mcopy(); let gas_after = vm.gas_left();