Fix SCW initialization bug in staker by ganeshvanahalli · Pull Request #3695 · OffchainLabs/nitro

ganeshvanahalli · 2025-09-24T21:03:39Z

This PR fixes a bug where AssertionChain when created uses the deep copy of txOpts which is txBuilder.SingleTxAuth(), in the case of using smart contract wallet (set using --node.staker.use-smart-contract-wallet) this address is initially 0x0 and is later filled once ExecuteTransactions is called on the txBuilder, which is incorrect hence we create the SCW when needed during initialization itself.

This is tested by enabling use of smart contract wallet by default on the nitro-testnode and verifying that assertions are being posted.

Pulls nitro-testnode PR- OffchainLabs/nitro-testnode#149
Resolves NIT-3859

rauljordan · 2025-09-29T14:53:12Z


 type ValidatorWalletInterface interface {
-	Initialize(context.Context) error
+	Initialize(context.Context, bool) error


In general, it is a bit of a code smell to use booleans like this as control variables. It makes it unclear what it does without needing to resort to comments and it would be more idiomatic to define to initialization functions. One that's called CreateAndInitialize, and another called Initialize()

I agree! fixed it

rauljordan

Pending some changes

github-actions · 2025-10-02T18:26:27Z

❌ 4 Tests Failed:

Tests completed	Failed	Passed	Skipped
2160	4	2156	0

View the top 3 failed tests by shortest run time

TestVersion30

Stack Traces | 7.310s run time

... [CONTENT TRUNCATED: Keeping last 20 lines]
�[90mstorage: deployed to 0xA02062aa4Cb0872315ACA3eA6dBF077d90B2DAC1�[0;0m
�[38;5;48;1myay!! we validated block 2 in 76.26ms�[0;0m
    precompile_inclusion_test.go:90: goroutine 455052 [running]:
        runtime/debug.Stack()
        	/opt/hostedtoolcache/go/1.25.3/x64/src/runtime/debug/stack.go:26 +0x5e
        github.com/offchainlabs/nitro/util/testhelpers.RequireImpl({0x408c530, 0xc0433d88c0}, {0x404a3c0, 0xc08ce896e0}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/util/testhelpers/testhelpers.go:29 +0x55
        github.com/offchainlabs/nitro/system_tests.Require(0xc0433d88c0, {0x404a3c0, 0xc08ce896e0}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/system_tests/common_test.go:1758 +0x5d
        github.com/offchainlabs/nitro/system_tests.testPrecompiles(0xc0433d88c0, 0x1e, {0xc1362fbdb0, 0x6, 0x48de7b?})
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:90 +0x371
        github.com/offchainlabs/nitro/system_tests.TestVersion30(0xc0433d88c0?)
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:67 +0x798
        testing.tRunner(0xc0433d88c0, 0x3cd38b0)
        	/opt/hostedtoolcache/go/1.25.3/x64/src/testing/testing.go:1934 +0xea
        created by testing.(*T).Run in goroutine 1
        	/opt/hostedtoolcache/go/1.25.3/x64/src/testing/testing.go:1997 +0x465
        
    precompile_inclusion_test.go:90: �[31;1m [] execution aborted (timeout = 5s) �[0;0m
--- FAIL: TestVersion30 (7.31s)

TestVersion40

Stack Traces | 7.570s run time

... [CONTENT TRUNCATED: Keeping last 20 lines]
        	/home/runner/work/nitro/nitro/util/testhelpers/testhelpers.go:29 +0x55
        github.com/offchainlabs/nitro/system_tests.Require(0xc0433d8a80, {0x404a3c0, 0xc127bbda40}, {0x0, 0x0, 0x0})
        	/home/runner/work/nitro/nitro/system_tests/common_test.go:1758 +0x5d
        github.com/offchainlabs/nitro/system_tests.testPrecompiles(0xc0433d8a80, 0x28, {0xc11ce81df8, 0x5, 0x39?})
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:90 +0x371
        github.com/offchainlabs/nitro/system_tests.TestVersion40(0xc0433d8a80?)
        	/home/runner/work/nitro/nitro/system_tests/precompile_inclusion_test.go:71 +0x64b
        testing.tRunner(0xc0433d8a80, 0x3cd38b8)
        	/opt/hostedtoolcache/go/1.25.3/x64/src/testing/testing.go:1934 +0xea
        created by testing.(*T).Run in goroutine 1
        	/opt/hostedtoolcache/go/1.25.3/x64/src/testing/testing.go:1997 +0x465
        
    precompile_inclusion_test.go:90: �[31;1m [] execution aborted (timeout = 5s) �[0;0m
--- FAIL: TestVersion40 (7.57s)
�[38;5;48;1myay!! we validated block 2 in 138.46ms�[0;0m
�[38;5;48;1myay!! we validated block 10 in 242.24ms�[0;0m
�[38;5;48;1myay!! we validated block 11 in 127.75ms�[0;0m
�[38;5;48;1myay!! we validated block 3 in 399.22ms�[0;0m
INFO [10-22|15:47:02.454] HTTP server stopped                      endpoint=127.0.0.1:38463
TRACE[10-22|15:47:02.454] P2P networking is spinning down

TestConsumeNextBid_DuplicateHandling

Stack Traces | 15.070s run time

... [CONTENT TRUNCATED: Keeping last 20 lines]
INFO [10-22|15:32:11.688] Starting work on payload                 id=0x035ab1226299337e
INFO [10-22|15:32:11.689] Updated payload                          id=0x035ab1226299337e                      number=15 hash=368ec4..7ef6f7 txs=1 withdrawals=0 gas=979,249   fees=9.79249e-07 root=dd14bf..a9d9c2 elapsed="883.383µs"
INFO [10-22|15:32:11.690] Stopping work on payload                 id=0x035ab1226299337e                      reason=delivery
INFO [10-22|15:32:11.691] Imported new potential chain segment     number=15 hash=368ec4..7ef6f7 blocks=1 txs=1 mgas=0.979 elapsed=1.194ms     mgasps=819.700  triediffs=39.88KiB triedirty=0.00B
INFO [10-22|15:32:11.691] Chain head was updated                   number=15 hash=368ec4..7ef6f7 root=dd14bf..a9d9c2 elapsed="105.255µs"
    auctioneer_bid_consumption_test.go:278: 
        	Error Trace:	/home/runner/work/nitro/nitro/timeboost/auctioneer_bid_consumption_test.go:278
        	Error:      	Received unexpected error:
        	            	invalid length, need 256 bits
        	            	opening wallet
        	            	github.com/offchainlabs/nitro/timeboost.NewAuctioneerServer
        	            		/home/runner/work/nitro/nitro/timeboost/auctioneer.go:207
        	            	github.com/offchainlabs/nitro/timeboost.TestConsumeNextBid_DuplicateHandling
        	            		/home/runner/work/nitro/nitro/timeboost/auctioneer_bid_consumption_test.go:277
        	            	testing.tRunner
        	            		/opt/hostedtoolcache/go/1.25.3/x64/src/testing/testing.go:1934
        	            	runtime.goexit
        	            		/opt/hostedtoolcache/go/1.25.3/x64/src/runtime/asm_amd64.s:1693
        	Test:       	TestConsumeNextBid_DuplicateHandling
--- FAIL: TestConsumeNextBid_DuplicateHandling (15.07s)

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

eljobe · 2025-10-06T05:34:46Z

@ganeshvanahalli, I feel like we're in a sort of "Chicken and Egg" situation here where this PR is pulling in OffchainLabs/nitro-testnode#149, but that PR is waiting on this one to be merged. Can you help bring things into a logical state?

ganeshvanahalli · 2025-10-06T15:38:53Z

@ganeshvanahalli, I feel like we're in a sort of "Chicken and Egg" situation here where this PR is pulling in OffchainLabs/nitro-testnode#149, but that PR is waiting on this one to be merged. Can you help bring things into a logical state?

@eljobe thanks for spotting this, I have testnode master and release sync PR here, once that is merged we can go ahead and merge OffchainLabs/nitro-testnode#149 as testnode master branch is meant to be mainly used in dev mode.

And in the future sync of master and release branch of testnode- this fix from nitro will be pulled in anyway. WDYT?

eljobe · 2025-10-22T07:49:58Z

 	"github.com/offchainlabs/nitro/staker/bold"
-	"github.com/offchainlabs/nitro/staker/legacy"
-	"github.com/offchainlabs/nitro/staker/multi_protocol"
+	legacystaker "github.com/offchainlabs/nitro/staker/legacy"


Why do these package aliases keep creeping back in. Are they needed?

Package aliases have been removed

Fix SCW initialization bug in staker

107ad0e

ganeshvanahalli mentioned this pull request Sep 24, 2025

Default to using smart contract wallet in staker OffchainLabs/nitro-testnode#149

Merged

ganeshvanahalli requested a review from rauljordan September 24, 2025 21:22

ganeshvanahalli assigned rauljordan Sep 24, 2025

Merge branch 'master' into fix-staker-scwbug

f75ebb1

ganeshvanahalli marked this pull request as ready for review September 26, 2025 15:14

Merge branch 'master' into fix-staker-scwbug

16651d3

ganeshvanahalli requested a review from amsanghi September 26, 2025 15:14

rauljordan reviewed Sep 29, 2025

View reviewed changes

rauljordan suggested changes Sep 29, 2025

View reviewed changes

address PR comments

0ff4ae4

ganeshvanahalli requested a review from rauljordan September 29, 2025 16:28

Merge branch 'master' into fix-staker-scwbug

2c16f7e

rauljordan previously approved these changes Oct 2, 2025

View reviewed changes

Merge branch 'master' into fix-staker-scwbug

df8d97c

rauljordan assigned ganeshvanahalli and unassigned rauljordan Oct 2, 2025

ganeshvanahalli assigned eljobe and unassigned ganeshvanahalli Oct 2, 2025

Merge branch 'master' into fix-staker-scwbug

9231203

eljobe previously approved these changes Oct 6, 2025

View reviewed changes

eljobe added the design-approved label Oct 6, 2025

eljobe enabled auto-merge October 6, 2025 05:24

eljobe assigned ganeshvanahalli and unassigned eljobe Oct 6, 2025

ganeshvanahalli disabled auto-merge October 6, 2025 15:38

ganeshvanahalli mentioned this pull request Oct 10, 2025

update testnode pin #3736

Merged

ganeshvanahalli added 2 commits October 10, 2025 10:48

Merge branch 'master' into fix-staker-scwbug

d2f8421

revert updating testnode pin

bdc268c

ganeshvanahalli dismissed stale reviews from eljobe and rauljordan via bdc268c October 10, 2025 15:49

ganeshvanahalli requested review from eljobe and rauljordan October 10, 2025 15:50

ganeshvanahalli assigned eljobe and unassigned ganeshvanahalli Oct 14, 2025

ganeshvanahalli and others added 3 commits October 21, 2025 12:01

Merge branch 'master' into fix-staker-scwbug

c3229a2

Merge branch 'master' into fix-staker-scwbug

52419aa

Merge branch 'master' into fix-staker-scwbug

9bbd291

eljobe previously requested changes Oct 22, 2025

View reviewed changes

eljobe assigned ganeshvanahalli and unassigned eljobe Oct 22, 2025

ganeshvanahalli added 2 commits October 22, 2025 10:19

Merge branch 'master' into fix-staker-scwbug

fe9bd57

remove unnecessary import aliases

3a4d91f

ganeshvanahalli assigned eljobe and unassigned ganeshvanahalli Oct 22, 2025

ganeshvanahalli requested a review from eljobe October 22, 2025 15:22

rauljordan approved these changes Oct 22, 2025

View reviewed changes

rauljordan assigned joshuacolvin0 Oct 22, 2025

joshuacolvin0 added this pull request to the merge queue Oct 22, 2025

Merged via the queue into master with commit eb3da91 Oct 22, 2025
23 checks passed

joshuacolvin0 deleted the fix-staker-scwbug branch October 22, 2025 16:36

Navigation Menu

Repository navigation

Fix SCW initialization bug in staker - #3695

Fix SCW initialization bug in staker#3695
joshuacolvin0 merged 14 commits into
masterfrom
fix-staker-scwbug

ganeshvanahalli commented Sep 24, 2025 •

edited

Loading

Uh oh!

rauljordan Sep 29, 2025

Uh oh!

ganeshvanahalli Sep 29, 2025

Uh oh!

rauljordan left a comment

Uh oh!

github-actions Bot commented Oct 2, 2025 •

edited

Loading

Uh oh!

eljobe commented Oct 6, 2025

Uh oh!

ganeshvanahalli commented Oct 6, 2025 •

edited

Loading

Uh oh!

eljobe Oct 22, 2025

Uh oh!

ganeshvanahalli Oct 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ganeshvanahalli commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rauljordan Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

ganeshvanahalli Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

rauljordan left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

❌ 4 Tests Failed:

Uh oh!

eljobe commented Oct 6, 2025

Uh oh!

ganeshvanahalli commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eljobe Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

ganeshvanahalli Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ganeshvanahalli commented Sep 24, 2025 •

edited

Loading

github-actions Bot commented Oct 2, 2025 •

edited

Loading

ganeshvanahalli commented Oct 6, 2025 •

edited

Loading