Skip to content

If batchFetcher returns error use existing LegacyBatchGasCost value#4448

Open
bragaigor wants to merge 4 commits intomasterfrom
braga/batchfetcher-always-err-on-err
Open

If batchFetcher returns error use existing LegacyBatchGasCost value#4448
bragaigor wants to merge 4 commits intomasterfrom
braga/batchfetcher-always-err-on-err

Conversation

@bragaigor
Copy link
Contributor

@bragaigor bragaigor commented Mar 2, 2026 •
edited
Loading

If batchFetcher returns error use existing LegacyBatchGasCost value

closes NIT-4598

bragaigor added 2 commits March 2, 2026 17:16
@bragaigor
Always return error if batchFetcher returns error
dc5bc95 
Signed-off-by: Igor Braga <5835477+bragaigor@users.noreply.github.com>
@bragaigor
add changelog file
57f9713 
Signed-off-by: Igor Braga <5835477+bragaigor@users.noreply.github.com>
@bragaigor bragaigor marked this pull request as ready for review March 2, 2026 22:23
@joshuacolvin0 joshuacolvin0 assigned gligneul and unassigned tsahee Mar 2, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026 •
edited
Loading

❌ 13 Tests Failed:

Tests completed Failed Passed Skipped
4245 13 4232 0
View the top 3 failed tests by shortest run time 
TestBlockValidatorSimpleOnchainWithPublishedMachine
Stack Traces | 0.060s run time 
=== RUN   TestBlockValidatorSimpleOnchainWithPublishedMachine
    block_validator_test.go:351: goroutine 832 [running]:
        runtime/debug.Stack()
        	/opt/hostedtoolcache/go/1.25.7/x64/src/runtime/debug/stack.go:26 +0x5e
        github.com/offchainlabs/nitro/util/testhelpers.RequireImpl({0x43f6070, 0xc001d80700}, {0x43b6240, 0xc000384d70}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/util/testhelpers/testhelpers.go:29 +0x9f
        github.com/offchainlabs/nitro/system_tests.Require(0xc001d80700, {0x43b6240, 0xc000384d70}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/system_tests/common_test.go:2075 +0x5d
        github.com/offchainlabs/nitro/system_tests.TestBlockValidatorSimpleOnchainWithPublishedMachine(0xc001d80700)
        	/home/runner/work/nitro/nitro/system_tests/block_validator_test.go:351 +0x51
        testing.tRunner(0xc001d80700, 0x402a448)
        	/opt/hostedtoolcache/go/1.25.7/x64/src/testing/testing.go:1934 +0xea
        created by testing.(*T).Run in goroutine 1
        	/opt/hostedtoolcache/go/1.25.7/x64/src/testing/testing.go:1997 +0x465
        
    block_validator_test.go:351: �[31;1m [] GET https://api.github.com/repos/OffchainLabs/nitro/releases?per_page=100: 502  [] �[0;0m
--- FAIL: TestBlockValidatorSimpleOnchainWithPublishedMachine (0.06s)

TestNitroNodeVersionAlerter
Stack Traces | 1.400s run time 
... [CONTENT TRUNCATED: Keeping last 20 lines]
INFO [03-04|17:42:39.714] Deploying rollup creator
INFO [03-04|17:42:39.714] Deploying bridge creator contracts
INFO [03-04|17:42:39.714] Deploying bridge template
INFO [03-04|17:42:39.714] Blockchain stopped
INFO [03-04|17:42:39.719] ExecutionEngine: Added DelayedMessages   msgIdx=182 delayedMsgIdx=64 block-header="&amp;{ParentHash:0xba4dc44f592ecfcf2c113ae579946c1a9b9a480b66e01a92468142c1d00579ec UncleHash:0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347 Coinbase:0xb386a74Dcab67b66F8AC07B4f08365d37495Dd23 Root:0x8c1fa03c0bd966cdab01780d365a75434370709af4906bfe3254981241033090 TxHash:0x7785a34b6e72209de1a50f6254b3e7838d8c0b149438250506e61c63d2c580c4 ReceiptHash:0xf08cf5553e1dae52e3df19b356b8320e17c39fb055f635739c31052db5c3e45e Bloom:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Difficulty:+1 Number:+182 GasLimit:1125899906842624 GasUsed:0 Time:1772646203 Extra:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] MixDigest:0x0000000000000000000000000000007500000000000000330000000000000000 Nonce:[0 0 0 0 0 0 0 65] BaseFee:+100000000 WithdrawalsHash:&lt;nil&gt; BlobGasUsed:&lt;nil&gt; ExcessBlobGas:&lt;nil&gt; ParentBeaconRoot:&lt;nil&gt; RequestsHash:&lt;nil&gt;}"
INFO [03-04|17:42:39.719] DelayedSequencer: Sequenced              msgnum=1  startpos=64
WARN [03-04|17:42:39.725] empty sequencer message
WARN [03-04|17:42:39.725] reading virtual delayed message segment  delayedMessagesRead=0 afterDelayedMessages=1
INFO [03-04|17:42:39.725] InboxTracker                             sequencerBatchCount=1  messageCount=1   l1Block=29  l1Timestamp=2026-03-04T17:43:02+0000
INFO [03-04|17:42:39.727] Submitted transaction                    hash=0x64660a20b2d31e9601955f942760ba47be03cbf6475e1e92db9d06128e7530b7 from=0x57Ff0F473737a1c161bfF9efDF016F7991585088 nonce=24  recipient=0xB211efa485d87C29b9486E7631D3140e16D28E08 value=0
INFO [03-04|17:42:39.728] Submitted contract creation              hash=0x0217f2d489f3e0b48a3e2e39d80eb68aba6ab610a5458fe0ff7cca538e3f7c15 from=0x57Ff0F473737a1c161bfF9efDF016F7991585088 nonce=10  contract=0xd8Ef51E29965b7c1388c0BFD630D4cFc46D661b5 value=0
INFO [03-04|17:42:39.731] Imported new potential chain segment     number=10  hash=7097cd..eceef3 blocks=1  txs=1  mgas=2.308  elapsed=28.022ms     mgasps=82.362   triediffs=31.66KiB  triedirty=0.00B
INFO [03-04|17:42:39.732] Chain head was updated                   number=10  hash=7097cd..eceef3 root=bf8fb3..6c73c8 elapsed="330.004µs"
INFO [03-04|17:42:39.733] Starting work on payload                 id=0x0378d098e0386b1a
INFO [03-04|17:42:39.750] Imported new potential chain segment     number=137 hash=46c2a2..7f1562 blocks=1  txs=1  mgas=0.021  elapsed=45.130ms     mgasps=0.465    triediffs=677.85KiB triedirty=8.66KiB
INFO [03-04|17:42:39.751] Chain head was updated                   number=137 hash=46c2a2..7f1562 root=bdff3e..b0ecbf elapsed="120.24µs"
INFO [03-04|17:42:39.755] Submitted contract creation              hash=0x61f78ed7484d562ec25d752a5b868d87786cc5c21bd3482b81469d671a2b4197 from=0x57Ff0F473737a1c161bfF9efDF016F7991585088 nonce=1   contract=0xC0d44eBf2024FAa79d5aa2F2b1a19329E53a8a77 value=0
INFO [03-04|17:42:39.760] Starting work on payload                 id=0x03e201ba352b86fe
INFO [03-04|17:42:39.766] Updated payload                          id=0x03e201ba352b86fe number=2   hash=5b5633..b7dbb3 txs=1  withdrawals=0 gas=1,501,105  fees=1.501105e-06   root=32d7b2..4025f3 elapsed=6.325ms
--- FAIL: TestNitroNodeVersionAlerter (1.40s)

TestRedisProduceComplex/two_producers,_some_consumers_killed,_one_retrying_consumer_take_over_their_work,_unequal_number_of_requests_from_producers
Stack Traces | 3.010s run time 
... [CONTENT TRUNCATED: Keeping last 20 lines]
�[36mDEBUG�[0m[03-04|17:28:25.150] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0  �[36mchecked�[0m=0
�[36mDEBUG�[0m[03-04|17:28:25.155] redis producer: check responses starting
�[36mDEBUG�[0m[03-04|17:28:25.155] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0  �[36mchecked�[0m=0
�[36mDEBUG�[0m[03-04|17:28:25.161] redis producer: check responses starting
�[36mDEBUG�[0m[03-04|17:28:25.161] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0  �[36mchecked�[0m=0
�[36mDEBUG�[0m[03-04|17:28:25.167] redis producer: check responses starting
�[36mDEBUG�[0m[03-04|17:28:25.167] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0  �[36mchecked�[0m=0
�[36mDEBUG�[0m[03-04|17:28:25.173] redis producer: check responses starting
�[36mDEBUG�[0m[03-04|17:28:25.173] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0  �[36mchecked�[0m=0
�[36mDEBUG�[0m[03-04|17:28:25.180] redis producer: check responses starting
�[36mDEBUG�[0m[03-04|17:28:25.180] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0  �[36mchecked�[0m=0
�[36mDEBUG�[0m[03-04|17:28:25.185] redis producer: check responses starting
�[36mDEBUG�[0m[03-04|17:28:25.185] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0  �[36mchecked�[0m=0
�[36mDEBUG�[0m[03-04|17:28:25.195] redis producer: check responses starting
�[36mDEBUG�[0m[03-04|17:28:25.195] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0  �[36mchecked�[0m=0
�[36mDEBUG�[0m[03-04|17:28:25.203] redis producer: check responses starting
�[36mDEBUG�[0m[03-04|17:28:25.204] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0  �[36mchecked�[0m=0
�[36mDEBUG�[0m[03-04|17:28:25.204] checkResponses                           �[36mresponded�[0m=105 �[36merrored�[0m=0  �[36mchecked�[0m=105
�[36mDEBUG�[0m[03-04|17:28:25.276] Error destroying a stream group          �[36merror�[0m="dial tcp 127.0.0.1:43069: connect: connection refused"
--- FAIL: TestRedisProduceComplex/two_producers,_some_consumers_killed,_one_retrying_consumer_take_over_their_work,_unequal_number_of_requests_from_producers (3.01s)

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

@codecov
Copy link

codecov bot commented Mar 3, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 32.32%. Comparing base (3380c2f) to head (d9795c1).
⚠️ Report is 21 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #4448      +/-   ##
==========================================
- Coverage   32.79%   32.32%   -0.48%     
==========================================
  Files         493      493              
  Lines       58290    58291       +1     
==========================================
- Hits        19118    18843     -275     
- Misses      35831    36101     +270     
- Partials     3341     3347       +6

tsahee
tsahee requested changes Mar 3, 2026
View reviewed changes
Copy link
Contributor

@tsahee tsahee left a comment

Choose a reason for hiding this comment

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

The current behavior is correct. No need to always return an error here.

@bragaigor
If batchFetcher returns error use existing LegacyBatchGasCost value
01557db 
Signed-off-by: Igor Braga <5835477+bragaigor@users.noreply.github.com>
@bragaigor bragaigor changed the title Always return error if batchFetcher returns error If batchFetcher returns error use existing LegacyBatchGasCost value Mar 3, 2026
@bragaigor bragaigor requested review from gligneul and tsahee March 3, 2026 09:47
gligneul
gligneul requested changes Mar 4, 2026
View reviewed changes
@gligneul gligneul assigned bragaigor and unassigned gligneul Mar 4, 2026
@bragaigor
add comment and return early
d9795c1 
Signed-off-by: Igor Braga <5835477+bragaigor@users.noreply.github.com>
@bragaigor bragaigor requested a review from gligneul March 4, 2026 14:07
@bragaigor bragaigor assigned gligneul and unassigned bragaigor Mar 4, 2026
@gligneul gligneul assigned tsahee and unassigned gligneul Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gligneul gligneul gligneul approved these changes

@diegoximenes diegoximenes Awaiting requested review from diegoximenes

@joshuacolvin0 joshuacolvin0 Awaiting requested review from joshuacolvin0

@tsahee tsahee Awaiting requested review from tsahee

Requested changes must be addressed to merge this pull request.

Assignees

@tsahee tsahee

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bragaigor @gligneul @tsahee