Fix nil batchFetcher dereference by joshuacolvin0 · Pull Request #4540 · OffchainLabs/nitro

joshuacolvin0 · 2026-03-23T03:44:40Z

Fix nil pointer dereference in FillInBatchGasFields when batchFetcher is nil and the message kind is BatchPostingReport.

On master, FillInBatchGasFields passes a nil batchFetcher to FromFallibleBatchFetcher, which wraps it in a non-nil closure that bypasses the downstream nil check and panics when called. This PR:

Returns an explicit error (ErrNilBatchFetcher) when batchFetcher is nil for BatchPostingReport messages, instead of silently leaving gas fields unpopulated
Passes real batchFetchers at call sites in inbox_tracker.go (via ParseIncomingL1Message) and transaction_streamer.go (reorg-resequence path) that previously passed nil
Adds sentinel errors (ErrNilBatchFetcher, ErrBatchHashMismatch, ErrParseBatchPostingReport) with %w wrapping for programmatic error checking
Adds warning/error logs for previously silent error paths: replay binary FillInBatchGasFields failure, GetMessage without inboxReader for BatchPostingReport, and corrects a misleading reorg log message
Provides a stub batchFetcher in init.go to satisfy the non-nil contract (only Initialize messages are needed at the deployment block)
Adds 22 unit tests covering nil fetcher, hash mismatch, truncated L2msg, fetcher errors with/without legacy cost, idempotency, empty batch data, and all message kinds

No consensus changes — the replay binary behavior is identical (batchFetcher is always non-nil there, and error-vs-no-error conditions are unchanged).

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

… is nil Guard against nil batchFetcher in FillInBatchGasFields before passing it to FromFallibleBatchFetcher, which wraps a nil function in a non-nil closure that bypasses the downstream nil check and panics when called. Also skip FillInBatchGasFields in ParseIncomingL1Message when batchFetcher is nil. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

codecov · 2026-03-23T03:58:11Z

Codecov Report

❌ Patch coverage is 33.33333% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.33%. Comparing base (5b41808) to head (57fa161).
⚠️ Report is 23 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4540      +/-   ##
==========================================
+ Coverage   34.52%   35.33%   +0.80%     
==========================================
  Files         498      498              
  Lines       59092    59107      +15     
==========================================
+ Hits        20400    20883     +483     
+ Misses      35067    34583     -484     
- Partials     3625     3641      +16

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions · 2026-03-23T04:30:48Z

❌ 23 Tests Failed:

Tests completed	Failed	Passed	Skipped
4745	23	4722	0

View the top 3 failed tests by shortest run time

TestAliasingFlaky

Stack Traces | -0.000s run time

... [CONTENT TRUNCATED: Keeping last 20 lines]
INFO [03-31|00:01:59.296] Persisted trie from memory database      nodes=25  flushnodes=0 size=4.56KiB  flushsize=0.00B time="178.765µs" flushtime=0s gcnodes=0 gcsize=0.00B gctime=0s          livenodes=68  livesize=12.57KiB
INFO [03-31|00:01:59.296] Writing snapshot state to disk           root=77ae46..2fbcae
INFO [03-31|00:01:59.296] Persisted trie from memory database      nodes=0   flushnodes=0 size=0.00B    flushsize=0.00B time="1.973µs"   flushtime=0s gcnodes=0 gcsize=0.00B gctime=0s          livenodes=68  livesize=12.57KiB
INFO [03-31|00:01:59.296] Submitted transaction                    hash=0x520e319193f7a349f4f2c3fe3cddabc25326058f4d7e2ce11865daa0683351e3 from=0xb386a74Dcab67b66F8AC07B4f08365d37495Dd23 nonce=4  recipient=0xEC73E2Ae7770565C91B6B7031437AeCEfDdC0686 value=0
INFO [03-31|00:01:59.297] Blockchain stopped
INFO [03-31|00:01:59.297] Imported new potential chain segment     number=45 hash=6e53e9..40a27e blocks=1  txs=0  mgas=0.000 elapsed="314.319µs" mgasps=0.000    triediffs=220.18KiB triedirty=0.00B
INFO [03-31|00:01:59.297] Starting work on payload                 id=0x037b4b04815b8bf6
INFO [03-31|00:01:59.297] Ethereum protocol stopped
INFO [03-31|00:01:59.297] Transaction pool stopped
INFO [03-31|00:01:59.297] Chain head was updated                   number=45 hash=6e53e9..40a27e root=ee286f..bfd791 elapsed="18.816µs"
INFO [03-31|00:01:59.297] Persisting dirty state                   head=34 root=09a9f5..cac24f layers=34
INFO [03-31|00:01:59.297] Submitted transaction                    hash=0x4d6064e44c7537bf33cf00d602fad132637ea9f1a2c03b60004b0ee87265d2db from=0xaF24Ca6c2831f4d4F629418b50C227DF0885613A nonce=7  recipient=0xaF24Ca6c2831f4d4F629418b50C227DF0885613A value=1,000,000,000,000
INFO [03-31|00:01:59.298] Persisted dirty state to disk            size=167.53KiB elapsed="788.204µs"
INFO [03-31|00:01:59.298] Blockchain stopped
INFO [03-31|00:01:59.298] Starting work on payload                 id=0x0320132297a9d031
INFO [03-31|00:01:59.298] Updated payload                          id=0x037b4b04815b8bf6                      number=35 hash=476c61..693445 txs=1  withdrawals=0 gas=2,821,360 fees=0.000141068    root=9c7658..568b18 elapsed=1.121ms
WARN [03-31|00:01:59.298] Error performing sealing work            err="chain rewind interrupted calculation of finalized block hash"
INFO [03-31|00:01:59.298] Stopping work on payload                 id=0x037b4b04815b8bf6                      reason=delivery
INFO [03-31|00:01:59.298] Updated payload                          id=0x0320132297a9d031                      number=46 hash=4c0b44..3465a3 txs=1  withdrawals=0 gas=21000     fees=0.002091872854 root=342f8e..a2a8dd elapsed="448.078µs"
INFO [03-31|00:01:59.298] Stopping work on payload                 id=0x0320132297a9d031                      reason=delivery

TestBatchPosterL1SurplusMatchesBatchGasFlaky

Stack Traces | 0.540s run time

... [CONTENT TRUNCATED: Keeping last 20 lines]
panic: runtime error: invalid memory address or nil pointer dereference [recovered, repanicked]
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x207b3b2]

goroutine 53 [running]:
testing.tRunner.func1.2({0x37e08e0, 0x61f49b0})
	/opt/hostedtoolcache/go/1.25.8/x64/src/testing/testing.go:1872 +0x237
testing.tRunner.func1()
	/opt/hostedtoolcache/go/1.25.8/x64/src/testing/testing.go:1875 +0x35b
panic({0x37e08e0?, 0x61f49b0?})
	/opt/hostedtoolcache/go/1.25.8/x64/src/runtime/panic.go:783 +0x132
github.com/offchainlabs/nitro/arbnode.(*InboxTracker).GetBatchCount(0x19853900?)
	/home/runner/work/nitro/nitro/arbnode/inbox_tracker.go:210 +0x12
github.com/offchainlabs/nitro/arbnode.(*InboxTracker).FindInboxBatchContainingMessage(0x0, 0x7)
	/home/runner/work/nitro/nitro/arbnode/inbox_tracker.go:225 +0x2f
github.com/offchainlabs/nitro/system_tests.TestBatchPosterL1SurplusMatchesBatchGasFlaky(0xc0003796c0)
	/home/runner/work/nitro/nitro/system_tests/batch_poster_test.go:839 +0x725
testing.tRunner(0xc0003796c0, 0x41b2718)
	/opt/hostedtoolcache/go/1.25.8/x64/src/testing/testing.go:1934 +0xea
created by testing.(*T).Run in goroutine 1
	/opt/hostedtoolcache/go/1.25.8/x64/src/testing/testing.go:1997 +0x465

TestRedisProduceComplex/one_producer,_all_consumers_are_active

Stack Traces | 1.240s run time

... [CONTENT TRUNCATED: Keeping last 20 lines]
�[36mDEBUG�[0m[03-30|23:57:38.872] consumer: xack                           �[36mcid�[0m=cadaf84f-6634-4880-9f1e-fc504bbc7a89 �[36mmessageId�[0m=1774915057749-4
�[36mDEBUG�[0m[03-30|23:57:38.872] Redis stream consuming                   �[36mconsumer_id�[0m=0a442b2c-4a16-48c1-b92a-8282dd4f5f6f �[36mmessage_id�[0m=1774915057749-5
�[36mDEBUG�[0m[03-30|23:57:38.872] consumer: setting result                 �[36mcid�[0m=0a442b2c-4a16-48c1-b92a-8282dd4f5f6f �[36mmsgIdInStream�[0m=1774915057749-5  �[36mresultKeyInRedis�[0m=result-key:stream:f62c831b-1372-415d-b39e-274513ab2fec.1774915057749-5
�[36mDEBUG�[0m[03-30|23:57:38.872] consumer: xdel                           �[36mcid�[0m=cadaf84f-6634-4880-9f1e-fc504bbc7a89 �[36mmessageId�[0m=1774915057749-4
�[36mDEBUG�[0m[03-30|23:57:38.872] consumer: xdel                           �[36mcid�[0m=4381d9eb-3a66-4fc7-afa6-ad530a3b4154 �[36mmessageId�[0m=1774915057749-2
�[36mDEBUG�[0m[03-30|23:57:38.872] consumer: xack                           �[36mcid�[0m=0a442b2c-4a16-48c1-b92a-8282dd4f5f6f �[36mmessageId�[0m=1774915057749-5
�[36mDEBUG�[0m[03-30|23:57:38.872] consumer: xdel                           �[36mcid�[0m=0a442b2c-4a16-48c1-b92a-8282dd4f5f6f �[36mmessageId�[0m=1774915057749-5
�[36mDEBUG�[0m[03-30|23:57:38.873] trimming                                 �[36mxTrimMinID�[0m=1774915057721-6 �[36mtrimmed�[0m=0 �[36mtrim-err�[0m=&lt;nil&gt;
�[33mWARN �[0m[03-30|23:57:38.883] XClaimJustID returned empty response when indicating heartbeat �[33mmsgID�[0m=1774915057721-3
�[36mDEBUG�[0m[03-30|23:57:38.885] consumer: xack                           �[36mcid�[0m=cc44bb3e-4e56-4846-b24c-e45dd9217ffd �[36mmessageId�[0m=1774915057721-13
�[36mDEBUG�[0m[03-30|23:57:38.888] consumer: xack                           �[36mcid�[0m=55ba2c69-c05a-4a8b-8baf-19f34623d975 �[36mmessageId�[0m=1774915057721-10
�[36mDEBUG�[0m[03-30|23:57:38.888] consumer: xdel                           �[36mcid�[0m=55ba2c69-c05a-4a8b-8baf-19f34623d975 �[36mmessageId�[0m=1774915057721-10
�[36mDEBUG�[0m[03-30|23:57:38.893] consumer: xdel                           �[36mcid�[0m=cc44bb3e-4e56-4846-b24c-e45dd9217ffd �[36mmessageId�[0m=1774915057721-13
�[36mDEBUG�[0m[03-30|23:57:38.895] consumer: xack                           �[36mcid�[0m=7fc7938a-a2c5-4bbf-995f-cadb4e330d08 �[36mmessageId�[0m=1774915057721-6
�[36mDEBUG�[0m[03-30|23:57:38.895] consumer: xdel                           �[36mcid�[0m=7fc7938a-a2c5-4bbf-995f-cadb4e330d08 �[36mmessageId�[0m=1774915057721-6
�[36mDEBUG�[0m[03-30|23:57:38.920] checkResponses                           �[36mresponded�[0m=58 �[36merrored�[0m=0 �[36mchecked�[0m=98
�[36mDEBUG�[0m[03-30|23:57:38.926] redis producer: check responses starting
�[36mDEBUG�[0m[03-30|23:57:38.946] checkResponses                           �[36mresponded�[0m=40 �[36merrored�[0m=0 �[36mchecked�[0m=40
�[31mERROR�[0m[03-30|23:57:38.950] Error from XpendingExt in getting PEL for auto claim �[31merr�[0m="context canceled" �[31mpendingLen�[0m=0
--- FAIL: TestRedisProduceComplex/one_producer,_all_consumers_are_active (1.24s)

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

bragaigor

LGTM

tsahee · 2026-03-23T18:14:25Z

arbos/arbostypes/incomingmessage.go

-	if err != nil {
-		return nil, err
+	if batchFetcher != nil {
+		err = msg.FillInBatchGasFields(batchFetcher)


nil check should be in FillInBatchGasFields, so that if the fields need to be filled the function will error out and not silently create wrong messages.

Both FillInBatchGasFields and FillInBatchGasFieldsWithParentBlock now return an error when batchFetcher is nil and the message kind is BatchPostingReport, instead of silently leaving gas fields unpopulated. Move the nil guard to the caller in delayed.go and pass the fetcher directly in inbox_tracker.go. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Let the nil check live in FillInBatchGasFieldsWithParentBlock so that FillInBatchGasFields delegates without duplicating the logic. This way a nil fetcher only errors when the message is a BatchPostingReport and the fields actually need to be filled, rather than being checked upfront. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Remove caller-side nil guards for batchFetcher Now that FillInBatchGasFieldsWithParentBlock handles nil batchFetcher internally, remove the redundant nil guards in delayed.go and transaction_streamer.go so that all call sites go through the centralized check. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Provide real fetcher in reorg path and add tests for fetcher error paths Wire up a real batchFetcher in the reorg-resequence LookupMessagesInRange call so BatchPostingReport messages get their gas fields filled. Add tests for fetcher error fallback: when LegacyBatchGasCost is already set the function succeeds (pre-arbos50 compat), and when neither field is set the fetcher error is propagated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Add tests for pre-set BatchDataStats and truncated L2msg edge cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Test that FillInBatchGasFields populates BatchDataStats and LegacyBatchGasCost with a working fetcher, and that it passes msg.Header.BlockNumber as parentChainBlockNumber through the FromFallibleBatchFetcher wrapper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ntBlock Include parentChainBlockNumber in the nil-fetcher error and add batchNum and err to the batch data fetch warning for easier debugging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

bragaigor

LGTM, only nitpick comments against err == nil test checks

arbos/arbostypes/incomingmessage_test.go

- Add ErrNilBatchFetcher, ErrBatchHashMismatch, ErrParseBatchPostingReport sentinel errors with %w wrapping for programmatic error checking - Remove redundant nil guard in delayed.go since FillInBatchGasFieldsWithParentBlock handles non-BatchPostingReport - Add edge-case tests: nil fetcher data, empty batch, idempotent calls, WithParentBlock nil data, and all non-BatchPostingReport message kinds - Add warning logs for swallowed FillInBatchGasFields errors in replay binary and GetMessage; fix misleading reorg log message - Use log.Error instead of log.Warn in replay binary (log level filtering) - Provide stub batchFetcher in init.go to satisfy non-nil contract Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

bragaigor

LGTM

joshuacolvin0 assigned KolbyML Mar 23, 2026

joshuacolvin0 marked this pull request as ready for review March 23, 2026 03:45

joshuacolvin0 assigned bragaigor and unassigned KolbyML Mar 23, 2026

Fix gofmt comment alignment in incomingmessage_test.go

43c8bf4

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

bragaigor previously approved these changes Mar 23, 2026

View reviewed changes

bragaigor assigned joshuacolvin0 and unassigned bragaigor Mar 23, 2026

tsahee requested changes Mar 23, 2026

View reviewed changes

joshuacolvin0 dismissed bragaigor’s stale review via 435ff68 March 23, 2026 19:03

joshuacolvin0 assigned tsahee and unassigned joshuacolvin0 Mar 23, 2026

joshuacolvin0 and others added 6 commits March 23, 2026 14:02

Merge branch 'master' into fix-nil-batch-fetcher

78f1390

Fix linter issue

1699ad8

Improve code clarity

db98c3d

Merge branch 'master' into fix-nil-batch-fetcher

f1f0477

joshuacolvin0 assigned joshuacolvin0 and unassigned tsahee Mar 27, 2026

joshuacolvin0 and others added 3 commits March 27, 2026 12:51

Merge branch 'master' into fix-nil-batch-fetcher

61b172e

Merge branch 'master' into fix-nil-batch-fetcher

9479a30

joshuacolvin0 assigned bragaigor and unassigned joshuacolvin0 Mar 30, 2026

bragaigor previously approved these changes Mar 30, 2026

View reviewed changes

arbos/arbostypes/incomingmessage_test.go Outdated Show resolved Hide resolved

arbos/arbostypes/incomingmessage_test.go Show resolved Hide resolved

arbos/arbostypes/incomingmessage_test.go Show resolved Hide resolved

arbos/arbostypes/incomingmessage_test.go Show resolved Hide resolved

bragaigor assigned joshuacolvin0 and unassigned bragaigor Mar 30, 2026

joshuacolvin0 dismissed bragaigor’s stale review via 57fa161 March 30, 2026 23:47

joshuacolvin0 assigned bragaigor and unassigned joshuacolvin0 Mar 30, 2026

bragaigor approved these changes Mar 31, 2026

View reviewed changes

bragaigor assigned joshuacolvin0 and unassigned bragaigor Mar 31, 2026

joshuacolvin0 assigned tsahee and unassigned joshuacolvin0 Mar 31, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix nil batchFetcher dereference#4540

Fix nil batchFetcher dereference#4540
joshuacolvin0 wants to merge 13 commits intomasterfrom
fix-nil-batch-fetcher

joshuacolvin0 commented Mar 23, 2026 •

edited

Loading

Uh oh!

codecov bot commented Mar 23, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Mar 23, 2026 •

edited

Loading

Uh oh!

bragaigor left a comment

Uh oh!

tsahee Mar 23, 2026

Uh oh!

joshuacolvin0 Mar 23, 2026

Uh oh!

bragaigor left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bragaigor left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

joshuacolvin0 commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 23 Tests Failed:

Uh oh!

bragaigor left a comment

Choose a reason for hiding this comment

Uh oh!

tsahee Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

joshuacolvin0 Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

bragaigor left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bragaigor left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joshuacolvin0 commented Mar 23, 2026 •

edited

Loading

codecov bot commented Mar 23, 2026 •

edited

Loading

github-actions bot commented Mar 23, 2026 •

edited

Loading

bragaigor left a comment •

edited

Loading