Skip to content

fix: avoid initializing syncer at node startup in sequencer mode #916

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

Merged
tomatoishealthy merged 1 commit into from
Mar 16, 2026
Merged

fix: avoid initializing syncer at node startup in sequencer mode #916

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions node/cmd/node/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,14 @@ func L2NodeMain(ctx *cli.Context) error {
dvNode.Start()
nodeConfig.Logger.Info("derivation node starting")
} else {
// ========== Create Syncer and L1 Sequencer Components ==========
syncer, err = node.NewSyncer(ctx, home, nodeConfig)
// ========== Create L1 Client ==========
l1RPC := ctx.GlobalString(flags.L1NodeAddr.Name)
l1Client, err := ethclient.Dial(l1RPC)
if err != nil {
return fmt.Errorf("failed to create syncer: %w", err)
return fmt.Errorf("failed to dial L1 node: %w", err)
}

tracker, verifier, signer, err = initL1SequencerComponents(ctx, syncer.L1Client(), nodeConfig.Logger)
tracker, verifier, signer, err = initL1SequencerComponents(ctx, l1Client, nodeConfig.Logger)
if err != nil {
return fmt.Errorf("failed to init L1 sequencer components: %w", err)
}
Expand All @@ -152,8 +153,7 @@ func L2NodeMain(ctx *cli.Context) error {
tmVal := privval.LoadOrGenFilePV(tmCfg.PrivValidatorKeyFile(), tmCfg.PrivValidatorStateFile())
pubKey, _ := tmVal.GetPubKey()

// Create executor with syncer
newSyncerFunc := func() (*sync.Syncer, error) { return syncer, nil } // Reuse existing syncer
newSyncerFunc := func() (*sync.Syncer, error) { return node.NewSyncer(ctx, home, nodeConfig) }
executor, err = node.NewExecutor(newSyncerFunc, nodeConfig, pubKey)
Comment on lines +156 to 157
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether Executor has a shutdown path for lazily created syncer.
rg -nP --type=go -C3 'type Executor struct|newSyncerFunc|syncer \*sync\.Syncer' node/core/executor.go
rg -nP --type=go -C3 'func\s*\(.*\*Executor.*\)\s*(Stop|Close|Shutdown)\s*\(' node/core/executor.go node/cmd/node/main.go
rg -nP --type=go -C3 'syncer\.Stop\s*\(' node/core/executor.go node/cmd/node/main.go

Repository: morph-l2/morph

Length of output: 1188

🏁 Script executed:

# Search for Executor methods that might handle shutdown
rg -nP --type=go 'func\s*\(\w+\s*\*Executor\)' node/core/executor.go | head -30

Repository: morph-l2/morph

Length of output: 1076

🏁 Script executed:

# Search for any Stop/Close/Shutdown methods in executor.go
rg -nP --type=go 'func.*Stop|func.*Close|func.*Shutdown' node/core/executor.go

Repository: morph-l2/morph

Length of output: 40

🏁 Script executed:

# Get the full Executor struct definition to understand its scope
ast-grep --pattern 'type Executor struct { $$$ }'

Repository: morph-l2/morph

Length of output: 2372

🏁 Script executed:

# Get context around line 156-157 where executor is created, and the cleanup section
sed -n '140,210p' node/cmd/node/main.go

Repository: morph-l2/morph

Length of output: 1960

🏁 Script executed:

# Check if tmNode.Stop() might handle executor cleanup
rg -nP --type=go -C5 'type.*Node|func.*Stop' node/consensus/sequencer/node.go | head -50

Repository: morph-l2/morph

Length of output: 130

🏁 Script executed:

# Check how executor is used in sequencer/node.go
rg -nP --type=go -B2 -A2 'executor' node/consensus/sequencer/node.go | head -40

Repository: morph-l2/morph

Length of output: 130

🏁 Script executed:

# Find the sequencer node setup function
rg -l 'func SetupNode' node/

Repository: morph-l2/morph

Length of output: 83

🏁 Script executed:

# Search for where tmNode might be defined or cleaned up
rg -nP --type=go 'type.*Node|SetupNode' node/ | grep -i setup

Repository: morph-l2/morph

Length of output: 324

🏁 Script executed:

# Get the SetupNode function implementation and the Node struct
sed -n '56,150p' node/sequencer/tm_node.go

Repository: morph-l2/morph

Length of output: 2043

🏁 Script executed:

# Also check the Stop method of the Node type
rg -nP --type=go -A10 'func.*\*Node.*Stop' node/sequencer/tm_node.go

Repository: morph-l2/morph

Length of output: 40

🏁 Script executed:

# Check if tmnode.Node has a Stop method and what it does
rg -nP --type=go 'type Node struct|func.*Stop' node/ | grep -A5 'tmnode\|Node.*Stop' | head -20

Repository: morph-l2/morph

Length of output: 40

🏁 Script executed:

# Let's search in the entire codebase for where executor.syncer is used
rg -nP --type=go 'executor\.syncer|\.syncer\.Stop' node/

Repository: morph-l2/morph

Length of output: 300

🏁 Script executed:

# Check if there are any references to stopping executor
rg -nP --type=go 'executor\.Stop|executor\.Close|executor\.Shutdown' node/

Repository: morph-l2/morph

Length of output: 40

🏁 Script executed:

# Check the context around line 186 in sequencers.go
sed -n '170,200p' node/core/sequencers.go

Repository: morph-l2/morph

Length of output: 859

🏁 Script executed:

# Get more context - check the full function
sed -n '150,210p' node/core/sequencers.go

Repository: morph-l2/morph

Length of output: 1476

🏁 Script executed:

# Verify the local syncer variable scope in main.go - is it the same as executor.syncer?
sed -n '1,160p' node/cmd/node/main.go | grep -n 'syncer'

Repository: morph-l2/morph

Length of output: 384

Add shutdown cleanup for executor's lazily created syncer.

The Executor creates a syncer internally in two scenarios: 1) during DevSequencer initialization (line 164 in executor.go), and 2) dynamically when the node transitions to sequencer role (sequencers.go). However, Executor has no Stop() method and the internally created syncer is never cleaned up on application shutdown. The local syncer cleanup at line 203 in main.go is for the Derivation Verifier's separate syncer instance, not the executor's internal syncer. Add an Executor.Stop() method that properly cleans up the internal syncer field.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@node/cmd/node/main.go` around lines 156 - 157, The Executor currently creates
an internal syncer (via NewExecutor / DevSequencer and sequencer transition) but
lacks lifecycle cleanup; add an Executor.Stop() method that safely closes and
nils its internal syncer field (checking for nil) and is concurrency-safe (use
the existing executor mutex or add one to the Executor struct if none exists) to
avoid races; update NewExecutor/Executor implementation so Stop() calls the
syncer's Stop/Close method (whichever applies) and ensure main.go invokes
executor.Stop() on application shutdown alongside other cleanup to fully release
the lazily created syncer.

if err != nil {
return err
Expand All @@ -175,7 +175,7 @@ func L2NodeMain(ctx *cli.Context) error {
}

// ========== Initialize BlockTagService ==========
blockTagSvc, err = initBlockTagService(ctx, syncer.L1Client(), executor, nodeConfig.Logger)
blockTagSvc, err = initBlockTagService(ctx, l1Client, executor, nodeConfig.Logger)
if err != nil {
return fmt.Errorf("failed to init BlockTagService: %w", err)
}
Expand Down
Loading