Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,13 @@ private static IEnumerable<TestCaseData> RoundtripCases()
Frame(),
])).SetName("Roundtrip_AllModesFlagsTargetsAndData");

yield return new TestCaseData(CreateFrameTx(frames:
[
Frame(gasLimit: 500_000, stateGasLimit: 183_600),
Frame(mode: TxFrame.ModeVerify, gasLimit: 90_000, stateGasLimit: 0),
Frame(mode: TxFrame.ModeSender, gasLimit: ulong.MaxValue - 1, stateGasLimit: 1),
])).SetName("Roundtrip_TwoDimensionalGasLimits");

yield return new TestCaseData(CreateFrameTx(signatures:
[
new TxFrameSignature(TxFrameSignature.SchemeArbitrary, null, default, FilledBytes(11, 0x77)),
Expand Down Expand Up @@ -479,7 +486,8 @@ private static void AssertFramesEqual(TxFrame[] actual, TxFrame[] expected)
Assert.That(actual[i].Mode, Is.EqualTo(expected[i].Mode), $"frame {i} mode");
Assert.That(actual[i].Flags, Is.EqualTo(expected[i].Flags), $"frame {i} flags");
Assert.That(actual[i].Target, Is.EqualTo(expected[i].Target), $"frame {i} target");
Assert.That(actual[i].GasLimit, Is.EqualTo(expected[i].GasLimit), $"frame {i} gas limit");
Assert.That(actual[i].ExecutionGasLimit, Is.EqualTo(expected[i].ExecutionGasLimit), $"frame {i} execution gas limit");
Assert.That(actual[i].StateGasLimit, Is.EqualTo(expected[i].StateGasLimit), $"frame {i} state gas limit");
Assert.That(actual[i].Value, Is.EqualTo(expected[i].Value), $"frame {i} value");
Assert.That(actual[i].Data.ToArray(), Is.EqualTo(expected[i].Data.ToArray()), $"frame {i} data");
}
Expand Down Expand Up @@ -510,8 +518,8 @@ private static Transaction CreateFrameTx(TxFrame[]? frames = null, TxFrameSignat
DecodedMaxFeePerGas = 30.GWei,
};

private static TxFrame Frame(byte mode = TxFrame.ModeDefault, byte flags = 0, Address? target = null, ulong gasLimit = 100_000, UInt256 value = default, byte[]? data = null) =>
new(mode, flags, target, gasLimit, value, data ?? Array.Empty<byte>());
private static TxFrame Frame(byte mode = TxFrame.ModeDefault, byte flags = 0, Address? target = null, ulong gasLimit = 100_000, ulong stateGasLimit = 0, UInt256 value = default, byte[]? data = null) =>
new(mode, flags, target, gasLimit, stateGasLimit, value, data ?? Array.Empty<byte>());

private static byte[] FilledBytes(int length, byte fill)
{
Expand Down
3 changes: 3 additions & 0 deletions src/Nethermind/Nethermind.Core.Test/FrameTxValidationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ private static IEnumerable<TestCaseData> ConstraintCases()
yield return Case("ExpiryFrameWithShortData_InvalidExpiryFrame",
static tx => tx.Frames = [Frame(mode: TxFrame.ModeVerify, target: Eip8141Constants.ExpiryVerifierAddress, data: new byte[Eip8141Constants.ExpiryDataLength - 1])],
FrameTxValidation.InvalidExpiryFrame);
yield return Case("ExpiryFrameWithStateGas_InvalidExpiryFrame",
static tx => tx.Frames = [new TxFrame(TxFrame.ModeVerify, flags: 0, Eip8141Constants.ExpiryVerifierAddress, 30_000, 1, UInt256.Zero, new byte[Eip8141Constants.ExpiryDataLength])],
FrameTxValidation.InvalidExpiryFrame);
yield return Case("TwoExpiryFrames_MultipleExpiryFrames",
static tx => tx.Frames = [SelfVerifyFrame(), ExpiryFrame(), ExpiryFrame()],
FrameTxValidation.MultipleExpiryFrames);
Expand Down
20 changes: 12 additions & 8 deletions src/Nethermind/Nethermind.Core/FrameTxValidation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Copy link
Copy Markdown
Contributor

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 != 0 is a new validity condition on the expiry verifier frame. Per .agents/rules/coding-style.md ("Non-obvious consensus rules or algorithms must reference the EIP number or Yellow Paper section"), please anchor it — the surrounding conditions predate the split and carry their justification in the enclosing docs, but this one is new. Also worth double-checking against the merged spec text whether the expiry frame is required to have limits.state == 0 specifically, or whether the spec constrains limits.execution too (in which case a fixed execution budget should be asserted here as well).

{
error = InvalidExpiryFrame;
return false;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High — MAX_VERIFY_GAS becomes bypassable while the runtime still uses a combined pool.

This now prices the validation prefix at limits.execution only, but the runtime seeds the frame's VM pool with the combined budget (TransactionProcessorBase.FrameTx.cs:594TGasPolicy.FromULong(frame.GasLimit) where GasLimit == execution + state), and the state-gas reservoir is unchanged. So a prefix frame declaring limits = [1, 100_000_000] scores ~1 gas here, sails past FrameTxVerifyGasFilter (FrameTxMaxVerifyGas = 100_000 by default), and still gets a 100M-gas pool of arbitrary work when the pool simulates the prefix. The declared state gas does not constrain what the frame spends it on until the dual-pool seeding lands.

The affordability check is not a mitigation: MAX_VERIFY_GAS exists precisely for the case where the attacker never pays (tx never included), so a funded sender can spam unbounded free validation work.

Since both the dual-pool seeding and MAX_VERIFY_STATE_GAS are explicitly out of scope here, the two halves of the split need to stay consistent. Options:

  • keep this on the combined frames[i].GasLimit until the runtime actually seeds gas_left = limits.execution, or
  • land the MAX_VERIFY_STATE_GAS bound in this PR alongside the execution-only change.

}

foreach (TxFrameSignature signature in transaction.FrameSignatures ?? [])
Expand Down Expand Up @@ -401,8 +404,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;
}
Expand Down
23 changes: 20 additions & 3 deletions src/Nethermind/Nethermind.Core/TxFrame.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low — GasLimit wraps silently, and it is read before validation.

The remark is true for frames that reach execution, but FrameTxDecoder.DecodeInternal reads frame.GasLimit while summing transaction.GasLimit (TxDecoders/FrameTxDecoder.cs:148) — i.e. at decode time, before IsWellFormed runs. With execution = ulong.MaxValue, state = 10 the property returns 9, so the saturating sum there is defeated and the tx is briefly visible with an absurdly small GasLimit. Validation does reject it afterwards, so there is no path to execution, but failing closed is cheap here:

Suggested change
public ulong GasLimit => ExecutionGasLimit + StateGasLimit;
public ulong GasLimit => ExecutionGasLimit > ulong.MaxValue - StateGasLimit ? ulong.MaxValue : ExecutionGasLimit + StateGasLimit;

(If you prefer to keep the plain sum, at least widen the remark to say the property may wrap for a transaction that has not yet passed static validation.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL] The combined GasLimit alias collapses independent reservations

Transaction.GasLimit is populated by summing this alias, while the unchanged EIP-8037 inclusion check feeds that scalar into both dimensions and the block picker compares the combined budget with one remaining-gas value. A transaction whose execution and state reservations each fit independently can therefore be rejected solely because their sum exceeds the block limit. The same alias lets the default-code keyed-nonce surcharge treat limits.state as execution headroom, affecting both block validity and frame execution.


public UInt256 Value { get; } = value;
public ReadOnlyMemory<byte> Data { get; } = data;

Expand Down
17 changes: 17 additions & 0 deletions src/Nethermind/Nethermind.Evm.Test/FrameTxProcessorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ public void Execute_TxParamSigHash_ExposesCanonicalHash()
[TestCase((byte)0x06, 3UL, TestName = "Execute_FrameParam_AllowedScope")]
[TestCase((byte)0x07, 0UL, TestName = "Execute_FrameParam_AtomicBatch")]
[TestCase((byte)0x08, 0UL, TestName = "Execute_FrameParam_Value")]
[TestCase((byte)0x09, 0UL, TestName = "Execute_FrameParam_StateGasLimit")]
public void Execute_FrameParamIntrospection_ReadsCompletedFrame(byte param, ulong expected)
{
DeploySmartSender(ApproveCode(TxFrame.ApproveExecutionAndPayment));
Expand All @@ -511,6 +512,22 @@ public void Execute_FrameParamIntrospection_ReadsCompletedFrame(byte param, ulon
AssertStorage(Observer, 0, (UInt256)expected);
}

[Test]
public void Execute_FrameParam_StateGasLimit_ReadsDeclaredStateBudget()
{
DeploySmartSender(ApproveCode(TxFrame.ApproveExecutionAndPayment));
DeployContract(Observer, Prepare.EvmCode
.PushData(0x09).PushData(1).Op(Instruction.FRAMEPARAM).PushData(0).Op(Instruction.SSTORE)
.Op(Instruction.STOP).Done);
Transaction tx = FrameTx(nonce: 0, SelfVerifyFrame(),
new TxFrame(TxFrame.ModeDefault, 0, Observer, 200_000, 50_000, UInt256.Zero, default));

TransactionResult result = Process(tx);

Assert.That(result.TransactionExecuted, Is.True);
AssertStorage(Observer, 0, (UInt256)50_000);
}

[Test]
public void Execute_FrameParamStatusOfCurrentFrame_ExceptionallyHalts()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 0x0A and 0x0B for a completed frame's execution and state usage. This bound returns BadInstruction for both. A later frame that introspects either usage therefore exceptionally halts instead of receiving the recorded value, which can change subsequent approvals and state.


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,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@

namespace Nethermind.Facade.Eth.RpcTransaction;

/// <summary>JSON-RPC view of an EIP-8141 frame: <c>[mode, flags, target, gas_limit, value, data]</c>.</summary>
/// <summary>JSON-RPC view of an EIP-8141 frame: <c>[mode, flags, target, limits, value, data]</c>,
/// where <c>limits = [execution, state]</c>.</summary>
public class FrameForRpc
{
public byte Mode { get; set; }
public byte Flags { get; set; }
public Address? Target { get; set; }
public ulong GasLimit { get; set; }
public ulong ExecutionGasLimit { get; set; }
public ulong StateGasLimit { get; set; }
public UInt256 Value { get; set; }
public byte[] Data { get; set; } = [];

Expand All @@ -26,12 +28,13 @@ public FrameForRpc(TxFrame frame)
Mode = frame.Mode;
Flags = frame.Flags;
Target = frame.Target;
GasLimit = frame.GasLimit;
ExecutionGasLimit = frame.ExecutionGasLimit;
StateGasLimit = frame.StateGasLimit;
Value = frame.Value;
Data = frame.Data.ToArray();
}

public TxFrame ToFrame() => new(Mode, Flags, Target, GasLimit, Value, Data);
public TxFrame ToFrame() => new(Mode, Flags, Target, ExecutionGasLimit, StateGasLimit, Value, Data);

public static FrameForRpc[]? FromFrames(TxFrame[]? frames) =>
frames?.Select(static f => new FrameForRpc(f)).ToArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ public void FrameTransactionForRpc_SerializesFrames()
Assert.That(frames[0].GetProperty("mode").GetInt32(), Is.EqualTo(TxFrame.ModeVerify));
Assert.That(frames[0].GetProperty("flags").GetInt32(), Is.EqualTo(TxFrame.ApproveExecutionAndPayment));
Assert.That(frames[0].GetProperty("target").GetString(), Is.EqualTo(TestItem.AddressB.ToString()));
Assert.That(frames[0].GetProperty("gasLimit").GetString(), Does.Match("^0x[0-9a-f]+$"));
Assert.That(frames[0].GetProperty("executionGasLimit").GetString(), Does.Match("^0x[0-9a-f]+$"));
Assert.That(frames[0].GetProperty("stateGasLimit").GetString(), Does.Match("^0x[0-9a-f]+$"));
}
}

Expand Down
25 changes: 20 additions & 5 deletions src/Nethermind/Nethermind.Serialization.Rlp/TxFrameDecoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
namespace Nethermind.Serialization.Rlp;

/// <summary>
/// Decodes the EIP-8141 frame tuple <c>[mode, flags, target, gas_limit, value, data]</c>.
/// Decodes the EIP-8141 frame tuple <c>[mode, flags, target, limits, value, data]</c>, where
/// <c>limits = [execution, state]</c>.
/// An empty target byte string decodes to null (resolves to the transaction sender).
/// </summary>
public sealed class TxFrameDecoder : RlpDecoder<TxFrame>
Expand All @@ -27,7 +28,16 @@ protected override TxFrame DecodeInternal(ref RlpReader decoderContext, RlpBehav
byte mode = decoderContext.DecodeByte();
byte flags = decoderContext.DecodeByte();
Address? target = decoderContext.DecodeAddressOrNull();
ulong gasLimit = decoderContext.DecodeULong();

int limitsLength = decoderContext.ReadSequenceLength();
int limitsCheck = limitsLength + decoderContext.Position;
ulong executionGasLimit = decoderContext.DecodeULong();
ulong stateGasLimit = decoderContext.DecodeULong();
if (!rlpBehaviors.HasFlag(RlpBehaviors.AllowExtraBytes))
{
decoderContext.Check(limitsCheck);
}
Comment on lines +36 to +39

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium — the inner limits check should not be gated on AllowExtraBytes.

AllowExtraBytes means "tolerate trailing bytes after the payload", which is why gating the outer check on line 44 is harmless — it is the last read. Here the check sits mid-stream: skipping it does not tolerate trailing data, it leaves Position inside/after the limits list and makes the subsequent value and data reads land at the wrong offsets, silently producing a different frame (and therefore a different tx hash) from limits = [exec, state, junk].

Today this is latent — RlpReader.DecodeArray calls decoder.Decode(ref this) with RlpBehaviors.None, so frames never see the flag — but any future call site (Rlp.Decode<TxFrame>(…, AllowExtraBytes), a DB path via KeyValueStoreRlpExtensions, which ORs the flag in) would hit it. Make it unconditional:

Suggested change
if (!rlpBehaviors.HasFlag(RlpBehaviors.AllowExtraBytes))
{
decoderContext.Check(limitsCheck);
}
decoderContext.Check(limitsCheck);


UInt256 value = decoderContext.DecodeUInt256();
ReadOnlyMemory<byte> data = decoderContext.DecodeByteArrayMemory(_dataRlpLimit);

Expand All @@ -36,7 +46,7 @@ protected override TxFrame DecodeInternal(ref RlpReader decoderContext, RlpBehav
decoderContext.Check(check);
}

return new TxFrame(mode, flags, target, gasLimit, value, data);
return new TxFrame(mode, flags, target, executionGasLimit, stateGasLimit, value, data);
}

public override void Encode<TWriter>(ref TWriter writer, TxFrame item, RlpBehaviors rlpBehaviors = RlpBehaviors.None)
Expand All @@ -45,7 +55,9 @@ public override void Encode<TWriter>(ref TWriter writer, TxFrame item, RlpBehavi
writer.Encode((ulong)item.Mode);
writer.Encode((ulong)item.Flags);
writer.Encode(item.Target);
writer.Encode(item.GasLimit);
writer.StartSequence(GetLimitsContentLength(item));
writer.Encode(item.ExecutionGasLimit);
writer.Encode(item.StateGasLimit);
writer.Encode(item.Value);
writer.Encode(item.Data);
}
Expand Down Expand Up @@ -85,7 +97,10 @@ private static int GetContentLength(TxFrame item) =>
Rlp.LengthOf((ulong)item.Mode)
+ Rlp.LengthOf((ulong)item.Flags)
+ Rlp.LengthOf(item.Target)
+ Rlp.LengthOf(item.GasLimit)
+ Rlp.LengthOfSequence(GetLimitsContentLength(item))
+ Rlp.LengthOf(item.Value)
+ Rlp.LengthOf(item.Data);

private static int GetLimitsContentLength(TxFrame item) =>
Rlp.LengthOf(item.ExecutionGasLimit) + Rlp.LengthOf(item.StateGasLimit);
}
Loading