-
Notifications
You must be signed in to change notification settings - Fork 720
feat(frames): two-dimensional gas limits per EIP-8141 #12847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -155,7 +155,7 @@ public static bool IsWellFormed(Transaction transaction, bool postTxEnabled, out | |
|
|
||
| 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) | ||
| { | ||
| error = InvalidExpiryFrame; | ||
| return false; | ||
|
|
@@ -170,8 +170,9 @@ public static bool IsWellFormed(Transaction transaction, bool postTxEnabled, out | |
| hasExpiryFrame = true; | ||
| } | ||
|
|
||
| ulong accumulated = totalFrameGas + frame.GasLimit; | ||
| if (accumulated < totalFrameGas) | ||
| ulong frameGas = frame.ExecutionGasLimit + frame.StateGasLimit; | ||
| ulong accumulated = totalFrameGas + frameGas; | ||
| if (frameGas < frame.ExecutionGasLimit || accumulated < totalFrameGas) | ||
| { | ||
| error = FrameGasOverflow; | ||
| return false; | ||
|
|
@@ -250,8 +251,10 @@ static bool BelongsToAtomicBatch(TxFrame[] frames, int i) => | |
| }; | ||
|
|
||
| /// <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 | ||
| /// limits (EIP-8141 <c>MAX_VERIFY_GAS</c>) of its validation prefix plus the cost of verifying its signatures, | ||
| /// saturating at <see cref="ulong.MaxValue"/>. The prefix's <c>limits.state</c> is bounded separately by | ||
| /// <c>MAX_VERIFY_STATE_GAS</c> and does not enter this budget. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Derived from the frame layout alone, so no state is read. Each layout of EIP-8141 "Public | ||
|
|
@@ -269,7 +272,7 @@ public static ulong ValidationWorkGas(Transaction transaction) | |
| ulong total = 0; | ||
| for (int i = 0; i < counted; i++) | ||
| { | ||
| total = Saturating(total, frames[i].GasLimit); | ||
| total = Saturating(total, frames[i].ExecutionGasLimit); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High — This now prices the validation prefix at The affordability check is not a mitigation: Since both the dual-pool seeding and
|
||
| } | ||
|
|
||
| foreach (TxFrameSignature signature in transaction.FrameSignatures ?? []) | ||
|
|
@@ -280,6 +283,27 @@ public static ulong ValidationWorkGas(Transaction transaction) | |
| return total; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// An upper bound on the state growth EIP-8141 admits through the public mempool for | ||
| /// <paramref name="transaction"/>: the sum of its validation prefix's <c>limits.state</c>, saturating at | ||
| /// <see cref="ulong.MaxValue"/>. Bounded separately by <c>MAX_VERIFY_STATE_GAS</c>; signature verification | ||
| /// uses no state gas, so it does not enter this budget. | ||
| /// </summary> | ||
| /// <param name="transaction">The frame transaction to price.</param> | ||
| public static ulong ValidationWorkStateGas(Transaction transaction) | ||
| { | ||
| TxFrame[] frames = transaction.Frames ?? []; | ||
| int counted = RecognizedPrefixLength(frames, transaction.SenderAddress) ?? frames.Length; | ||
|
|
||
| ulong total = 0; | ||
| for (int i = 0; i < counted; i++) | ||
| { | ||
| total = Saturating(total, frames[i].StateGasLimit); | ||
| } | ||
|
|
||
| return total; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// The number of leading frames forming a validation prefix EIP-8141 recognizes for the public | ||
| /// mempool, or <c>null</c> when the layout matches none of them. | ||
|
|
@@ -401,8 +425,9 @@ private static bool CalculateGasBudget(Transaction transaction, IReleaseSpec spe | |
| tokens += CountCalldataTokens(frame.Data.Span, spec); | ||
| dataLength += (ulong)frame.Data.Length; | ||
|
|
||
| ulong accumulated = totalFrameGas + frame.GasLimit; | ||
| if (accumulated < totalFrameGas) | ||
| ulong frameGas = frame.ExecutionGasLimit + frame.StateGasLimit; | ||
| ulong accumulated = totalFrameGas + frameGas; | ||
| if (frameGas < frame.ExecutionGasLimit || accumulated < totalFrameGas) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,10 +7,11 @@ | |||||
| namespace Nethermind.Core; | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// A single frame of an EIP-8141 frame transaction: <c>[mode, flags, target, gas_limit, value, data]</c>. | ||||||
| /// A single frame of an EIP-8141 frame transaction: <c>[mode, flags, target, limits, value, data]</c>, | ||||||
| /// where <c>limits = [execution, state]</c>. | ||||||
| /// https://eips.ethereum.org/EIPS/eip-8141 | ||||||
| /// </summary> | ||||||
| public class TxFrame(byte mode, byte flags, Address? target, ulong gasLimit, UInt256 value, ReadOnlyMemory<byte> data) | ||||||
| public class TxFrame(byte mode, byte flags, Address? target, ulong executionGasLimit, ulong stateGasLimit, UInt256 value, ReadOnlyMemory<byte> data) | ||||||
| { | ||||||
| public const byte ModeDefault = 0; | ||||||
| public const byte ModeVerify = 1; | ||||||
|
|
@@ -26,13 +27,29 @@ public class TxFrame(byte mode, byte flags, Address? target, ulong gasLimit, UIn | |||||
| public const byte ApproveScopeMask = ApproveExecutionAndPayment; | ||||||
| public const byte AtomicBatchFlag = 0x4; | ||||||
|
|
||||||
| /// <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; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Low — The remark is true for frames that reach execution, but
Suggested change
(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.)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL] The combined GasLimit alias collapses independent reservations
|
||||||
|
|
||||||
| public UInt256 Value { get; } = value; | ||||||
| public ReadOnlyMemory<byte> Data { get; } = data; | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,10 @@ public struct EthereumGasPolicy : IGasPolicy<EthereumGasPolicy> | |
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static EthereumGasPolicy FromULong(ulong value) => new() { Value = value }; | ||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static EthereumGasPolicy FromFrameLimits(ulong executionGasLimit, ulong stateGasLimit) => | ||
| new() { Value = executionGasLimit, StateReservoir = (long)stateGasLimit }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL] Frame state charges can spill into the execution budget
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit Not reachable today (a frame reaching execution has passed affordability and block-gas-limit checks, which bound the limits far below |
||
|
|
||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static EthereumGasPolicy CreateSystemTransactionIntrinsicGas(ulong blockGasLimit) => | ||
| new() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -217,21 +217,22 @@ public static EvmExceptionType InstructionFrameParam<TGasPolicy, TTracingInst>(V | |
| // Spec stack order: frameIndex on top, param second. | ||
| 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; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL] FRAMEPARAM rejects the required gas-usage selectors The updated EIP-8141 table defines |
||
|
|
||
| int index = (int)frameIndex.u0; | ||
| TxFrame frame = ctx.Frames[index]; | ||
| return param.u0 switch | ||
| { | ||
| 0x00 => stack.PushAddress<TTracingInst>(ctx.ResolvedTarget(index)), | ||
| 0x01 => stack.PushUInt256<TTracingInst>((UInt256)frame.GasLimit), | ||
| 0x01 => stack.PushUInt256<TTracingInst>((UInt256)frame.ExecutionGasLimit), | ||
| 0x02 => stack.PushUInt32<TTracingInst>(frame.Mode), | ||
| 0x03 => stack.PushUInt32<TTracingInst>(frame.Flags), | ||
| 0x04 => stack.PushUInt256<TTracingInst>((UInt256)frame.Data.Length), | ||
| 0x05 => FrameStatus<TTracingInst>(ctx, index, ref stack), | ||
| 0x06 => stack.PushUInt32<TTracingInst>(frame.AllowedApproveScope), | ||
| 0x07 => stack.PushUInt32<TTracingInst>((uint)(frame.IsAtomicBatch ? 1 : 0)), | ||
| 0x08 => stack.PushUInt256<TTracingInst>(frame.Value), | ||
| 0x09 => stack.PushUInt256<TTracingInst>((UInt256)frame.StateGasLimit), | ||
| _ => EvmExceptionType.BadInstruction, | ||
| }; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Low — new consensus rule without a spec citation.
frame.StateGasLimit != 0is 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 havelimits.state == 0specifically, or whether the spec constrainslimits.executiontoo (in which case a fixed execution budget should be asserted here as well).