Skip to content
This repository has been archived by the owner on Jan 9, 2025. It is now read-only.

Commit

Permalink
fix: cached storage keys (#1012)
Browse files Browse the repository at this point in the history
<!--- Please provide a general summary of your changes in the title
above -->

<!-- Give an estimate of the time you spent on this PR in terms of work
days.
Did you spend 0.5 days on this PR or rather 2 days?  -->

Time spent on this PR: 0.4d

## Pull request type

<!-- Please try to limit your pull request to one type,
submit multiple pull requests if needed. -->

Please check the type of change your PR introduces:

- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying,
or link to a relevant issue. -->

Resolves #<Issue number>

## What is the new behavior?

<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Addresses of keys were correctly set inside the storage dict, but no
value was written. As such, the value post-cache was still 0, treated as
a cold storage slot. Here, we write the value fetched from SN in the
storage dict when caching access lists.
-
-

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg"
height="34" align="absmiddle"
alt="Reviewable"/>](https://reviewable.io/reviews/kkrt-labs/kakarot/1012)
<!-- Reviewable:end -->
  • Loading branch information
enitrat authored Mar 6, 2024
1 parent 01a8adc commit efb810d
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 36 deletions.
27 changes: 0 additions & 27 deletions blockchain-tests-skip.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,33 +48,6 @@ testname:
- Call1024PreCalls_d0g2v0_Shanghai
- CallRecursiveBombPreCall_d0g0v0_Shanghai
- Delegatecall1024_d0g0v0_Shanghai
stEIP2930:
- manualCreate_d2g0v0_Shanghai
- storageCosts_d0g0v0_Shanghai
- storageCosts_d1g0v0_Shanghai
- storageCosts_d2g0v0_Shanghai
- storageCosts_d32g0v0_Shanghai
- storageCosts_d35g0v0_Shanghai
- storageCosts_d3g0v0_Shanghai
- storageCosts_d4g0v0_Shanghai
- storageCosts_d5g0v0_Shanghai
- variedContext_d0g0v0_Shanghai
- variedContext_d10g0v0_Shanghai
- variedContext_d12g0v0_Shanghai
- variedContext_d16g0v0_Shanghai
- variedContext_d18g0v0_Shanghai
- variedContext_d20g0v0_Shanghai
- variedContext_d22g0v0_Shanghai
- variedContext_d24g0v0_Shanghai
- variedContext_d26g0v0_Shanghai
- variedContext_d28g0v0_Shanghai
- variedContext_d2g0v0_Shanghai
- variedContext_d30g0v0_Shanghai
- variedContext_d32g0v0_Shanghai
- variedContext_d34g0v0_Shanghai
- variedContext_d5g0v0_Shanghai
- variedContext_d7g0v0_Shanghai
- variedContext_d8g0v0_Shanghai
stEIP150singleCodeGasPrices:
- gasCostExp_d7g0v0_Shanghai
stExtCodeHash:
Expand Down
35 changes: 27 additions & 8 deletions src/kakarot/account.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -482,13 +482,13 @@ namespace Account {
// @dev This is used for access list transactions that provide a list of preaccessed keys
// @param storage_keys_len The number of storage keys to cache.
// @param storage_keys The pointer to the first storage key.
func cache_storage_keys{pedersen_ptr: HashBuiltin*, range_check_ptr}(
func cache_storage_keys{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
self: model.Account*, storage_keys_len: felt, storage_keys: Uint256*
) -> model.Account* {
alloc_locals;
let storage_ptr = self.storage;
with storage_ptr {
Internals._cache_storage_keys(storage_keys_len, storage_keys);
Internals._cache_storage_keys(self.address.evm, storage_keys_len, storage_keys);
}
tempvar self = new model.Account(
address=self.address,
Expand Down Expand Up @@ -516,17 +516,36 @@ namespace Internals {
return (res=res);
}

func _cache_storage_keys{pedersen_ptr: HashBuiltin*, range_check_ptr, storage_ptr: DictAccess*}(
storage_keys_len: felt, storage_keys: Uint256*
) {
func _cache_storage_keys{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, storage_ptr: DictAccess*
}(evm_address: felt, storage_keys_len: felt, storage_keys: Uint256*) {
alloc_locals;
if (storage_keys_len == 0) {
return ();
}

let key = storage_keys;
let (storage_addr) = Internals._storage_addr(key);
dict_read{dict_ptr=storage_ptr}(key=storage_addr);
let (local storage_addr) = Internals._storage_addr(key);
// Cache value read from Starknet storage

let starknet_address = Account.get_registered_starknet_address(evm_address);
if (starknet_address != 0) {
let (value) = IContractAccount.storage(
contract_address=starknet_address, storage_addr=storage_addr
);
tempvar value_ptr = new Uint256(value.low, value.high);
tempvar syscall_ptr = syscall_ptr;
tempvar pedersen_ptr = pedersen_ptr;
tempvar range_check_ptr = range_check_ptr;
// Otherwise returns 0
} else {
tempvar value_ptr = new Uint256(0, 0);
tempvar syscall_ptr = syscall_ptr;
tempvar pedersen_ptr = pedersen_ptr;
tempvar range_check_ptr = range_check_ptr;
}
dict_write{dict_ptr=storage_ptr}(key=storage_addr, new_value=cast(value_ptr, felt));

return _cache_storage_keys(storage_keys_len - 1, storage_keys + Uint256.SIZE);
return _cache_storage_keys(evm_address, storage_keys_len - 1, storage_keys + Uint256.SIZE);
}
}
12 changes: 12 additions & 0 deletions src/kakarot/accounts/eoa/externally_owned_account.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ from starkware.cairo.common.cairo_builtins import HashBuiltin, SignatureBuiltin,
from starkware.cairo.common.alloc import alloc
from starkware.starknet.common.syscalls import get_tx_info, get_caller_address
from starkware.cairo.common.math import assert_le
from starkware.cairo.common.uint256 import Uint256

from kakarot.accounts.eoa.library import ExternallyOwnedAccount
from kakarot.account import Account
Expand Down Expand Up @@ -137,6 +138,17 @@ func bytecode_len{
return (len=0);
}

// @notice Read a given storage key
// @dev
// @param key The storage address, with storage_var being storage_(key: Uint256)
// @return 0 as EOAs don't have storage
@view
func storage{
syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
}(storage_addr: felt) -> (value: Uint256) {
return (value=Uint256(0, 0));
}

// @notice Returns the account type
@view
func account_type{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}() -> (
Expand Down
3 changes: 3 additions & 0 deletions src/kakarot/interfaces/interfaces.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ namespace IAccount {
func bytecode() -> (bytecode_len: felt, bytecode: felt*) {
}

func storage(storage_addr: felt) -> (value: Uint256) {
}

func account_type() -> (type: felt) {
}
}
Expand Down
4 changes: 4 additions & 0 deletions tests/src/kakarot/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ def test_should_cache_access_list(self, cairo_run, transaction):
for address, storage_keys in expected_result.items():
assert state["accounts"].get(address) is not None
assert set(state["accounts"][address]["storage"].keys()) == storage_keys
assert all(
state["accounts"][address]["storage"][key] is not None
for key in storage_keys
)

class TestCopyAccounts:
def test_should_handle_null_pointers(self, cairo_run):
Expand Down
2 changes: 1 addition & 1 deletion tests/utils/serde.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def serialize_dict(self, dict_ptr, value_scope=None):
output[key] = (
self.serialize_scope(value_scope, value_ptr)
if value_ptr != 0
else ""
else None
)
return output

Expand Down

0 comments on commit efb810d

Please sign in to comment.