Add support for broken-down intrinsic gas, change execution event gas recording#1985
Add support for broken-down intrinsic gas, change execution event gas recording#1985
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors intrinsic gas computation to support broken-down gas cost reporting and improves execution event gas accounting. It introduces a new C structure monad_c_eth_intrinsic_gas to represent individual components of intrinsic gas costs, refactors static_validate_transaction to accept pre-computed intrinsic gas as a parameter, and updates event recording to correctly handle top-level call frame gas accounting.
Key Changes
- Introduced
monad_c_eth_intrinsic_gasstruct to break down intrinsic gas into base, data, creation, init_code, access_list, and authorizations components - Modified
static_validate_transactionto accept intrinsic gas as a parameter, eliminating the need for a workaround with system transactions - Updated top-level call frame gas accounting in execution events to correctly subtract intrinsic gas
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| category/execution/ethereum/core/eth_ctypes.h | Added new monad_c_eth_intrinsic_gas struct definition with component fields |
| category/execution/ethereum/transaction_gas.hpp | Added declarations for intrinsic_gas_breakdown and sum functions |
| category/execution/ethereum/transaction_gas.cpp | Refactored intrinsic_gas to use intrinsic_gas_breakdown and added sum function |
| category/execution/ethereum/validate_transaction.hpp | Updated static_validate_transaction signature to accept intrinsic gas parameter |
| category/execution/ethereum/validate_transaction.cpp | Modified validation to use passed-in intrinsic gas instead of computing it |
| category/execution/ethereum/execute_transaction.hpp | Added intrinsic_gas_ member to store breakdown |
| category/execution/ethereum/execute_transaction.cpp | Updated to compute and store intrinsic gas breakdown, pass it to validation and events |
| category/execution/monad/execute_system_transaction.cpp | Simplified system transaction validation by passing zero intrinsic gas |
| category/execution/ethereum/event/record_txn_events.hpp | Updated signature to accept gas_left and intrinsic gas breakdown |
| category/execution/ethereum/event/record_txn_events.cpp | Improved top-level frame gas accounting using intrinsic gas breakdown |
| category/rpc/monad_executor.cpp | Updated eth_call to compute and pass intrinsic gas |
| test/ethereum_test/src/transaction_test.cpp | Updated test to pass intrinsic gas to validation |
| category/execution/ethereum/test/test_validation.cpp | Updated all validation tests to compute and pass intrinsic gas |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| template <Traits traits> | ||
| monad_c_eth_intrinsic_gas intrinsic_gas_breakdown(Transaction const &) noexcept; | ||
|
|
||
| uint64_t sum(monad_c_eth_intrinsic_gas const &) noexcept; // Found by ADL |
There was a problem hiding this comment.
The declaration of the sum function is in the global namespace, but the definition in transaction_gas.cpp (line 166) is inside the monad namespace. This namespace mismatch will cause a linkage error. Either move this declaration inside MONAD_NAMESPACE_BEGIN/END to match the definition, or move the definition in the .cpp file outside the monad namespace to match this declaration.
02eb2bd to
1fdfb89
Compare
The sole function which computes intrinsic gas now organizes it per category. The `sum` function, found via argument dependent lookup, aggregates them to produce the total intrinsic gas, and the old intrinsic_gas function calls it. In a future commit, the entire intrinsic gas accounting structure will be recorded to an execution event.
The broken down intrinsic_gas_ is stored in the ExecuteTransaction functor, so that it can be passed in full to the event recording; eventually it will be exposed publicly via both RPC and the exec event ring, but this is an API (for RPC) and ABI (for events) affecting change, so it will happen later
1fdfb89 to
1721f9d
Compare
There are several related things happening in this PR. First we add a new C type, for representing the broken-down sources of intrinsic gas cost:
There are a few changes:
The sole function which computes intrinsic gas (
intrinsic_gas_breakdown) now produces one of the above structures. Thesumfunction, whose overload for this type is found via argument dependent lookup, aggregates a breakdown structure to produce the total intrinsic gas; the oldintrinsic_gasfunction is now a convenience wrapper forsum(intrinsic_gas_breakdown<traits>(tx))static_validate_transactionno longer computes the intrinsic gas itself, but requires the caller to pass it in. This was done for two reasons (1) it removes a hack for system transactions, where we used to pass a (temporary) fakegas_limitto trick the intrinsic validation step and (2) now we're computing this once and passing it around, no need to keep recomputing it (although sometimes we re-sum it)The broken down
intrinsic_gas_is stored in theExecuteTransactionfunctor, so that it can be passed in full to the event recording; eventually it will be exposed publicly via both RPC and the exec event ring, but this is an API (for RPC) and ABI (for events) affecting change, so it will happen laterTop-level call frame gas accounting is changed for execution events; eventually when we break the ABI, we'll report the intrinsic gas breakdown too