From 05760fd44551706e75e1405d48031685bd4acde3 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Wed, 6 Mar 2024 10:35:12 +0100 Subject: [PATCH] fix: eip1559 proper validation (#1010) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Time spent on this PR: 0.4d ## Pull request type 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? Resolves # ## What is the new behavior? - Add proper EIP-1159 validation on block gas / tx gas / account value - - - - - This change is [Reviewable](https://reviewable.io/reviews/kkrt-labs/kakarot/1010) --- blockchain-tests-skip.yml | 17 ----------------- scripts/utils/kakarot.py | 5 +++-- src/kakarot/accounts/eoa/library.cairo | 24 +++++++++++++++++++----- src/kakarot/interfaces/interfaces.cairo | 12 ++++++++++++ src/kakarot/interpreter.cairo | 3 +++ tests/end_to_end/test_kakarot.py | 4 ++-- tests/src/kakarot/test_kakarot.py | 3 ++- tests/utils/constants.py | 4 ++-- 8 files changed, 43 insertions(+), 29 deletions(-) diff --git a/blockchain-tests-skip.yml b/blockchain-tests-skip.yml index 9826035c5..0dfe04b19 100644 --- a/blockchain-tests-skip.yml +++ b/blockchain-tests-skip.yml @@ -69,23 +69,6 @@ testname: stEIP150singleCodeGasPrices: - gasCostExp_d7g0v0_Shanghai - gasCostReturn_d0g0v0_Shanghai - stEIP1559: - - lowFeeCap_d0g0v0_Shanghai - - lowGasLimit_d0g0v0_Shanghai - - lowGasPriceOldTypes_d0g0v0_Shanghai - - lowGasPriceOldTypes_d1g0v0_Shanghai - - outOfFundsOldTypes_d0g1v1_Shanghai - - outOfFundsOldTypes_d1g1v1_Shanghai - - outOfFunds_d0g1v1_Shanghai - - senderBalance_d0g0v0_Shanghai - - tipTooHigh_d0g0v0_Shanghai - - transactionIntinsicBug_d0g0v0_Shanghai - - valCausesOOF_d0g0v1_Shanghai - - valCausesOOF_d0g2v0_Shanghai - - valCausesOOF_d0g2v1_Shanghai - - valCausesOOF_d1g0v1_Shanghai - - valCausesOOF_d1g2v0_Shanghai - - valCausesOOF_d1g2v1_Shanghai stEIP2930: - manualCreate_d2g0v0_Shanghai - storageCosts_d0g0v0_Shanghai diff --git a/scripts/utils/kakarot.py b/scripts/utils/kakarot.py index 8037c9a97..2bb46d079 100644 --- a/scripts/utils/kakarot.py +++ b/scripts/utils/kakarot.py @@ -35,6 +35,7 @@ from scripts.utils.starknet import get_deployments from scripts.utils.starknet import invoke as _invoke_starknet from scripts.utils.starknet import wait_for_transaction +from tests.utils.constants import TRANSACTION_GAS_LIMIT from tests.utils.helpers import rlp_encode_signed_data from tests.utils.uint256 import int_to_uint256 @@ -133,7 +134,7 @@ async def deploy( value = kwargs.pop("value", 0) receipt, response, success, gas_used = await eth_send_transaction( to=0, - gas=int(3e6), + gas=int(TRANSACTION_GAS_LIMIT), data=contract.constructor(*args, **kwargs).data_in_transaction, caller_eoa=caller_eoa, max_fee=max_fee, @@ -206,7 +207,7 @@ def _wrap_kakarot(fun: str, caller_eoa: Optional[Account] = None): async def _wrapper(self, *args, **kwargs): abi = self.get_function_by_name(fun).abi gas_price = kwargs.pop("gas_price", 1_000) - gas_limit = kwargs.pop("gas_limit", 1_000_000_000) + gas_limit = kwargs.pop("gas_limit", TRANSACTION_GAS_LIMIT) value = kwargs.pop("value", 0) caller_eoa_ = kwargs.pop("caller_eoa", caller_eoa) max_fee = kwargs.pop("max_fee", None) diff --git a/src/kakarot/accounts/eoa/library.cairo b/src/kakarot/accounts/eoa/library.cairo index c7c2f2b32..6ed7807fc 100644 --- a/src/kakarot/accounts/eoa/library.cairo +++ b/src/kakarot/accounts/eoa/library.cairo @@ -1,14 +1,16 @@ %lang starknet from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin -from starkware.starknet.common.syscalls import CallContract, get_tx_info -from starkware.cairo.common.uint256 import Uint256, uint256_not +from starkware.starknet.common.syscalls import CallContract, get_tx_info, get_contract_address +from starkware.cairo.common.uint256 import Uint256, uint256_not, uint256_add, uint256_le from starkware.cairo.common.alloc import alloc from starkware.cairo.common.bool import TRUE, FALSE from starkware.cairo.common.memcpy import memcpy from starkware.cairo.common.math_cmp import is_le +from starkware.cairo.common.math import split_felt from kakarot.account import Account +from kakarot.errors import Errors from kakarot.interfaces.interfaces import IERC20, IKakarot from utils.eth_transaction import EthTransaction from utils.utils import Helpers @@ -164,17 +166,29 @@ namespace ExternallyOwnedAccount { let tx = EthTransaction.decode([call_array].data_len, calldata + [call_array].data_offset); let (_kakarot_address) = kakarot_address.read(); + let (block_gas_limit) = IKakarot.get_block_gas_limit(_kakarot_address); + let tx_gas_fits_in_block = is_le(tx.gas_limit, block_gas_limit); + let (base_fee) = IKakarot.get_base_fee(_kakarot_address); + let (native_token_address) = IKakarot.get_native_token(_kakarot_address); + let (contract_address) = get_contract_address(); + let (balance) = IERC20.balanceOf(native_token_address, contract_address); // ensure that the user was willing to at least pay the base fee let enough_fee = is_le(base_fee, tx.max_fee_per_gas); let max_fee_greater_priority_fee = is_le(tx.max_priority_fee_per_gas, tx.max_fee_per_gas); - if (enough_fee * max_fee_greater_priority_fee == 0) { - let (return_data: felt*) = alloc(); + let max_gas_fee = tx.gas_limit * tx.max_fee_per_gas; + let (max_fee_high, max_fee_low) = split_felt(max_gas_fee); + let (tx_cost, carry) = uint256_add(tx.amount, Uint256(low=max_fee_low, high=max_fee_high)); + assert carry = 0; + let (is_balance_enough) = uint256_le(tx_cost, balance); + + if (enough_fee * max_fee_greater_priority_fee * is_balance_enough * tx_gas_fits_in_block == 0) { + let (return_data_len, return_data) = Errors.eth_validation_failed(); tempvar range_check_ptr = range_check_ptr; tempvar syscall_ptr = syscall_ptr; tempvar pedersen_ptr = pedersen_ptr; - tempvar return_data_len = 0; + tempvar return_data_len = return_data_len; tempvar return_data = return_data; tempvar success = FALSE; tempvar gas_used = 0; diff --git a/src/kakarot/interfaces/interfaces.cairo b/src/kakarot/interfaces/interfaces.cairo index 82b25b12e..da2ace727 100644 --- a/src/kakarot/interfaces/interfaces.cairo +++ b/src/kakarot/interfaces/interfaces.cairo @@ -92,6 +92,18 @@ namespace IKakarot { func get_coinbase() -> (coinbase: felt) { } + func set_block_gas_limit(gas_limit_: felt) { + } + + func get_block_gas_limit() -> (block_gas_limit: felt) { + } + + func set_prev_randao(prev_randao_: Uint256) { + } + + func get_prev_randao() -> (prev_randao: Uint256) { + } + func deploy_externally_owned_account(evm_address: felt) { } diff --git a/src/kakarot/interpreter.cairo b/src/kakarot/interpreter.cairo index 6d4713b8c..e376439bb 100644 --- a/src/kakarot/interpreter.cairo +++ b/src/kakarot/interpreter.cairo @@ -837,6 +837,7 @@ namespace Interpreter { return (evm, stack, memory, state, gas_limit); } + // TODO: same as below let (is_value_le_balance) = uint256_le([value], [sender.balance]); if (is_value_le_balance == FALSE) { let evm = EVM.halt_validation_failed(evm); @@ -844,6 +845,8 @@ namespace Interpreter { } let (balance_post_value_transfer) = uint256_sub([sender.balance], [value]); + // TODO: since we check in EOA execute whether tx.amount + maxFeePerGas *tx.gas_limit <= + // tx.sender.balance this might be superfluous let effective_gas_fee = gas_limit * env.gas_price; let (fee_high, fee_low) = split_felt(effective_gas_fee); let fee_u256 = Uint256(low=fee_low, high=fee_high); diff --git a/tests/end_to_end/test_kakarot.py b/tests/end_to_end/test_kakarot.py index 90a585d2f..bb17336fa 100644 --- a/tests/end_to_end/test_kakarot.py +++ b/tests/end_to_end/test_kakarot.py @@ -8,7 +8,7 @@ from scripts.utils.starknet import wait_for_transaction from tests.end_to_end.bytecodes import test_cases -from tests.utils.constants import PRE_FUND_AMOUNT +from tests.utils.constants import PRE_FUND_AMOUNT, TRANSACTION_GAS_LIMIT from tests.utils.helpers import ( extract_memory_from_execute, generate_random_evm_address, @@ -197,7 +197,7 @@ async def test_eth_call_should_succeed( "is_some": 1, "value": int(generate_random_evm_address(seed=3), 16), }, - gas_limit=1_000_000_000, + gas_limit=TRANSACTION_GAS_LIMIT, gas_price=1_000, value=1_000, data=bytes(), diff --git a/tests/src/kakarot/test_kakarot.py b/tests/src/kakarot/test_kakarot.py index 6ded01fb0..396a921fe 100644 --- a/tests/src/kakarot/test_kakarot.py +++ b/tests/src/kakarot/test_kakarot.py @@ -11,6 +11,7 @@ from web3.exceptions import NoABIFunctionsFound from scripts.ef_tests.fetch import EF_TESTS_PARSED_DIR +from tests.utils.constants import TRANSACTION_GAS_LIMIT from tests.utils.syscall_handler import SyscallHandler, parse_state CONTRACT_ADDRESS = 1234 @@ -26,7 +27,7 @@ def _factory(contract_app, contract_name): def _wrap_cairo_run(fun): def _wrapper(self, *args, **kwargs): origin = kwargs.pop("origin", 0) - gas_limit = kwargs.pop("gas_limit", int(1e9)) + gas_limit = kwargs.pop("gas_limit", int(TRANSACTION_GAS_LIMIT)) gas_price = kwargs.pop("gas_price", 0) value = kwargs.pop("value", 0) data = self.get_function_by_name(fun)( diff --git a/tests/utils/constants.py b/tests/utils/constants.py index 6a69c4975..211f871d8 100644 --- a/tests/utils/constants.py +++ b/tests/utils/constants.py @@ -26,8 +26,8 @@ # TRANSACTION # TODO: handle tx gas limit properly and remove this constant -# Temporarily set tx gas limit to 1M gas -TRANSACTION_GAS_LIMIT = 1_000_000 +# Temporarily set tx gas limit to 20M gas (= block gas limit) +TRANSACTION_GAS_LIMIT = BLOCK_GAS_LIMIT # PRECOMPILES LAST_PRECOMPILE_ADDRESS = 0x09