Skip to content
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

fix: no resequence if l1 sync enabled #1578

Merged
merged 1 commit into from
Dec 18, 2024
+14 −9
Merged

fix: no resequence if l1 sync enabled #1578

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion eth/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ func New(ctx context.Context, stack *node.Node, config *ethconfig.Config, logger
isSequencer := sequencer.IsSequencer()

// if the L1 block sync is set we're in recovery so can't run as a sequencer
if cfg.L1SyncStartBlock > 0 {
if cfg.IsL1Recovery() {
if !isSequencer {
panic("you cannot launch in l1 sync mode as an RPC node")
}
Expand Down
4 changes: 4 additions & 0 deletions eth/ethconfig/config_zkevm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,7 @@ func (c *Zk) HasExecutors() bool {
func (c *Zk) ShouldImportInitialBatch() bool {
return c.InitialBatchCfgFile != ""
}

func (c *Zk) IsL1Recovery() bool {
return c.L1SyncStartBlock > 0
}
11 changes: 8 additions & 3 deletions zk/stages/stage_sequence_execute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ func SpawnSequencingStage(
}

if lastBatch < highestBatchInDs {
return resequence(s, u, ctx, cfg, historyCfg, lastBatch, highestBatchInDs)
if err = cfg.dataStreamServer.UnwindToBatchStart(lastBatch + 1); err != nil {
return err
}
Comment on lines +50 to +52
Copy link

Choose a reason for hiding this comment

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

Is this going to work in resequence mode? We retrieve the data to be resequenced from ds. If ds is unwound, we won't have anything to resequence.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should flag activate the ability to resequence from the ds? This behaviour is needed when l1 recovering - are both scenarios valid simultaneously?

Copy link

@cffls cffls Dec 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, we should read data from ds when resequencing, but only one mode (from resequence, l1recover, limbo, sequence) can be activated at one time.
Looks like resequencing is broken in the latest CI and also in this PR, but somehow the CI job didn't expose the error.

if !cfg.zk.IsL1Recovery() {
return resequence(s, u, ctx, cfg, historyCfg, lastBatch, highestBatchInDs)
}
}

if cfg.zk.SequencerResequence {
Expand Down Expand Up @@ -108,15 +113,15 @@ func sequencingBatchStep(
return nil
}

batchNumberForStateInitialization, err := prepareBatchNumber(sdb, forkId, lastBatch, cfg.zk.L1SyncStartBlock > 0)
batchNumberForStateInitialization, err := prepareBatchNumber(sdb, forkId, lastBatch, cfg.zk.IsL1Recovery())
if err != nil {
return err
}

var block *types.Block
runLoopBlocks := true
batchContext := newBatchContext(ctx, &cfg, &historyCfg, s, sdb)
batchState := newBatchState(forkId, batchNumberForStateInitialization, executionAt+1, cfg.zk.HasExecutors(), cfg.zk.L1SyncStartBlock > 0, cfg.txPool, resequenceBatchJob)
batchState := newBatchState(forkId, batchNumberForStateInitialization, executionAt+1, cfg.zk.HasExecutors(), cfg.zk.IsL1Recovery(), cfg.txPool, resequenceBatchJob)
blockDataSizeChecker := NewBlockDataChecker(cfg.zk.ShouldCountersBeUnlimited(batchState.isL1Recovery()))
streamWriter := newSequencerBatchStreamWriter(batchContext, batchState)

Expand Down
4 changes: 0 additions & 4 deletions zk/stages/stage_sequence_execute_resequence.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ func resequence(
return err
}

if err = cfg.dataStreamServer.UnwindToBatchStart(lastBatch + 1); err != nil {
return err
}

log.Info(fmt.Sprintf("[%s] Resequence from batch %d to %d in data stream", s.LogPrefix(), lastBatch+1, highestBatchInDs))
for _, batch := range batches {
batchJob := NewResequenceBatchJob(batch)
Expand Down
2 changes: 1 addition & 1 deletion zk/stages/stage_sequencer_l1_block_sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func SpawnSequencerL1BlockSyncStage(
log.Info(fmt.Sprintf("[%s] Starting L1 block sync stage", logPrefix))
defer log.Info(fmt.Sprintf("[%s] Finished L1 block sync stage", logPrefix))

if cfg.zkCfg.L1SyncStartBlock == 0 {
if !cfg.zkCfg.IsL1Recovery() {
log.Info(fmt.Sprintf("[%s] Skipping L1 block sync stage", logPrefix))
return nil
}
Expand Down
Loading