feat(frames): two-dimensional gas limits per EIP-8141 (devnet7) - #12942
Conversation
Split each frame's single gas_limit into limits = [execution, state] end to end, and the frame receipt's gas_used into [execution, state], per EIP-8141 (ethereum/EIPs#12062), which adopts the EIP-8037 two-dimensional gas model. Core: - TxFrame carries ExecutionGasLimit and StateGasLimit; the combined GasLimit is their sum. TxFrameReceipt carries ExecutionGasUsed and StateGasUsed. - Per-frame independent gas pools: a state charge exceeding limits.state halts the frame and is never paid from execution gas. - State-gas attribution and cross-frame refills are journaled at call, frame and atomic-batch boundaries; a refill reduces the owning frame's receipt. - Static validity: per-dimension overflow, expiry frame limits.state == 0, value_transfer_cost (EIP-2780 TX_VALUE_COST) folded into the intrinsic, and the EIP-7825 cap enforced on intrinsic + sum(limits.execution). - The Bogota devnet fork now derives from Amsterdam so EIP-8037 is active under frame transactions. Wire and settlement: - RLP: frame limits = [execution, state]; receipt gas_used = [execution, state]. - Settlement binds the calldata floor to the execution dimension only; state gas is charged on top. Block inclusion, mempool admission and block production check the execution and state dimensions separately. Introspection: - FRAMEPARAM exposes limits.state and per-dimension gas_used. TXPARAM state_gas_left is assigned 0x11 to avoid the EIP-8250 legacy-nonce 0x0C collision (a spec update will follow separately). Payment approval that must create an absent sender charges NEW_ACCOUNT from the approving frame's state budget, atomically before the nonce increment; a frame that cannot afford it halts with no approval effects.
core-repository-dispatch-app
Bot
added
BREAKING
eip
evm
new feature
optimism
rpc
labels
|
Claude finished @AnkushinDaniil's task in 12m 48s —— View job Review — EIP-8141 two-dimensional gas limits (devnet7)
Careful, well-documented port. I traced the state-gas journal end-to-end (SSTORE charge → cross-frame refill → call/frame/batch/POST_TX rollback → settlement) and the design holds up: the checkpoint is captured at every boundary that restores world state, Findings below; 9 inline comments posted. Medium
Low
Notes / questions (no action implied)
I did not build or run the suites (CI covers that); findings are from reading |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| if (!frame.Value.IsZero && frame.Target is not null && frame.Target != transaction.SenderAddress) | ||
| { | ||
| valueTransferCost += GasCostOf.TxValueCostEip2780; |
There was a problem hiding this comment.
Medium — TX_VALUE_COST is not gated on spec.IsEip2780Enabled, and the self-transfer predicate uses the wrong debit side.
Two separate issues in this block:
-
Missing spec gate.
EthereumGasPolicy.Eip2780ExtraGasstarts withif (!spec.IsEip2780Enabled) return 0;. Here the 6000 is added unconditionally. It happens to be harmless today only because this PR also re-parentsBogotaontoAmsterdam; a chainspec that schedulesbogotaTimeover an Osaka-based network (exactly the layout the pre-changeBogotadoc described) would silently overcharge intrinsic gas by 6000 per value-bearing frame — a consensus divergence, not a config wart. -
The self-transfer discount is keyed on the wrong account. EIP-2780 skips
TX_VALUE_COSTbecause a self-transfer coalesces into the sender leaf write already priced intoTX_BASE_COST. In the frame model the debit side isENTRY_POINTfor every frame whose mode is notSENDER— not the tx sender. So:- a
DEFAULTframe withTarget == null(resolves to the sender) andvalue > 0moves valueENTRY_POINT → sender: two distinct leaves, no charge; - a
DEFAULTframe withTarget == SenderAddressandvalue > 0: same, no charge.
Both look like undercharges. The predicate probably wants to compare against the frame's resolved caller (
frame.Mode == ModeSender ? sender : ENTRY_POINT) rather thantransaction.SenderAddress. - a
Please confirm against the EIP-8141 text and add the spec gate either way.
| } | ||
|
|
||
| frameContext.Payer = resolvedTarget; | ||
| if (spec.UseHotAndColdStorage) accessTracker.WarmUp(resolvedTarget); |
There was a problem hiding this comment.
Medium — new consensus-visible warm/cold effect with no spec reference.
accessTracker.WarmUp(resolvedTarget) on payment approval is new in this PR and changes the gas cost of every subsequent access to the payer in the same transaction. That is a consensus rule, and per .agents/rules/coding-style.md ("Non-obvious consensus rules or algorithms must reference the EIP number or Yellow Paper section") it needs an EIP-8141 citation in a comment — or removal if the spec does not warm the payer. Note the surrounding code is meticulous about this elsewhere (e.g. the create_evm_from_frame and resolve_delegated_code_address comments in ExecuteFrame), so the omission stands out.
Also worth noting: this warms into the shared tracker rather than the frame tracker, so it survives a later frame revert. That is probably intended (the payment itself survives), but it deserves a word.
| long correction = frameContext.StateGasCorrectionFor(f); | ||
| if (correction > 0) | ||
| { | ||
| TxFrameReceipt corrected = frameReceipts[f]; | ||
| ulong reducedState = corrected.StateGasUsed > (ulong)correction ? corrected.StateGasUsed - (ulong)correction : 0; | ||
| frameReceipts[f] = new TxFrameReceipt(corrected.Status, corrected.ExecutionGasUsed, reducedState, corrected.Logs); | ||
| } | ||
| } | ||
|
|
||
| ulong grossGasBeforeCorrection = intrinsicGas + totalFrameGasUsed; | ||
| ulong stateGasCorrectionApplied = (ulong)Math.Max(0, stateGasCorrection); | ||
| ulong grossGas = grossGasBeforeCorrection > stateGasCorrectionApplied ? grossGasBeforeCorrection - stateGasCorrectionApplied : 0; | ||
| ulong gasAfterRefund = grossGas - RefundHelper.CalculateClaimableRefund(grossGas, (ulong)refundCounter, spec); |
There was a problem hiding this comment.
Medium — the per-frame correction is clamped at zero but the transaction-level one is not.
reducedState saturates at 0 when correction > corrected.StateGasUsed, while stateGasCorrectionApplied (line 503) subtracts the full TotalStateGasCorrection from grossGas, and blockStateGas (line 507) subtracts the full un-clamped stateGasCorrection. If the two ever diverge, the receipts and the amount actually billed disagree, silently — and blockStateGas would be understated, i.e. the block's state dimension undercounts.
I could not construct a reachable case (ownership is cleared on refill and the journal is restored at every rollback boundary, so correction[f] <= _frameStateGasUsed[f] appears to hold), which is exactly why the clamp is worrying: it is defending against a state the rest of the design says is impossible, and it does so inconsistently. Either
- drop the clamp and let it be an invariant (with a
Debug.Assert), or - derive the transaction-level total from the clamped per-frame values so the two can never disagree:
long stateGasCorrection = 0;
for (int f = 0; f < frameReceipts.Length; f++)
{
long correction = Math.Min(frameContext.StateGasCorrectionFor(f), (long)frameReceipts[f].StateGasUsed);
if (correction <= 0) continue;
stateGasCorrection += correction;
TxFrameReceipt corrected = frameReceipts[f];
frameReceipts[f] = new TxFrameReceipt(corrected.Status, corrected.ExecutionGasUsed, corrected.StateGasUsed - (ulong)correction, corrected.Logs);
}The second form also makes grossGas/blockStateGas provably non-negative without the two > guards below.
|
|
||
| AddingTxEventArgs CanAddTransaction(Block block, Transaction currentTx, | ||
| IReadOnlySet<Transaction> transactionsInBlock, IReadOnlyStateProvider stateProvider); | ||
| IReadOnlySet<Transaction> transactionsInBlock, IReadOnlyStateProvider stateProvider, ulong cumulativeStateGas = 0); |
There was a problem hiding this comment.
Medium — an optional parameter on a public interface method fails open.
ulong cumulativeStateGas = 0 means any caller or implementor that has not been updated (out-of-tree plugins implement IBlockProductionTransactionPicker; the PR is already labelled BREAKING) still compiles and silently gets stateGasRemaining == block.Header.GasLimit on every call — i.e. the EIP-8037 state-dimension bound is disabled for the whole block, and production can emit a block that BlockAccessListManager then rejects. A missing argument should be a compile error here, not a silent 0.
Since there is exactly one call site (BlockProductionTransactionsExecutor), making the parameter required costs nothing in-tree and turns the failure mode from "wrong blocks" into "won't build". Same for the BlockProductionTransactionPicker.CanAddTransaction override and the Optimism override.
| 0x0E when TEip8250.IsActive => stack.PushBytes<TTracingInst>(ctx.NonceKeysHash.BytesAsSpan), | ||
| 0x10 when TEip8250.IsActive => stack.PushUInt256<TTracingInst>(ctx.NonceKeys is { } keys ? keys[0] : UInt256.Zero), | ||
| 0x0F when TEip8272.IsActive => stack.PushUInt256<TTracingInst>((UInt256)ctx.RecentRootReferences.Length), | ||
| 0x11 => stack.PushUInt256<TTracingInst>((UInt256)(ulong)Math.Max(0, TGasPolicy.GetStateReservoir(in gas))), |
There was a problem hiding this comment.
Low — 0x11 is the only TXPARAM case with no activation guard.
Every other conditional param in this switch is written 0x0E when TEip8250.IsActive => … and falls through to BadInstruction when its EIP is off. 0x11 (state_gas_left) is unconditional, so on a policy without a state dimension it pushes a hard 0 instead of rejecting. Since the state dimension only exists under EIP-8037, 0x11 when TEip8037.IsActive (or whatever flag is threaded here) would match the surrounding convention and keep an inactive param from looking like a real answer of zero.
| /// <summary>Constructs a frame whose entire budget is execution gas, with <c>limits.state == 0</c>.</summary> | ||
| public TxFrame(byte mode, byte flags, Address? target, ulong gasLimit, UInt256 value, ReadOnlyMemory<byte> data) | ||
| : this(mode, flags, target, gasLimit, 0, value, data) | ||
| { | ||
| } | ||
|
|
||
| public byte Mode { get; } = mode; | ||
| public byte Flags { get; } = flags; | ||
|
|
||
| /// <summary>Null resolves to the transaction sender during execution.</summary> | ||
| public Address? Target { get; } = target; | ||
|
|
||
| public ulong GasLimit { get; } = gasLimit; | ||
| /// <summary>EIP-8141 <c>limits.execution</c>: the frame's execution-gas budget.</summary> | ||
| public ulong ExecutionGasLimit { get; } = executionGasLimit; | ||
|
|
||
| /// <summary>EIP-8141 <c>limits.state</c>: the frame's state-gas budget (EIP-8037).</summary> | ||
| public ulong StateGasLimit { get; } = stateGasLimit; | ||
|
|
||
| /// <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 — the back-compat ctor and GasLimit are now test-only, and both are footguns.
After this PR, production code constructs TxFrame only via the 7-arg ctor (TxFrameDecoder, CapFrameGas) and reads GasLimit nowhere; both survive purely for the existing test corpus. Two problems:
- The 6-arg ctor makes
new TxFrame(mode, flags, target, 400_000, value, data)mean "400k execution, zero state". That reads as "400k total" at the call site, and the diff shows the trap in action —FrameTxBlockGasTestshad to change several such calls toexecutionGasLimit: 200_000, stateGasLimit: 200_000to keep the tests meaningful, while others were left as-is. Any test still using the 6-arg form is now silently exercising alimits.state == 0frame. GasLimit => ExecutionGasLimit + StateGasLimitis an uncheckedulongadd whose XML doc asserts it "never wraps for a frame reaching execution" — true for validated frames, but the property is public on a type that exists in decoded-but-unvalidated form.
AGENTS.md asks that changes remove code made unused and avoid growing public surface. Suggest deleting both and updating the tests to the explicit two-argument form — that also makes each test's state budget visible, which is the whole point of the PR.
| bool approvesPayment = (scope & TxFrame.ApprovePayment) != 0; | ||
| bool applyPayment = false; | ||
| bool usesAccountNonce = false; | ||
| long newAccountCost = 0; | ||
|
|
||
| if (approvesPayment) | ||
| { | ||
| frameContext.SenderApproved = true; | ||
| UInt256[]? keys = frameContext.NonceKeys; | ||
| usesAccountNonce = keys is null || !KeyedNonceManager.UsesKeyedDomain(keys); | ||
| if (usesAccountNonce && WorldState.GetNonce(frameContext.Sender) >= Eip8250Constants.MaxNonceSeq) | ||
| { | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
Low — the nonce-exhaustion check moved in front of the guards it used to sit behind, and turned from a no-op into a frame failure.
Old order was: bail out if Payer is not null || !SenderApproved, then bail out if the balance is short, then check nonce >= MaxNonceSeq and return (frame still succeeds, SenderApproved already set). New order checks the nonce first and return falses, which the callers turn into EvmExceptionType.OutOfGas for the whole frame — and SenderApproved is never set because that assignment now sits below.
So a second payment approval (Payer is not null, no nonce would have been consumed) or an under-funded target now hard-fails the frame when the sender's account nonce is exhausted. The deleted comment said the opposite: "an approval that cannot consume its nonce performs no approval effects at all."
Eip8250Constants.MaxNonceSeq == ulong.MaxValue, so this is unreachable in practice — hence Low — but it is a consensus-visible reordering that the PR description does not mention. Either restore the guard order or update the comment to state the new rule and cite the spec line it follows.
(The usesAccountNonce extraction to KeyedNonceManager.UsesKeyedDomain is exactly equivalent to the old inline predicate — nice cleanup.)
| if (_maxVerifyStateGas != 0) | ||
| { | ||
| ulong verifyStateGas = FrameTxValidation.ValidationWorkStateGas(tx); | ||
| if (verifyStateGas > _maxVerifyStateGas) | ||
| { | ||
| Metrics.PendingTransactionsFrameTxVerifyGasTooHigh++; | ||
| if (logger.IsTrace) logger.Trace($"Skipped adding transaction {tx.ToString(" ")}, validation prefix budgets {verifyStateGas} state gas (max {_maxVerifyStateGas})."); | ||
| return AcceptTxResult.FrameTxVerifyGasTooHigh; | ||
| } |
There was a problem hiding this comment.
Low — the state bound is indistinguishable from the execution bound in metrics and RPC.
The new branch reuses Metrics.PendingTransactionsFrameTxVerifyGasTooHigh and AcceptTxResult.FrameTxVerifyGasTooHigh. Since MAX_VERIFY_GAS and MAX_VERIFY_STATE_GAS are independently configurable knobs, an operator seeing that counter climb cannot tell which one to raise, and the AcceptTxResult a submitter gets back says "verify gas" for a state-gas rejection. A sibling metric plus a distinct AcceptTxResult (or at least .WithMessage(...)) would make the two knobs debuggable.
| if (frameSucceeded && frameContext.ApprovalScopeSignal != 0) | ||
| { | ||
| long remainingStateGas = (long)frame.StateGasLimit - frameStateGas; | ||
| if (!TryApplyApproval(frameContext, resolvedTarget, spec, in accessTracker, remainingStateGas, out long approvalStateGas)) | ||
| { | ||
| frameSucceeded = false; | ||
| substate = new TransactionSubstate(EvmExceptionType.OutOfGas, tracer.IsTracingInstructions); | ||
| frameGasUsed = frame.ExecutionGasLimit; | ||
| frameStateGas = 0; | ||
| } | ||
| else | ||
| { | ||
| frameGasUsed += (ulong)approvalStateGas; | ||
| frameStateGas += approvalStateGas; | ||
| } | ||
| } |
There was a problem hiding this comment.
Medium — the same ~15-line approval-settlement block is now written three times, with different failure handling each time.
Identical-looking copies at lines 298–313 (frame loop), 674–687 (SimulateVerifyPrefix) and 951–964 (ExecuteFrame). They are not actually identical:
- the
ExecuteFramecopy falls intoWorldState.Restore(snapshot)/frameTracker.Restore()on failure; - the frame-loop copy does not restore anything (safe today only because it is reachable solely via
ExecuteDefaultVerifyCode, which writes no state — an invariant nothing in the code states); - the
SimulateVerifyPrefixcopy also does not restore, and additionally recomputesverifyGasUsed += frameGasUsed - (ulong)frameStateGas.
.agents/rules/coding-style.md asks for 5+ line repeated blocks to be extracted, and this is consensus code where a future fix applied to one copy and not the others is a chain split. Suggest folding the "settle approval, or fail the frame with gasUsed = ExecutionGasLimit" shape into one helper returning the adjusted (substate, gasUsed, stateGasUsed) and calling it from all three, with the state restore made explicit at each site.
Relatedly: the frame-loop copy is dead for every path except default verify code, because TryApplyApproval clears ApprovalScopeSignal on entry and ExecuteFrame already ran it. A comment saying so would save the next reader the trace I just did.
There was a problem hiding this comment.
These three sites aren't equivalent. The outer loop, the verify prefix, and ExecuteFrame differ in what rolls back and whether a failure voids the whole transaction. They already share the overflow clamp. Merging the bodies would couple three consensus control flows, and a wrong merge is a chain split. I'm keeping them separate.
| int gasUsedEnd = decoderContext.ReadSequenceLength() + decoderContext.Position; | ||
| ulong executionGasUsed = decoderContext.DecodeULong(); | ||
| ulong stateGasUsed = decoderContext.DecodeULong(); | ||
| decoderContext.Check(gasUsedEnd); |
There was a problem hiding this comment.
Low (operational) — unversioned change to the on-disk receipt format.
gas_used goes from a scalar to a 2-element list in ReceiptStorageDecoder, CompactReceiptStorageDecoder and ReceiptMessageDecoder. The storage decoders read the local receipts DB, so any node that already has frame-tx receipts written by the current devnet branch will throw RlpException when serving them — there is no version byte or fallback.
For a devnet branch that is probably fine and a resync is the answer, but it should be called out in the PR description (and, if devnet7 nodes are already running against eip8141-frame-txs-devnet7, in the release notes) so operators don't discover it via eth_getTransactionReceipt 500s.
There was a problem hiding this comment.
Fixed in #12957: the two storage decoders now fall back to the pre-2D scalar gas_used when reading an old-format frame-tx receipt, so a node with devnet7 receipts already on disk decodes them instead of throwing. Split into its own PR to keep the storage-compat change separate from the consensus change.
EVM Opcode Benchmark DiffAggregated runs: base=3, pr=3 Regressions (1)
New / Removed (1)
|
EIP-8141 now owns its spec-canonical introspection indices; the sibling extensions move off the collisions so a pure-8141 fork matches the spec. - TXPARAM 0x0C = STATE_GAS_LEFT (8141), was 0x11 - opcode 0xb5 = SIGDATACOPY (8141) as a distinct opcode; SIGPARAM (0xb4) is read-only again (no copy overload) - EIP-8250 legacy nonce TXPARAM 0x0C -> 0x11 (gated on 8250) - EIP-8272 RECENTROOTREFLOAD opcode 0xb5 -> 0xb6 - wire: fees is a nested [max_priority_fee, max_fee, max_fee_per_blob_gas] list; intrinsic FRAME_TX_INTRINSIC_COST 15000 -> 12000 Verified against execution-spec fixtures (EELS #3396) at fork Bogota: 201/201 pass. Unit suites green (Evm/Core/TxPool/Blockchain frame tests). The matching EIP-8250 / EIP-8272 index moves ship as separate spec PRs.
flcl42
left a comment
flcl42
left a comment
There was a problem hiding this comment.
Found 4 issues (1 critical and 3 medium) in src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessorBase.FrameTx.cs, src/Nethermind/Nethermind.Consensus/Processing/BlockProcessor.BlockProductionTransactionsExecutor.cs, and src/Nethermind/Nethermind.Consensus/Processing/BlockProcessor.IBlockProductionTransactionPicker.cs.
| ulong grossGas = grossGasBeforeCorrection > stateGasCorrectionApplied ? grossGasBeforeCorrection - stateGasCorrectionApplied : 0; | ||
| ulong gasAfterRefund = grossGas - RefundHelper.CalculateClaimableRefund(grossGas, (ulong)refundCounter, spec); | ||
| ulong blockStateGas = (ulong)Math.Max(0, totalFrameStateGasUsed - stateGasCorrection); | ||
| ulong blockRegularGas = Eip8037BlockGasInclusionCheck.CalculateBlockExecutionGas(gasAfterRefund, blockStateGas, floorGas); |
There was a problem hiding this comment.
[CRITICAL] Frame refunds reduce the pre-refund block execution dimension
When a successful frame earns an EIP-3529 refund, for example by clearing an existing nonzero slot, this passes gasAfterRefund to a helper whose parameter and regular-transaction usage are explicitly pre-refund. The result is stored in tx.BlockGasUsed and accumulated into header gas, so the refund reduces the block execution dimension and leaves extra capacity. Nethermind can consequently derive a different gasUsed or block-validity result from clients applying EIP-7778 pre-refund accounting.
There was a problem hiding this comment.
Agreed on the substance. The root cause is upstream of the client: EIP-8141 activates on a fork with EIP-7778 but carries EIP-8037's pre-7778 settlement, so block_execution_gas_used is accounted from gas_used_after_refund. That re-opens the refund-based block-limit circumvention EIP-7778 closes.
The fix belongs in the spec, not one client: ethereum/EIPs#12226 moves the block execution dimension to gas_used_before_refund (mirroring EIP-8037's EIP-7778 integration) and keeps the payer charge post-refund — https://github.com/AnkushinDaniil/EIPs/blob/79ec62a8c4104f13c0d03d27eed21c0ba5af0851/EIPS/eip-8141.md#L621-L628
This PR tracks the currently released fixtures (post-refund) to stay consensus-conformant. I'll switch the client to pre-refund once #12226 merges and fixtures regenerate.
| currentTx, | ||
| transactionsInBlock, | ||
| stateProvider, | ||
| receiptsTracer.BlockStateGasUsed); |
There was a problem hiding this comment.
[MEDIUM] State-heavy prefixes hide unused execution capacity from the producer
The new call passes only cumulative state gas. BlockProductionTransactionPicker still computes execution remaining from block.GasUsed, while BlockReceiptsTracer sets that value to the maximum of cumulative execution and state gas. With a 1,000,000 limit and a 100,000/600,000 execution/state prefix, a frame reserving 500,000 execution and no state fits both dimensions but is skipped because only 400,000 is seen as execution headroom. State-heavy payloads are therefore underfilled.
|
|
||
| AddingTxEventArgs CanAddTransaction(Block block, Transaction currentTx, | ||
| IReadOnlySet<Transaction> transactionsInBlock, IReadOnlyStateProvider stateProvider); | ||
| IReadOnlySet<Transaction> transactionsInBlock, IReadOnlyStateProvider stateProvider, ulong cumulativeStateGas = 0); |
There was a problem hiding this comment.
[MEDIUM] The optional picker parameter breaks existing plugin implementations
Optional parameters are a caller-side compile-time feature; adding this parameter changes the CLR interface signature. A plugin implementing the former four-argument member, or overriding the former virtual picker method, no longer supplies the invoked member and can fail to load or compile, while only in-tree implementations are migrated here. A four-argument compatibility bridge would preserve existing picker extensions.
| stateGasUsed = Math.Max(0, TGasPolicy.GetStateGasUsed(in state.Gas)) + entryState; | ||
| if (substate.IsError || substate.ShouldRevert) | ||
| { | ||
| TGasPolicy.ResetForHalt(ref state.Gas, (long)frame.StateGasLimit, 0); |
There was a problem hiding this comment.
[MEDIUM] Frame halt reset casts limits.state to long without the clamp FromFrameLimits applies
EthereumGasPolicy.FromFrameLimits seeds the frame reservoir with stateGasLimit > long.MaxValue ? long.MaxValue : (long)stateGasLimit, but the halt/revert reset here passes (long)frame.StateGasLimit unclamped. For limits.state >= 2^63 the cast is negative, so StateReservoir becomes about -2^63 and the following GetPreRefundGas(in state.Gas, combinedLimit) computes combinedLimit - remainingGas - stateReservoir, which lands outside ulong.
Such a transaction is well formed at the consensus layer: FrameTxValidation.IsWellFormed only rejects a wrapping limits.execution + limits.state sum, and FrameTxFieldsTxValidator bounds the execution reservation while discarding the state reservation (out _). Locally, TxValidator.IsWellFormed returned success on Bogota.Instance for a two-frame transaction whose second frame declares limits.execution = 200_000 and limits.state = 2^63, and executing it with a reverting target reached this line and produced Gas invariant violated: pre-refund gas (18446744073709554622) must fit in ulong for gas limit (9223372036854975808), remaining gas (196994), and state reservoir (-9223372036854775808).
In assertion-enabled builds the Debug.Assert in IGasPolicy.GetPreRefundGas aborts the process on that input, which is reachable from a peer's block or from an eth_call/eth_estimateGas payload since FrameForRpc carries stateGasLimit straight through. In release builds the documented conservative fallback returns txGasLimit, so a frame that merely reverted is billed its whole combined limit (about 9.2e18 gas), and that figure flows into totalFrameGasUsed, tx.BlockGasUsed, the payer settlement and the beneficiary credit before the block-level gas check rejects the block. Mirroring the FromFrameLimits clamp at this call site, or rejecting an out-of-range limits.state during static validation, would keep the reservoir non-negative and the invariant intact.
| return error!; | ||
| } | ||
|
|
||
| if (!FrameTxValidation.TryCalculateBlockGasReservations(transaction, releaseSpec, out ulong executionReservation, out _) |
There was a problem hiding this comment.
An overflow in TryCalculateBlockGasReservations and a genuinely over-cap budget both return FrameExecutionGasExceedsCap with no numbers, so a rejected transaction cannot be diagnosed without re-deriving the budget by hand — and FrameGasOverflow already exists for the first case. IntrinsicGasTxValidator fifty lines up shows the pattern, including the IsEip8037Enabled gate this check omits before applying the cap.
if (!FrameTxValidation.TryCalculateBlockGasReservations(transaction, releaseSpec, out ulong executionReservation, out _))
{
return FrameTxValidation.FrameGasOverflow;
}
if (releaseSpec.IsEip8037Enabled && executionReservation > Eip7825Constants.DefaultTxGasLimitCap)
{
return FrameTxValidation.FrameExecutionGasExceedsCap(executionReservation, Eip7825Constants.DefaultTxGasLimitCap);
}| /// </summary> | ||
| public void RecordStateChargeOwner(in StorageCell slot, int frame) | ||
| { | ||
| int previousOwner = _stateChargeOwner.TryGetValue(slot, out int existing) ? existing : NoOwner; |
There was a problem hiding this comment.
Both of these do two dictionary probes where one would do, on the SSTORE path — TryGetValue plus an indexer set here, TryGetValue plus Remove in TryResolveStateChargeOwner. GetValueRefOrAddDefault and Dictionary.Remove(key, out value) collapse each to a single lookup. Note NoOwner is -1 while the added default is 0, so the out bool exists flag has to drive previousOwner, not a sentinel comparison.
ref int owner = ref CollectionsMarshal.GetValueRefOrAddDefault(_stateChargeOwner, slot, out bool existed);
int previousOwner = existed ? owner : NoOwner;
owner = frame; // write through the ref before touching the journal
_stateGasJournal.Add(new StateGasJournalEntry(StateGasJournalKind.OwnerSet, slot, previousOwner, 0));| bool ssetOutOfGas = !TGasPolicy.ConsumeStorageWrite<TEip8037, OnFlag>(ref gas, spec); | ||
| if (ssetOutOfGas) goto OutOfGas; | ||
| FrameTxContext? chargeFrameCtx = vm.TxExecutionContext.FrameTxContext; | ||
| if (chargeFrameCtx is not null) |
There was a problem hiding this comment.
The refill side at line 573 is gated on TEip8037.IsActive but this record side is not, so on a spec with 8141 and no 8037 the ownership map and journal would only ever grow. The preceding ConsumeStorageWrite<TEip8037, ...> already conditions the charge itself on that flag, so the asymmetry looks unintended. Gating also lets the JIT drop the load and call entirely from the non-8037 specialisation.
if (TEip8037.IsActive && vm.TxExecutionContext.FrameTxContext is { } chargeFrameCtx)
{
chargeFrameCtx.RecordStateChargeOwner(in storageCell, chargeFrameCtx.CurrentFrameIndex);
}| { | ||
| for (int k = _stateGasJournal.Count - 1; k >= checkpoint; k--) | ||
| { | ||
| StateGasJournalEntry entry = _stateGasJournal[k]; |
There was a problem hiding this comment.
This is called at every rollback boundary, and the common case is that nothing was journaled since the checkpoint — which still costs a call into a loop, a switch and a RemoveRange. The indexer also copies the whole StateGasJournalEntry, StorageCell included. An early exit plus CollectionsMarshal.AsSpan and a ref read removes both; take the span after the early exit and let it go before the RemoveRange.
int count = _stateGasJournal.Count;
if (count == checkpoint) return;
Span<StateGasJournalEntry> entries = CollectionsMarshal.AsSpan(_stateGasJournal);
for (int k = count - 1; k >= checkpoint; k--)
{
ref StateGasJournalEntry entry = ref entries[k];
switch (entry.Kind) { }
}
_stateGasJournal.RemoveRange(checkpoint, count - checkpoint);
| cumulativeState, | ||
| tx.GasLimit); | ||
| Eip8037BlockGasInclusionCheck.Outcome outcome; | ||
| if (tx.SupportsFrames && FrameTxValidation.TryCalculateBlockGasReservations(tx, spec, out ulong executionReservation, out ulong stateReservation)) |
There was a problem hiding this comment.
nit: use ? ... : ... syntax
There was a problem hiding this comment.
The two branches call different Validate overloads. The frame path passes exact per-dimension reservations; the other passes a single worst-case budget. A ternary would still repeat the shared arguments across a multi-line call, so if/else reads clearer here.
| ulong stateGasRemaining = block.Header.GasLimit.SaturatingSub(cumulativeStateGas); | ||
| ulong executionReservation; | ||
| ulong stateReservation; | ||
| if (currentTx.SupportsFrames) |
| /// <summary> | ||
| /// An upper bound on the public-mempool validation work of <paramref name="transaction"/>: the gas limits | ||
| /// of its validation prefix plus the cost of verifying its signatures, saturating at <see cref="ulong.MaxValue"/>. | ||
| /// An upper bound on the public-mempool validation work of <paramref name="transaction"/>: the execution-gas |
| return total; | ||
| } | ||
|
|
||
| /// <summary> |
| /// <summary>EIP-8141 <c>limits.state</c>: the frame's state-gas budget (EIP-8037).</summary> | ||
| public ulong StateGasLimit { get; } = stateGasLimit; | ||
|
|
||
| /// <summary>The combined gas the frame reserves against the payer, <c>limits.execution + limits.state</c>.</summary> |
| 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 > 0x0B) return EvmExceptionType.BadInstruction; |
There was a problem hiding this comment.
maybe better if not hardcoded? (I know it was like this before though)
There was a problem hiding this comment.
Pre-existing on the base branch; leaving verbatim to keep the diff minimal.
| if (!stack.PopUInt256(out UInt256 signatureIndex, out UInt256 param)) return EvmExceptionType.StackUnderflow; | ||
| if (signatureIndex >= (UInt256)ctx.Signatures.Length) return EvmExceptionType.BadInstruction; | ||
| if (param > 0x04) return EvmExceptionType.BadInstruction; | ||
| if (param > 0x03) return EvmExceptionType.BadInstruction; |
There was a problem hiding this comment.
maybe better if not hardcoded? (I know it was like this before though)
There was a problem hiding this comment.
Pre-existing on the base branch; leaving verbatim to keep the diff minimal.
| totalFrameGasUsed += frameGasUsed; | ||
|
|
||
| bool frameSucceeded = !substate.ShouldRevert && !substate.IsError; | ||
| if (frameSucceeded && frameContext.ApprovalScopeSignal != 0) |
There was a problem hiding this comment.
this function is massive, can we refactor with some helpers?
There was a problem hiding this comment.
It's large because it's the frame-tx settlement path. Splitting it moves consensus-visible gas accounting, so the risk outweighs the readability gain. Leaving it as one function.
| } | ||
|
|
||
| /// <summary> | ||
| /// Journal position covering the outstanding SSTORE-charge ownership map and the per-frame |
There was a problem hiding this comment.
a bunch of comments in this file too long or not needed
|
|
||
| /// <summary> | ||
| /// Devnet fork enabling EIP-8141 frame transactions on top of Osaka. Matches the frame | ||
| /// Devnet fork enabling EIP-8141 frame transactions on top of Amsterdam. Matches the frame |
There was a problem hiding this comment.
don't need to talk about the devnet in a comment
…m/cold cite; track post-refund block gas pending EIP-8141 spec fix
Reviewer findings addressed in this commit: claude-bot (Low): - Separate MAX_VERIFY_STATE_GAS rejection (AcceptTxResult, error message, metric) so a state-bound mempool reject is distinguishable from an execution-bound one. - TxParam 0x11 activation guard already present (gated on TEip8250). wurdum: - TxValidator: split overflow (FrameGasOverflow) from over-cap (FrameExecutionGasExceedsCap now carries the reservation and cap), add the IsEip8037Enabled gate mirroring IntrinsicGasTxValidator. - EvmInstructions.Storage: gate RecordStateChargeOwner on TEip8037.IsActive, matching the refill side; otherwise the ownership map/journal grow on a spec with 8141 but not 8037. - FrameTxContext: single dictionary probe on the SSTORE path (GetValueRefOrAddDefault / Remove(key, out)); early-exit + AsSpan + ref read in RestoreStateGasJournal. - Remove the now-unused TotalStateGasCorrection; accumulate the per-frame correction total in the existing settlement loop instead of a second pass. Marchhill (nits): trim member docs to summary + one-line cite per repo style. Deliberately kept (explained in review threads): the three-decoder gas_used wire framing and the receipt-format decoder are consensus-wire shapes, not mechanically deduped under pressure; the three block-gas tests are not identical (success+nonzero vs two halt+zero paths); the hardcoded operands at EvmInstructions.FrameTx 221/279 are pre-existing.
A frame-tx receipt persisted under eip8141-frame-txs-devnet7 stored gas_used as a scalar. After PR #12942 changed it to an [execution, state] list, a node reading its own receipts DB would throw RlpException (eth_getTransactionReceipt /eth_getLogs 500s) with no fallback. The two storage decoders (ReceiptStorageDecoder, CompactReceiptStorageDecoder) now detect the shape at gas_used via a shared FrameReceiptGasRlp.DecodeGasUsed: a sequence decodes as [execution, state]; a scalar decodes as execution with state = 0. The scalar path preserves the per-frame total but is not wire-faithful when re-encoded for GetReceipts; that trade-off is documented on the helper's remarks. The network ReceiptMessageDecoder is intentionally left strict list-only. Regression fixtures are golden blobs produced by the base-branch encoder (c265e33): a standalone pre-2D receipt, and an array [legacy, pre-2D frame, legacy] exercised through both full decode and DecodeStructRef iteration.
A frame-tx receipt persisted under eip8141-frame-txs-devnet7 stored gas_used as a scalar. After PR #12942 changed it to an [execution, state] list, a node reading its own receipts DB would throw RlpException (eth_getTransactionReceipt /eth_getLogs 500s) with no fallback. The two storage decoders (ReceiptStorageDecoder, CompactReceiptStorageDecoder) now detect the shape at gas_used via a shared FrameReceiptGasRlp.DecodeGasUsed: a sequence decodes as [execution, state]; a scalar decodes as execution with state = 0. The scalar path preserves the per-frame total but is not wire-faithful when re-encoded for GetReceipts; that trade-off is documented on the helper's remarks. The network ReceiptMessageDecoder is intentionally left strict list-only. Regression fixtures are golden blobs produced by the base-branch encoder (c265e33): a standalone pre-2D receipt, and an array [legacy, pre-2D frame, legacy] exercised through both full decode and DecodeStructRef iteration.
A frame-tx receipt persisted under eip8141-frame-txs-devnet7 stored gas_used as a scalar. After PR #12942 changed it to an [execution, state] list, a node reading its own receipts DB would throw RlpException (eth_getTransactionReceipt /eth_getLogs 500s) with no fallback. The two storage decoders (ReceiptStorageDecoder, CompactReceiptStorageDecoder) now detect the shape at gas_used via a shared FrameReceiptGasRlp.DecodeGasUsed: a sequence decodes as [execution, state]; a scalar decodes as execution with state = 0. The scalar path preserves the per-frame total but is not wire-faithful when re-encoded for GetReceipts; that trade-off is documented on the helper's remarks. The network ReceiptMessageDecoder is intentionally left strict list-only. Regression fixtures are golden blobs produced by the base-branch encoder (c265e33): a standalone pre-2D receipt, and an array [legacy, pre-2D frame, legacy] exercised through both full decode and DecodeStructRef iteration.
…rocessor-check The base's two-dimensional gas model (#12942) reshaped the processor's frame preamble, so the hoisted structural check is re-applied on top of it: - FrameTxValidation.IsWellFormed now runs ahead of the validation-prefix simulation as well, so both entry points share the precondition. - The base's in-processor mode bound, POST_TX fork gate and atomic-batch approval-scope loop are dropped: IsWellFormed enforces all three with the same error constants, and duplicating them would only diverge the detail. - FrameTransactionForRpc keeps the base's nonce_keys mapping alongside the null-entry rejection and the frame gas cap. - The test helper CallAndRestore(Transaction) the base grew replaces the one this branch added; no caller needed the tracer overload.
Re-applies the deploy-frame carve-outs on the base's two-dimensional gas model (#12942), which rewrote the prefix simulation loop: - The frame now reports its state gas, and approval is applied by the base's TryApplyApproval ahead of the revert check, so the trailing ApplyApproval call this branch carried is gone. The codeless-sender refusal stays where it was. - The deploy frame is the one prefix frame that writes state, so its test helper budgets limits.state; with limits.state == 0 every deployment halted. - Both EIP8141-GAP markers announcing the carve-outs as unimplemented are removed with the change that implements them.
The shared max_cost helper survives; the arithmetic under it is the base's. TryCalculateMaxCost is now purely additive over the two-dimensional TryCalculateGasBudget (#12942), so the admission bound and the simulated APPROVE gate both price through the base's formula. - CalculateSimulatedMaxCost is gone: its sole caller already reads the shared helper, so the base's copy was dead. - The base extracted TryGetPayerReservation to share the release amount with the DEBUG bookkeeping check. Rather than bypass it, it now reads the exposure admission recorded, so the check and the release agree on the stored value.
…ng-cap - AcceptTxResult id 29 is taken by the base's FrameTxVerifyStateGasTooHigh (#12942), so the paymaster-cap result moves to 30. Ids compare by value alone, so the collision made the two results equal and the cap's own tests would have accepted the wrong rejection. - OnRemovedTx keeps the base's expiry-count assertion and decrements the paymaster count after it, ahead of the exposure release.
4 participants
Changes
Ports the EIP-8141 two-dimensional per-frame gas model onto
eip8141-frame-txs-devnet7. Each frame declareslimits = [execution, state]and each frame receipt reports
gas_used = [execution, state], perEIP-8141 adopting the
EIP-8037 split.
TxFrame/TxFrameReceipt: split gas fields, back-compatible single-argctor for existing callers.
limits.statehalts the frame and never spends execution gas.journaled and restored at call / frame / atomic-batch rollback boundaries.
split into
ExecutionGasUsed/StateGasUsed.charged on top. Block inclusion, mempool admission and block production check
each dimension separately.
limits.state == 0,value_transfer_cost(EIP-2780TX_VALUE_COST) folded into the intrinsic,EIP-7825 cap on
intrinsic + sum(limits.execution).NEW_ACCOUNTfrom theapproving frame's state budget, atomically before the nonce increment.
Bogotaderives fromAmsterdamso EIP-8037 is active under frame txs.FRAMEPARAMexposeslimits.stateand per-dimensiongas_used;TXPARAMstate_gas_leftuses0x11to avoid the EIP-82500x0Clegacy-noncecollision (spec update to follow).
Deferred to separate coordinated PRs (they change bytes / opcode slots and must
be switched cross-client in lock-step, out of scope here): the outer
feesnested-list RLP migration and
SIGDATACOPY (0xb5).Types of changes
Testing
Notes on testing
Frame suites green: Evm.Test
~FrameTx205,~Eip814125; Core.Test~FrameTx109; TxPool.Test~FrameTx80; Blockchain.Test~Frame34;JsonRpc.Test
~Frame18. RED/GREEN control confirmed for the value-transfercharge and the payment-approval NEW_ACCOUNT charge. execution-specs (#3396
frame-tx fixtures) and a cross-client Kurtosis devnet with ethrex are being run
against this branch.
Documentation