From 6ac70e80efc1d31b013972754f4fe0cdc4ca109c Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Thu, 6 Feb 2025 16:13:47 +0700 Subject: [PATCH] fix: process-transaction src & strategy (#670) - fixes the strategy of process_transaction to only generate valid gas values regarding basefee - fixes the preaccessed addresses & keys to be instanciated with a default dict with default 0 - fixes the state used in reads post-execution to be the updated on from the env - saturate blob gas / gas price values to 2**64 - stop early taylor exponentiation when accumulated numerator is > 2**128 - limit excess_blob_gas to 10*target in strategy, to avoid the pythonic taylor exponentiation to run forever. - fix the assertions made in `process_transaction` to handle raises AND exceptions in return values --- cairo/ethereum/cancun/fork.cairo | 9 ++-- cairo/ethereum/cancun/utils/message.cairo | 2 +- cairo/ethereum/cancun/vm/gas.cairo | 17 +++++- cairo/ethereum/cancun/vm/interpreter.cairo | 2 +- cairo/ethereum/utils/numeric.cairo | 11 +++- cairo/tests/ethereum/cancun/test_fork.py | 54 ++++++++++++++----- cairo/tests/ethereum/cancun/vm/test_gas.py | 12 +++-- cairo/tests/ethereum/utils/test_numeric.py | 41 +++++++++++++- .../src/cairo_addons/hints/dict.py | 6 --- .../src/cairo_addons/hints/hashdict.py | 4 +- 10 files changed, 125 insertions(+), 33 deletions(-) diff --git a/cairo/ethereum/cancun/fork.cairo b/cairo/ethereum/cancun/fork.cairo index a7908ae60..62950a8d4 100644 --- a/cairo/ethereum/cancun/fork.cairo +++ b/cairo/ethereum/cancun/fork.cairo @@ -7,6 +7,7 @@ from starkware.cairo.common.cairo_builtins import ( ModBuiltin, ) from starkware.cairo.common.dict_access import DictAccess +from starkware.cairo.common.default_dict import default_dict_new from starkware.cairo.common.math import assert_not_zero, split_felt, assert_le_felt from starkware.cairo.common.math_cmp import is_le, is_le_felt from starkware.cairo.common.registers import get_fp_and_pc @@ -93,7 +94,7 @@ from ethereum.cancun.transactions import recover_sender from cairo_core.comparison import is_zero from src.utils.array import count_not_zero -from src.utils.dict import dict_new_empty, hashdict_write +from src.utils.dict import hashdict_write const ELASTICITY_MULTIPLIER = 2; const BASE_FEE_MAX_CHANGE_DENOMINATOR = 8; @@ -369,7 +370,7 @@ func process_transaction{ EnvImpl.set_state{env=env}(state); // Create preaccessed addresses and write coinbase - let (preaccessed_addresses_ptr) = dict_new_empty(); + let (preaccessed_addresses_ptr) = default_dict_new(0); tempvar preaccessed_addresses_ptr_start = preaccessed_addresses_ptr; let address = env.value.coinbase; hashdict_write{dict_ptr=preaccessed_addresses_ptr}(1, &address.value, 1); @@ -380,7 +381,7 @@ func process_transaction{ ), ); // Create preaccessed storage keys - let (preaccessed_storage_keys_ptr) = dict_new_empty(); + let (preaccessed_storage_keys_ptr) = default_dict_new(0); tempvar preaccessed_storage_keys = SetTupleAddressBytes32( new SetTupleAddressBytes32Struct( dict_ptr_start=cast(preaccessed_storage_keys_ptr, SetTupleAddressBytes32DictAccess*), @@ -429,6 +430,8 @@ func process_transaction{ preaccessed_storage_keys, ); let output = process_message_call{env=env}(message); + // Rebind env's state modified in `process_message_call` + let state = env.value.state; // Calculate gas refund with_attr error_message("OverflowError") { diff --git a/cairo/ethereum/cancun/utils/message.cairo b/cairo/ethereum/cancun/utils/message.cairo index 47667e814..3afb49c12 100644 --- a/cairo/ethereum/cancun/utils/message.cairo +++ b/cairo/ethereum/cancun/utils/message.cairo @@ -20,7 +20,7 @@ from starkware.cairo.common.registers import get_fp_and_pc, get_label_location from starkware.cairo.common.memcpy import memcpy from starkware.cairo.common.dict_access import DictAccess -from src.utils.dict import dict_new_empty, hashdict_write +from src.utils.dict import hashdict_write const PRECOMPILED_ADDRESSES_SIZE = 10; diff --git a/cairo/ethereum/cancun/vm/gas.cairo b/cairo/ethereum/cancun/vm/gas.cairo index 5fd8344bf..2567de3c6 100644 --- a/cairo/ethereum/cancun/vm/gas.cairo +++ b/cairo/ethereum/cancun/vm/gas.cairo @@ -258,31 +258,46 @@ func calculate_excess_blob_gas{range_check_ptr}(parent_header: Header) -> U64 { return excess_blob_gas; } +// @dev: Saturates at 2**64 - 1 func calculate_total_blob_gas{range_check_ptr}(tx: Transaction) -> Uint { if (tx.value.blob_transaction.value != 0) { - let total_blob_gas = Uint( + tempvar total_blob_gas = Uint( GasConstants.GAS_PER_BLOB * tx.value.blob_transaction.value.blob_versioned_hashes.value.len, ); + let saturate = is_le_felt(2 ** 64, total_blob_gas.value); + if (saturate != 0) { + tempvar res = Uint(2 ** 64 - 1); + return res; + } return total_blob_gas; } let total_blob_gas = Uint(0); return total_blob_gas; } +// @dev: Saturates at 2**64 - 1 func calculate_blob_gas_price{range_check_ptr}(excess_blob_gas: U64) -> Uint { let blob_gas_price = taylor_exponential( Uint(GasConstants.MIN_BLOB_GASPRICE), Uint(excess_blob_gas.value), Uint(GasConstants.BLOB_GASPRICE_UPDATE_FRACTION), ); + let saturate = is_le_felt(2 ** 64, blob_gas_price.value); + if (saturate != 0) { + tempvar res = Uint(2 ** 64 - 1); + return res; + } return blob_gas_price; } func calculate_data_fee{range_check_ptr}(excess_blob_gas: U64, tx: Transaction) -> Uint { alloc_locals; + // saturate at 2**64 - 1 let total_blob_gas = calculate_total_blob_gas(tx); + // saturate at 2**64 - 1 let blob_gas_price = calculate_blob_gas_price(excess_blob_gas); + // fits in (2**128-1) let data_fee = Uint(total_blob_gas.value * blob_gas_price.value); return data_fee; } diff --git a/cairo/ethereum/cancun/vm/interpreter.cairo b/cairo/ethereum/cancun/vm/interpreter.cairo index 4b1f0b2a2..dbdd2258b 100644 --- a/cairo/ethereum/cancun/vm/interpreter.cairo +++ b/cairo/ethereum/cancun/vm/interpreter.cairo @@ -59,7 +59,7 @@ from ethereum.cancun.state import ( touch_account, ) -from src.utils.dict import dict_new_empty, hashdict_write, dict_squash +from src.utils.dict import hashdict_write, dict_squash struct MessageCallOutput { value: MessageCallOutputStruct*, diff --git a/cairo/ethereum/utils/numeric.cairo b/cairo/ethereum/utils/numeric.cairo index 9839d105f..a0d3be56a 100644 --- a/cairo/ethereum/utils/numeric.cairo +++ b/cairo/ethereum/utils/numeric.cairo @@ -1,4 +1,4 @@ -from starkware.cairo.common.math_cmp import is_le, is_not_zero +from starkware.cairo.common.math_cmp import is_le, is_not_zero, is_le_felt from starkware.cairo.common.uint256 import uint256_reverse_endian from ethereum_types.numeric import Uint, U256, U256Struct, bool, U64 from ethereum_types.bytes import Bytes32, Bytes32Struct, Bytes20, Bytes @@ -86,6 +86,7 @@ func ceil32{range_check_ptr}(value: Uint) -> Uint { return result; } +// @dev: Saturates when the numerator_accumulated * numerator is greater than 2**128 - 1 func taylor_exponential{range_check_ptr}(factor: Uint, numerator: Uint, denominator: Uint) -> Uint { let output = 0; let i = 1; @@ -109,6 +110,14 @@ func _taylor_exponential{range_check_ptr}( let output = output + numerator_accumulated; let value = numerator_accumulated * numerator; let div = denominator * i; + + let saturate = is_le_felt(2 ** 128, value); + if (saturate != 0) { + // Return current accumulated output/denominator when we hit saturation + let (res, _) = divmod(output, denominator); + return res; + } + let (numerator_accumulated, _) = divmod(value, div); let i = i + 1; diff --git a/cairo/tests/ethereum/cancun/test_fork.py b/cairo/tests/ethereum/cancun/test_fork.py index 26dfdb86e..74914e332 100644 --- a/cairo/tests/ethereum/cancun/test_fork.py +++ b/cairo/tests/ethereum/cancun/test_fork.py @@ -28,10 +28,11 @@ signing_hash_4844, ) from ethereum.cancun.vm import Environment +from ethereum.cancun.vm.gas import TARGET_BLOB_GAS_PER_BLOCK from ethereum.exceptions import EthereumException from ethereum_types.bytes import Bytes, Bytes20 from ethereum_types.numeric import U64, U256, Uint -from hypothesis import assume, given, settings +from hypothesis import assume, given from hypothesis import strategies as st from hypothesis.strategies import composite, integers @@ -127,6 +128,22 @@ def tx_with_sender_in_state( return tx, state +@composite +def env_with_valid_gas_price(draw): + env = draw(st.from_type(Environment)) + if env.gas_price < env.base_fee_per_gas: + env.gas_price = draw( + st.integers(min_value=int(env.base_fee_per_gas), max_value=2**64 - 1).map( + Uint + ) + ) + # Values too high would cause taylor_exponential to run indefinitely. + env.excess_blob_gas = draw( + st.integers(0, 10 * int(TARGET_BLOB_GAS_PER_BLOCK)).map(U64) + ) + return env + + class TestFork: @given( block_gas_limit=..., @@ -201,22 +218,33 @@ def test_make_receipt( "make_receipt", tx, error, cumulative_gas_used, logs ) - @given(env=..., tx=tx_without_code()) - @settings(max_examples=100) + @given(env=env_with_valid_gas_price(), tx=tx_without_code()) def test_process_transaction(self, cairo_run, env: Environment, tx: Transaction): + # The Cairo Runner will raise if an exception is in the return values OR if + # an assert expression fails (e.g. InvalidBlock) try: - gas_used_cairo, logs_cairo, error_cairo = cairo_run( - "process_transaction", env, tx - ) - except Exception as e: - with strict_raises(type(e)): - process_transaction(env, tx) + env_cairo, gas_cairo, logs_cairo = cairo_run("process_transaction", env, tx) + except Exception as cairo_e: + # 1. Handle exceptions thrown + try: + output_py = process_transaction(env, tx) + except Exception as thrown_exception: + assert type(cairo_e) is type(thrown_exception) + return + + # 2. Handle exceptions in return values + assert env_cairo == env + assert gas_cairo == output_py[0] + assert logs_cairo == output_py[1] + with strict_raises(type(cairo_e)): + if len(output_py) == 3: + raise output_py[2] return - gas_used, logs, error = process_transaction(env, tx) - assert gas_used_cairo == gas_used - assert logs_cairo == logs - assert error_cairo == error + gas_used, logs = process_transaction(env, tx) + assert env_cairo == env + assert gas_used == gas_cairo + assert logs == logs_cairo @given( data=tx_with_sender_in_state(), diff --git a/cairo/tests/ethereum/cancun/vm/test_gas.py b/cairo/tests/ethereum/cancun/vm/test_gas.py index 16e8d1ece..3ff6e5456 100644 --- a/cairo/tests/ethereum/cancun/vm/test_gas.py +++ b/cairo/tests/ethereum/cancun/vm/test_gas.py @@ -126,13 +126,19 @@ def test_calculate_total_blob_gas(self, cairo_run, tx: BlobTransaction): @given(excess_blob_gas=excess_blob_gas) def test_calculate_blob_gas_price(self, cairo_run, excess_blob_gas): - assert calculate_blob_gas_price(excess_blob_gas) == cairo_run( + """Saturates at 2**64 - 1""" + blob_gas_price_py = min( + calculate_blob_gas_price(excess_blob_gas), Uint(2**64 - 1) + ) + assert blob_gas_price_py == cairo_run( "calculate_blob_gas_price", excess_blob_gas ) @given(excess_blob_gas=excess_blob_gas, tx=...) def test_calculate_data_fee(self, cairo_run, excess_blob_gas, tx: BlobTransaction): + """Saturates at (2**64 - 1)**2""" assume(len(tx.blob_versioned_hashes) > 0) - assert calculate_data_fee(excess_blob_gas, tx) == cairo_run( - "calculate_data_fee", excess_blob_gas, tx + data_fee_py = min( + calculate_data_fee(excess_blob_gas, tx), Uint((2**64 - 1) ** 2) ) + assert data_fee_py == cairo_run("calculate_data_fee", excess_blob_gas, tx) diff --git a/cairo/tests/ethereum/utils/test_numeric.py b/cairo/tests/ethereum/utils/test_numeric.py index 1fd794ffd..feaf83a74 100644 --- a/cairo/tests/ethereum/utils/test_numeric.py +++ b/cairo/tests/ethereum/utils/test_numeric.py @@ -1,5 +1,9 @@ from ethereum.cancun.fork_types import Address -from ethereum.cancun.vm.gas import BLOB_GASPRICE_UPDATE_FRACTION, MIN_BLOB_GASPRICE +from ethereum.cancun.vm.gas import ( + BLOB_GASPRICE_UPDATE_FRACTION, + MIN_BLOB_GASPRICE, + TARGET_BLOB_GAS_PER_BLOCK, +) from ethereum.utils.numeric import ceil32, taylor_exponential from ethereum_types.bytes import Bytes32 from ethereum_types.numeric import U256, Uint @@ -11,6 +15,24 @@ from tests.utils.strategies import uint128 +def taylor_exponential_limited( + factor: Uint, numerator: Uint, denominator: Uint +) -> Uint: + """Limited version of taylor_exponential that saturates when `numerator_accumulated * numerator` is greater than 2**128 - 1""" + i = Uint(1) + output = Uint(0) + numerator_accumulated = factor * denominator + while numerator_accumulated > Uint(0): + output += numerator_accumulated + value = numerator_accumulated * numerator + div = denominator * i + if value > Uint(2**128 - 1): + return output // denominator + numerator_accumulated = value // div + i += Uint(1) + return output // denominator + + class TestNumeric: @given(a=uint128, b=uint128) @@ -34,7 +56,9 @@ def test_ceil32(self, cairo_run, value: Uint): @given( factor=st.just(MIN_BLOB_GASPRICE), - numerator=st.integers(min_value=1, max_value=100_000).map(Uint), + numerator=st.integers( + min_value=1, max_value=10 * int(TARGET_BLOB_GAS_PER_BLOCK) + ).map(Uint), denominator=st.just(BLOB_GASPRICE_UPDATE_FRACTION), ) def test_taylor_exponential( @@ -44,6 +68,19 @@ def test_taylor_exponential( "taylor_exponential", factor, numerator, denominator ) + @given( + factor=st.just(MIN_BLOB_GASPRICE), + numerator=st.integers(min_value=1, max_value=100_000_000_000_000_000).map(Uint), + denominator=st.just(BLOB_GASPRICE_UPDATE_FRACTION), + ) + def test_taylor_exponential_limited( + self, cairo_run, factor: Uint, numerator: Uint, denominator: Uint + ): + """Compares to our limited version of taylor_exponential that saturates when `numerator_accumulated * numerator` is greater than 2**128 - 1""" + assert taylor_exponential_limited(factor, numerator, denominator) == cairo_run( + "taylor_exponential", factor, numerator, denominator + ) + @given(bytes=...) def test_U256_from_be_bytes32(self, cairo_run, bytes: Bytes32): expected = U256.from_be_bytes(bytes) diff --git a/python/cairo-addons/src/cairo_addons/hints/dict.py b/python/cairo-addons/src/cairo_addons/hints/dict.py index 026709206..5831a5280 100644 --- a/python/cairo-addons/src/cairo_addons/hints/dict.py +++ b/python/cairo-addons/src/cairo_addons/hints/dict.py @@ -87,9 +87,6 @@ def copy_dict_segment( def merge_dict_tracker_with_parent( dict_manager: DictManager, ids: VmConsts, - segments: MemorySegmentManager, - memory: MemoryDict, - ap: RelocatableValue, ): current_dict_tracker = dict_manager.get_tracker(ids.dict_ptr) parent_dict_tracker = dict_manager.get_tracker(ids.parent_dict_end) @@ -100,9 +97,6 @@ def merge_dict_tracker_with_parent( def update_dict_tracker( dict_manager: DictManager, ids: VmConsts, - segments: MemorySegmentManager, - memory: MemoryDict, - ap: RelocatableValue, ): dict_tracker = dict_manager.get_tracker(ids.current_tracker_ptr) dict_tracker.current_ptr = ids.new_tracker_ptr.address_ diff --git a/python/cairo-addons/src/cairo_addons/hints/hashdict.py b/python/cairo-addons/src/cairo_addons/hints/hashdict.py index 8a58fa20c..1c716c447 100644 --- a/python/cairo-addons/src/cairo_addons/hints/hashdict.py +++ b/python/cairo-addons/src/cairo_addons/hints/hashdict.py @@ -42,9 +42,9 @@ def hashdict_write(dict_manager: DictManager, ids: VmConsts, memory: MemoryDict) preimage = tuple([memory[ids.key + i] for i in range(ids.key_len)]) prev_value = dict_tracker.data.get(preimage) if prev_value: - ids.prev_value = prev_value + ids.dict_ptr.prev_value = prev_value else: - ids.prev_value = dict_tracker.data.default_factory() + ids.dict_ptr.prev_value = dict_tracker.data.default_factory() dict_tracker.data[preimage] = ids.new_value