Skip to content

Commit

Permalink
fix: create tx nonce overflow (#995)
Browse files Browse the repository at this point in the history
  • Loading branch information
enitrat authored Sep 30, 2024
1 parent 77bfa66 commit 3d92e66
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 13 deletions.
5 changes: 4 additions & 1 deletion crates/evm/src/backend/validation.cairo
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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::<u64>::MAX, 'Nonce overflow');

// Validate gas
let gas_limit = tx.gas_limit();
Expand Down
11 changes: 6 additions & 5 deletions crates/evm/src/create_helpers.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion crates/evm/src/instructions/system_operations.cairo
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 3 additions & 6 deletions crates/evm/src/interpreter.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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::<u64>::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);
Expand Down

0 comments on commit 3d92e66

Please sign in to comment.