From 1b9d320b7bbb93e6cdb68303e6c429e2ed368fa4 Mon Sep 17 00:00:00 2001 From: Oba Date: Tue, 8 Oct 2024 14:26:02 +0200 Subject: [PATCH] fix: use MAX_SAFE_CHAIN_ID (#1472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Time spent on this PR: ## Pull request type 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? Resolves # ## What is the new behavior? Use MAX_SAFE_CHAIN_ID for cairo / cairo-zero See https://gist.github.com/rekmarks/a47bd5f2525936c4b8eee31a16345553 - - - This change is [Reviewable](https://reviewable.io/reviews/kkrt-labs/kakarot/1472) --- .../contracts/src/kakarot_core/eth_rpc.cairo | 28 +++++++++---------- .../crates/utils/src/constants.cairo | 2 ++ cairo_zero/kakarot/constants.cairo | 3 ++ cairo_zero/kakarot/library.cairo | 3 +- cairo_zero/tests/src/kakarot/test_kakarot.py | 13 +++++++-- kakarot_scripts/constants.py | 6 ++-- tests/utils/constants.py | 3 +- 7 files changed, 37 insertions(+), 21 deletions(-) diff --git a/cairo/kakarot-ssj/crates/contracts/src/kakarot_core/eth_rpc.cairo b/cairo/kakarot-ssj/crates/contracts/src/kakarot_core/eth_rpc.cairo index b5259b65d..f25ddc966 100644 --- a/cairo/kakarot-ssj/crates/contracts/src/kakarot_core/eth_rpc.cairo +++ b/cairo/kakarot-ssj/crates/contracts/src/kakarot_core/eth_rpc.cairo @@ -10,7 +10,7 @@ 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::constants::MAX_SAFE_CHAIN_ID; use utils::eth_transaction::transaction::{Transaction, TransactionTrait}; #[starknet::interface] @@ -128,7 +128,7 @@ pub impl EthRPC< fn eth_chain_id(self: @TContractState) -> u64 { let tx_info = get_tx_info().unbox(); let tx_chain_id: u64 = tx_info.chain_id.try_into().unwrap(); - tx_chain_id % POW_2_53.try_into().unwrap() + tx_chain_id % MAX_SAFE_CHAIN_ID.try_into().unwrap() } fn eth_call( @@ -268,7 +268,7 @@ mod tests { 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::constants::MAX_SAFE_CHAIN_ID; fn set_up() -> KakarotCore::ContractState { // Define the kakarot state to access contract functions @@ -301,29 +301,29 @@ mod tests { } #[test] - fn test_eth_chain_id_returns_input_when_less_than_pow_2_53() { + fn test_eth_chain_id_returns_input_when_less_than_max_safe_chain_id() { let kakarot_state = KakarotCore::unsafe_new_contract_state(); - // Convert POW_2_53 - 1 to u64 since POW_2_53 is defined as u128 - let chain_id: u64 = (POW_2_53 - 1).try_into().unwrap(); + let chain_id: u64 = MAX_SAFE_CHAIN_ID - 1; start_cheat_chain_id_global(chain_id.into()); assert_eq!( kakarot_state.eth_chain_id(), chain_id, - "Should return original chain ID when below 2^53" + "Should return original chain ID when below MAX_SAFE_CHAIN_ID" ); tear_down(); } #[test] - fn test_eth_chain_id_returns_modulo_when_greater_than_or_equal_to_pow_2_53() { - // Test with a value equal to 2^53 + fn test_eth_chain_id_returns_modulo_when_greater_than_or_equal_to_max_safe_chain_id() { + // Test with a value equal to MAX_SAFE_CHAIN_ID let kakarot_state = set_up(); - let chain_id: u64 = POW_2_53.try_into().unwrap(); - start_cheat_chain_id_global(chain_id.into()); - assert_eq!(kakarot_state.eth_chain_id(), 0, "Should return 0 when chain ID is 2^53"); + start_cheat_chain_id_global(MAX_SAFE_CHAIN_ID.into()); + assert_eq!( + kakarot_state.eth_chain_id(), 0, "Should return 0 when chain ID is MAX_SAFE_CHAIN_ID" + ); - // Test with a value greater than 2^53 - let chain_id: u64 = (POW_2_53 + 53).try_into().unwrap(); + // Test with a value greater than MAX_SAFE_CHAIN_ID + let chain_id: u64 = MAX_SAFE_CHAIN_ID + 53; start_cheat_chain_id_global(chain_id.into()); assert_eq!( kakarot_state.eth_chain_id(), 53, "Should return correct value after modulo operation" diff --git a/cairo/kakarot-ssj/crates/utils/src/constants.cairo b/cairo/kakarot-ssj/crates/utils/src/constants.cairo index 0b4b1c676..5f9b52a8b 100644 --- a/cairo/kakarot-ssj/crates/utils/src/constants.cairo +++ b/cairo/kakarot-ssj/crates/utils/src/constants.cairo @@ -14,6 +14,8 @@ pub const BLOCK_GAS_LIMIT: u64 = 7_000_000; pub const MIN_BASE_FEE_PER_BLOB_GAS: u64 = 1; // CHAIN_ID = KKRT (0x4b4b5254) in ASCII pub const CHAIN_ID: u64 = 1263227476; +// see https://gist.github.com/rekmarks/a47bd5f2525936c4b8eee31a16345553 +pub const MAX_SAFE_CHAIN_ID: u64 = 4503599627370476; // STACK pub const STACK_MAX_DEPTH: usize = 1024; diff --git a/cairo_zero/kakarot/constants.cairo b/cairo_zero/kakarot/constants.cairo index 34131f538..c38f29ab2 100644 --- a/cairo_zero/kakarot/constants.cairo +++ b/cairo_zero/kakarot/constants.cairo @@ -22,6 +22,9 @@ namespace Constants { const EMPTY_CODE_HASH_LOW = 0xe500b653ca82273b7bfad8045d85a470; const EMPTY_CODE_HASH_HIGH = 0xc5d2460186f7233c927e7db2dcc703c0; const BURN_ADDRESS = 0xdead; + + // See https://gist.github.com/rekmarks/a47bd5f2525936c4b8eee31a16345553 + const MAX_SAFE_CHAIN_ID = 4503599627370476; } // See model.Opcode: diff --git a/cairo_zero/kakarot/library.cairo b/cairo_zero/kakarot/library.cairo index d1e75bc58..acc3e0984 100644 --- a/cairo_zero/kakarot/library.cairo +++ b/cairo_zero/kakarot/library.cairo @@ -13,6 +13,7 @@ from starkware.cairo.common.math import split_felt from backend.starknet import Starknet from kakarot.account import Account +from kakarot.constants import Constants from kakarot.storages import ( Kakarot_uninitialized_account_class_hash, Kakarot_account_contract_class_hash, @@ -125,7 +126,7 @@ namespace Kakarot { chain_id: felt ) { let (tx_info) = get_tx_info(); - let (_, chain_id) = unsigned_div_rem(tx_info.chain_id, 2 ** 53); + let (_, chain_id) = unsigned_div_rem(tx_info.chain_id, Constants.MAX_SAFE_CHAIN_ID); return (chain_id=chain_id); } diff --git a/cairo_zero/tests/src/kakarot/test_kakarot.py b/cairo_zero/tests/src/kakarot/test_kakarot.py index 0ba9d54c6..6cd1328dd 100644 --- a/cairo_zero/tests/src/kakarot/test_kakarot.py +++ b/cairo_zero/tests/src/kakarot/test_kakarot.py @@ -17,7 +17,12 @@ from web3.exceptions import NoABIFunctionsFound from kakarot_scripts.ef_tests.fetch import EF_TESTS_PARSED_DIR -from tests.utils.constants import CHAIN_ID, TRANSACTION_GAS_LIMIT, TRANSACTIONS +from tests.utils.constants import ( + CHAIN_ID, + MAX_SAFE_CHAIN_ID, + TRANSACTION_GAS_LIMIT, + TRANSACTIONS, +) from tests.utils.errors import cairo_error from tests.utils.helpers import felt_to_signed_int, rlp_encode_signed_data from tests.utils.syscall_handler import SyscallHandler, parse_state @@ -494,10 +499,12 @@ def test_failing_contract(self, cairo_run): class TestEthChainIdEntrypoint: @given(chain_id=integers(min_value=0, max_value=2**64 - 1)) - def test_should_return_chain_id_modulo_53(self, cairo_run, chain_id): + def test_should_return_chain_id_modulo_max_safe_chain_id( + self, cairo_run, chain_id + ): with patch.dict(SyscallHandler.tx_info, {"chain_id": chain_id}): res = cairo_run("test__eth_chain_id") - assert res == chain_id % 2**53 + assert res == chain_id % MAX_SAFE_CHAIN_ID class TestEthSendRawTransactionEntrypoint: @SyscallHandler.patch("Pausable_paused", 1) diff --git a/kakarot_scripts/constants.py b/kakarot_scripts/constants.py index 676cc59ef..a1bf745bb 100644 --- a/kakarot_scripts/constants.py +++ b/kakarot_scripts/constants.py @@ -22,6 +22,8 @@ BLOCK_GAS_LIMIT = 7_000_000 DEFAULT_GAS_PRICE = 1 BEACON_ROOT_ADDRESS = "0x000F3df6D732807Ef1319fB7B8bB8522d0Beac02" +# see https://gist.github.com/rekmarks/a47bd5f2525936c4b8eee31a16345553 +MAX_SAFE_CHAIN_ID = 4503599627370476 class NetworkType(Enum): @@ -192,7 +194,7 @@ class NetworkType(Enum): if WEB3.is_connected(): chain_id = WEB3.eth.chain_id else: - chain_id = starknet_chain_id % 2**53 + chain_id = starknet_chain_id % MAX_SAFE_CHAIN_ID except ( requests.exceptions.ConnectionError, requests.exceptions.MissingSchema, @@ -202,7 +204,7 @@ class NetworkType(Enum): f"⚠️ Could not get chain Id from {NETWORK['rpc_url']}: {e}, defaulting to KKRT" ) starknet_chain_id = int.from_bytes(b"KKRT", "big") - chain_id = starknet_chain_id % 2**53 + chain_id = starknet_chain_id % MAX_SAFE_CHAIN_ID class ChainId(IntEnum): diff --git a/tests/utils/constants.py b/tests/utils/constants.py index 7807302b2..bdab8bf09 100644 --- a/tests/utils/constants.py +++ b/tests/utils/constants.py @@ -3,13 +3,14 @@ import pytest -from kakarot_scripts.constants import BLOCK_GAS_LIMIT +from kakarot_scripts.constants import BLOCK_GAS_LIMIT, MAX_SAFE_CHAIN_ID BLOCK_GAS_LIMIT = BLOCK_GAS_LIMIT MIN_BASE_FEE_PER_BLOB_GAS = 1 CHAIN_ID = int.from_bytes(b"KKRT", "big") # KKRT (0x4b4b5254) in ASCII BIG_CHAIN_ID = int.from_bytes(b"SN_SEPOLIA", "big") +MAX_SAFE_CHAIN_ID = MAX_SAFE_CHAIN_ID # Class hash of the cairo1 helpers CAIRO1_HELPERS_CLASS_HASH = 0xDEADBEEFABDE1E11A5