Skip to content

Commit

Permalink
line up deterministic addressing for the zero nonce case (#638)
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:

## 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:

- [X] 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?

We currently increment account nonce before create / execution. 

I also added the first fixture address to line up with anvil, to make it
easier to diff results.

Resolves #637 

## What is the new behavior?

- Now we do so after, along with fixing a subtle bug in how we were
encoding the case of the nonce being zero to rlp, see:
https://ethereum.stackexchange.com/posts/61021/revisions
-
-

## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

---------

Co-authored-by: johann makram salib bestowrous <[email protected]>
  • Loading branch information
jobez and johann makram salib bestowrous authored Jul 15, 2023
1 parent c3efa55 commit ad8daf2
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 18 deletions.
3 changes: 1 addition & 2 deletions scripts/utils/starknet.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,14 @@

from scripts.constants import (
BUILD_DIR,
CLIENT,
BUILD_DIR_FIXTURES,
CLIENT,
CONTRACTS,
CONTRACTS_FIXTURES,
DEPLOYMENTS_DIR,
ETH_TOKEN_ADDRESS,
NETWORK,
SOURCE_DIR,
SOURCE_DIR_FIXTURES,
)

logging.basicConfig()
Expand Down
4 changes: 2 additions & 2 deletions src/kakarot/accounts/eoa/library.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,6 @@ namespace ExternallyOwnedAccount {
return (response_len=0);
}

Accounts.increment_nonce();

let (
nonce, gas_price, gas_limit, destination, amount, payload_len, payload, tx_hash, v, r, s
) = EthTransaction.decode([call_array].data_len, calldata + [call_array].data_offset);
Expand All @@ -153,6 +151,8 @@ namespace ExternallyOwnedAccount {
calldata,
response + return_data_len,
);

Accounts.increment_nonce();
return (response_len=return_data_len + response_len);
}
}
7 changes: 4 additions & 3 deletions src/kakarot/instructions/system_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -811,13 +811,14 @@ namespace CreateHelper {
// so we use popped_len to derive the way we should handle
// the creation of evm addresses
if (popped_len != 4) {
// Increment happens before we fetch nonce
// see https://github.com/ethereum/EIPs/blob/master/EIPS/eip-161.md
let (nonce) = IContractAccount.increment_nonce(ctx.starknet_contract_address);
let (nonce) = IContractAccount.get_nonce(ctx.starknet_contract_address);
let (evm_contract_address) = CreateHelper.get_create_address(
ctx.evm_contract_address, nonce
);
let (nonce) = IContractAccount.increment_nonce(ctx.starknet_contract_address);
let (contract_account_class_hash_) = contract_account_class_hash.read();
let (starknet_contract_address) = Accounts.create(
contract_account_class_hash_, evm_contract_address
Expand Down
7 changes: 7 additions & 0 deletions src/utils/rlp.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ namespace RLP {
range_check_ptr,
}(item: felt, rlp_len: felt, rlp: felt*) -> (rlp_len: felt) {
alloc_locals;
if (item == 0) {
// a single byte of value zero is encoded as '0x80',
// which is a special prefix denoting an empty byte array.
assert [rlp + rlp_len] = 0x80;
return (rlp_len + 1,);
}

// 127 is the largest value that can be represented by one byte
let item_is_single_byte = is_le(item, 127);

Expand Down
2 changes: 1 addition & 1 deletion src/utils/utils.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ namespace Helpers {
return split_word_little((value - low_part) / 256, len - 1, dst + 1);
}

// @notice Splits a felt into 16 bytes, big-endien, and outputs to `dst`.
// @notice Splits a felt into 16 bytes, big-endian, and outputs to `dst`.
func split_word_128{range_check_ptr}(start_value: felt, dst: felt*) {
// Fill dst using only hints with no opcodes.
let value = start_value;
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/bytecodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@
"value": 0,
"code": "3300",
"calldata": "",
"stack": "552990106838132297007705540970627247012975398931",
"stack": "1390849295786071768276380950238675083608645509734",
"memory": "",
"return_value": "",
},
Expand Down
14 changes: 12 additions & 2 deletions tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from starkware.starknet.testing.contract import DeclaredClass, StarknetContract
from starkware.starknet.testing.starknet import Starknet

from tests.utils.helpers import generate_random_private_key
from tests.utils.helpers import generate_random_private_key, private_key_from_hex

Wallet = namedtuple(
"Wallet", ["address", "private_key", "starknet_contract", "starknet_address"]
Expand Down Expand Up @@ -52,7 +52,17 @@ async def addresses(starknet, kakarot, externally_owned_account_class) -> List[W
- address: the hex string of the EVM address (20 bytes)
- starknet_address: the corresponding address for starknet (same value but as int)
"""
private_keys = [generate_random_private_key(seed=i) for i in range(4)]
# Predefined private key shared with anvil,
# so we can more easily compare results for validity
predefined_private_key = private_key_from_hex(
"ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"
)

# Randomly generated private keys
private_keys = [predefined_private_key] + [
generate_random_private_key(seed=i) for i in range(3)
]

wallets = []
for private_key in private_keys:
evm_address = private_key.public_key.to_checksum_address()
Expand Down
8 changes: 4 additions & 4 deletions tests/src/kakarot/instructions/test_system_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,17 @@ async def test_call__should_transfer_value(self, system_operations, mint):
system_operations.contract_address
)

@pytest.mark.parametrize("salt", [127, 256, 2**55 - 1])
@pytest.mark.parametrize("salt", [0, 127, 256, 2**55 - 1])
async def test_create(self, system_operations, salt):
evm_caller_address_int = 15
# given we start with the first anvil test account
evm_caller_address_int = 0xF39FD6E51AAD88F6F4CE6AB8827279CFFFB92266
evm_caller_address_bytes = evm_caller_address_int.to_bytes(20, byteorder="big")
evm_caller_address = to_checksum_address(evm_caller_address_bytes)
expected_create_addr = get_create_address(evm_caller_address, salt)

await system_operations.test__exec_create__should_return_a_new_context_with_bytecode_from_memory_at_expected_address(
evm_caller_address_int,
# Nonce will be incremented before exec_create is executed, therefore we subtract 1
salt - 1,
salt,
from_bytes(decode_hex(expected_create_addr)),
).call()

Expand Down
10 changes: 7 additions & 3 deletions tests/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from eth_account import Account
from eth_keys import keys
from eth_keys.datatypes import PrivateKey
from eth_utils import decode_hex, keccak, to_bytes, to_checksum_address
from eth_utils import decode_hex, keccak, to_checksum_address
from hexbytes import HexBytes

from tests.utils.constants import CHAIN_ID
Expand Down Expand Up @@ -76,12 +76,12 @@ def get_domain_separator(name: str, token_address: str) -> bytes:
)


def get_create_address(sender_address: str, salt: int) -> str:
def get_create_address(sender_address: str, nonce: int) -> str:
"""
See [CREATE](https://www.evm.codes/#f0)
"""
return to_checksum_address(
keccak(rlp.encode([decode_hex(sender_address), to_bytes(salt)]))[-20:]
keccak(rlp.encode([decode_hex(sender_address), nonce]))[-20:]
)


Expand Down Expand Up @@ -136,6 +136,10 @@ def encode_price(reserve_0: int, reserve_1: int) -> list:
]


def private_key_from_hex(hex_key: str):
return keys.PrivateKey(bytes.fromhex(hex_key))


def generate_random_private_key(seed=0):
random.seed(seed)
return keys.PrivateKey(int.to_bytes(random.getrandbits(256), 32, "big"))
Expand Down

0 comments on commit ad8daf2

Please sign in to comment.