fix(l2): reserve L1 gas upfront in prepare_execution to prevent DA fee griefing#6264
fix(l2): reserve L1 gas upfront in prepare_execution to prevent DA fee griefing#6264avilagaston9 wants to merge 9 commits intomainfrom
Conversation
avilagaston9
changed the title
🤖 Kimi Code ReviewReview FeedbackCritical Issues
Minor Issues
Suggested Fix for Critical Issuesfn validate_gas_limit_covers_l1_fee(
vm: &VM<'_>,
l1_fee_config: &Option<L1FeeConfig>,
) -> Result<(), TxValidationError> {
let l1_gas = calculate_l1_fee_gas(vm, l1_fee_config)
.map_err(|_| TxValidationError::InsufficientGasLimit)?; // Fix item 2
let gas_remaining: u64 = vm.current_call_frame.gas_remaining.try_into()
.map_err(|_| TxValidationError::InternalError)?; // Fix item 3
let intrinsic_gas = vm
.current_call_frame
.gas_limit
.checked_sub(gas_remaining) // Fix item 1
.ok_or(TxValidationError::InternalError)?;
let min_gas_limit = intrinsic_gas
.checked_add(l1_gas)
.ok_or(TxValidationError::IntrinsicGasTooHigh)?; // More specific error
if vm.current_call_frame.gas_limit < min_gas_limit {
return Err(TxValidationError::InsufficientGasLimit);
}
Ok(())
}The overall approach is sound and addresses an important economic vulnerability in L2 transaction processing. Automated review by Kimi (Moonshot AI) · custom prompt |
🤖 Codex Code ReviewFindings:
If you want line-precise comments, I can annotate with exact line numbers after you share the current file with Automated review by OpenAI Codex · custom prompt |
Lines of code reportTotal lines added: Detailed view |
🤖 Claude Code ReviewHere is my review of PR 6264: Review:
|
Benchmark Results ComparisonBenchmark Results: MstoreBench
Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
avilagaston9
changed the title
to prevent a griefing vector where transactions with gas_limit = intrinsic_gas pass validation but revert at finalize time with l1_gas recalculated to 0, leaving the sequencer to absorb DA costs with no reimbursement. The new validate_gas_limit_covers_l1_fee function checks that gas_limit >= intrinsic_gas + l1_gas during prepare_execution, rejecting underfunded transactions before they enter the blob. Privileged transactions are unaffected (they return early before the check). The finalize revert path is kept as a safety net for legitimate cases where execution gas consumption pushes actual_gas + l1_gas over gas_limit. Closes #6053
…g it, so execution physically cannot consume the L1 fee budget. Previously the validate-only approach still allowed a contract to burn enough execution gas to squeeze the L1 fee at finalize time. Now increase_consumed_gas(l1_gas) is called during prepare_execution, making the revert-for-l1-gas path in finalize_non_privileged_execution dead code (removed). The refund cap (EIP-3529) is applied to execution gas only since l1_gas is not refundable.
…non_privileged_execution
…consumed_gas(0) is a no-op
…sic_plus_l1 to a single iteration with an assertion, and add test_no_l1_fee_config_21000_is_enough to confirm gas_limit=21000 suffices when L1 fee is disabled.
avilagaston9
force-pushed
the
fix/l2-validate-gas-limit-covers-l1-fee
branch
from
01ddfd3 to
9fd98ce
Compare
gas_used - l1_gas separation works correctly when a contract consumes execution gas. The L1 fee vault must still receive the full l1_gas payment.
…tract call reverts due to out-of-gas, the L1 fee vault still receives the full l1_gas payment thanks to the upfront reservation in prepare_execution.
🤖 Kimi Code ReviewReview SummaryThis PR introduces a critical security fix for L2 gas reservation by reserving L1 data availability gas upfront during transaction preparation. The implementation correctly prevents a griefing vector where users could set Key Changes Review1. Security Fix: L1 Gas Reservation ✅
The new 2. Simplified Gas Accounting ✅
The removal of the complex post-execution gas limit check logic is appropriate since L1 gas is now reserved upfront. The new approach:
3. Test Coverage ✅
Comprehensive test suite covers:
Minor Issues1. Potential Overflow in
|
Greptile SummaryThis PR fixes a DA fee griefing vulnerability by reserving L1 gas upfront during Key changes:
The implementation calls Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| crates/vm/levm/src/hooks/l2_hook.rs | Implements upfront L1 gas reservation in prepare_execution to prevent DA fee griefing, removes dead revert path, and correctly applies refund cap to execution gas only |
| test/tests/levm/l2_gas_reservation_tests.rs | Comprehensive test suite covering upfront rejection, exact gas limits, L1 vault payment, contract execution, and OOG revert scenarios |
Sequence Diagram
sequenceDiagram
participant User
participant VM
participant Hook as L2Hook
participant CallFrame
participant Vault as L1FeeVault
User->>VM: Submit tx (gas_limit)
VM->>Hook: prepare_execution()
Hook->>Hook: calculate_l1_fee_gas(tx)
Note over Hook: l1_gas = (tx_size * l1_fee_per_byte) / gas_price
Hook->>CallFrame: increase_consumed_gas(l1_gas)
alt gas_limit < intrinsic_gas + l1_gas
CallFrame-->>Hook: OutOfGas error
Hook-->>VM: IntrinsicGasTooLow
VM-->>User: Transaction rejected
else sufficient gas
CallFrame->>CallFrame: gas_remaining -= l1_gas
Note over CallFrame: L1 gas reserved, cannot be consumed by execution
Hook-->>VM: Success
VM->>VM: Execute transaction
VM->>Hook: finalize_execution(ctx_result)
Hook->>Hook: calculate_l1_fee_gas(tx)
Note over Hook: Same value as prepare
Hook->>Hook: execution_gas = ctx_result.gas_used - l1_gas
Hook->>Hook: Apply refund cap to execution_gas only
Hook->>Vault: pay_to_l1_fee_vault(l1_gas * gas_price)
Vault->>Vault: Receives full L1 fee
Hook-->>VM: Success
end
Last reviewed commit: 5396341
🤖 Claude Code ReviewNow I have a complete picture of the code. Let me write up the review. PR Review:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses an L2 griefing vector where a transaction could be included in DA/blob data but end up paying zero to the L1 fee vault at finalize time by exhausting the execution gas budget. The fix reserves the computed l1_gas upfront during prepare_execution, so execution cannot consume the L1 DA fee budget.
Changes:
- Reserve
l1_gasduringprepare_executionfor non-privileged L2 transactions. - Update finalize accounting to treat reserved
l1_gasas non-refundable and simplify/remove the previous “cap/revert” path. - Add a dedicated test suite covering L2 gas reservation behavior and L1 fee vault payment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
crates/vm/levm/src/hooks/l2_hook.rs |
Reserves L1 DA gas upfront and updates finalize gas/refund accounting to separate execution gas from reserved l1_gas. |
test/tests/levm/l2_gas_reservation_tests.rs |
Adds tests for upfront rejection, exact-budget success, vault payment correctness, and revert/OOG scenarios. |
test/tests/levm/mod.rs |
Registers the new l2_gas_reservation_tests module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let l1_gas = calculate_l1_fee_gas(vm, l1_fee_config)?; | ||
| vm.current_call_frame | ||
| .increase_consumed_gas(l1_gas) | ||
| .map_err(|_| TxValidationError::IntrinsicGasTooLow)?; | ||
| Ok(()) |
There was a problem hiding this comment.
reserve_l1_gas currently only ensures gas_limit >= intrinsic_gas + l1_gas (because it runs after add_intrinsic_gas). On Prague+ forks, compute_actual_gas_used can charge max(execution_gas, min_gas_used) (EIP-7623 floor), so a tx can pass reservation but later be charged min_gas_used + l1_gas, exceeding gas_limit and breaking refund accounting. Reservation should also enforce gas_limit >= min_gas_used + l1_gas (or an equivalent check).
| let actual_gas_used = execution_gas | ||
| .checked_add(l1_gas) | ||
| .ok_or(InternalError::Overflow)?; | ||
|
|
There was a problem hiding this comment.
The removed actual_gas_used > gas_limit cap/revert branch relies on the assumption that reservation makes actual_gas_used <= gas_limit always true. With Prague+ EIP-7623, compute_actual_gas_used can raise execution_gas to min_gas_used, so actual_gas_used = execution_gas + l1_gas can still exceed gas_limit unless reservation also accounts for the min-gas floor. Add a guard here or tighten reserve_l1_gas accordingly.
| // Ensure total gas used does not exceed gas limit after Prague+ (EIP-7623). | |
| if actual_gas_used > vm.env.gas_limit { | |
| return Err(VMError::Internal(InternalError::Custom( | |
| "actual gas used exceeds gas limit after min-gas adjustment".to_owned(), | |
| ))); | |
| } |
| /// A transaction with gas_limit exactly covering intrinsic_gas + l1_gas should succeed. | ||
| /// The execution gas budget is 0, but for a simple EOA transfer that's fine. | ||
| #[test] | ||
| fn test_gas_limit_exactly_covers_intrinsic_plus_l1() { | ||
| // Build the tx once, compute l1_gas, then rebuild with the exact gas_limit. | ||
| // One iteration is enough since a small gas_limit change barely affects RLP size. | ||
| let tx = make_tx(22_000); | ||
| let l1_gas = compute_l1_gas(&tx); | ||
| assert!(l1_gas > 0, "l1_gas should be positive for this test"); |
There was a problem hiding this comment.
Current tests don’t cover the Prague+ EIP-7623 floor case where min_gas_used can exceed intrinsic (e.g., non-empty calldata with many non-zero bytes). Add a test that sets calldata to trigger min_gas_used > intrinsic, then asserts: (1) gas_limit < min_gas_used + l1_gas is rejected upfront and (2) gas_limit = min_gas_used + l1_gas succeeds while still paying the full L1 fee vault amount.
…prevent an underflow in refund_sender when the EIP-7623 calldata gas floor exceeds intrinsic_gas. Without this check, a transaction with heavy non-zero calldata could pass both validate_min_gas_limit (gas_limit >= floor) and the l1_gas reservation (gas_limit >= intrinsic + l1_gas) individually, but at finalize max(execution_gas, floor) + l1_gas would exceed gas_limit.
|
|
||
| let gas_refunded: u64 = default_hook::compute_gas_refunded(vm, ctx_result)?; | ||
| // Refund cap based on execution gas only (EIP-3529) | ||
| let gas_refunded: u64 = vm |
There was a problem hiding this comment.
Good that this doesn't use compute_gas_refunded(vm, ctx_result) — that helper caps refunds at ctx_result.gas_used / 5, which now includes l1_gas since it was consumed from gas_remaining. Using execution_gas_pre_refund here correctly excludes l1_gas from the denominator.
Worth noting for future maintainability: a "simplification" to call compute_gas_refunded would silently inflate the refund cap by l1_gas / 5, letting users reclaim gas that should cover the L1 fee.
Motivation
A user can send a transaction with
gas_limit = intrinsic_gas(e.g., 21000). The tx passes validation, gets included in the blob, but at finalize timeactual_gas + l1_gas > gas_limittriggers a revert. The recalculatedl1_gas = gas_limit - actual_gas = 0, so the L1 fee vault receives nothing. The sequencer pays DA costs and gets nothing back — a griefing vector.Closes #6053
Description
Reserve
l1_gasfromgas_remainingduringprepare_executionby callingincrease_consumed_gas(l1_gas), so execution physically cannot consume the L1 fee budget. This replaces the previous validate-only approach (validate_gas_limit_covers_l1_fee) which rejected transactions wheregas_limit < intrinsic_gas + l1_gasbut didn't prevent a contract from burning enough execution gas to squeeze the L1 fee at finalize time.With reservation, the L1 fee vault always receives the full
l1_gaspayment:gas_limit < intrinsic_gas + l1_gas, the tx is rejected upfront withIntrinsicGasTooLowactual_gas_used <= gas_limitis guaranteed)l1_gasis not refundableDesign note:
calculate_l1_fee_gasis called in both prepare and finalizecalculate_l1_fee_gasis computed twice: once inreserve_l1_gas(prepare) and once infinalize_non_privileged_execution(finalize). Both calls usevm.txandvm.env.gas_pricewhich are immutable during execution, so they always return the same value. An alternative would be storing the reservedl1_gason the VM/environment to avoid recomputation, but we intentionally keep the current approach to avoid modifying the environment struct for now.How to Test
cargo test -p ethrex-test -- levm::l2_gas_reservation cargo clippy -p ethrex-levm -- -D warnings cargo fmt --all -- --checkChecklist
STORE_SCHEMA_VERSION(crates/storage/lib.rs) if the PR includes breaking changes to theStorerequiring a re-sync.