Skip to content

Commit

Permalink
Store COINBASE in storage_var (kkrt-labs#1069)
Browse files Browse the repository at this point in the history
Made coinbase storage var a constructor argument rather than a hardcoded
value.

Time spent on this PR: 1 hour
(methodology: searched the whole source tree for "coinbase", "0xCA40",
"precompiles", and "uninitialized")

## Pull request type

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

## What is the current behavior?

Currently Coinbase is a hardcoded constant value
(0xCA40796aFB5472abaeD28907D5ED6FC74c04954a) duplicated in a few places
in the .py and .cairo sources.

Resolves kkrt-labs#1011

## What is the new behavior?

That constant is now defined in a single location in constants.py and
imported as needed. Accordingly, codebase is also passed as a new
argument in the "kakarot" and "EVM" constructors.

<!-- 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/1069)
<!-- Reviewable:end -->

---------

Co-authored-by: Rene Doursat <[email protected]>
  • Loading branch information
ClementWalter and prof-farworse authored Apr 4, 2024
1 parent d965e91 commit 42bbc4e
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 11 deletions.
1 change: 1 addition & 0 deletions kakarot_scripts/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ class ChainId(IntEnum):
NETWORK["chain_id"] = ChainId.chain_id

ETH_TOKEN_ADDRESS = 0x49D36570D4E46F48E99674BD3FCC84644DDD6B96F7C741B1562B82F9E004DC7
COINBASE = 0xCA40796AFB5472ABAED28907D5ED6FC74C04954A
SOURCE_DIR = Path("src")
SOURCE_DIR_FIXTURES = Path("tests/fixtures")
CONTRACTS = {p.stem: p for p in list(SOURCE_DIR.glob("**/*.cairo"))}
Expand Down
3 changes: 3 additions & 0 deletions kakarot_scripts/deploy_kakarot.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from kakarot_scripts.constants import (
BLOCK_GAS_LIMIT,
COINBASE,
DECLARED_CONTRACTS,
ETH_TOKEN_ADDRESS,
EVM_ADDRESS,
Expand Down Expand Up @@ -59,6 +60,7 @@ async def main():
class_hash["account_contract"], # account_contract_class_hash_
class_hash["uninitialized_account"], # uninitialized_account_class_hash_
class_hash["Cairo1Helpers"],
COINBASE,
BLOCK_GAS_LIMIT,
)

Expand All @@ -69,6 +71,7 @@ async def main():
class_hash["account_contract"], # account_contract_class_hash_
class_hash["uninitialized_account"], # uninitialized_account_class_hash_
class_hash["Cairo1Helpers"],
COINBASE,
BLOCK_GAS_LIMIT,
)

Expand Down
2 changes: 2 additions & 0 deletions src/kakarot/kakarot.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func constructor{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr
account_contract_class_hash: felt,
uninitialized_account_class_hash: felt,
precompiles_class_hash: felt,
coinbase: felt,
block_gas_limit: felt,
) {
return Kakarot.constructor(
Expand All @@ -36,6 +37,7 @@ func constructor{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr
account_contract_class_hash,
uninitialized_account_class_hash,
precompiles_class_hash,
coinbase,
block_gas_limit,
);
}
Expand Down
4 changes: 3 additions & 1 deletion src/kakarot/library.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,22 @@ namespace Kakarot {
// @param account_contract_class_hash The clash hash of the contract account.
// @param uninitialized_account_class_hash The class hash of the uninitialized account used for deterministic address calculation.
// @param precompiles_class_hash The precompiles class hash for precompiles not implemented in Kakarot.
// @param coinbase The EOA whose key is owned by the deployer (or known to be owned by Coinbase)
func constructor{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}(
owner: felt,
native_token_address,
account_contract_class_hash,
uninitialized_account_class_hash,
precompiles_class_hash,
coinbase,
block_gas_limit,
) {
Ownable.initializer(owner);
Kakarot_native_token_address.write(native_token_address);
Kakarot_account_contract_class_hash.write(account_contract_class_hash);
Kakarot_uninitialized_account_class_hash.write(uninitialized_account_class_hash);
Kakarot_precompiles_class_hash.write(precompiles_class_hash);
Kakarot_coinbase.write(0xCA40796aFB5472abaeD28907D5ED6FC74c04954a);
Kakarot_coinbase.write(coinbase);
Kakarot_block_gas_limit.write(block_gas_limit);
return ();
}
Expand Down
4 changes: 2 additions & 2 deletions tests/end_to_end/bytecodes.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from kakarot_scripts.constants import NETWORK
from kakarot_scripts.constants import COINBASE, NETWORK

test_cases = [
{
Expand Down Expand Up @@ -749,7 +749,7 @@
"value": 0,
"code": "4100",
"calldata": "",
"stack": f"{0xca40796afb5472abaed28907d5ed6fc74c04954a}",
"stack": f"{COINBASE}",
"memory": "",
"return_data": "",
"success": 1,
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/EVM.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@ func constructor{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr
account_contract_class_hash: felt,
uninitialized_account_class_hash: felt,
precompiles_class_hash: felt,
coinbase: felt,
block_gas_limit: felt,
) {
Kakarot_native_token_address.write(native_token_address);
Kakarot_account_contract_class_hash.write(account_contract_class_hash);
Kakarot_uninitialized_account_class_hash.write(uninitialized_account_class_hash);
Kakarot_precompiles_class_hash.write(precompiles_class_hash);
Kakarot_coinbase.write(0xCA40796aFB5472abaeD28907D5ED6FC74c04954a);
Kakarot_coinbase.write(coinbase);
Kakarot_block_gas_limit.write(block_gas_limit);
return ();
}
Expand Down
10 changes: 3 additions & 7 deletions tests/src/kakarot/instructions/test_block_information.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import pytest

from kakarot_scripts.constants import COINBASE
from tests.utils.constants import BIG_CHAIN_ID, BLOCK_GAS_LIMIT, CHAIN_ID, Opcodes
from tests.utils.syscall_handler import SyscallHandler

Expand All @@ -11,10 +12,7 @@ class TestBlockInformation:
@pytest.mark.parametrize(
"opcode,expected_result",
[
(
Opcodes.COINBASE,
0xCA40796AFB5472ABAED28907D5ED6FC74C04954A,
),
(Opcodes.COINBASE, COINBASE),
(Opcodes.TIMESTAMP, 0x1234),
(Opcodes.NUMBER, SyscallHandler.block_number),
(Opcodes.PREVRANDAO, 0),
Expand All @@ -25,9 +23,7 @@ class TestBlockInformation:
(Opcodes.BLOBBASEFEE, 0),
],
)
@SyscallHandler.patch(
"Kakarot_coinbase", 0xCA40796AFB5472ABAED28907D5ED6FC74C04954A
)
@SyscallHandler.patch("Kakarot_coinbase", COINBASE)
@SyscallHandler.patch("Kakarot_block_gas_limit", BLOCK_GAS_LIMIT)
@patch.object(SyscallHandler, "block_timestamp", 0x1234)
def test__exec_block_information(self, cairo_run, opcode, expected_result):
Expand Down

0 comments on commit 42bbc4e

Please sign in to comment.