Skip to content

Commit

Permalink
fix: mcopy opcode (#939)
Browse files Browse the repository at this point in the history 
* fix: mcopy opcode

* adapt tests

* fmt
  • Loading branch information
@enitrat
enitrat authored Sep 13, 2024
1 parent 0b913f6 commit b1f7f8c
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 214 deletions.
19 changes: 13 additions & 6 deletions crates/evm/src/instructions/memory_operations.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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(());
Expand Down Expand Up @@ -1039,16 +1042,20 @@ 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');

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();
Expand Down
209 changes: 1 addition & 208 deletions crates/evm/src/interpreter.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<EthAddress> = 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::<u256>(eoa_starknet_address, selector!("get_nonce"), 0);
start_mock_call::<Span<u8>>(eoa_starknet_address, selector!("bytecode"), [].span());
start_mock_call::<u256>(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::<u256>(contract_starknet_address, selector!("get_nonce"), 1);
let counter_evm_bytecode = counter_evm_bytecode();
start_mock_call::<
Span<u8>
>(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::<u256>(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());
}
}

0 comments on commit b1f7f8c

Please sign in to comment.