update ssj

cairo/kakarot-ssj/crates/contracts/src/account_contract.cairo

@@ -68,7 +68,7 @@ pub mod AccountContract {
     use crate::storage::StorageBytecode;
     use openzeppelin::token::erc20::interface::{IERC20CamelDispatcher, IERC20CamelDispatcherTrait};
     use super::OutsideExecution;
-    use utils::eth_transaction::transaction::TransactionUnsignedTrait;
+    use utils::eth_transaction::transaction::TransactionTrait;
     use utils::serialization::{deserialize_signature, deserialize_bytes, serialize_bytes};
     use utils::traits::DefaultSignature;
 
@@ -249,18 +249,13 @@ pub mod AccountContract {
             let mut encoded_tx_data = deserialize_bytes((*outside_execution.calls[0]).calldata)
                 .expect('conversion to Span<u8> failed')
                 .span();
-            let unsigned_transaction = TransactionUnsignedTrait::decode_enveloped(
-                ref encoded_tx_data
-            )
-                .expect('EOA: could not decode tx');
+            let unsigned_transaction_hash = TransactionTrait::compute_hash(encoded_tx_data);
 
             let address = self.Account_evm_address.read();
-            verify_eth_signature(unsigned_transaction.hash, signature, address);
+            verify_eth_signature(unsigned_transaction_hash, signature, address);
 
-            //TODO: refactor this to call eth_send_raw_unsigned_tx. Only the transactions bytes are
-            //passed.
             let (success, return_data, gas_used) = kakarot
-                .eth_send_transaction(unsigned_transaction.transaction);
+                .eth_send_raw_unsigned_tx(encoded_tx_data);
             let return_data = serialize_bytes(return_data).span();
 
             // See Argent account

cairo/kakarot-ssj/crates/contracts/src/kakarot_core/eth_rpc.cairo

@@ -1,16 +1,17 @@
 use core::num::traits::Zero;
 use core::starknet::get_tx_info;
-use core::starknet::{EthAddress, get_caller_address};
+use core::starknet::{EthAddress, get_caller_address, ContractAddress};
 use crate::account_contract::{IAccountDispatcher, IAccountDispatcherTrait};
 use crate::kakarot_core::interface::IKakarotCore;
 use crate::kakarot_core::kakarot::{KakarotCore, KakarotCore::{KakarotCoreState}};
 use evm::backend::starknet_backend;
 use evm::backend::validation::validate_eth_tx;
+use evm::model::account::AccountTrait;
 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;
+use utils::eth_transaction::transaction::{Transaction, TransactionTrait};
 
 #[starknet::interface]
 pub trait IEthRPC<T> {
@@ -84,21 +85,6 @@ pub trait IEthRPC<T> {
     /// * The estimated gas as a u64
     fn eth_estimate_gas(self: @T, origin: EthAddress, tx: Transaction) -> (bool, Span<u8>, u64);
 
-    //TODO: make this an internal function. The account contract should call
-    //eth_send_raw_transaction.
-    /// Executes a transaction and possibly modifies the state.
-    ///
-    /// # Arguments
-    ///
-    /// * `tx` - The transaction object
-    ///
-    /// # Returns
-    ///
-    /// A tuple containing:
-    /// * A boolean indicating success
-    /// * The return data as a Span<u8>
-    /// * The amount of gas used as a u64
-    fn eth_send_transaction(ref self: T, tx: Transaction) -> (bool, Span<u8>, u64);
 
     /// Executes an unsigned transaction.
     ///
@@ -153,9 +139,7 @@ pub impl EthRPC<
             core::panic_with_felt252('fn must be called, not invoked');
         };
 
-        let origin = Address {
-            evm: origin, starknet: kakarot_state.compute_starknet_address(origin)
-        };
+        let origin = Address { evm: origin, starknet: kakarot_state.get_starknet_address(origin) };
 
         let TransactionResult { success, return_data, gas_used, state: _state } =
             EVMTrait::process_transaction(
@@ -171,16 +155,49 @@ pub impl EthRPC<
         panic!("unimplemented")
     }
 
-    //TODO: make this one internal, and the eth_send_raw_unsigned_tx one public
-    fn eth_send_transaction(
-        ref self: TContractState, mut tx: Transaction
+    //TODO: we can't really unit-test this with foundry because we can't generate the RLP-encoding
+    //in Cairo Find another way - perhaps test-data gen with python?
+    fn eth_send_raw_unsigned_tx(
+        ref self: TContractState, mut tx_data: Span<u8>
     ) -> (bool, Span<u8>, u64) {
+        let tx = TransactionTrait::decode_enveloped(ref tx_data).expect('EOA: could not decode tx');
+        EthRPCInternal::eth_send_transaction(ref self, tx)
+    }
+}
+
+trait EthRPCInternal<T> {
+    /// Executes a transaction and possibly modifies the state.
+    ///
+    /// This function implements the `eth_sendTransaction` method as described in the Ethereum
+    /// JSON-RPC specification.
+    /// The nonce is taken from the corresponding account contract.
+    ///
+    /// # Arguments
+    ///
+    /// * `tx` - A `Transaction` struct
+    ///
+    /// # Returns
+    ///
+    /// A tuple containing:
+    /// * A boolean indicating success (TRUE if the transaction succeeded, FALSE otherwise)
+    /// * The return data as a `Span<u8>`
+    /// * The amount of gas used by the transaction as a `u64`
+    fn eth_send_transaction(ref self: T, tx: Transaction) -> (bool, Span<u8>, u64);
+}
+
+impl EthRPCInternalImpl<
+    TContractState, impl KakarotState: KakarotCoreState<TContractState>, +Drop<TContractState>
+> of EthRPCInternal<TContractState> {
+    fn eth_send_transaction(ref self: TContractState, tx: Transaction) -> (bool, Span<u8>, u64) {
         let mut kakarot_state = KakarotState::get_state();
         let intrinsic_gas = validate_eth_tx(@kakarot_state, tx);
 
         let starknet_caller_address = get_caller_address();
-        let account = IAccountDispatcher { contract_address: starknet_caller_address };
-        let origin = Address { evm: account.get_evm_address(), starknet: starknet_caller_address };
+        // panics if the caller is a spoofer of an EVM address.
+        //TODO: e2e test this! :) Send a transaction from an account that is not Kakarot's account
+        //(e.g. deploy an account but not from Kakarot)
+        let origin_evm_address = safe_get_evm_address(@self, starknet_caller_address);
+        let origin = Address { evm: origin_evm_address, starknet: starknet_caller_address };
 
         let TransactionResult { success, return_data, gas_used, mut state } =
             EVMTrait::process_transaction(
@@ -189,26 +206,40 @@ pub impl EthRPC<
         starknet_backend::commit(ref state).expect('Committing state failed');
         (success, return_data, gas_used)
     }
-
-    fn eth_send_raw_unsigned_tx(
-        ref self: TContractState, tx_data: Span<u8>
-    ) -> (bool, Span<u8>, u64) {
-        panic!("unimplemented")
-    }
 }
 
-trait IEthRPCInternal<T> {
-    fn eth_send_transaction(
-        ref self: T, origin: EthAddress, tx: Transaction
-    ) -> (bool, Span<u8>, u64);
-}
 
-impl EthRPCInternalImpl<TContractState, +Drop<TContractState>> of IEthRPCInternal<TContractState> {
-    fn eth_send_transaction(
-        ref self: TContractState, origin: EthAddress, tx: Transaction
-    ) -> (bool, Span<u8>, u64) {
-        panic!("unimplemented")
-    }
+/// Returns the EVM address associated with a Starknet account deployed by Kakarot.
+///
+/// This function prevents cases where a Starknet account has an entrypoint `get_evm_address()`
+/// but isn't part of the Kakarot system. It also mitigates re-entrancy risk with the Cairo Interop
+/// module.
+///
+/// # Arguments
+///
+/// * `starknet_address` - The Starknet address of the account
+///
+/// # Returns
+///
+/// * `EthAddress` - The associated EVM address
+///
+/// # Panics
+///
+/// Panics if the declared corresponding EVM address (retrieved with `get_evm_address`)
+/// does not recompute into the actual caller address.
+fn safe_get_evm_address<
+    TContractState, impl KakarotState: KakarotCoreState<TContractState>, +Drop<TContractState>
+>(
+    self: @TContractState, starknet_address: ContractAddress
+) -> EthAddress {
+    let account = IAccountDispatcher { contract_address: starknet_address };
+    let evm_address = account.get_evm_address();
+    let safe_starknet_address = AccountTrait::get_starknet_address(evm_address);
+    assert!(
+        safe_starknet_address == starknet_address,
+        "Kakarot: caller contract is not a Kakarot Account"
+    );
+    evm_address
 }
 
 fn is_view(self: @KakarotCore::ContractState) -> bool {
@@ -225,16 +256,19 @@ fn is_view(self: @KakarotCore::ContractState) -> bool {
 
 #[cfg(test)]
 mod tests {
+    use core::ops::DerefMut;
+    use core::starknet::EthAddress;
+    use core::starknet::storage::{StoragePathEntry, StoragePointerWriteAccess};
     use crate::kakarot_core::KakarotCore;
     use crate::kakarot_core::eth_rpc::IEthRPC;
-    use crate::kakarot_core::interface::IExtendedKakarotCoreDispatcherTrait;
+    use crate::kakarot_core::interface::{IKakarotCore, IExtendedKakarotCoreDispatcherTrait};
     use crate::test_utils::{setup_contracts_for_testing, fund_account_with_native_token};
     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 super::safe_get_evm_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
@@ -250,12 +284,7 @@ mod tests {
     #[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
-        );
+        let starknet_address = kakarot_state.get_starknet_address(evm_address());
         start_mock_call::<u256>(starknet_address, selector!("get_nonce"), 1);
         assert_eq!(kakarot_state.eth_get_transaction_count(evm_address()), 1);
     }
@@ -301,4 +330,33 @@ mod tests {
         );
         tear_down();
     }
+
+    #[test]
+    fn test_safe_get_evm_address_succeeds() {
+        let kakarot_state = set_up();
+        // no registry - returns the computed address
+        let starknet_address = kakarot_state.get_starknet_address(evm_address());
+        start_mock_call::<
+            EthAddress
+        >(starknet_address, selector!("get_evm_address"), evm_address());
+        let safe_evm_address = safe_get_evm_address(@kakarot_state, starknet_address);
+        assert_eq!(safe_evm_address, evm_address());
+    }
+
+    #[test]
+    #[should_panic(expected: "Kakarot: caller contract is not a Kakarot Account")]
+    fn test_safe_get_evm_address_panics_when_caller_is_not_kakarot_account() {
+        let mut kakarot_state = set_up();
+        let mut kakarot_storage = kakarot_state.deref_mut();
+
+        // Calling get_evm_address() on a fake starknet account that will return `evm_address()`.
+        // Then, when computing the deterministic starknet_address with get_starknet_address(), it
+        // will return a different address.
+        // This should fail.
+        let fake_starknet_account = 'fake_account'.try_into().unwrap();
+        start_mock_call::<
+            EthAddress
+        >(fake_starknet_account, selector!("get_evm_address"), evm_address());
+        safe_get_evm_address(@kakarot_state, fake_starknet_account);
+    }
 }

cairo/kakarot-ssj/crates/contracts/src/kakarot_core/interface.cairo

@@ -9,11 +9,6 @@ pub trait IKakarotCore<TContractState> {
     /// Gets the native token used by the Kakarot smart contract
     fn get_native_token(self: @TContractState) -> ContractAddress;
 
-    /// 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;
-
     /// Checks into KakarotCore storage if an EOA or a CA has been deployed for
     /// a particular EVM address and. If so returns its corresponding address,
     /// otherwise returns 0
@@ -47,7 +42,18 @@ pub trait IKakarotCore<TContractState> {
     /// Setter for the base fee
     fn set_base_fee(ref self: TContractState, base_fee: u64);
 
-    // Getter for the Starknet Address
+    /// Returns the corresponding Starknet address for a given EVM address.
+    ///
+    /// Returns the registered address if there is one, otherwise returns the deterministic
+    /// address got when Kakarot deploys an account.
+    ///
+    /// # Arguments
+    ///
+    /// * `evm_address` - The EVM address to transform to a starknet address
+    ///
+    /// # Returns
+    ///
+    /// * `ContractAddress` - The Starknet Account Contract address
     fn get_starknet_address(self: @TContractState, evm_address: EthAddress) -> ContractAddress;
 }
 
@@ -59,11 +65,6 @@ pub trait IExtendedKakarotCore<TContractState> {
     /// Gets the native token used by the Kakarot smart contract
     fn get_native_token(self: @TContractState) -> ContractAddress;
 
-    /// 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;
-
     /// Checks into KakarotCore storage if an EOA or a CA has been deployed for
     /// a particular EVM address and. If so returns its corresponding address,
     /// otherwise returns 0
@@ -88,6 +89,10 @@ pub trait IExtendedKakarotCore<TContractState> {
     /// Executes an EVM transaction and possibly modifies the state
     fn eth_send_transaction(ref self: TContractState, tx: Transaction) -> (bool, Span<u8>, u64);
 
+    fn eth_send_raw_unsigned_tx(
+        ref self: TContractState, encoded_tx_data: Span<u8>
+    ) -> (bool, Span<u8>, u64);
+
     // Returns the transaction count (nonce) of the specified address
     fn eth_get_transaction_count(self: @TContractState, address: EthAddress) -> u64;
 

cairo/kakarot-ssj/crates/contracts/src/kakarot_core/kakarot.cairo

@@ -5,6 +5,7 @@ const INVOKE_ETH_CALL_FORBIDDEN: felt252 = 'KKT: Cannot invoke eth_call';
 pub mod KakarotCore {
     use core::num::traits::Zero;
     use core::starknet::event::EventEmitter;
+    use core::starknet::get_caller_address;
     use core::starknet::storage::{
         Map, StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess,
         StoragePointerWriteAccess
@@ -15,6 +16,7 @@ pub mod KakarotCore {
     use crate::kakarot_core::eth_rpc;
     use crate::kakarot_core::interface::IKakarotCore;
     use evm::backend::starknet_backend;
+    use evm::model::account::AccountTrait;
     use utils::helpers::compute_starknet_address;
 
     component!(path: ownable_component, storage: ownable, event: OwnableEvent);
@@ -133,15 +135,6 @@ pub mod KakarotCore {
             self.Kakarot_native_token_address.read()
         }
 
-        fn compute_starknet_address(
-            self: @ContractState, evm_address: EthAddress
-        ) -> ContractAddress {
-            let kakarot_address = get_contract_address();
-            compute_starknet_address(
-                kakarot_address, evm_address, self.Kakarot_uninitialized_account_class_hash.read()
-            )
-        }
-
         fn address_registry(self: @ContractState, evm_address: EthAddress) -> ContractAddress {
             self.Kakarot_evm_to_starknet_address.read(evm_address)
         }
@@ -183,9 +176,14 @@ pub mod KakarotCore {
             let existing_address = self.Kakarot_evm_to_starknet_address.read(evm_address);
             assert(existing_address.is_zero(), 'Account already exists');
 
-            let starknet_address = self.compute_starknet_address(evm_address);
-            //TODO: enable this assertion. Will require changing test runner to snfoundry
-            // assert!(starknet_address == caller, "Account must be registered by the caller");
+            let starknet_address = compute_starknet_address(
+                get_contract_address(),
+                evm_address,
+                self.Kakarot_uninitialized_account_class_hash.read()
+            );
+            assert!(
+                starknet_address == get_caller_address(), "Account must be registered by the caller"
+            );
 
             self.Kakarot_evm_to_starknet_address.write(evm_address, starknet_address);
             self.emit(AccountDeployed { evm_address, starknet_address });
@@ -204,19 +202,9 @@ pub mod KakarotCore {
             self.Kakarot_base_fee.read()
         }
 
-        // @notice Returns the corresponding Starknet address for a given EVM address.
-        // @dev Returns the registered address if there is one, otherwise returns the deterministic
-        //      address got when Kakarot deploys an account.
-        // @param evm_address The EVM address to transform to a starknet address
-        // @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()) {
-                return registered_starknet_address;
-            }
 
-            let computed_starknet_address = self.compute_starknet_address(evm_address);
-            return computed_starknet_address;
+        fn get_starknet_address(self: @ContractState, evm_address: EthAddress) -> ContractAddress {
+            AccountTrait::get_starknet_address(evm_address)
         }
     }