Skip to content

Commit d900d77

Browse files
fix(pr-review): address wurdum's PR 700 review comments
- Simplify GrowBacklog call by removing switch statement (comment 2) GrowBacklog internally routes based on GetGasModelToUse() - Split GetGasModelToUse test into 3 focused tests (comment 3) - Remove #region/#endregion blocks from L2PricingStateTests (comment 4) - Delete PrecompileContextTests.cs with duplicate tests (comment 5) feat(tests): add multi-gas refund integration tests Port tests mirroring Nitro's tx_processor_multigas_test.go: - MultiGasRefund_WhenExpensiveResourceNotUsed_ProducesPositiveRefund - MultiGasRefund_ForRetryableTx_RefundToAddressReceivesRefund - MultiGasRefund_WhenAllGasIsComputation_NoRefund These verify multi-gas refund calculation for e2e compatibility.
1 parent 1f4cc4e commit d900d77

5 files changed

Lines changed: 219 additions & 186 deletions

File tree

src/Nethermind

Submodule Nethermind updated 380 files

src/Nethermind.Arbitrum.Test/Arbos/Storage/L2PricingStateTests.cs

Lines changed: 39 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -673,55 +673,54 @@ public void ShrinkBacklog_MultiGasConstraints_ReducesWeightedBacklog()
673673
}
674674

675675
[Test]
676-
public void GetGasModelToUse_BasedOnVersionAndConstraints_ReturnsCorrectModel()
676+
public void GetGasModelToUse_WhenArbOS49_ReturnsLegacy()
677677
{
678-
// Legacy model (v49, no constraints)
679-
{
680-
IWorldState worldState = TestWorldStateFactory.CreateForTest();
681-
using IDisposable disposer = worldState.BeginScope(IWorldState.PreGenesis);
682-
_ = ArbOSInitialization.Create(worldState);
678+
IWorldState worldState = TestWorldStateFactory.CreateForTest();
679+
using IDisposable disposer = worldState.BeginScope(IWorldState.PreGenesis);
680+
_ = ArbOSInitialization.Create(worldState);
683681

684-
PrecompileTestContextBuilder context = new PrecompileTestContextBuilder(worldState, ulong.MaxValue)
685-
.WithArbosState()
686-
.WithArbosVersion(ArbosVersion.FortyNine)
687-
.WithReleaseSpec();
682+
PrecompileTestContextBuilder context = new PrecompileTestContextBuilder(worldState, ulong.MaxValue)
683+
.WithArbosState()
684+
.WithArbosVersion(ArbosVersion.FortyNine)
685+
.WithReleaseSpec();
688686

689-
context.ArbosState.L2PricingState.GetGasModelToUse().Should().Be(GasModel.Legacy);
690-
}
687+
context.ArbosState.L2PricingState.GetGasModelToUse().Should().Be(GasModel.Legacy);
688+
}
691689

692-
// Single constraints model (v50 with single-gas constraints)
693-
{
694-
IWorldState worldState = TestWorldStateFactory.CreateForTest();
695-
using IDisposable disposer = worldState.BeginScope(IWorldState.PreGenesis);
696-
_ = ArbOSInitialization.Create(worldState);
690+
[Test]
691+
public void GetGasModelToUse_WhenArbOS50WithConstraints_ReturnsSingleGasConstraints()
692+
{
693+
IWorldState worldState = TestWorldStateFactory.CreateForTest();
694+
using IDisposable disposer = worldState.BeginScope(IWorldState.PreGenesis);
695+
_ = ArbOSInitialization.Create(worldState);
697696

698-
PrecompileTestContextBuilder context = new PrecompileTestContextBuilder(worldState, ulong.MaxValue)
699-
.WithArbosState()
700-
.WithArbosVersion(ArbosVersion.Fifty)
701-
.WithReleaseSpec();
697+
PrecompileTestContextBuilder context = new PrecompileTestContextBuilder(worldState, ulong.MaxValue)
698+
.WithArbosState()
699+
.WithArbosVersion(ArbosVersion.Fifty)
700+
.WithReleaseSpec();
702701

703-
context.ArbosState.L2PricingState.AddConstraint(7_000_000, 60, 0);
704-
context.ArbosState.L2PricingState.GetGasModelToUse().Should().Be(GasModel.SingleGasConstraints);
705-
}
702+
context.ArbosState.L2PricingState.AddConstraint(7_000_000, 60, 0);
703+
context.ArbosState.L2PricingState.GetGasModelToUse().Should().Be(GasModel.SingleGasConstraints);
704+
}
706705

707-
// Multi-gas constraints model (v60 with multi-gas constraints)
708-
{
709-
IWorldState worldState = TestWorldStateFactory.CreateForTest();
710-
using IDisposable disposer = worldState.BeginScope(IWorldState.PreGenesis);
711-
_ = ArbOSInitialization.Create(worldState);
706+
[Test]
707+
public void GetGasModelToUse_WhenArbOS60WithMultiGasConstraints_ReturnsMultiGasConstraints()
708+
{
709+
IWorldState worldState = TestWorldStateFactory.CreateForTest();
710+
using IDisposable disposer = worldState.BeginScope(IWorldState.PreGenesis);
711+
_ = ArbOSInitialization.Create(worldState);
712712

713-
PrecompileTestContextBuilder context = new PrecompileTestContextBuilder(worldState, ulong.MaxValue)
714-
.WithArbosState()
715-
.WithArbosVersion(ArbosVersion.Sixty)
716-
.WithReleaseSpec();
713+
PrecompileTestContextBuilder context = new PrecompileTestContextBuilder(worldState, ulong.MaxValue)
714+
.WithArbosState()
715+
.WithArbosVersion(ArbosVersion.Sixty)
716+
.WithReleaseSpec();
717717

718-
Dictionary<ResourceKind, ulong> weights = new()
719-
{
720-
{ ResourceKind.Computation, 1 },
721-
};
722-
context.ArbosState.L2PricingState.AddMultiGasConstraint(7_000_000, 60, 0, weights);
723-
context.ArbosState.L2PricingState.GetGasModelToUse().Should().Be(GasModel.MultiGasConstraints);
724-
}
718+
Dictionary<ResourceKind, ulong> weights = new()
719+
{
720+
{ ResourceKind.Computation, 1 },
721+
};
722+
context.ArbosState.L2PricingState.AddMultiGasConstraint(7_000_000, 60, 0, weights);
723+
context.ArbosState.L2PricingState.GetGasModelToUse().Should().Be(GasModel.MultiGasConstraints);
725724
}
726725

727726
[Test]
@@ -785,7 +784,6 @@ public void GasPoolUpdateCost_ArbOS51WithConstraints_ReturnsCorrectCost()
785784
l2Pricing.GasPoolUpdateCost().Should().Be(constraintCost);
786785
}
787786

788-
#region Coverage: Exception Guards
789787

790788
[Test]
791789
public void CalcMultiGasConstraintsExponents_DivisorZero_ThrowsException()
@@ -841,9 +839,7 @@ public void UpdatePricingModelMultiConstraints_DivisorZero_ThrowsException()
841839
.WithMessage("*divisor is zero*");
842840
}
843841

844-
#endregion
845842

846-
#region Coverage: Migration Path
847843

848844
[Test]
849845
public void SetMultiGasConstraintsFromSingleGasConstraints_WhenCalled_ConvertsCorrectly()
@@ -958,9 +954,7 @@ public void SetMultiGasConstraintsFromSingleGasConstraints_WhenExistingConstrain
958954
mc.MaxWeight.Should().Be(1); // Not 5 from the old constraint
959955
}
960956

961-
#endregion
962957

963-
#region Coverage: Legacy Pricing Paths
964958

965959
[Test]
966960
public void GasPoolUpdateCost_ArbOS50_ReturnsCorrectCost()
@@ -1133,9 +1127,7 @@ public void ShrinkBacklog_SingleGasConstraints_ReducesAllConstraintBacklogs()
11331127
l2Pricing.OpenConstraintAt(1).Backlog.Should().Be(1700);
11341128
}
11351129

1136-
#endregion
11371130

1138-
#region Coverage: Multi-Gas Fees and Base Fee Calculation
11391131

11401132
[Test]
11411133
public void CommitMultiGasFees_WhenNotMultiGasConstraints_DoesNothing()
@@ -1335,5 +1327,4 @@ public void UpdatePricingModelMultiGasConstraints_ZeroExponent_UsesMinBaseFee()
13351327
l2Pricing.MultiGasFees.GetNextBlockFee(ResourceKind.Computation).Should().Be(minPrice);
13361328
}
13371329

1338-
#endregion
13391330
}

src/Nethermind.Arbitrum.Test/Execution/TransactionProcessorMultiGasTests.cs

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,18 @@
33

44
using FluentAssertions;
55
using Nethermind.Arbitrum.Arbos;
6+
using Nethermind.Arbitrum.Arbos.Storage;
67
using Nethermind.Arbitrum.Evm;
78
using Nethermind.Arbitrum.Execution;
89
using Nethermind.Arbitrum.Test.Infrastructure;
910
using Nethermind.Core;
11+
using Nethermind.Core.Test;
1012
using Nethermind.Core.Test.Builders;
1113
using Nethermind.Evm;
1214
using Nethermind.Evm.State;
1315
using Nethermind.Evm.Test;
1416
using Nethermind.Evm.TransactionProcessing;
17+
using Nethermind.Int256;
1518

1619
namespace Nethermind.Arbitrum.Test.Execution;
1720

@@ -115,4 +118,179 @@ public void Execute_SufficientGas_ChargesPosterGasAsL1Calldata()
115118

116119
gas.Get(ResourceKind.L1Calldata).Should().BeGreaterThan(0UL, "expected L1Calldata > 0");
117120
}
121+
122+
/// <summary>
123+
/// Tests multi-gas refund calculation for normal transactions.
124+
/// Mirrors Nitro's TestEndTxHookMultiGasRefundNormalTx from tx_processor_multigas_test.go.
125+
///
126+
/// When multi-gas constraints make certain resources expensive (e.g., StorageGrowth weight=10),
127+
/// transactions that don't use those expensive resources should have multiDimensionalCost less than
128+
/// singleGasCost, resulting in a positive refund.
129+
/// </summary>
130+
[Test]
131+
public void MultiGasRefund_WhenExpensiveResourceNotUsed_ProducesPositiveRefund()
132+
{
133+
IWorldState worldState = TestWorldStateFactory.CreateForTest();
134+
using IDisposable disposer = worldState.BeginScope(IWorldState.PreGenesis);
135+
_ = ArbOSInitialization.Create(worldState);
136+
137+
PrecompileTestContextBuilder context = new PrecompileTestContextBuilder(worldState, ulong.MaxValue)
138+
.WithArbosState()
139+
.WithArbosVersion(ArbosVersion.Sixty)
140+
.WithReleaseSpec();
141+
142+
L2PricingState l2Pricing = context.ArbosState.L2PricingState;
143+
144+
// Set up constraint that makes StorageGrowth expensive (weight=10) vs Computation (weight=1)
145+
// This mirrors Nitro's test setup where StorageGrowth is heavily constrained
146+
Dictionary<ResourceKind, ulong> weights = new()
147+
{
148+
{ ResourceKind.Computation, 1 },
149+
{ ResourceKind.StorageGrowth, 10 },
150+
};
151+
152+
// target=100000, window=10, backlog=200_000_000_000 (high backlog to elevate prices)
153+
l2Pricing.AddMultiGasConstraint(100000, 10, 200_000_000_000, weights);
154+
155+
// Update pricing model to produce different per-resource fees
156+
l2Pricing.UpdatePricingModel(100);
157+
l2Pricing.CommitMultiGasFees();
158+
159+
UInt256 baseFee = l2Pricing.BaseFeeWeiStorage.Get();
160+
baseFee.Should().BeGreaterThan(UInt256.Zero, "baseFee should be elevated due to high backlog");
161+
162+
// Simulate transaction that uses only Computation and StorageAccess (NOT StorageGrowth)
163+
// This means the expensive resource (StorageGrowth) is not used
164+
const ulong gasUsed = 1_000_000;
165+
MultiGas usedMultiGas = default;
166+
usedMultiGas.Increment(ResourceKind.Computation, gasUsed / 2);
167+
usedMultiGas.Increment(ResourceKind.StorageAccess, gasUsed / 2);
168+
169+
// Calculate costs
170+
UInt256 singleGasCost = baseFee * gasUsed;
171+
UInt256 multiDimensionalCost = l2Pricing.MultiDimensionalPriceForRefund(usedMultiGas);
172+
173+
// Since StorageGrowth (the expensive resource) wasn't used, multiDimensionalCost should be less
174+
multiDimensionalCost.Should().BeLessThan(singleGasCost,
175+
"multiDimensionalCost should be less than singleGasCost when expensive resources aren't used");
176+
177+
// Calculate refund
178+
UInt256 expectedRefund = singleGasCost - multiDimensionalCost;
179+
expectedRefund.Should().BeGreaterThan(UInt256.Zero,
180+
"refund should be positive when multiDimensionalCost < singleGasCost");
181+
}
182+
183+
/// <summary>
184+
/// Tests multi-gas refund calculation for retryable transactions.
185+
/// Mirrors Nitro's TestEndTxHookMultiGasRefundRetryableTx from tx_processor_multigas_test.go.
186+
///
187+
/// For retryable transactions, the refund goes to RefundTo address (up to MaxRefund),
188+
/// with any excess going to the sender.
189+
/// </summary>
190+
[Test]
191+
public void MultiGasRefund_ForRetryableTx_RefundToAddressReceivesRefund()
192+
{
193+
IWorldState worldState = TestWorldStateFactory.CreateForTest();
194+
using IDisposable disposer = worldState.BeginScope(IWorldState.PreGenesis);
195+
_ = ArbOSInitialization.Create(worldState);
196+
197+
PrecompileTestContextBuilder context = new PrecompileTestContextBuilder(worldState, ulong.MaxValue)
198+
.WithArbosState()
199+
.WithArbosVersion(ArbosVersion.Sixty)
200+
.WithReleaseSpec();
201+
202+
L2PricingState l2Pricing = context.ArbosState.L2PricingState;
203+
204+
// Set up constraint that makes StorageGrowth expensive
205+
Dictionary<ResourceKind, ulong> weights = new()
206+
{
207+
{ ResourceKind.Computation, 1 },
208+
{ ResourceKind.StorageGrowth, 10 },
209+
};
210+
l2Pricing.AddMultiGasConstraint(100000, 10, 200_000_000_000, weights);
211+
212+
// Update pricing model
213+
l2Pricing.UpdatePricingModel(100);
214+
l2Pricing.CommitMultiGasFees();
215+
216+
UInt256 baseFee = l2Pricing.BaseFeeWeiStorage.Get();
217+
218+
// Simulate retryable transaction gas usage (similar to normal tx)
219+
const ulong gasUsed = 1_000_000;
220+
MultiGas usedMultiGas = default;
221+
usedMultiGas.Increment(ResourceKind.Computation, gasUsed / 2);
222+
usedMultiGas.Increment(ResourceKind.StorageAccess, gasUsed / 2);
223+
224+
// For retryables, gasFeeCap is used as the simple gas price
225+
UInt256 gasFeeCap = baseFee;
226+
UInt256 simpleGasCost = gasFeeCap * gasUsed;
227+
UInt256 multiDimensionalCost = l2Pricing.MultiDimensionalPriceForRefund(usedMultiGas);
228+
229+
UInt256 expectedRefund = simpleGasCost - multiDimensionalCost;
230+
expectedRefund.Should().BeGreaterThan(UInt256.Zero,
231+
"retryable tx should have positive refund when expensive resources aren't used");
232+
233+
// Verify refund distribution logic:
234+
// If MaxRefund >= expectedRefund: entire refund goes to RefundTo
235+
// If MaxRefund < expectedRefund: MaxRefund to RefundTo, remainder to From
236+
UInt256 maxRefund = expectedRefund * 10; // Large MaxRefund
237+
UInt256 toRefundTo = UInt256.Min(expectedRefund, maxRefund);
238+
UInt256 toFrom = expectedRefund - toRefundTo;
239+
240+
toRefundTo.Should().Be(expectedRefund, "with large MaxRefund, entire refund goes to RefundTo");
241+
toFrom.Should().Be(UInt256.Zero, "with large MaxRefund, nothing goes to From");
242+
243+
// Test with small MaxRefund
244+
UInt256 smallMaxRefund = expectedRefund / 2;
245+
UInt256 toRefundToSmall = UInt256.Min(expectedRefund, smallMaxRefund);
246+
UInt256 toFromSmall = expectedRefund - toRefundToSmall;
247+
248+
toRefundToSmall.Should().Be(smallMaxRefund, "with small MaxRefund, only MaxRefund goes to RefundTo");
249+
toFromSmall.Should().Be(expectedRefund - smallMaxRefund, "remainder goes to From");
250+
}
251+
252+
/// <summary>
253+
/// Tests that when all gas goes to computation only, there's no refund
254+
/// (multiDimensionalCost equals singleGasCost).
255+
/// </summary>
256+
[Test]
257+
public void MultiGasRefund_WhenAllGasIsComputation_NoRefund()
258+
{
259+
IWorldState worldState = TestWorldStateFactory.CreateForTest();
260+
using IDisposable disposer = worldState.BeginScope(IWorldState.PreGenesis);
261+
_ = ArbOSInitialization.Create(worldState);
262+
263+
PrecompileTestContextBuilder context = new PrecompileTestContextBuilder(worldState, ulong.MaxValue)
264+
.WithArbosState()
265+
.WithArbosVersion(ArbosVersion.Sixty)
266+
.WithReleaseSpec();
267+
268+
L2PricingState l2Pricing = context.ArbosState.L2PricingState;
269+
270+
// Set up constraint with only Computation (weight=1)
271+
Dictionary<ResourceKind, ulong> weights = new()
272+
{
273+
{ ResourceKind.Computation, 1 },
274+
};
275+
l2Pricing.AddMultiGasConstraint(7_000_000, 60, 0, weights);
276+
277+
// Use the initialized base fee from state (0.1 gwei = 100_000_000 wei)
278+
UInt256 baseFee = l2Pricing.BaseFeeWeiStorage.Get();
279+
280+
// Set multi-gas base fee for computation to match
281+
l2Pricing.SetNextBlockMultiGasBaseFee(ResourceKind.Computation, baseFee);
282+
l2Pricing.CommitMultiGasFees();
283+
284+
// All gas goes to Computation
285+
const ulong gasUsed = 100_000;
286+
MultiGas usedMultiGas = default;
287+
usedMultiGas.Increment(ResourceKind.Computation, gasUsed);
288+
289+
UInt256 singleGasCost = baseFee * gasUsed;
290+
UInt256 multiDimensionalCost = l2Pricing.MultiDimensionalPriceForRefund(usedMultiGas);
291+
292+
// When all gas is computation at same base fee, costs should be equal
293+
multiDimensionalCost.Should().Be(singleGasCost,
294+
"when all gas is computation at base fee, no refund should occur");
295+
}
118296
}

0 commit comments

Comments
 (0)