feat: rakita/state-gas branch without ee-tests #3521
Open
Conversation
Add state gas accounting with reservoir model across interpreter, handler, context, and precompile crates. Includes comprehensive EIP-8037 test suite with 35+ test vectors covering SSTORE, CALL, CREATE, SELFDESTRUCT, reservoir refill, and gas opcode behavior.
… refill - Rename ResultGas::spent to regular_gas_spent to distinguish from state gas after EIP-8037 - Add total_gas_spent() method and deprecate old spent()/set_spent()/with_spent() accessors - Simplify reservoir refill logic: replace handler_reservoir_refill() with inline max() - Remove unnecessary reservoir manipulation in precompile path - Update op-revm handler to use simplified reservoir refill - Update Cargo.lock dependencies
…nd update test data
Consolidate the EIP-8037 auth list refund splitting logic from run_without_catch_error into apply_eip7702_auth_list, avoiding duplication between handler and inspector. pre_execution now takes init_and_floor_gas as a mutable reference and returns only the regular refund portion.
Introduce PrecompileFailure wrapper that carries both the error and an optional GasTracker, allowing precompiles that charge state gas (EIP-8037) to propagate gas info back to the caller on failure for proper reservoir refill.
Update all precompile Err returns to use .into() for converting PrecompileError to PrecompileFailure, enabling state gas tracking on precompile failures.
Commit 257d261 introduced a regression in gas accounting by incorrectly erasing both remaining and reservoir gas from total_gas_spent. The reservoir should not be erased since it's tracked separately via state_gas_spent. This fix reverts the erase_cost line from erase_cost(remaining + reservoir) to erase_cost(remaining), which correctly accounts for: - Regular gas: deducted from remaining calculation - State gas: tracked separately in state_gas_spent field Results: - State test failures reduced from 20 to 4 (pre-regression count) - All 408 workspace tests now pass - Updated EIP-8037 test data and assertions to match correct values
Refactor to improve code organization by moving the state gas deduction logic from the execution method into the first_frame_input method, where frame initialization concerns are naturally grouped together. This keeps frame setup logic cohesive and simplifies the execution method to focus on the execution loop rather than frame setup details. No functional change - same gas accounting and execution behavior.
Four fixes for EIP-8037 state gas interactions: 1. post_execution: Fix EIP-7623 floor gas check to account for EIP-8037 reservoir. Per EIP-8037, gas_used = tx.gas - gas_left - reservoir. The floor check now subtracts reservoir when computing gas used. 2. handler: Simplify reservoir handling in last_frame_result to always use the frame's final reservoir value directly. It already reflects child frame restorations via handle_reservoir_remaining_gas. 3. handler: Add EIP-8037 floor gas validation when gas_limit exceeds TX_MAX_GAS_LIMIT. The maximum gas_used is capped at TX_MAX_GAS_LIMIT since excess goes to reservoir, so floor_gas must not exceed that cap. 4. inspector: Remove double deduction of initial_state_gas in inspect_execution. The first_frame_input method already handles the deduction; the explicit deduction in the inspector was redundant.
… and EIP-7702 - tx_gas_used now subtracts unused reservoir gas (tx.gas - gas_left - state_gas_left) - Child revert/halt returns all state gas to parent reservoir (matching Python spec) - EIP-7702 state gas refund goes to reservoir instead of reducing initial_state_gas - Refund cap uses gas_used excluding reservoir - Precompile provider preserves reservoir across gas tracker replacement - Updated EIP-8037 test expectations and test data
…recovery Move require_non_staticcall! after gas charging in the CREATE/CREATE2 instruction handler. In the execution-specs, the static check is inside generic_create() after all gas (including state gas) has been charged. The previous ordering prevented state gas from being recorded on the frame, so on child error the parent could not recover it via handle_reservoir_remaining_gas.
…x reservoir handling - Remove unused `initial_reservoir` parameter from `last_frame_result` across all handlers - Fix precompile provider to save reservoir before overwriting gas tracker - Add deprecated `gas_used()` method on ExecutionResult pointing to `tx_gas_used()`
Static call check doesn't need to be after gas charging - CREATE in a static context is always an error regardless of gas accounting.
On revert/halt, state changes are rolled back so state gas should be refunded back to the reservoir. Only on success should the frame's final reservoir be used as-is.
klkvr
reviewed
Mar 24, 2026
| } | ||
|
|
||
| // EIP-8037: State creation gas cost increase | ||
| if spec.is_enabled_in(SpecId::AMSTERDAM) { |
Collaborator
There was a problem hiding this comment.
@rakita how are we planning to trigger this on tempo side? we can't just enable AMSTERDAM i think because its going to be changing further?
Member
Author
There was a problem hiding this comment.
Tempo has its own specific gas params here
EIP-8037 should use cfg.enable_amsterdam_eip8037 in all other places
fgimenez
reviewed
Mar 24, 2026
fgimenez
reviewed
Mar 24, 2026
Two bugs in state_gas_spent reporting: 1. build_result_gas added full initial_state_gas without subtracting eip7702_reservoir_refund, overcounting state gas for existing authority accounts. This caused block_state_gas_used to be too high and block_regular_gas_used to be too low. 2. On revert/halt, last_frame_result set state_gas_spent before restoring it to the reservoir, leaving a stale execution value. Now correctly zeroed since state changes are rolled back.
Refactor precompile provider to properly handle reservoir refill logic for both success and error/halt cases, ensuring state gas accounting is correct when precompiles don't track reservoir. Also apply rustfmt formatting across multiple files.
… API PrecompileOutput has gas_used (not gas: GasTracker) and PrecompileError is a flat enum (no .error/.gas subfields). Use record_regular_cost() to avoid deprecation warning.
…, flatten PrecompileError Remove gas_limit field from PrecompileOutput (only gas_used needed) and eliminate the PrecompileError wrapper layer so errors are returned directly as the enum variant without .into() conversion.
rakita
force-pushed
the
rakita/state-gas-min
branch
2 times, most recently
from
c495311 to
9028e22
Compare
Merging this PR will degrade performance by 7.06%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | EXTCODESIZE_50 |
38.4 µs | 40.8 µs | -6.02% |
| ❌ | GASLIMIT_50 |
18.4 µs | 19.1 µs | -3.41% |
| ❌ | GASPRICE_50 |
18.8 µs | 19.4 µs | -3.06% |
| ❌ | GAS_50 |
18.5 µs | 19.1 µs | -3.56% |
| ❌ | EXTCODECOPY_50 |
50.3 µs | 52.6 µs | -4.5% |
| ❌ | JUMPDEST_50 |
15.7 µs | 16.5 µs | -4.48% |
| ❌ | JUMP_50 |
18 µs | 18.7 µs | -3.56% |
| ❌ | CALLDATASIZE_50 |
18.5 µs | 19.2 µs | -3.7% |
| ❌ | DELEGATECALL_50 |
85.7 µs | 90.1 µs | -4.9% |
| ❌ | COINBASE_50 |
18.6 µs | 19.2 µs | -3.08% |
| ❌ | CALLVALUE_50 |
18.5 µs | 19.1 µs | -3.11% |
| ❌ | DIFFICULTY_50 |
18.8 µs | 19.4 µs | -3.35% |
| ❌ | CALL_50 |
90.4 µs | 94.6 µs | -4.4% |
| ❌ | CODESIZE_50 |
18.6 µs | 19.2 µs | -3.39% |
| ❌ | CHAINID_50 |
18.5 µs | 19.1 µs | -3.4% |
| ❌ | MSIZE_50 |
18.4 µs | 19.2 µs | -3.85% |
| ❌ | MCOPY_50 |
21 µs | 21.7 µs | -3% |
| ❌ | MSTORE_50 |
19.9 µs | 20.6 µs | -3.31% |
| ❌ | PC_50 |
18.6 µs | 19.2 µs | -3.54% |
| ❌ | PUSH11_50 |
18.9 µs | 19.6 µs | -3.33% |
| ... | ... | ... | ... | ... |
ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.
Comparing rakita/state-gas-min (280fbbc) with main (99c74f2)
… cleanup Update 4 EIP-8037 tests to match current reservoir refill behavior: on revert/halt at the top level, state_gas_spent is zeroed and state gas is restored to the reservoir (state changes rolled back). Regenerate testdata. Also complete the PrecompileOutput/PrecompileError simplification from the previous commit: remove reverted field, flatten error matching in tests.
rakita
force-pushed
the
rakita/state-gas-min
branch
from
9028e22 to
03986fc
Compare
…event underflow - Remove saturating_sub of reservoir from total_gas_spent in build_result_gas - Use saturating_sub in block_regular_gas_used to prevent underflow
This reverts commit 03986fc.
Compute total_gas_spent as limit - remaining - reservoir in build_result_gas to correctly exclude unused state gas. Use saturating_sub in Gas::spent/total_gas_spent to prevent underflow. Add std feature to ee-tests revm dependency.
rakita
force-pushed
the
rakita/state-gas-min
branch
2 times, most recently
from
9fc771d to
e41d52f
Compare
rakita
commented
Mar 25, 2026
| gas.limit() | ||
| .saturating_sub(gas.remaining()) | ||
| .saturating_sub(gas.reservoir()), | ||
| ) |
Member
Author
There was a problem hiding this comment.
This is confusing, and I am not sure if it is correct.
stategas tests is passing but on hive ResultGas::block_regular_gas_used that does total_gas_spent - state_gas wound underflow (Still checking it)
Would love additional eyes on it. Gas is constructed in handler.rs in fn last_frame_result lines 440-480
Replace manual ResultGas construction with the shared build_result_gas function for consistency with the main execution path.
rakita
force-pushed
the
rakita/state-gas-min
branch
from
e41d52f to
280fbbc
Compare
rakita
force-pushed
the
rakita/state-gas-min
branch
from
280fbbc to
7efb2be
Compare
3 tasks
fgimenez
reviewed
Mar 31, 2026
| /// Refunded gas. This is used only at the end of execution. | ||
| refunded: i64, | ||
| /// Tracker for gas during execution. | ||
| tracker: GasTracker, |
Collaborator
There was a problem hiding this comment.
iiuc Gas::limit and GasTracker::gas_limit store the same value, Gas never reads tracker.limit(), it always uses its own self.limit correct?
if so maybe we could drop gas_limit from GasTracker or just inline the tracker fields directly into Gas, it's the only consumer, wdyt?
| /// | ||
| /// | Getter | Source | Description | | ||
| /// |-------------------|------------------------------------|-------------------------------------------| | ||
| /// | [`limit()`] | `Gas::limit()` | Transaction gas limit | |
Collaborator
There was a problem hiding this comment.
this doc table is stale, limit(), intrinsic_gas(), and remaining() are removed from ResultGas, spent() deprecated
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removal of tests as github can't handle big PRs