Skip to content

Fix expected surplus in sequencer #4038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix expected surplus in sequencer #4038

wants to merge 1 commit into from

Conversation

@alexandrosfilios
Copy link
Contributor

@alexandrosfilios alexandrosfilios commented Nov 20, 2025

No description provided.

@github-actions
Copy link

github-actions bot commented Nov 20, 2025 •
edited
Loading

❌ 4 Tests Failed:

Tests completed Failed Passed Skipped
2172 4 2168 0
View the top 3 failed tests by shortest run time 
TestRedisProduceComplex/two_producers,_some_consumers_killed,_others_should_take_over_their_work,_unequal_number_of_requests_from_producers
Stack Traces | 2.029s run time 
... [CONTENT TRUNCATED: Keeping last 20 lines]
�[36mDEBUG�[0m[11-20|14:12:47.021] redis producer: check responses starting
�[36mDEBUG�[0m[11-20|14:12:47.021] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0 �[36mchecked�[0m=4
�[36mDEBUG�[0m[11-20|14:12:47.024] redis producer: check responses starting
�[36mDEBUG�[0m[11-20|14:12:47.024] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0 �[36mchecked�[0m=0
�[36mDEBUG�[0m[11-20|14:12:47.027] redis producer: check responses starting
�[36mDEBUG�[0m[11-20|14:12:47.028] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0 �[36mchecked�[0m=4
�[36mDEBUG�[0m[11-20|14:12:47.029] redis producer: check responses starting
�[36mDEBUG�[0m[11-20|14:12:47.029] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0 �[36mchecked�[0m=0
�[36mDEBUG�[0m[11-20|14:12:47.033] redis producer: check responses starting
�[36mDEBUG�[0m[11-20|14:12:47.033] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0 �[36mchecked�[0m=4
�[36mDEBUG�[0m[11-20|14:12:47.034] redis producer: check responses starting
�[36mDEBUG�[0m[11-20|14:12:47.034] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=0 �[36mchecked�[0m=0
�[36mDEBUG�[0m[11-20|14:12:47.038] redis producer: check responses starting
�[36mDEBUG�[0m[11-20|14:12:47.038] request timed out waiting for response   �[36mmsgId�[0m=1763647965036-15 �[36mallowedOldestId�[0m=1763647965038-0
�[36mDEBUG�[0m[11-20|14:12:47.038] request timed out waiting for response   �[36mmsgId�[0m=1763647965036-12 �[36mallowedOldestId�[0m=1763647965038-0
�[36mDEBUG�[0m[11-20|14:12:47.038] request timed out waiting for response   �[36mmsgId�[0m=1763647965036-14 �[36mallowedOldestId�[0m=1763647965038-0
�[36mDEBUG�[0m[11-20|14:12:47.039] request timed out waiting for response   �[36mmsgId�[0m=1763647965036-13 �[36mallowedOldestId�[0m=1763647965038-0
�[36mDEBUG�[0m[11-20|14:12:47.039] checkResponses                           �[36mresponded�[0m=0   �[36merrored�[0m=4 �[36mchecked�[0m=4
    pubsub_test.go:320: Error awaiting responses from promises 1: [139 140 141 142]
--- FAIL: TestRedisProduceComplex/two_producers,_some_consumers_killed,_others_should_take_over_their_work,_unequal_number_of_requests_from_producers (2.03s)

TestVersion30
Stack Traces | 7.510s run time 
... [CONTENT TRUNCATED: Keeping last 20 lines]
�[38;5;48;1mpaid network fee:    322770000000�[0;0m
�[38;5;48;1mpaid fee:            2422770000000�[0;0m
    precompile_inclusion_test.go:90: goroutine 455443 [running]:
        runtime/debug.Stack()
        	/opt/hostedtoolcache/go/1.25.4/x64/src/runtime/debug/stack.go:26 +0x5e
        github.com/offchainlabs/nitro/util/testhelpers.RequireImpl({0x40e3230, 0xc082a34380}, {0x40a0a80, 0xc102b82360}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/util/testhelpers/testhelpers.go:29 +0x55
        github.com/offchainlabs/nitro/system_tests.Require(0xc082a34380, {0x40a0a80, 0xc102b82360}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/system_tests/common_test.go:1759 +0x5d
        github.com/offchainlabs/nitro/system_tests.testPrecompiles(0xc082a34380, 0x1e, {0xc10fb61db0, 0x6, 0xc0086d5d10?})
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:90 +0x371
        github.com/offchainlabs/nitro/system_tests.TestVersion30(0xc082a34380?)
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:67 +0x798
        testing.tRunner(0xc082a34380, 0x3d274c8)
        	/opt/hostedtoolcache/go/1.25.4/x64/src/testing/testing.go:1934 +0xea
        created by testing.(*T).Run in goroutine 1
        	/opt/hostedtoolcache/go/1.25.4/x64/src/testing/testing.go:1997 +0x465
        
    precompile_inclusion_test.go:90: �[31;1m [] execution aborted (timeout = 5s) �[0;0m
--- FAIL: TestVersion30 (7.51s)

TestVersion40
Stack Traces | 8.830s run time 
... [CONTENT TRUNCATED: Keeping last 20 lines]
=== PAUSE TestVersion40
=== CONT  TestVersion40
    precompile_inclusion_test.go:90: goroutine 455444 [running]:
        runtime/debug.Stack()
        	/opt/hostedtoolcache/go/1.25.4/x64/src/runtime/debug/stack.go:26 +0x5e
        github.com/offchainlabs/nitro/util/testhelpers.RequireImpl({0x40e3230, 0xc082a34540}, {0x40a0a80, 0xc136217a40}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/util/testhelpers/testhelpers.go:29 +0x55
        github.com/offchainlabs/nitro/system_tests.Require(0xc082a34540, {0x40a0a80, 0xc136217a40}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/system_tests/common_test.go:1759 +0x5d
        github.com/offchainlabs/nitro/system_tests.testPrecompiles(0xc082a34540, 0x28, {0xc09b709df8, 0x5, 0x39?})
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:90 +0x371
        github.com/offchainlabs/nitro/system_tests.TestVersion40(0xc082a34540?)
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:71 +0x64b
        testing.tRunner(0xc082a34540, 0x3d274d0)
        	/opt/hostedtoolcache/go/1.25.4/x64/src/testing/testing.go:1934 +0xea
        created by testing.(*T).Run in goroutine 1
        	/opt/hostedtoolcache/go/1.25.4/x64/src/testing/testing.go:1997 +0x465
        
    precompile_inclusion_test.go:90: �[31;1m [] execution aborted (timeout = 5s) �[0;0m
--- FAIL: TestVersion40 (8.83s)

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

@alexandrosfilios alexandrosfilios marked this pull request as ready for review November 20, 2025 14:04
@alexandrosfilios alexandrosfilios force-pushed the afilios/fix-expected-surplus branch from 64e9da2 to 6cb625f Compare November 20, 2025 14:04
MishkaRogachev
MishkaRogachev reviewed Nov 20, 2025
View reviewed changes
execution/gethexec/sequencer.go
} else {
// l1GasPrice can be zero because of roundings, hence backlogCost is calculated separately
backlogFee := big.NewInt(backlogCallDataUnits)
backlogFee.Mul(backlogFee, blobFeePerByte)
Copy link
Contributor

@MishkaRogachev MishkaRogachev Nov 20, 2025

Choose a reason for hiding this comment

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

I don't have enough context to understand this line change. Could you please add a brief description?

Copy link
Contributor

@tsahee tsahee Nov 26, 2025

Choose a reason for hiding this comment

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

now I look at the entire computation and not sure I get it.

can you add a comment with the full computation for computing backlog cost here?

tsahee
tsahee requested changes Nov 26, 2025
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.

question

execution/gethexec/sequencer.go
} else {
// l1GasPrice can be zero because of roundings, hence backlogCost is calculated separately
backlogFee := big.NewInt(backlogCallDataUnits)
backlogFee.Mul(backlogFee, blobFeePerByte)
Copy link
Contributor

@tsahee tsahee Nov 26, 2025

Choose a reason for hiding this comment

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

now I look at the entire computation and not sure I get it.

can you add a comment with the full computation for computing backlog cost here?

@tsahee tsahee assigned alexandrosfilios and unassigned tsahee Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MishkaRogachev MishkaRogachev MishkaRogachev left review comments

@tsahee tsahee tsahee requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@alexandrosfilios alexandrosfilios

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alexandrosfilios @MishkaRogachev @tsahee