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] 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::<u64>::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::<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);