overwrite fix to codehash (#1654)

Overwrite fix to codehash done in #1644

Motivation:

 I prefer a solution that avoids having invalid states altogether.

- All accounts have a code_hash set to keccak256("") when deployed
- Any modification to the account's code will also modify it's code_hash
- extcodehash simply check the account_exists = has_code_or_nonce or
has_balance condition to know whether to return 0 or code_hash

which is more inline with the spec. The `codehash` of an account is
_never_ zero, the `extcodehash` is.

cairo_zero/backend/starknet.cairo

@@ -239,11 +239,10 @@ namespace Internals {
 
         // Update bytecode and jumpdests if required (newly created account)
         if (self.created != FALSE) {
-            IAccount.write_bytecode(starknet_address, self.code_len, self.code);
+            IAccount.write_bytecode(starknet_address, [self.code_hash], self.code_len, self.code);
             Internals._save_valid_jumpdests(
                 starknet_address, self.valid_jumpdests_start, self.valid_jumpdests
             );
-            IAccount.set_code_hash(starknet_address, [self.code_hash]);
             return ();
         }
 

cairo_zero/kakarot/account.cairo

@@ -123,11 +123,15 @@ namespace Account {
             tempvar address = new model.Address(starknet=starknet_address, evm=evm_address);
             let balance = fetch_balance(address);
             assert balance_ptr = new Uint256(balance.low, balance.high);
+            // empty code hash see https://eips.ethereum.org/EIPS/eip-1052
+            tempvar code_hash_ptr = new Uint256(
+                low=Constants.EMPTY_CODE_HASH_LOW, high=Constants.EMPTY_CODE_HASH_HIGH
+            );
             let account = Account.init(
                 address=address,
                 code_len=0,
                 code=bytecode,
-                code_hash=cast(0, Uint256*),
+                code_hash=code_hash_ptr,
                 nonce=0,
                 balance=balance_ptr,
             );

cairo_zero/kakarot/accounts/account_contract.cairo

@@ -190,14 +190,16 @@ func __execute__{
 }
 
 // @notice Store the bytecode of the contract.
+// @param code_hash The hash of the bytecode to store.
 // @param bytecode_len The length of the bytecode.
 // @param bytecode The bytecode of the contract.
 @external
 func write_bytecode{
     syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
-}(bytecode_len: felt, bytecode: felt*) {
+}(code_hash: Uint256, bytecode_len: felt, bytecode: felt*) {
     // Access control check.
     Ownable.assert_only_owner();
+    AccountContract.set_code_hash(code_hash);
     return AccountContract.write_bytecode(bytecode_len, bytecode);
 }
 
@@ -294,17 +296,6 @@ func get_code_hash{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_p
     return (code_hash,);
 }
 
-// @notice Set the code hash of the account.
-// @param code_hash The code hash of the account.
-@external
-func set_code_hash{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
-    code_hash: Uint256
-) {
-    Ownable.assert_only_owner();
-    AccountContract.set_code_hash(code_hash);
-    return ();
-}
-
 // @notice Authorizes a pre-eip155 transaction by message hash.
 // @param message_hash The hash of the message.
 @external

cairo_zero/kakarot/accounts/library.cairo

@@ -103,6 +103,10 @@ namespace AccountContract {
         let infinite = Uint256(Constants.UINT128_MAX, Constants.UINT128_MAX);
         IERC20.approve(native_token_address, kakarot_address, infinite);
 
+        // Write the empty code hash in storage, as this account does not currently have code.
+        tempvar code_hash = Uint256(Constants.EMPTY_CODE_HASH_LOW, Constants.EMPTY_CODE_HASH_HIGH);
+        AccountContract.set_code_hash(code_hash);
+
         // Register the account in the Kakarot mapping
         IKakarot.register_account(kakarot_address, evm_address);
         return ();

cairo_zero/kakarot/instructions/environmental_information.cairo

@@ -11,7 +11,6 @@ from starkware.cairo.common.math_cmp import is_not_zero, is_nn
 from starkware.cairo.common.uint256 import Uint256, uint256_le
 
 from kakarot.account import Account
-from kakarot.constants import Constants
 
 from kakarot.evm import EVM
 from kakarot.errors import Errors
@@ -479,27 +478,18 @@ namespace EnvironmentalInformation {
             return evm;
         }
 
+        let account = State.get_account(evm_address);
+        let has_code_or_nonce = Account.has_code_or_nonce(account);
+        let account_exists = has_code_or_nonce + account.balance.low + account.balance.high;
         // Relevant cases:
         // https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L392
-        let account = State.get_account(evm_address);
-
-        // If the account has code, return the code_hash stored in the account storage.
-        if (account.code_len != 0) {
-            Stack.push_uint256([account.code_hash]);
+        if (account_exists == FALSE) {
+            Stack.push_uint128(0);
             return evm;
         }
 
-        // If the account has no code but a balance or nonce, return the hash of the empty code hash.
-        if (account.nonce + account.balance.low + account.balance.high != 0) {
-            let empty_code_hash = Uint256(
-                low=Constants.EMPTY_CODE_HASH_LOW, high=Constants.EMPTY_CODE_HASH_HIGH
-            );
-            Stack.push_uint256(empty_code_hash);
-            return evm;
-        }
+        Stack.push_uint256([account.code_hash]);
 
-        // Account is empty (EIP-161), return 0
-        Stack.push_uint128(0);
         return evm;
     }
 }

cairo_zero/kakarot/interfaces/interfaces.cairo

@@ -48,7 +48,7 @@ namespace IAccount {
     func bytecode() -> (bytecode_len: felt, bytecode: felt*) {
     }
 
-    func write_bytecode(bytecode_len: felt, bytecode: felt*) {
+    func write_bytecode(code_hash: Uint256, bytecode_len: felt, bytecode: felt*) {
     }
 
     func storage(storage_addr: felt) -> (value: Uint256) {
@@ -80,9 +80,6 @@ namespace IAccount {
     func get_code_hash() -> (code_hash: Uint256) {
     }
 
-    func set_code_hash(code_hash: Uint256) {
-    }
-
     func execute_from_outside(
         outside_execution: OutsideExecution,
         call_array_len: felt,

cairo_zero/tests/src/kakarot/accounts/test_account_contract.cairo

@@ -16,7 +16,6 @@ from kakarot.accounts.account_contract import (
     set_nonce,
     set_authorized_pre_eip155_tx,
     execute_starknet_call,
-    set_code_hash,
     execute_from_outside,
     upgrade,
 )
@@ -52,13 +51,19 @@ func test__write_bytecode{
 
     // Given
     local bytecode_len: felt;
+    local code_hash: Uint256;
     let (bytecode: felt*) = alloc();
     %{
-        ids.bytecode_len = len(program_input["bytecode"])
-        segments.write_arg(ids.bytecode, program_input["bytecode"])
+        from ethereum.crypto.hash import keccak256
+        bytecode = program_input["bytecode"]
+        ids.bytecode_len = len(bytecode)
+        segments.write_arg(ids.bytecode, bytecode)
+        code_hash = keccak256(bytes(bytecode))
+        ids.code_hash.low = int.from_bytes(code_hash[0:32], "big")
+        ids.code_hash.high = int.from_bytes(code_hash[32:64], "big")
     %}
 
-    write_bytecode(bytecode_len, bytecode);
+    write_bytecode(code_hash, bytecode_len, bytecode);
 
     return ();
 }
@@ -219,7 +224,7 @@ func test__set_code_hash{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_c
         ids.code_hash.high = program_input["code_hash"][1]
     %}
 
-    set_code_hash(code_hash);
+    AccountContract.set_code_hash(code_hash);
 
     return ();
 }

cairo_zero/tests/src/kakarot/accounts/test_account_contract.py

@@ -213,21 +213,12 @@ def test__should_return_if_jumpdest_valid(
 
     class TestCodeHash:
         @given(code_hash=integers(min_value=0, max_value=2**256 - 1))
-        @SyscallHandler.patch("Ownable_owner", 0xDEAD)
-        def test_should_assert_only_owner(self, cairo_run, code_hash):
-            with cairo_error(message="Ownable: caller is not the owner"):
-                cairo_run("test__set_code_hash", code_hash=int_to_uint256(code_hash))
-
-        @given(code_hash=integers(min_value=0, max_value=2**256 - 1))
-        @SyscallHandler.patch("Ownable_owner", SyscallHandler.caller_address)
         def test__should_set_code_hash(self, cairo_run, code_hash):
             with patch.object(SyscallHandler, "mock_storage") as mock_storage:
                 low, high = int_to_uint256(code_hash)
                 cairo_run("test__set_code_hash", code_hash=(low, high))
                 code_hash_address = get_storage_var_address("Account_code_hash")
-                ownable_address = get_storage_var_address("Ownable_owner")
                 calls = [
-                    call(address=ownable_address),
                     call(address=code_hash_address, value=low),
                     call(address=code_hash_address + 1, value=high),
                 ]

cairo_zero/tests/src/kakarot/test_account.cairo

@@ -174,7 +174,7 @@ func test__fetch_original_storage__state_modified{
     let starknet_address = Account.compute_starknet_address(evm_address);
     tempvar address = new model.Address(starknet_address, evm_address);
     let (local code: felt*) = alloc();
-    tempvar code_hash = new Uint256(0, 0);
+    tempvar code_hash = new Uint256(Constants.EMPTY_CODE_HASH_LOW, Constants.EMPTY_CODE_HASH_HIGH);
     tempvar balance = new Uint256(0, 0);
     let account = Account.init(address, 0, code, code_hash, 0, balance);