feat(frames): two-dimensional gas limits per EIP-8141 - #12847
feat(frames): two-dimensional gas limits per EIP-8141#12847AnkushinDaniil wants to merge 2 commits into
Conversation
Split each frame's single gas_limit into limits = [execution, state] to match the merged ethereum/EIPs#12062 spec update. Encoding, static validation, intrinsic budget, FRAMEPARAM introspection and the JSON-RPC view now carry both dimensions; runtime remains combined-pool (execution + state) pending the explicit dual-pool follow-up.
|
Claude finished @AnkushinDaniil's task in 4m 37s —— View job Review: two-dimensional gas limits per EIP-8141Read the full diff against
The shape of the change is good: the overflow guard FindingsHigh —
// TransactionProcessorBase.FrameTx.cs:594
TGasPolicy.FromULong(frame.GasLimit), // == execution + stateA prefix frame declaring Either keep this on the combined Medium — inner
Medium — test coverage for the changed behaviours The added tests cover the round-trip, the expiry rejection, and
Low — The remark holds for frames reaching execution, but Low — new consensus rule without an EIP citation ·
Low — RPC surface
Low — naming and duplication
Note (not a finding): VerdictNot mergeable as-is: the I did not build or run tests — this environment blocks |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| for (int i = 0; i < counted; i++) | ||
| { | ||
| total = Saturating(total, frames[i].GasLimit); | ||
| total = Saturating(total, frames[i].ExecutionGasLimit); |
There was a problem hiding this comment.
High — MAX_VERIFY_GAS becomes bypassable while the runtime still uses a combined pool.
This now prices the validation prefix at limits.execution only, but the runtime seeds the frame's VM pool with the combined budget (TransactionProcessorBase.FrameTx.cs:594 → TGasPolicy.FromULong(frame.GasLimit) where GasLimit == execution + state), and the state-gas reservoir is unchanged. So a prefix frame declaring limits = [1, 100_000_000] scores ~1 gas here, sails past FrameTxVerifyGasFilter (FrameTxMaxVerifyGas = 100_000 by default), and still gets a 100M-gas pool of arbitrary work when the pool simulates the prefix. The declared state gas does not constrain what the frame spends it on until the dual-pool seeding lands.
The affordability check is not a mitigation: MAX_VERIFY_GAS exists precisely for the case where the attacker never pays (tx never included), so a funded sender can spam unbounded free validation work.
Since both the dual-pool seeding and MAX_VERIFY_STATE_GAS are explicitly out of scope here, the two halves of the split need to stay consistent. Options:
- keep this on the combined
frames[i].GasLimituntil the runtime actually seedsgas_left = limits.execution, or - land the
MAX_VERIFY_STATE_GASbound in this PR alongside the execution-only change.
| if (!rlpBehaviors.HasFlag(RlpBehaviors.AllowExtraBytes)) | ||
| { | ||
| decoderContext.Check(limitsCheck); | ||
| } |
There was a problem hiding this comment.
Medium — the inner limits check should not be gated on AllowExtraBytes.
AllowExtraBytes means "tolerate trailing bytes after the payload", which is why gating the outer check on line 44 is harmless — it is the last read. Here the check sits mid-stream: skipping it does not tolerate trailing data, it leaves Position inside/after the limits list and makes the subsequent value and data reads land at the wrong offsets, silently producing a different frame (and therefore a different tx hash) from limits = [exec, state, junk].
Today this is latent — RlpReader.DecodeArray calls decoder.Decode(ref this) with RlpBehaviors.None, so frames never see the flag — but any future call site (Rlp.Decode<TxFrame>(…, AllowExtraBytes), a DB path via KeyValueStoreRlpExtensions, which ORs the flag in) would hit it. Make it unconditional:
| if (!rlpBehaviors.HasFlag(RlpBehaviors.AllowExtraBytes)) | |
| { | |
| decoderContext.Check(limitsCheck); | |
| } | |
| decoderContext.Check(limitsCheck); |
| /// <summary>The combined gas the frame reserves against the payer, <c>limits.execution + limits.state</c>.</summary> | ||
| /// <remarks>Static validation rejects a transaction whose frame reservations overflow, so this sum never | ||
| /// wraps for a frame reaching execution.</remarks> | ||
| public ulong GasLimit => ExecutionGasLimit + StateGasLimit; |
There was a problem hiding this comment.
Low — GasLimit wraps silently, and it is read before validation.
The remark is true for frames that reach execution, but FrameTxDecoder.DecodeInternal reads frame.GasLimit while summing transaction.GasLimit (TxDecoders/FrameTxDecoder.cs:148) — i.e. at decode time, before IsWellFormed runs. With execution = ulong.MaxValue, state = 10 the property returns 9, so the saturating sum there is defeated and the tx is briefly visible with an absurdly small GasLimit. Validation does reject it afterwards, so there is no path to execution, but failing closed is cheap here:
| public ulong GasLimit => ExecutionGasLimit + StateGasLimit; | |
| public ulong GasLimit => ExecutionGasLimit > ulong.MaxValue - StateGasLimit ? ulong.MaxValue : ExecutionGasLimit + StateGasLimit; |
(If you prefer to keep the plain sum, at least widen the remark to say the property may wrap for a transaction that has not yet passed static validation.)
| if (frame.Mode == TxFrame.ModeVerify && frame.Target == Eip8141Constants.ExpiryVerifierAddress) | ||
| { | ||
| if (frame.Flags != 0 || !frame.Value.IsZero || frame.Data.Length != Eip8141Constants.ExpiryDataLength) | ||
| if (frame.Flags != 0 || !frame.Value.IsZero || frame.StateGasLimit != 0 || frame.Data.Length != Eip8141Constants.ExpiryDataLength) |
There was a problem hiding this comment.
Low — new consensus rule without a spec citation.
frame.StateGasLimit != 0 is a new validity condition on the expiry verifier frame. Per .agents/rules/coding-style.md ("Non-obvious consensus rules or algorithms must reference the EIP number or Yellow Paper section"), please anchor it — the surrounding conditions predate the split and carry their justification in the enclosing docs, but this one is new. Also worth double-checking against the merged spec text whether the expiry frame is required to have limits.state == 0 specifically, or whether the spec constrains limits.execution too (in which case a fixed execution budget should be asserted here as well).
EVM Opcode Benchmark DiffAggregated runs: base=3, pr=3 Regressions (1)
Improvements (1)
|
Seed each frame's execution pool from limits.execution and its state-gas reservoir from limits.state, so state work is drawn independently of the execution budget. The per-frame charge folds execution gas used plus the reservoir-funded state gas, keeping the payer charge and the block state/ regular split unchanged. Add the MAX_VERIFY_STATE_GAS public-mempool bound (sum of the validation prefix limits.state) alongside MAX_VERIFY_GAS.
AnkushinDaniil
changed the title
flcl42
left a comment
flcl42
left a comment
There was a problem hiding this comment.
Found 7 issues (6 critical and 1 high) in src/Nethermind/Nethermind.Evm/GasPolicy/EthereumGasPolicy.cs, src/Nethermind/Nethermind.Core/TxFrame.cs, src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessorBase.FrameTx.cs, and src/Nethermind/Nethermind.Evm/Instructions/EvmInstructions.FrameTx.cs.
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static EthereumGasPolicy FromFrameLimits(ulong executionGasLimit, ulong stateGasLimit) => | ||
| new() { Value = executionGasLimit, StateReservoir = (long)stateGasLimit }; |
There was a problem hiding this comment.
[CRITICAL] Frame state charges can spill into the execution budget
FromFrameLimits seeds the existing EIP-8037 reservoir policy, whose ConsumeStateGas deducts any reservoir shortfall from Value. EIP-8141's explicit frame pools are independent and exclude this spill behavior. A state charge above StateGasLimit therefore succeeds whenever ExecutionGasLimit covers the difference, whereas a conforming client exceptionally halts the frame; committed state and receipts then diverge.
| /// <summary>The combined gas the frame reserves against the payer, <c>limits.execution + limits.state</c>.</summary> | ||
| /// <remarks>Static validation rejects a transaction whose frame reservations overflow, so this sum never | ||
| /// wraps for a frame reaching execution.</remarks> | ||
| public ulong GasLimit => ExecutionGasLimit + StateGasLimit; |
There was a problem hiding this comment.
[CRITICAL] The combined GasLimit alias collapses independent reservations
Transaction.GasLimit is populated by summing this alias, while the unchanged EIP-8037 inclusion check feeds that scalar into both dimensions and the block picker compares the combined budget with one remaining-gas value. A transaction whose execution and state reservations each fit independently can therefore be rejected solely because their sum exceeds the block limit. The same alias lets the default-code keyed-nonce surcharge treat limits.state as execution headroom, affecting both block validity and frame execution.
| @@ -428,9 +428,10 @@ private TransactionResult ExecuteFrameTx(Transaction tx, ITxTracer tracer, Execu | |||
| ulong grossGas = intrinsicGas + totalFrameGasUsed; | |||
| ulong spentGas = Math.Max(grossGas - RefundHelper.CalculateClaimableRefund(grossGas, (ulong)refundCounter, spec), floorGas); | |||
There was a problem hiding this comment.
[CRITICAL] The calldata floor absorbs frame state gas
For a floor-bound frame transaction, this applies Math.Max to the combined execution-and-state charge. EIP-8141 instead floors the execution component and then adds state gas. With 20k execution gas, 30k state gas, and a 100k floor, this path charges and reports 100k instead of 130k, changing the payer refund and cumulative receipt gas.
| gasUsed = frame.GasLimit - remainingGas; | ||
| ulong combinedLimit = frame.ExecutionGasLimit + frame.StateGasLimit; | ||
| gasUsed = substate.IsError | ||
| ? combinedLimit - (ulong)Math.Max(0, TGasPolicy.GetStateReservoir(in state.Gas)) |
There was a problem hiding this comment.
[CRITICAL] Exceptional halts retain rolled-back state charges
The top-level VM does not call ResetForHalt, so this error branch subtracts the remaining reservoir before the frame snapshot is restored. If a frame consumes state gas and later executes INVALID, gasUsed becomes the full execution limit plus the consumed state gas. EIP-8141 restores the state pool to frame entry and records zero state usage on an exceptional halt, burning only the execution pool, so payer balance and cumulative gas diverge.
|
|
||
| ulong remainingGas = substate.IsError ? 0 : TGasPolicy.GetRemainingGas(in state.Gas); | ||
| gasUsed = frame.GasLimit - remainingGas; | ||
| ulong combinedLimit = frame.ExecutionGasLimit + frame.StateGasLimit; |
There was a problem hiding this comment.
[CRITICAL] Frame receipts still encode gas usage as one scalar
The two budgets are collapsed into gasUsed here and passed to the existing scalar TxFrameReceipt.GasUsed; the receipt encoders consequently emit one RLP integer. The amended receipt format is gas_used = [execution, state] for every frame receipt, including zero-state frames. Any block containing a frame transaction therefore gets a different receipt payload and receipts root from a conforming client.
| if (!stack.PopUInt256(out UInt256 frameIndex, out UInt256 param)) return EvmExceptionType.StackUnderflow; | ||
| if (frameIndex >= (UInt256)ctx.Frames.Length) return EvmExceptionType.BadInstruction; | ||
| if (param > 0x08) return EvmExceptionType.BadInstruction; | ||
| if (param > 0x09) return EvmExceptionType.BadInstruction; |
There was a problem hiding this comment.
[CRITICAL] FRAMEPARAM rejects the required gas-usage selectors
The updated EIP-8141 table defines 0x0A and 0x0B for a completed frame's execution and state usage. This bound returns BadInstruction for both. A later frame that introspects either usage therefore exceptionally halts instead of receiving the recorded value, which can change subsequent approvals and state.
| ulong combinedLimit = frame.ExecutionGasLimit + frame.StateGasLimit; | ||
| gasUsed = substate.IsError | ||
| ? combinedLimit - (ulong)Math.Max(0, TGasPolicy.GetStateReservoir(in state.Gas)) | ||
| : TGasPolicy.GetPreRefundGas(in state.Gas, combinedLimit); |
There was a problem hiding this comment.
[HIGH] Reverted frames keep their rolled-back state gas in the payer charge
For a frame that consumes state gas and then exits via REVERT (ShouldRevert, not an exceptional halt), gasUsed is computed as GetPreRefundGas(state.Gas, combinedLimit) = combinedLimit - remainingGas - stateReservoir, which equals consumed execution gas plus consumed state gas. The consumed state gas therefore stays in totalFrameGasUsed and is charged to the payer even though the frame's writes were rolled back. Two behaviors in the same codebase indicate that rolled-back state owes no state gas: the regular EIP-8037 path restores a reverted child's state gas (RestoreChildStateGas: "On explicit REVERT, restore the child's remaining state reservoir plus its reverted state gas usage"), and the atomic-batch unroll in this very method resets totalFrameStateGasUsed to its batch-start value because "the unrolled frames' writes are gone with the snapshot, so their state charges are not owed either." The block dimension already excludes the reverted frame (totalFrameStateGasUsed only accumulates on success, and Execute_PayloadFrameReverts_OwesNoStateGas asserts BlockStateGas == 0), but the payer dimension still bills it, so a frame such as SSTORE followed by REVERT leaves the payer charged for state growth that never committed. A client that restores the state pool to frame entry on revert would compute a smaller payer charge and cumulative gas, so payer balances and receipts diverge.
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static EthereumGasPolicy FromFrameLimits(ulong executionGasLimit, ulong stateGasLimit) => | ||
| new() { Value = executionGasLimit, StateReservoir = (long)stateGasLimit }; |
There was a problem hiding this comment.
Nit (long)stateGasLimit is an unchecked narrowing cast: a stateGasLimit > long.MaxValue silently becomes a negative StateReservoir, and ConsumeStateGas/CalculateStateGasSpill treat a non-positive reservoir as "spill everything into execution". The sibling default IGasPolicy.FromFrameLimits handles the same overflow domain by saturating at ulong.MaxValue, so the two implementations disagree on out-of-range input.
Not reachable today (a frame reaching execution has passed affordability and block-gas-limit checks, which bound the limits far below long.MaxValue), hence a nit — but the inconsistency is a latent trap if a future caller invokes this off the execution path. Consider clamping the reservoir the same way the default does. (Distinct from the already-raised spill-semantics concern on this method.)
| [ConfigItem(DefaultValue = "100000", Description = "EIP-8141 `MAX_VERIFY_GAS`: the max gas a frame transaction's validation prefix and signature verification may cost for the transaction to be accepted into the public mempool. `0` to lift the limit. Raise it only on a test network.")] | ||
| ulong FrameTxMaxVerifyGas { get; set; } | ||
|
|
||
| [ConfigItem(DefaultValue = "500000", Description = "EIP-8141 `MAX_VERIFY_STATE_GAS`: the max state gas a frame transaction's validation prefix may budget across its `limits.state` for the transaction to be accepted into the public mempool. `0` to lift the limit. Raise it only on a test network.")] |
There was a problem hiding this comment.
Nit Worth confirming the default 500000 matches the merged EIP-8141 MAX_VERIFY_STATE_GAS value — the description asserts equality with the spec constant, and per coding-style.md config keys should document defaults accurately. MAX_VERIFY_GAS above defaults to 100000; a quick check that the state bound's magnitude relative to it is intentional (5×) would avoid pinning a placeholder.
|
Superseded by #12850, which carries the same two-dimensional gas change against feature/frames-devnet-0. Continuing the review there. |
3 participants
What
Implements EIP-8141 two-dimensional frame gas
limits = [execution, state](ethereum/EIPs#12062) end to end, in one change: the field split plus the runtime semantics that give it meaning.Field split
TxFramecarriesExecutionGasLimitandStateGasLimit;GasLimitstays as their sum for the combined budget (max-cost reservation, overflow checks).limitsas a nested[execution, state]list; the decoder reads both.execution + stateper frame with overflow guards; an expiry-verifier frame must keeplimits.state == 0.standard_gas_limitaccounts for both dimensions.FRAMEPARAM:0x01returnslimits.execution, new0x09returnslimits.state.executionGasLimit/stateGasLimit.Runtime two-pool semantics
gas_left = limits.executionand its state-gas reservoir seeded fromlimits.state, so state work is drawn from the state dimension independently of the execution budget. Before this, a frame rented an empty reservoir and every state charge spilled into execution.GetPreRefundGas(gas, execution + state)on the success/revert path andexecution + state − unspent_reservoiron an exceptional halt, mirroring the standard EIP-8037 halt formula (gas_leftis burned; only the unspent reservoir escapes the charge).blockStateGas,CalculateBlockExecutionGas) are unchanged. The change is behavioral only where a frame's state budget now lets it do state work its execution budget could not have absorbed.Public-mempool bound
MAX_VERIFY_STATE_GAS(default 500,000, configFrameTxMaxVerifyStateGas,0lifts it): the sum of the validation prefix'slimits.statemust not exceed it, enforced at ingress alongsideMAX_VERIFY_GAS. State gas does not measure node validation work, so it stays out of theMAX_VERIFY_GASbudget.Testing
SSetStatecharge succeeds whenlimits.statecovers it and bills the same state dimension; the negative control withstate = 0spills into execution and halts out of gas, committing nothing.MAX_VERIFY_STATE_GASingress bound (at, over, and behind a recognized prefix).Behavior on the settlement and halt paths is consensus-affecting, so this is queued for the reproducible stand and a cross-client Kurtosis run before it leaves the family branch.
Open question
The reference spec lists
FRAMEPARAM0x0A/0x0B(gas_used.execution/gas_used.state) and a two-field frame receipt. Those are gas-used introspection rather than the limits split, and are not implemented here — should they land in this change or a follow-up? Happy to fold them in if you'd prefer one card.