feat(frames): catch frames-devnet-0 up to the EIP-8141 reference (execution-specs #3396) - #12918
feat(frames): catch frames-devnet-0 up to the EIP-8141 reference (execution-specs #3396)#12918AnkushinDaniil wants to merge 25 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. (cherry picked from commit 40aa68a)
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. (cherry picked from commit 232c4f8)
Our devnet-0 base charges frame entry gas (cold account access, EIP-8037 new-account cost); the merged PR-12847 dual-pool runtime was written against the family branch, which does not. Charge the entry cost through TryConsumeStateAndExecutionGas on the seeded frame budget so cold access draws limits.execution and the new-account cost draws limits.state (spilling into execution), leaving the per-frame gasUsed/stateGasUsed read straight off the policy as PR-12847 does.
…ests fixture The fee-collector regression test mutates _spec, absent from the 8141-only fixture, so the project did not compile. Wrap the prototype in an OverridableReleaseSpec as the sibling FrameTxBlockGasTests fixture does.
An exceptionally halted payload frame (a fresh SSTORE followed by INVALID) consumed state gas from its reservoir and then rolled the write back, but the frame processor left ResetForHalt to the ordinary transaction processor and never called it, so the reverted state usage was billed to the payer and added to the block state dimension. Snap the frame's state gas back to its seed on the error path, matching the ordinary halt path, so gas_used is the execution budget alone and the frame owes zero state gas. Adds a regression test.
Within a frame transaction EthereumGasPolicy applied the EIP-8037 reservoir spill, so a state charge exceeding limits.state was paid from limits.execution and the frame could commit state its state budget could not afford. EIP-8141 makes the two pools independent: a charge exceeding its pool halts, and execution gas is never spent on state charges. Add an IndependentStatePool flag seeded by FromFrameLimits and propagated to child call frames; ConsumeStateGas halts instead of spilling when it is set. Envelope EIP-8037 behaviour is unchanged. Observation frames in the frame tests declare a state budget for their recording writes, and a regression test proves the no-spill halt.
…a floor max_gas and settlement folded the calldata floor over the whole transaction, so a frame tx whose floor exceeded its execution component under-reserved max_cost and was charged only the floor, silently dropping the state gas. Reserve max(standard_gas_limit, calldata_floor_gas + sum(limits.state)) and settle tx_execution_gas = max(gas_after_refund - tx_state_gas, calldata_floor_gas) with gas_used = tx_execution_gas + tx_state_gas, so the floor binds on the execution dimension alone; this also applies the storage refund to the block execution dimension. Adds a regression test.
EIP-8141 encodes each frame's gas_used as a two-element [execution, state] list mirroring the frame's limits, but the receipt carried a single scalar, so every frame-transaction receipt hashed to a different receipts root than a conforming client even when execution agreed. Split TxFrameReceipt into ExecutionGasUsed and StateGasUsed, encode gas_used as a nested sequence in the network, storage, and compact receipt decoders, and populate both dimensions from the frame's execution and state usage. A batch-unrolled frame keeps its execution gas but reports zero state gas, per unroll_atomic_batch.
…d it EIP-8141 attributes every SSTORE state charge to the frame that pays it and, when a later frame reverses that slot, refunds the state gas by reducing the paying frame's receipt rather than the reversing frame's. The receipt carried the charge on whichever frame happened to reverse the slot, so a cross-frame reversal produced a different receipts root than a conforming client. Track the outstanding SSTORE-charge owner per (address, slot) on the frame context and journal it, together with the per-frame gas_used.state corrections, at the same rollback boundaries as world state: the EVM call revert/halt seams, a frame revert/halt, and an atomic-batch unroll. On a reversal, reduce the owner frame's gas_used.state and, only when the owner is the current frame, credit its state_gas_left; a cross-frame reversal never credits the reversing frame. At settlement the accumulated correction drops both the gross and the state dimension by the same amount, so a refill lowers gas_used directly and outside the EIP-3529 refund cap. Adds regression tests: a cross-frame reversal reduces the creating frame's state gas and not the reversing frame's; a reversal in a frame that reverts, or in an inner call that reverts, leaves the creating frame's state gas intact.
…seed Three defense-in-depth fixes on the frame-transaction decode and gas-policy seams flagged in review, none reachable under a validated block: - saturate each frame's execution+state limit before summing them into the transaction-level GasLimit, so a per-frame limit that overflows ulong no longer wraps to a small value that pre-execution consumers under-count; - check the frame limits sub-list length unconditionally, so an over-long limits list is rejected rather than silently shifting the parse when a caller passes AllowExtraBytes; - saturate the state reservoir seed in FromFrameLimits so a state limit above long.MaxValue, reachable only through an unvalidated eth_call, cannot seed a negative reservoir.
A frame-entry state charge, account creation on a value transfer or sender creation by APPROVE, was metered against limits.state but allowed to spill the shortfall into limits.execution, so a frame declaring too little state gas was admitted and, on the codeless sender path, transferred value and created the account. EIP-8141 meters every state-dependent charge inside limits.state and fails the frame when it is exhausted, with no spill, since the pools are independent within a frame transaction. Seed the frame policy once and consume the entry charge through it, failing the frame on an underfunded state budget and removing the duplicated affordability rule the VM entry consume had drifted from. Adds a regression test; the transfer scenarios now declare the new-account state budget the spec requires.
Adopts EIP-8141 revision ethereum/EIPs#12187: signature-byte copy moves off SIGPARAM's param 0x04 into a dedicated SIGDATACOPY (0xb5) opcode so SIGPARAM's stack effect is static. SIGPARAM now accepts param 0x00-0x03 only, and 0x03 (len(signature)) is restricted to ARBITRARY entries. SIGDATACOPY copies an ARBITRARY entry's raw bytes with CALLDATACOPY semantics; a non-ARBITRARY scheme or out-of-bounds index halts.
Bogota is the frames-devnet-0 target fork. Base it on Amsterdam (not Osaka) and register the "Bogota" spec name so the frame-transaction fixtures resolve to it.
EIP-8141 encodes the transaction fees as a nested list rather than flat fields. Decode the nested structure so spec wire vectors round-trip.
Catch up to the EIP-8141 gas revision (ethereum/EIPs#12062): the frame transaction intrinsic is 12000, and each value-bearing frame targeting a foreign account adds a 6000 value-transfer cost to the mandatory gas.
FRAMEPARAM 0x0A/0x0B report a completed frame's execution and state gas, so record each frame's gas in the transaction context.
Add the state-gas-left transaction parameter (TXPARAM 0x0C) and the completed-frame execution/state-gas frame parameters (FRAMEPARAM 0x0A/0x0B), guarded to frames before the current one.
A frame transaction reserves execution and state gas separately: execution = max(intrinsic + sum of execution budgets, floor), state = sum of state budgets. Reserve by dimension instead of the legacy worst-case from the combined gas limit.
A frame transaction has no transaction-level status (ethereum/EIPs#12061): both status and log set come from the frame receipts. Derive Logs alongside the already-derived StatusCode so a failed-frame receipt keeps its logs and the receipts root is unchanged.
- dispatch a frame targeting a precompile without 7702 delegation - resolve 7702 delegation and charge delegate access at frame entry - on atomic-batch unroll, drop the batch's state gas from both the state dimension and the combined accumulator, so it does not leak into the execution dimension of the header - apply a payment approval before the frame receipt is built, creating the sender and charging NEW_ACCOUNT from the frame's state budget - derive the transaction status from the frames (MarkAsSuccess/Failed)
Update the frame-transaction tests to the EIP-8037/8141 revisions: intrinsic 12000, precompile-target dispatch cost, and explicit state-gas budgets for state-touching frames (SSTORE, dead-account transfer, deploy).
|
Claude finished @AnkushinDaniil's task in 12m 0s —— View job Review: EIP-8141 spec catch-up (frames-devnet-0)
Well-structured change overall — the two-dimensional plumbing is coherent, the state-gas ownership journal is correctly hooked into every rollback boundary I could find, and the reservation math is a sound upper bound on settlement. Findings below; details are in the inline comments. 1 High, 5 Medium, 6 Low. HighH1 — a post-hoc approval failure leaves the frame's state committed.
MediumM1 — a reverted frame now owes its entry The old line forced M2 — the one unclamped Lines 349 and 357 both clamp; line 354 does not. The clamp on 349 is itself an admission that a correction may exceed the state gas it reverses. If it ever does in aggregate, M3 — The 6000 value-transfer charge is skipped whenever the target resolves to the sender. That reasoning holds only for M4 — the EIP-7825 per-tx cap is silently dropped for frame txs in the inclusion check. The non-frame branch keeps M5 — the RPC receipt surface was not split alongside the tx surface.
Low
Verified as correct
Note on the PR description"no other-EIP surface introduced" doesn't quite hold:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (frameSucceeded && frameContext.ApprovalScopeSignal != 0) | ||
| { | ||
| long remainingStateBudget = (long)frame.StateGasLimit - frameStateGas; | ||
| if (!TryApplyApproval(frameContext, resolvedTarget, spec, in accessTracker, remainingStateBudget, out long approvalStateGas)) | ||
| { | ||
| frameSucceeded = false; | ||
| substate = new TransactionSubstate(EvmExceptionType.OutOfGas, tracer.IsTracingInstructions); | ||
| } | ||
| else | ||
| { | ||
| frameStateGas += approvalStateGas; | ||
| frameGasUsed += (ulong)approvalStateGas; | ||
| } | ||
| } |
There was a problem hiding this comment.
High — a post-hoc approval failure leaves the frame's state committed.
ExecuteFrame decides whether to roll back from the substate it computed (if (substate.ShouldRevert || substate.IsError) { WorldState.Restore(snapshot); frameTracker.Restore(); }, line 587). By the time TryApplyApproval returns false here, ExecuteFrame has already returned success, so nothing restores the snapshot or the access-tracker touches.
Result for a non-VERIFY, non-batch frame that writes state and then APPROVEs payment while the sender account does not exist and the frame's remaining limits.state is below NEW_ACCOUNT:
- the frame's storage writes / balance changes stay committed,
- its receipt says
StatusFailureand its logs are dropped (frameLogs = []), so the receipt/bloom no longer describes the state that was actually applied, frameGasUsedis the amount actually consumed rather than the fulllimits.executionan exceptional halt owes,- the warm/cold journal keeps the frame's touches.
The VERIFY case is masked because a failed VERIFY frame aborts the whole tx via WorldState.Restore(txSnapshot); batch frames are masked by the unroll. Everything else diverges from the reference on both state root and receipts.
Cheapest fix that keeps one rollback boundary: decide the approval inside ExecuteFrame, before its ShouldRevert || IsError restore — or take a snapshot around the frame here and restore it (plus frameTracker) when TryApplyApproval fails, and charge frame.ExecutionGasLimit.
| if (substate.IsError) | ||
| { | ||
| // EIP-8141 (ethereum/EIPs#12062): an exceptionally halted frame grows no state and owes zero state gas. | ||
| TGasPolicy.ResetForHalt(ref state.Gas, (long)frame.StateGasLimit, 0); | ||
| } | ||
|
|
||
| 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); | ||
| // Clamp: a reverted or errored frame carries no non-negativity guarantee on its state gas. | ||
| stateGasUsed = Math.Max(0, TGasPolicy.GetStateGasUsed(in state.Gas)); |
There was a problem hiding this comment.
Medium — a reverted frame now owes its entry NEW_ACCOUNT state gas.
The old code was stateGasUsed = substate.ShouldRevert || substate.IsError ? 0 : entryState + GetStateGasUsed(...). The reset above only covers IsError; for ShouldRevert the value now comes straight from the policy.
Trace a top-level revert: entryState is drawn through TryConsumeStateAndExecutionGas (line 487) before RentTopLevel, so InitialStateGasUsed == entryState. PrepareTopLevelSubstate → RefundRevertedTopLevelStateGas refunds only down to that floor (RefundStateGas(..., stateGasFloor: InitialStateGasUsed)), so GetStateGasUsed is entryState, not 0.
So a frame with value > 0 targeting a dead account that then REVERTs is billed NEW_ACCOUNT state gas into totalStateGas → blockStateGas, even though WorldState.Restore(snapshot) on line 589 undid the creation. That contradicts the "a reverted frame commits no state, so it grows none" invariant the halt path upholds, and it is inconsistent with the nested-call treatment in VirtualMachine.cs:305-310, which explicitly credits NEW_ACCOUNT back for a reverted *CALL.
Execute_PayloadFrameReverts_OwesNoStateGas does not catch it — its frame carries zero value, so entryState == 0. Worth either extending ResetForHalt to the revert case or adding the missing test case if the reference really does keep the charge.
| long stateGasCorrection = frameContext.TotalStateGasCorrection; | ||
| for (int f = 0; f < frameReceipts.Length; f++) | ||
| { | ||
| 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 grossGas = intrinsicGas + totalFrameGasUsed - (ulong)stateGasCorrection; | ||
| ulong gasAfterRefund = grossGas - RefundHelper.CalculateClaimableRefund(grossGas, (ulong)refundCounter, spec); | ||
| // EIP-8141 (ethereum/EIPs#12062): the calldata floor bounds the execution dimension alone, and state gas is added on top rather than absorbed. State gas is clamped since later-frame refills can drive the running total negative. | ||
| ulong blockStateGas = (ulong)Math.Max(0, totalStateGas - stateGasCorrection); | ||
| ulong executionAfterState = gasAfterRefund > blockStateGas ? gasAfterRefund - blockStateGas : 0; | ||
| ulong blockExecutionGas = Math.Max(executionAfterState, floorGas); | ||
| ulong spentGas = blockExecutionGas + blockStateGas; |
There was a problem hiding this comment.
Medium — the one unclamped subtraction of stateGasCorrection can wrap ulong.
Both neighbouring uses of the correction defend themselves:
- line 349 —
corrected.StateGasUsed > (ulong)correction ? ... : 0 - line 357 —
(ulong)Math.Max(0, totalStateGas - stateGasCorrection)
but line 354 does not. The clamp on line 349 is an explicit statement that a per-frame correction may exceed that frame's recorded state gas; if that ever happens across the whole set, intrinsicGas + totalFrameGasUsed - (ulong)stateGasCorrection wraps to ~2^64, and the fallout is not a wrong number in a log line:
gasAfterRefund≈ 2^64,executionAfterState≈ 2^64,spentGas≈ 2^64;chargedCost > maxCost, so the payer gets no refund of the pre-chargedMaxCost;WorldState.AddToBalanceAndCreateIfNotExists(header.GasBeneficiary!, premiumPerGas * (UInt256)spentGas, spec)credits the beneficiary against a fabricated gas figure — ether creation.
A long cast of stateGasCorrection that turns out negative (a journal restore outrunning its record) lands in the same place. Given the blast radius, this deserves the same Math.Max(0, ...) treatment as line 357, or a hard invariant check that fails the transaction.
| long stateGasCorrection = frameContext.TotalStateGasCorrection; | |
| for (int f = 0; f < frameReceipts.Length; f++) | |
| { | |
| 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 grossGas = intrinsicGas + totalFrameGasUsed - (ulong)stateGasCorrection; | |
| ulong gasAfterRefund = grossGas - RefundHelper.CalculateClaimableRefund(grossGas, (ulong)refundCounter, spec); | |
| // EIP-8141 (ethereum/EIPs#12062): the calldata floor bounds the execution dimension alone, and state gas is added on top rather than absorbed. State gas is clamped since later-frame refills can drive the running total negative. | |
| ulong blockStateGas = (ulong)Math.Max(0, totalStateGas - stateGasCorrection); | |
| ulong executionAfterState = gasAfterRefund > blockStateGas ? gasAfterRefund - blockStateGas : 0; | |
| ulong blockExecutionGas = Math.Max(executionAfterState, floorGas); | |
| ulong spentGas = blockExecutionGas + blockStateGas; | |
| long stateGasCorrection = frameContext.TotalStateGasCorrection; | |
| for (int f = 0; f < frameReceipts.Length; f++) | |
| { | |
| 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); | |
| } | |
| } | |
| // The correction can never exceed the state gas it reverses, but clamp rather than wrap: a | |
| // wrapped grossGas would skip the payer refund and over-credit the beneficiary. | |
| ulong grossCharge = intrinsicGas + totalFrameGasUsed; | |
| ulong correctionApplied = stateGasCorrection > 0 ? (ulong)stateGasCorrection : 0; | |
| ulong grossGas = grossCharge > correctionApplied ? grossCharge - correctionApplied : 0; | |
| ulong gasAfterRefund = grossGas - RefundHelper.CalculateClaimableRefund(grossGas, (ulong)refundCounter, spec); | |
| // EIP-8141 (ethereum/EIPs#12062): the calldata floor bounds the execution dimension alone, and state gas is added on top rather than absorbed. State gas is clamped since later-frame refills can drive the running total negative. | |
| ulong blockStateGas = (ulong)Math.Max(0, totalStateGas - stateGasCorrection); | |
| ulong executionAfterState = gasAfterRefund > blockStateGas ? gasAfterRefund - blockStateGas : 0; | |
| ulong blockExecutionGas = Math.Max(executionAfterState, floorGas); | |
| ulong spentGas = blockExecutionGas + blockStateGas; |
| // EIP-8141 (ethereum/EIPs#12062): a value-bearing frame with an explicit target other than the sender pays a flat value-transfer cost. | ||
| if (!frame.Value.IsZero && frame.Target is not null && !frame.Target.Equals(transaction.SenderAddress)) | ||
| { | ||
| valueTransferCost += Eip8141Constants.ValueTransferGasCost; | ||
| } |
There was a problem hiding this comment.
Medium — Target == sender is not the same as "self-transfer" outside SENDER mode.
The guard skips the 6000 charge whenever the target resolves to the sender, on the reasoning that such a frame moves value to itself. That only holds for ModeSender, where caller == sender (TransactionProcessorBase.FrameTx.cs:186). For ModeDefault / ModeVerify the caller is Eip8141Constants.EntryPointAddress, so a value-bearing frame whose target is the sender is an ENTRY_POINT → sender transfer — a real transfer, priced here as free.
Two sub-cases both fall through the same hole:
frame.Target is not null && frame.Target.Equals(SenderAddress)inDEFAULTmode;frame.Target is null(resolves to the sender) inDEFAULTmode — excluded by theis not nullclause.
The PR description states the rule as a flat "per-value-frame transfer cost 6000" with no target qualifier, which is what makes me suspect the reference keys purely on frame.value != 0. If that is right, this under-reserves and under-charges those frames and diverges on gas. (It is not masked by the ENTRY_POINT balance check on line 449 — that reverts the frame, but the static budget is still computed from the frame layout and still feeds intrinsicGas/maxGas.)
Please confirm against execution-specs#3396 which predicate the reference uses, and add a TestCase for a value-bearing DEFAULT frame targeting the sender either way.
| if (tx.SupportsFrames && tx.Frames is not null && FrameTxValidation.TryCalculateGasBudget(tx, spec, out ulong frameIntrinsic, out ulong frameFloor, out _)) | ||
| { | ||
| // EIP-8141 frames declare exact per-dimension budgets: the execution reservation is the | ||
| // intrinsic execution plus the frames' execution budgets (bounded by the calldata floor); | ||
| // the state reservation is the frames' state budgets. | ||
| ulong totalFrameExecution = 0; | ||
| ulong totalFrameState = 0; | ||
| foreach (TxFrame frame in tx.Frames) | ||
| { | ||
| totalFrameExecution += frame.ExecutionGasLimit; | ||
| totalFrameState += frame.StateGasLimit; | ||
| } | ||
|
|
||
| ulong executionReservation = Math.Max(frameIntrinsic + totalFrameExecution, frameFloor); | ||
| outcome = Eip8037BlockGasInclusionCheck.Validate( | ||
| block.Header.GasLimit, | ||
| cumulativeExecution, | ||
| cumulativeState, | ||
| executionReservation, | ||
| totalFrameState); | ||
| } |
There was a problem hiding this comment.
Medium — the EIP-7825 per-tx cap is silently dropped for frame transactions.
The else branch still goes through Validate(..., txGas), which applies Math.Min(Eip7825Constants.DefaultTxGasLimitCap, txGas) to the execution reservation. The frames branch bypasses that entirely: executionReservation = Math.Max(frameIntrinsic + totalFrameExecution, frameFloor), uncapped.
That makes the check strictly more demanding for a frame tx whose intrinsic + Σlimits.execution exceeds TX_GAS_LIMIT_CAP — and this is CheckPerTxInclusion, i.e. a block-validation rule. If the reference keeps the cap here, we reject blocks other clients accept, which is the worse failure direction. Please state the reference behaviour in the comment (the current comment justifies the exactness of the budgets but is silent on the cap) and, if the cap does apply, restore it.
Two things that do check out, for the record: frameIntrinsic + totalFrameExecution cannot overflow, because TryCalculateGasBudget already rejected any tx whose intrinsic + Σ(execution + state) wraps; and the reservation is a sound upper bound on the settlement's blockExecutionGas / blockStateGas.
| /// <summary> | ||
| /// Seeds an EIP-8141 frame budget from its two-dimensional <c>limits = [execution, state]</c>: the execution | ||
| /// dimension funds <c>gas_left</c> and the state dimension funds the state reservoir. Pre-EIP-8037 policies, | ||
| /// having no state dimension, fall back to a single combined budget. | ||
| /// </summary> | ||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| static virtual TSelf FromFrameLimits(ulong executionGasLimit, ulong stateGasLimit) => |
There was a problem hiding this comment.
Low — dead default implementation.
EthereumGasPolicy is the only struct ... : IGasPolicy<...> in the tree and it overrides FromFrameLimits, so this body never runs. The "pre-EIP-8037 policies fall back to a single combined budget" fallback it documents has no subject.
Not wrong, just extra interface surface for a hypothetical implementor — and AGENTS.md ranks "adds code without adding surface area" above this. If it stays, the saturating branch is worth a one-line note on why saturation (rather than rejection) is the right failure mode, since CalculateGasBudget rejects the same overflow a few layers up.
| /// <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 — this property has no production consumer left, and its stated invariant is not enforced where it is read.
After this PR the only remaining reads of TxFrame.GasLimit are in tests; every production site now uses ExecutionGasLimit / StateGasLimit explicitly (FrameTxDecoder computes its own saturating sum, FrameTxValidation and ExecuteFrame add the two components with their own overflow handling). AGENTS.md asks for code a change leaves unused to be removed — and a silently-wrapping ulong sum that only tests exercise is exactly the surface that grows a real caller later.
The <remarks> is also load-bearing but unverifiable at the read site: IsWellFormed does reject a per-frame overflow, but it is CalculateGasBudget (not IsWellFormed) that gates ExecuteFrameTx. They happen to make the same check — worth saying so explicitly if the property stays.
Same shape applies to TxFrameReceipt.GasUsed, which does still have one consumer — see the FrameReceiptForRpc point in the review summary.
Address review of PR #12918. A payment APPROVE that must create a non-existent sender charges NEW_ACCOUNT from the approving frame's state pool; a pool that cannot cover it halts the frame, and the reference (ethereum/EIPs#12062 attempt_approval) discards every approval effect. The approval ran after ExecuteFrame had already committed the frame, so a shortfall left the frame's writes, logs, and warm touches committed while its receipt reported failure. Move the approval inside ExecuteFrame's rollback boundary, and defer every context mutation past the budget check so a shortfall applies nothing. A fail-first regression proves it: a second sponsor sets the payer so the transaction stays valid and the divergence is observable rather than masked by the unset-payer revert. Two defensive corrections to the same accounting: - a reverted frame resets its state gas like an exceptional halt, so it never carries an entry NEW_ACCOUNT charge (the reference rolls the entry charge back on revert; no reachable change for coded targets today). - clamp the state-gas correction subtraction so a correction exceeding the running total cannot wrap ulong.
The lint gate fails the build on IDE0005 for the unused Nethermind.State directive.
feature/frames-devnet-0 no longer built on its own, so every PR against it was red regardless of its own diff. None of this is a frame change; it is folded into PR #12918 to turn CI green. - Eth72ProtocolHandlerTests: pass the LightTransaction ctor's TxType, which it gained without this call site being updated (CS7036). - Eth68ProtocolHandlerTests: the Eth68 handler now takes an IChainHeadSpecProvider, not an ISpecProvider (CS1503). - drop an unused using in FrameTxVerifyDosMeasurement and a duplicate one in TxPoolSourceTests, and an unused using in FrameTxProducerRetryMeasurement (IDE0005 / CS0105 fail the lint gate). - suppress the SSH.NET GHSA-q939-rpr3-3284 audit for Nethermind.IntegrationTests only. The advisory is a ScpClient recursive-download path traversal in a test-only transitive dependency (via Testcontainers) that the suite never exercises; the fixed 2026.0.0 forces a repo-wide BouncyCastle major bump, out of scope here.
| if (frameSucceeded && frameContext.ApprovalScopeSignal != 0) | ||
| { | ||
| long remainingStateBudget = (long)frame.StateGasLimit - frameStateGas; | ||
| if (!TryApplyApproval(frameContext, resolvedTarget, spec, in accessTracker, remainingStateBudget, out long approvalStateGas)) |
There was a problem hiding this comment.
When TryApplyApproval returns false the frame is relabelled as an exceptional halt, but ExecuteFrame has already returned and committed it, so its storage writes, balance moves and warm/cold touches stay while the receipt says StatusFailure and drops the frame's logs. That is a state-root and receipts-root divergence, and the frame is charged actual gas rather than its execution limit. Restoring a snapshot taken around the frame, or deciding the approval inside ExecuteFrame, fixes both.
Snapshot frameSnapshot = WorldState.TakeSnapshot();
StackAccessTracker frameApprovalTracker = accessTracker;
frameApprovalTracker.TakeSnapshot();
// ... ExecuteFrame ...
if (!TryApplyApproval(frameContext, resolvedTarget, spec, in accessTracker, remainingStateBudget, out long approvalStateGas))
{
WorldState.Restore(frameSnapshot);
frameApprovalTracker.Restore();
frameGasUsed = frame.ExecutionGasLimit;
frameStateGas = 0;
frameSucceeded = false;
substate = new TransactionSubstate(EvmExceptionType.OutOfGas, tracer.IsTracingInstructions);
}| _codeInfoRepository.GetCachedCodeInfo(resolvedTarget, followDelegation: false, vmSpec: spec, delegationAddress: out delegated); | ||
| if (spec.UseHotAndColdStorage && delegated is not null) | ||
| { | ||
| entryExecution += accessTracker.IsCold(delegated) && !spec.IsPrecompile(delegated) |
There was a problem hiding this comment.
The designation charge reads IsCold(delegated) before the target has been warmed, so a target delegating to itself is priced cold twice. ConsumeAccountAccessGas warms first, which makes the regular CALL path charge cold plus warm for the identical access, so the two paths disagree on gas_used.execution by ColdAccountAccess - WarmAccess. Treating a designation back to the target as warm aligns them.
bool delegatedCold = !delegated.Equals(resolvedTarget)
&& accessTracker.IsCold(delegated)
&& !spec.IsPrecompile(delegated);
entryExecution += delegatedCold
? (spec.IsEip8038Enabled ? Eip8038Constants.ColdAccountAccess : GasCostOf.ColdAccountAccess)
: Eip8038Constants.WarmAccess;| } | ||
| else | ||
| { | ||
| tracer.MarkAsFailed(Eip8141Constants.EntryPointAddress, in gasConsumed, [], error: null); |
There was a problem hiding this comment.
The new failure branch passes error: null, so SimulateTxTracer renders "execution reverted: " for a frame transaction that never reverted, and BuildFailedReceipt stores a null Error on a receipt whose status is failure. MarkAsFailed also carries no logs, so every tracer except BlockReceiptsTracer loses the frame-derived log set it used to get. A concrete reason string fixes the first half.
tracer.MarkAsFailed(Eip8141Constants.EntryPointAddress, in gasConsumed, [],
error: "frame transaction has a failed frame");| { | ||
| bool ssetOutOfGas = !TGasPolicy.ConsumeStorageWrite<TEip8037, OnFlag>(ref gas, spec); | ||
| if (ssetOutOfGas) goto OutOfGas; | ||
| FrameTxContext? chargeFrameCtx = vm.TxExecutionContext.FrameTxContext; |
There was a problem hiding this comment.
The ownership recording added to the SSTORE charge branch is not behind TEip8037.IsActive, although the reversal branch that is its only reader is. On specs without EIP-8037 the JIT therefore still emits the context load, the null check and the call on every fresh zero-slot write instead of deleting them. Wrapping it in if (TEip8037.IsActive) matches the reversal branch.
if (TEip8037.IsActive)
{
FrameTxContext? chargeFrameCtx = vm.TxExecutionContext.FrameTxContext;
chargeFrameCtx?.RecordStateChargeOwner(in storageCell, chargeFrameCtx.CurrentFrameIndex);
}| /// </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 ownership methods hash and probe the StorageCell key twice — TryGetValue then an indexer set, and TryGetValue then Remove — on a path driven directly by SSTORE. CollectionsMarshal.GetValueRefOrAddDefault and Remove(key, out value) collapse each to one operation, and the dictionary is a plain single-threaded Dictionary so both are safe here.
public void RecordStateChargeOwner(in StorageCell slot, int frame)
{
ref int owner = ref CollectionsMarshal.GetValueRefOrAddDefault(_stateChargeOwner, slot, out bool exists);
_stateGasJournal.Add(new StateGasJournalEntry(StateGasJournalKind.OwnerSet, slot, exists ? owner : NoOwner, 0));
owner = frame;
}
public bool TryResolveStateChargeOwner(in StorageCell slot, out int owner)
{
if (!_stateChargeOwner.Remove(slot, out owner)) return false;
_stateGasJournal.Add(new StateGasJournalEntry(StateGasJournalKind.OwnerCleared, slot, owner, 0));
return true;
}
…d fees wire Three tests still encoded the pre-catch-up shape and failed on CI: - FrameTxDecoderTests hand-built payloads carried the three fee scalars flat; the decoder now reads fees as a nested list, so a padded payload hit the fee sequence-prefix check before the trailing-signature guard. - The two max-gas gating tests (TxPool picker and block-production picker) fixed their boundary case to the old 15,000 intrinsic; with the spec-correct 12,000 intrinsic the 115,000 frame now fits the 130,000 block, so the reject/skip case needed a higher limit. No production change; the wire and intrinsic are already covered by the #3396 fixtures (200/200).
flcl42
left a comment
flcl42
left a comment
There was a problem hiding this comment.
Found 5 issues (2 medium and 3 low) in src/Nethermind/Nethermind.Core/FrameTxValidation.cs, src/Nethermind/Nethermind.Core/TxFrameReceipt.cs, src/Nethermind/Nethermind.Specs/Forks/Bogota.cs, src/Nethermind/Nethermind.Core.Test/Encoding/FrameTxDecoderTests.cs, and 1 other file.
| valueTransferCost += Eip8141Constants.ValueTransferGasCost; | ||
| } | ||
|
|
||
| ulong frameGas = frame.ExecutionGasLimit + frame.StateGasLimit; |
There was a problem hiding this comment.
[MEDIUM] Scalar admission rejects independently fitting frame gas dimensions
This budget now adds execution and state limits together for payer escrow, while EIP-8037 validation admits a frame transaction when each dimension independently fits. The unchanged GasLimitTxFilter and block-production picker compare this combined value with one scalar block limit, so an empty 30M block rejects a transaction reserving about 4M execution and 27M state even though received-block validation accepts it.
| public ulong ExecutionGasUsed { get; } = executionGasUsed; | ||
|
|
||
| /// <summary>State gas attributed to the frame (<c>gas_used.state</c>) after all refills and rollbacks.</summary> | ||
| public ulong StateGasUsed { get; } = stateGasUsed; |
There was a problem hiding this comment.
[MEDIUM] Frame receipt RPC output collapses the state-gas dimension
For any frame that consumes state gas, the consensus receipt now records separate execution and state values, but FrameReceiptForRpc still assigns their sum to a single gasUsed field. Consequently eth_getTransactionReceipt cannot represent the [execution, state] pair emitted by the RLP receipt, and receipts with identical totals but different splits serialize identically; the adapted RPC test uses zero state gas and checks only status.
| /// <c>bogotaTime</c>, a step after <c>amsterdamTime</c>. Not scheduled on any public network. | ||
| /// </summary> | ||
| public class Bogota() : NamedReleaseSpec<Bogota>(Osaka.Instance) | ||
| public class Bogota() : NamedReleaseSpec<Bogota>(Amsterdam.Instance) |
There was a problem hiding this comment.
[LOW] The Bogota genesis guidance still targets Osaka
Bogota now inherits Amsterdam, but GethGenesisConfigJson.BogotaTime is still documented as “frame-tx opcodes over Osaka.” Because hard-fork labels expand only their own Apply delta, an Osaka-plus-Bogota genesis following that guidance is accepted with EIP-8141 enabled while Amsterdam’s EIP-8037 remains disabled, which does not match Bogota.Instance or the new two-dimensional frame gas behavior.
| Rlp.Encode(0L), // max_priority_fee_per_gas | ||
| Rlp.Encode(0L), // max_fee_per_gas | ||
| Rlp.Encode(0L), // max_fee_per_blob_gas | ||
| Rlp.Encode(Rlp.Encode(0L), Rlp.Encode(0L), Rlp.Encode(0L)), // fees |
There was a problem hiding this comment.
[LOW] The trailing-signature test still documents a nine-field payload
Nesting the three fee scalars on this line reduces the valid payload to seven top-level elements, but the comment immediately above still says both the payload and strict clients read exactly nine. That contradicts the exact-element invariant the padding test is intended to preserve and can misdirect future decoder changes.
| 0x07 => stack.PushUInt32<TTracingInst>((uint)(frame.IsAtomicBatch ? 1 : 0)), | ||
| 0x08 => stack.PushUInt256<TTracingInst>(frame.Value), | ||
| 0x09 => stack.PushUInt256<TTracingInst>((UInt256)frame.StateGasLimit), | ||
| 0x0A => FrameExecutionGasUsed<TTracingInst>(ctx, index, ref stack), |
There was a problem hiding this comment.
[LOW] Frame gas-usage introspection parameters have no test coverage
FRAMEPARAM 0x0A and 0x0B (per-frame execution and state gas-used reads) and TXPARAM 0x0C (state gas left) are introduced here but have no corresponding test case; the FRAMEPARAM parameterized test stops at 0x09 and the TXPARAM test stops at 0x0B. FRAMEPARAM 0x0B returns StateGasUsedFor, which nets the cross-frame refill correction (_frameStateGasCorrection) that this change adds, so an inverted sign or a clamping error in that correction would be observable only through these untested reads. The SIGPARAM 0x03 restriction to the ARBITRARY scheme (non-ARBITRARY now halts instead of returning a length) is likewise untested. A test that creates a slot in one frame, reverses it in a later frame, and reads FRAMEPARAM 0x0B from a third frame would pin the corrected value.
3 participants
Changes
Catch the 8141-only frames-devnet-0 line up to the latest EIP-8141 reference (execution-specs PR #3396) and make it pass the EEST fixtures built from that PR (200/200). Twenty-one commits on top of
feature/frames-devnet-0, each compiling on its own.Grouped by theme:
BogotaasAmsterdam + EIP-8141; parse the"Bogota"spec name.feesfield as a nested list.12000and per-value-frame transfer cost6000; track per-frame gas used inFrameTxContextforFRAMEPARAM 0x0A/0x0B; addTXPARAM 0x0C(STATE_GAS_LEFT).execution = max(intrinsic + Σexec, floor),state = Σstate) instead of a worst-case from the combined gas limit.NEW_ACCOUNTstate gas to the approving frame and warms the payer.Types of changes
Testing
fill --fork Bogota).FrameTx | Eip8141 | Eip8037.Nethermind.Evm.Testsuite: 5147 total, 0 failed.rebase --execbuild check: every one of the 21 commits compiles independently.Notes
Isolated branch; not merged into
feature/frames-devnet-0. Diagnostics removed; no other-EIP surface introduced.