Stop charging gas for retryable redeem gas pool update on ArbOS 60+ #4101

MishkaRogachev · 2025-12-02T16:47:44Z

Fixes NIT-4152
pulls in OffchainLabs/go-ethereum#599

Changes:

Add WithUnmeteredGasAccounting to the precompile context to ignore gas charges within the ShrinkBacklog call.
Parametrise Retryable-Redeem test scenarios to prove behaviour for different ArbOS versions

github-actions · 2025-12-02T17:16:35Z

❌ 4 Tests Failed:

Tests completed	Failed	Passed	Skipped
4451	4	4447	0

View the top 3 failed tests by shortest run time

TestSequencerPriceAdjustsFrom10Gwei

Stack Traces | 37.860s run time

=== RUN   TestSequencerPriceAdjustsFrom10Gwei
=== PAUSE TestSequencerPriceAdjustsFrom10Gwei
=== CONT  TestSequencerPriceAdjustsFrom10Gwei
    common_test.go:711: BuildL1 deployConfig: DeployBold=true, DeployReferenceDAContracts=false
    fees_test.go:253: �[31;1m [batch count didn't update in time] �[0;0m
--- FAIL: TestSequencerPriceAdjustsFrom10Gwei (37.86s)

TestOutOfGasInStorageCacheFlush

Stack Traces | 52.930s run time

... [CONTENT TRUNCATED: Keeping last 20 lines]
    program_test.go:2834: goroutine 844055 [running]:
        runtime/debug.Stack()
        	/opt/hostedtoolcache/go/1.25.5/x64/src/runtime/debug/stack.go:26 +0x5e
        github.com/offchainlabs/nitro/util/testhelpers.RequireImpl({0x40fdbd0, 0xc0218fe8c0}, {0x40ba9c0, 0xc2894e5e40}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/util/testhelpers/testhelpers.go:29 +0x55
        github.com/offchainlabs/nitro/system_tests.Require(0xc0218fe8c0, {0x40ba9c0, 0xc2894e5e40}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/system_tests/common_test.go:2034 +0x5d
        github.com/offchainlabs/nitro/system_tests.TestOutOfGasInStorageCacheFlush(0xc0218fe8c0)
        	/home/runner/work/nitro/nitro/system_tests/program_test.go:2834 +0x1006
        testing.tRunner(0xc0218fe8c0, 0x3d3c260)
        	/opt/hostedtoolcache/go/1.25.5/x64/src/testing/testing.go:1934 +0xea
        created by testing.(*T).Run in goroutine 1
        	/opt/hostedtoolcache/go/1.25.5/x64/src/testing/testing.go:1997 +0x465
        
    program_test.go:2834: �[31;1m [] failed calculating position for validation: batch not found on L1 yet �[0;0m
ERROR[12-24|16:08:08.249] Dangling trie nodes after full cleanup
�[38;5;48;1myay!! we validated block 25 in 510.14ms�[0;0m
INFO [12-24|16:08:08.410] HTTP server stopped                      endpoint=127.0.0.1:34415
TRACE[12-24|16:08:08.410] P2P networking is spinning down
--- FAIL: TestOutOfGasInStorageCacheFlush (52.93s)

TestBatchPosterPostsReportOnlyBatchAfterMaxEmptyBatchDelay

Stack Traces | 57.170s run time

=== RUN   TestBatchPosterPostsReportOnlyBatchAfterMaxEmptyBatchDelay
=== PAUSE TestBatchPosterPostsReportOnlyBatchAfterMaxEmptyBatchDelay
=== CONT  TestBatchPosterPostsReportOnlyBatchAfterMaxEmptyBatchDelay
    common_test.go:711: BuildL1 deployConfig: DeployBold=true, DeployReferenceDAContracts=false
    batch_poster_test.go:963: 
        	Error Trace:	/home/runner/work/nitro/nitro/system_tests/batch_poster_test.go:963
        	Error:      	Should be true
        	Test:       	TestBatchPosterPostsReportOnlyBatchAfterMaxEmptyBatchDelay
        	Messages:   	expected second batch to be posted by MaxEmptyBatchDelay
--- FAIL: TestBatchPosterPostsReportOnlyBatchAfterMaxEmptyBatchDelay (57.17s)

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

gligneul

Looks good, just minor comments.

arbos/l2pricing/model.go

precompiles/ArbOwner.go

tsahee · 2025-12-10T23:03:21Z

precompiles/ArbRetryableTx_test.go

 }

-func TestRetryableRedeem(t *testing.T) {
+func overrideStateArbOSVersion(evm *vm.EVM, arbosVersion uint64) error {


instead of that, use newMockEVMForTestingWithVersion

I tried it, but it does not work because of the way the arbos version is obtained here. Probably it could be fixed by presetting the version here. I suggest to open a separate ticket for it

I don't follw. If newMockEVMForTestingWithVersion needs to be fixed, let's fix it. Let's talk about it later.

tsahee · 2025-12-10T23:07:34Z

precompiles/ArbRetryableTx.go

+	// L2PricingState().ShrinkBacklog(). BacklogUpdateCost() returns
+	// that amount based on the storage read/write mix used by ShrinkBacklog().
+	backlogUpdateCost := uint64(0)
+	if c.State.L2PricingState().ArbosVersion >= params.ArbosVersion_60 {


in all of those this shouldn't be ArbosVersion60, but arbosVersion_RedeemChargingFix or something similar.
So we could easily change the version in which this applies (can be a number only after it's issued)

I fixed it to ArbosVersion_MultiGasConstraintsVersion to exclude the possibility of an intermediate state under which there is no implementation.

tsahee · 2025-12-10T23:13:38Z

precompiles/ArbRetryableTx.go

+	backlogUpdateCost := uint64(0)
+	if c.State.L2PricingState().ArbosVersion >= params.ArbosVersion_60 {
+		// Charge static price for any pricer starting from ArbOS 60
+		backlogUpdateCost = l2pricing.ArbOS60StaticBacklogUpdateCost


why not just add that logic into L2PricingState().BacklogUpdateCost() ?

tsahee · 2025-12-10T23:30:24Z

precompiles/ArbRetryableTx.go

-	return retryTxHash, c.State.L2PricingState().ShrinkBacklog(gasToDonate, multigas.ComputationGas(gasToDonate))
+	// Starting from ArbOS 60, we don't charge gas for the ShrinkBacklog call here
+	if c.State.L2PricingState().ArbosVersion >= params.ArbosVersion_60 {
+		err = c.WithUnmeteredGasAccounting(func() error {


I get the idea and kind of like it.. however, since the context only exists for a specific call and will be removed once you return from this function (it is created in "func (p *Precompile) Call"), I think this adds more complexity then safety, and it'll probably be simpler to just have a context.StopCollectingGas() function or something similar. This will also allow you to reuse the call to ShrinkBacklog between the options and help us in case we do anything else in the end of the function make sure we don't spend (and need to allocate) more gas.

I agree, but there is one nuance: after the method completes, the context is still in use, and I see a difference of 3 gas units if I leave it free. I corrected it to a compromise option with defer c.SetUnmeteredGasAccounting(false)

tsahee · 2025-12-18T03:53:16Z

precompiles/ArbOwner.go


-	if c.State.ArbOSVersion() >= params.ArbosVersion_MultiConstraintFix {
+	arbosVersion := c.State.ArbOSVersion()
+	if arbosVersion >= params.ArbosVersion_MultiConstraintFix && arbosVersion < params.ArbosVersion_60 {


this should be ArbosVersion_MultiGasConstraintsVersion instead of ArbosVersion_60

tsahee · 2025-12-18T03:54:39Z

precompiles/ArbOwner.go

 	evm mech,
 	constraints []MultiGasConstraint,
 ) error {
-	limit := l2pricing.MultiGasConstraintsMaxNum


better to make this depend on arbosVersion then remove entirely (I think this check won't spend gas so won't be a problem to add)

Removing this check was a desirable change, as we only introduced it to avoid breaking retyrable-redeem. We did not release a version with this code yet, so I believe it's safe to remove it. (There is also similar check for signle-gas constraints, but this one should be kept)

tsahee · 2025-12-18T04:07:35Z

precompiles/ArbRetryableTx_test.go

 }

-func TestRetryableRedeem(t *testing.T) {
+func overrideStateArbOSVersion(evm *vm.EVM, arbosVersion uint64) error {


I don't follw. If newMockEVMForTestingWithVersion needs to be fixed, let's fix it. Let's talk about it later.

codecov · 2025-12-22T10:43:35Z

Codecov Report

❌ Patch coverage is 10.71429% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.95%. Comparing base (ba80222) to head (e458d6d).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4101      +/-   ##
==========================================
- Coverage   33.25%   32.95%   -0.30%     
==========================================
  Files         459      459              
  Lines       55826    55835       +9     
==========================================
- Hits        18567    18403     -164     
- Misses      34015    34210     +195     
+ Partials     3244     3222      -22

MishkaRogachev · 2025-12-23T16:20:53Z

precompiles/context.go

 }

+func testContextWithVersion(caller addr, evm mech, version uint64) *Context {
+	versionOffset := uint64(0)


Initially I put this version present into testContext, but there is at least one test TestArbInfraFeeAccount which assumes arbosState starts with initial version, so I believe testContextWithVersion is a better fit for retryable edge case

MishkaRogachev force-pushed the revisit-retryable-redeem-gas-budgeting-fro-arbos-60 branch 3 times, most recently from 4fa5689 to 85e2c9c Compare December 3, 2025 11:43

MishkaRogachev changed the title ~~WIP: considering free storage access from precompile context~~ Stop charging gas for retryable redeem gas pool update on ArbOS 60+ Dec 3, 2025

MishkaRogachev force-pushed the revisit-retryable-redeem-gas-budgeting-fro-arbos-60 branch 3 times, most recently from 32ca70c to 2bc088b Compare December 5, 2025 11:56

MishkaRogachev marked this pull request as ready for review December 5, 2025 12:29

MishkaRogachev requested review from eljobe, gligneul and tsahee December 5, 2025 12:29

MishkaRogachev assigned gligneul Dec 5, 2025

gligneul requested changes Dec 5, 2025

View reviewed changes

arbos/l2pricing/model.go Show resolved Hide resolved

precompiles/ArbOwner.go Outdated Show resolved Hide resolved

gligneul assigned MishkaRogachev and unassigned gligneul Dec 5, 2025

gligneul previously approved these changes Dec 8, 2025

View reviewed changes

MishkaRogachev assigned tsahee and unassigned MishkaRogachev Dec 8, 2025

MishkaRogachev force-pushed the revisit-retryable-redeem-gas-budgeting-fro-arbos-60 branch 2 times, most recently from 749e9c7 to 29a5c43 Compare December 9, 2025 10:16

tsahee requested changes Dec 10, 2025

View reviewed changes

tsahee assigned MishkaRogachev and unassigned tsahee Dec 10, 2025

MishkaRogachev mentioned this pull request Dec 11, 2025

WIP: Free storage slots for pricing constraints #4104

Closed

MishkaRogachev dismissed gligneul’s stale review via 48b4998 December 11, 2025 14:48

MishkaRogachev force-pushed the revisit-retryable-redeem-gas-budgeting-fro-arbos-60 branch from 29a5c43 to 48b4998 Compare December 11, 2025 14:48

MishkaRogachev mentioned this pull request Dec 11, 2025

params: Move constraints arbos versions to config_arbitrum OffchainLabs/go-ethereum#599

Open

MishkaRogachev assigned tsahee and unassigned MishkaRogachev Dec 11, 2025

MishkaRogachev requested review from gligneul and tsahee December 11, 2025 17:41

MishkaRogachev force-pushed the revisit-retryable-redeem-gas-budgeting-fro-arbos-60 branch from 48b4998 to 789b552 Compare December 12, 2025 16:51

gligneul previously approved these changes Dec 12, 2025

View reviewed changes

tsahee requested changes Dec 18, 2025

View reviewed changes

tsahee assigned MishkaRogachev and unassigned tsahee Dec 18, 2025

MishkaRogachev dismissed gligneul’s stale review via 35ce79a December 22, 2025 10:27

MishkaRogachev force-pushed the revisit-retryable-redeem-gas-budgeting-fro-arbos-60 branch 3 times, most recently from 1ddc554 to 020720a Compare December 22, 2025 10:31

MishkaRogachev force-pushed the revisit-retryable-redeem-gas-budgeting-fro-arbos-60 branch 4 times, most recently from bf4cf7d to b6f8080 Compare December 22, 2025 17:37

MishkaRogachev assigned tsahee and unassigned MishkaRogachev Dec 23, 2025

MishkaRogachev requested a review from tsahee December 23, 2025 12:29

MishkaRogachev commented Dec 23, 2025

View reviewed changes

MishkaRogachev added 9 commits December 24, 2025 16:40

stop charging gas for retryable redeem gas pool update on ArbOS 60+

15110ab

Add static gas update cost for ArbOS60

6b444f4

Correct BacklogUpdateCost for ArbOs 60

63a92c4

Remove count limit for gas constraints

24bf507

Improve retryable tests and return back GasConstraintsMaxNum

b1ebf85

Fix TestMultiGasConstraintsStorage order-dependent assertions

a9f6e6b

Review fixes: support old behabiour in SetGasPricingConstraints

9b94a85

Review fixes: corect arbos version, simplify redeem gas accounting

4e6cba6

Add changelog and move arbos version preset to testContext

e458d6d

MishkaRogachev force-pushed the revisit-retryable-redeem-gas-budgeting-fro-arbos-60 branch from 6f44f27 to e458d6d Compare December 24, 2025 15:47

Stop charging gas for retryable redeem gas pool update on ArbOS 60+ #4101

Are you sure you want to change the base?

Stop charging gas for retryable redeem gas pool update on ArbOS 60+ #4101

Conversation

MishkaRogachev commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 4 Tests Failed:

Uh oh!

gligneul left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MishkaRogachev Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MishkaRogachev commented Dec 2, 2025 •

edited

Loading

github-actions bot commented Dec 2, 2025 •

edited

Loading

codecov bot commented Dec 22, 2025 •

edited

Loading

MishkaRogachev Dec 23, 2025 •

edited

Loading