Skip to content

Commit 0e530bb

Browse files
Marchhillclaude
andcommitted
fix(evm): address devnet-6 review findings in the state-gas policy
- Make advanced state-gas refund revocation exact: track how much spill refund each advance marked (VmState.StateGasSpillRefundAdvanced) and undo it on revert; the removal drains the reservoir only to zero, then recorded usage, and restores any net-spill debt the credit had filled. - Set the OutOfGas flag when ConsumeStateGas fails on the spill path. - Saturate the EIP-7825 cap subtraction in CreateAvailableFromIntrinsic (replacing the debug assert) so an over-cap intrinsic moves the excess to the reservoir; regression tests for all three. - Extract the shared destroy/recreate logic into DestroyAccount on the non-generic TransactionProcessorBase. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0f70752 commit 0e530bb

6 files changed

Lines changed: 92 additions & 45 deletions

File tree

src/Nethermind/Nethermind.Evm.Test/Eip8037Tests.cs

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,46 @@ public void ConsumeStateGas_oog_does_not_zero_reservoir()
213213

214214
bool consumed = EthereumGasPolicy.ConsumeStateGas(ref gas, 70);
215215

216-
Assert.That((consumed, gas.Value, gas.StateReservoir, gas.StateGasUsed, gas.StateGasSpill),
217-
Is.EqualTo((false, 10L, 50L, 0L, 0L)));
216+
Assert.That((consumed, gas.Value, gas.StateReservoir, gas.StateGasUsed, gas.StateGasSpill, EthereumGasPolicy.IsOutOfGas(in gas)),
217+
Is.EqualTo((false, 10UL, 50L, 0L, 0L, true)));
218+
}
219+
220+
[Test]
221+
public void Revoking_advanced_refund_restores_net_spill_reservoir_and_spill_tracking()
222+
{
223+
// A net-spill (negative) reservoir, as left by RestoreChildStateGas after nested spills.
224+
EthereumGasPolicy gas = new() { Value = 400, StateReservoir = -300, StateGasUsed = 0, StateGasSpill = 300, StateGasSpillRefunded = 0 };
225+
226+
long tracked = EthereumGasPolicy.AddStateGasRefundToReservoir(ref gas, 200, trackSpillRefund: true);
227+
Assert.That((tracked, gas.StateReservoir, gas.StateGasSpillRefunded), Is.EqualTo((200L, -100L, 200L)));
228+
229+
EthereumGasPolicy.RemoveStateGasRefundFromReservoir(ref gas, 200, tracked);
230+
231+
Assert.That((gas.StateReservoir, gas.StateGasUsed, gas.StateGasSpillRefunded), Is.EqualTo((-300L, 0L, 0L)));
232+
}
233+
234+
[Test]
235+
public void Revoking_consumed_advanced_refund_deducts_usage_without_fabricating_spill_debt()
236+
{
237+
EthereumGasPolicy gas = new() { Value = 400, StateReservoir = 0, StateGasUsed = 100 };
238+
239+
long tracked = EthereumGasPolicy.AddStateGasRefundToReservoir(ref gas, 200, trackSpillRefund: true);
240+
Assert.That(EthereumGasPolicy.ConsumeStateGas(ref gas, 200), Is.True);
241+
242+
EthereumGasPolicy.RemoveStateGasRefundFromReservoir(ref gas, 200, tracked);
243+
244+
Assert.That((gas.StateReservoir, gas.StateGasUsed), Is.EqualTo((0L, 100L)));
245+
}
246+
247+
[Test]
248+
public void Intrinsic_regular_gas_above_the_tx_cap_saturates_into_the_reservoir()
249+
{
250+
EthereumGasPolicy intrinsic = new() { Value = Eip7825Constants.DefaultTxGasLimitCap + 1, StateReservoir = 0 };
251+
252+
EthereumGasPolicy gas = EthereumGasPolicy.CreateAvailableFromIntrinsic(
253+
Eip7825Constants.DefaultTxGasLimitCap + 2, in intrinsic, Amsterdam.Instance);
254+
255+
Assert.That((gas.Value, gas.StateReservoir), Is.EqualTo((0UL, 1L)));
218256
}
219257

220258
[Test]

src/Nethermind/Nethermind.Evm/GasPolicy/EthereumGasPolicy.cs

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ public static bool ConsumeStateGas(ref EthereumGasPolicy gas, long stateGasCost)
137137
ulong spillAmount = CalculateStateGasSpill(in gas, stateGasCost);
138138
if (!TryConsume(ref gas, spillAmount))
139139
{
140+
gas.OutOfGas = true;
140141
return false;
141142
}
142143

@@ -385,34 +386,38 @@ public static long DiscardStateGas(ref EthereumGasPolicy gas, long amount, long
385386
}
386387

387388
[MethodImpl(MethodImplOptions.AggressiveInlining)]
388-
public static void AddStateGasRefundToReservoir(ref EthereumGasPolicy gas, long amount, bool trackSpillRefund)
389+
public static long AddStateGasRefundToReservoir(ref EthereumGasPolicy gas, long amount, bool trackSpillRefund)
389390
{
390-
if (trackSpillRefund)
391-
{
392-
TrackStateGasSpillRefund(ref gas, amount);
393-
}
394-
391+
long trackedSpillRefund = trackSpillRefund ? TrackStateGasSpillRefund(ref gas, amount) : 0;
395392
gas.StateReservoir += amount;
393+
return trackedSpillRefund;
396394
}
397395

398396
[MethodImpl(MethodImplOptions.AggressiveInlining)]
399-
public static void RemoveStateGasRefundFromReservoir(ref EthereumGasPolicy gas, long amount)
397+
public static void RemoveStateGasRefundFromReservoir(ref EthereumGasPolicy gas, long amount, long trackedSpillRefund)
400398
{
401-
long fromReservoir = Math.Min(amount, gas.StateReservoir);
399+
gas.StateGasSpillRefunded -= Math.Min(trackedSpillRefund, gas.StateGasSpillRefunded);
400+
401+
// Revoke what is still parked in the reservoir (never fabricating spill debt), then
402+
// usage the credit funded; any remainder refilled a net-spill hole, so restore the debt.
403+
long fromReservoir = Math.Max(0, Math.Min(amount, gas.StateReservoir));
402404
gas.StateReservoir -= fromReservoir;
403405
amount -= fromReservoir;
404406

405407
if (amount > 0)
406408
{
407-
gas.StateGasUsed -= Math.Min(amount, gas.StateGasUsed);
409+
long fromUsed = Math.Min(amount, gas.StateGasUsed);
410+
gas.StateGasUsed -= fromUsed;
411+
gas.StateReservoir -= amount - fromUsed;
408412
}
409413
}
410414

411415
[MethodImpl(MethodImplOptions.AggressiveInlining)]
412-
private static void TrackStateGasSpillRefund(ref EthereumGasPolicy gas, long amount)
416+
private static long TrackStateGasSpillRefund(ref EthereumGasPolicy gas, long amount)
413417
{
414-
long unrefundedSpill = GetUnrefundedStateGasSpill(in gas);
415-
gas.StateGasSpillRefunded += Math.Min(amount, unrefundedSpill);
418+
long tracked = Math.Min(amount, GetUnrefundedStateGasSpill(in gas));
419+
gas.StateGasSpillRefunded += tracked;
420+
return tracked;
416421
}
417422

418423
[MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -538,16 +543,13 @@ public static EthereumGasPolicy CreateAvailableFromIntrinsic(ulong gasLimit, in
538543
// before calling; if they don't, the subtraction wraps silently.
539544
Debug.Assert(gasLimit >= intrinsicGas.Value + (ulong)intrinsicGas.StateReservoir,
540545
$"gasLimit ({gasLimit}) < intrinsicRegular ({intrinsicGas.Value}) + intrinsicState ({intrinsicGas.StateReservoir})");
541-
Debug.Assert(!spec.IsEip8037Enabled || Eip7825Constants.DefaultTxGasLimitCap >= intrinsicGas.Value,
542-
"Eip8037 enabled but intrinsicRegular exceeds tx gas cap.");
543-
544546
ulong executionGas = gasLimit - intrinsicGas.Value - (ulong)intrinsicGas.StateReservoir;
545547
ulong reservoir = 0;
546548

547549
if (spec.IsEip8037Enabled)
548550
{
549551
// EIP-8037: cap gas_left at TX_MAX_GAS_LIMIT - intrinsic_regular, overflow goes to reservoir
550-
ulong maxGasLeft = Eip7825Constants.DefaultTxGasLimitCap - intrinsicGas.Value;
552+
ulong maxGasLeft = Eip7825Constants.DefaultTxGasLimitCap.SaturatingSub(intrinsicGas.Value);
551553
reservoir = executionGas.SaturatingSub(maxGasLeft);
552554
executionGas -= reservoir;
553555
}

src/Nethermind/Nethermind.Evm/GasPolicy/IGasPolicy.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,12 +228,16 @@ static virtual void RefundStateGas(ref TSelf gas, long amount, long stateGasFloo
228228
[MethodImpl(MethodImplOptions.AggressiveInlining)]
229229
static virtual long DiscardStateGas(ref TSelf gas, long amount, long stateGasFloor, bool trackSpillRefund) => amount;
230230

231+
// Returns the spill-refund amount it marked so a later revocation can unmark it exactly.
231232
[MethodImpl(MethodImplOptions.AggressiveInlining)]
232-
static virtual void AddStateGasRefundToReservoir(ref TSelf gas, long amount, bool trackSpillRefund) =>
233+
static virtual long AddStateGasRefundToReservoir(ref TSelf gas, long amount, bool trackSpillRefund)
234+
{
233235
TSelf.UpdateGasUp(ref gas, (ulong)amount);
236+
return 0;
237+
}
234238

235239
[MethodImpl(MethodImplOptions.AggressiveInlining)]
236-
static virtual void RemoveStateGasRefundFromReservoir(ref TSelf gas, long amount) { }
240+
static virtual void RemoveStateGasRefundFromReservoir(ref TSelf gas, long amount, long trackedSpillRefund) { }
237241

238242
// EIP-8037 top-level halt: snap state-gas back to (R0, intrinsicStateUsed, 0); the
239243
// post-reset StateGasUsed feeds SpentGas so the user doesn't pay for uncommitted state.

src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,22 @@ public bool TryCalculateBlobFees(BlockHeader header, Transaction transaction,
7373
public abstract class TransactionProcessorBase
7474
{
7575
internal static bool ForceSimpleTransferDisabled;
76+
77+
private protected static void DestroyAccount(IWorldState worldState, Address toBeDestroyed, in UInt256 balance, bool commit, bool removeSelfdestructBurn)
78+
{
79+
// Build-up rounds (!commit) span the whole block: later txs may redeploy this address,
80+
// so the order-preserving journaled clear is required; the O(1) mark needs a commit after.
81+
if (commit) worldState.MarkStorageDestroyed(toBeDestroyed);
82+
else worldState.ClearStorage(toBeDestroyed);
83+
worldState.DeleteAccount(toBeDestroyed);
84+
85+
// EIP-8246: preserve any remaining balance as a fresh nonce-0, code-less account;
86+
// an empty account stays deleted via EIP-161.
87+
if (removeSelfdestructBurn && !balance.IsZero)
88+
{
89+
worldState.CreateAccount(toBeDestroyed, balance);
90+
}
91+
}
7692
}
7793

7894
public abstract class TransactionProcessorBase<TGasPolicy> : TransactionProcessorBase, ITransactionProcessor
@@ -380,16 +396,7 @@ static void FinalizeDestroyedAccount(IWorldState worldState, in TransactionSubst
380396
substate.Logs.Add(TransferLog.CreateBurn(toBeDestroyed, balance));
381397
}
382398

383-
if (commit) worldState.MarkStorageDestroyed(toBeDestroyed);
384-
else worldState.ClearStorage(toBeDestroyed);
385-
worldState.DeleteAccount(toBeDestroyed);
386-
387-
// EIP-8246: preserve any remaining balance as a fresh nonce-0,
388-
// code-less account; an empty account stays deleted via EIP-161.
389-
if (removeSelfdestructBurn && !balance.IsZero)
390-
{
391-
worldState.CreateAccount(toBeDestroyed, balance);
392-
}
399+
DestroyAccount(worldState, toBeDestroyed, in balance, commit, removeSelfdestructBurn);
393400
}
394401
}
395402

@@ -1386,19 +1393,7 @@ private int ExecuteEvmCall<TTracingInst>(
13861393
substate.Logs.Add(TransferLog.CreateSelfDestruct(toBeDestroyed, balance));
13871394
}
13881395

1389-
// Build-up rounds (!commit) span the whole block: later txs may
1390-
// redeploy this address, so the order-preserving journaled clear
1391-
// is required; the O(1) mark is only valid when a commit follows.
1392-
if (commit) WorldState.MarkStorageDestroyed(toBeDestroyed);
1393-
else WorldState.ClearStorage(toBeDestroyed);
1394-
WorldState.DeleteAccount(toBeDestroyed);
1395-
1396-
// EIP-8246: preserve any remaining balance as a fresh nonce-0,
1397-
// code-less account; an empty account stays deleted via EIP-161.
1398-
if (removeSelfdestructBurn && !balance.IsZero)
1399-
{
1400-
WorldState.CreateAccount(toBeDestroyed, balance);
1401-
}
1396+
DestroyAccount(WorldState, toBeDestroyed, in balance, commit, removeSelfdestructBurn);
14021397

14031398
if (tracingRefunds)
14041399
{

src/Nethermind/Nethermind.Evm/VirtualMachine.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,9 +687,10 @@ internal void CreditStateGasRefund(ref TGasPolicy gas, long amount, bool trackSp
687687
{
688688
// Restored state gas paid by an ancestor frame stays spendable here; the
689689
// state-gas-used reduction must propagate upward separately.
690-
TGasPolicy.AddStateGasRefundToReservoir(ref gas, pendingRefund, trackSpillRefund);
690+
long trackedSpillRefund = TGasPolicy.AddStateGasRefundToReservoir(ref gas, pendingRefund, trackSpillRefund);
691691
vmState.StateGasRefundPending += pendingRefund;
692692
vmState.StateGasRefundAdvanced += pendingRefund;
693+
vmState.StateGasSpillRefundAdvanced += trackedSpillRefund;
693694
}
694695
}
695696

@@ -709,10 +710,12 @@ private void IncorporateChildStateGasRefunds(VmState<TGasPolicy> childState)
709710
{
710711
_currentState.StateGasRefundPending += unappliedRefund;
711712
_currentState.StateGasRefundAdvanced += unappliedRefund;
713+
_currentState.StateGasSpillRefundAdvanced += Math.Min(childState.StateGasSpillRefundAdvanced, unappliedRefund);
712714
}
713715

714716
childState.StateGasRefundPending = 0;
715717
childState.StateGasRefundAdvanced = 0;
718+
childState.StateGasSpillRefundAdvanced = 0;
716719
}
717720
}
718721

@@ -721,10 +724,11 @@ private static void RemoveAdvancedStateGasRefund(VmState<TGasPolicy> vmState, re
721724
{
722725
if (vmState.StateGasRefundAdvanced > 0)
723726
{
724-
TGasPolicy.RemoveStateGasRefundFromReservoir(ref gas, vmState.StateGasRefundAdvanced);
727+
TGasPolicy.RemoveStateGasRefundFromReservoir(ref gas, vmState.StateGasRefundAdvanced, vmState.StateGasSpillRefundAdvanced);
725728
vmState.StateGasRefundAdvanced = 0;
726729
}
727730

731+
vmState.StateGasSpillRefundAdvanced = 0;
728732
vmState.StateGasRefundPending = 0;
729733
}
730734

src/Nethermind/Nethermind.Evm/VmState.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ private static readonly
3737
// State-gas refund already made spendable in this frame while its accounting correction
3838
// still has to reach the ancestor frame that originally paid the state gas.
3939
public long StateGasRefundAdvanced;
40+
public long StateGasSpillRefundAdvanced;
4041
internal long OutputDestination { get; private set; } // TODO: move to CallEnv
4142
internal long OutputLength { get; private set; } // TODO: move to CallEnv
4243
public long Refund { get; set; }
@@ -156,10 +157,12 @@ private void Initialize(
156157
_accessTracker.TakeSnapshot();
157158
Debug.Assert(StateGasRefundPending == 0, "Pooled VmState returned with uncleared StateGasRefundPending.");
158159
Debug.Assert(StateGasRefundAdvanced == 0, "Pooled VmState returned with uncleared StateGasRefundAdvanced.");
160+
Debug.Assert(StateGasSpillRefundAdvanced == 0, "Pooled VmState returned with uncleared StateGasSpillRefundAdvanced.");
159161
Gas = gas;
160162
InitialStateGasUsed = TGasPolicy.GetStateGasUsed(in gas);
161163
StateGasRefundPending = 0;
162164
StateGasRefundAdvanced = 0;
165+
StateGasSpillRefundAdvanced = 0;
163166
OutputDestination = outputDestination;
164167
OutputLength = outputLength;
165168
Refund = 0;
@@ -230,6 +233,7 @@ public void Dispose()
230233
_snapshot = default;
231234
StateGasRefundPending = 0;
232235
StateGasRefundAdvanced = 0;
236+
StateGasSpillRefundAdvanced = 0;
233237

234238
_statePool.Enqueue(this);
235239

0 commit comments

Comments
 (0)