diff --git a/op-node/rollup/driver/sync_deriver.go b/op-node/rollup/driver/sync_deriver.go index 752a70595be..41e1092a0b1 100644 --- a/op-node/rollup/driver/sync_deriver.go +++ b/op-node/rollup/driver/sync_deriver.go @@ -227,6 +227,18 @@ func (s *SyncDeriver) SyncStep() { s.tryBackupUnsafeReorg() + if s.Engine.IsEngineInitialELSyncing() { + // On startup in --syncmode=execution-layer, syncStatus is always + // syncStatusWillStartEL regardless of whether the engine is actually + // unsynced. Probe the engine's finalized head here so op-node can skip + // EL sync if the engine is already synced from a previous run + // (fixes #18468). Transitions out of syncStatusWillStartEL when + // applicable and triggers a head-initialization reset. + if err := s.Engine.MaybeSkipELSyncIfEngineAlreadySynced(s.Ctx); err != nil { + s.Log.Warn("Failed to probe engine finalized head on EL-sync startup", "err", err) + } + } + if s.Engine.IsEngineInitialELSyncing() { // The pipeline cannot move forwards if doing initial EL sync. s.Log.Debug("Rollup driver is backing off because execution engine is performing initial EL sync.", diff --git a/op-node/rollup/engine/engine_controller.go b/op-node/rollup/engine/engine_controller.go index 5db7a6c7f30..1cdb5f6005f 100644 --- a/op-node/rollup/engine/engine_controller.go +++ b/op-node/rollup/engine/engine_controller.go @@ -308,6 +308,74 @@ func (e *EngineController) isEngineInitialELSyncing() bool { e.syncStatus == syncStatusFinishedELButNotFinalized } +// checkEngineAlreadySynced queries the engine's finalized head to decide +// whether EL sync should be skipped. synced=true means the engine has a +// non-genesis finalized head and the operator did not opt into +// post-finalization EL sync; in that case EL sync must be skipped. synced=false +// indicates EL sync is needed (engine reports no finalized head, only genesis +// is finalized, or SupportsPostFinalizationELSync is set). +func (e *EngineController) checkEngineAlreadySynced(ctx context.Context) (synced bool, finalized eth.L2BlockRef, err error) { + finalized, err = e.engine.L2BlockRefByLabel(ctx, eth.Finalized) + if errors.Is(err, ethereum.NotFound) { + return false, eth.L2BlockRef{}, nil + } + if err != nil { + return false, eth.L2BlockRef{}, err + } + if finalized.Hash == e.rollupCfg.Genesis.L2.Hash { + return false, finalized, nil + } + if e.syncCfg.SupportsPostFinalizationELSync { + return false, finalized, nil + } + return true, finalized, nil +} + +// MaybeSkipELSyncIfEngineAlreadySynced is a startup guard for +// --syncmode=execution-layer: if the engine is already synced (has a +// non-genesis finalized head), transition syncStatus directly to +// syncStatusFinishedEL and emit a ResetEngineRequestEvent so op-node's +// in-memory heads are initialized from the engine via FindL2Heads. +// +// Without this, op-node restarted against an already-synced engine sits +// in syncStatusWillStartEL indefinitely because SyncStep backs off on +// isEngineInitialELSyncing() and the only place that transitions out of +// syncStatusWillStartEL is insertUnsafePayload — which requires a fresh +// unsafe payload that may never arrive (the sequencer's gossip is for +// blocks the engine already has). See #18468. +// +// No-op when syncStatus is not syncStatusWillStartEL. +func (e *EngineController) MaybeSkipELSyncIfEngineAlreadySynced(ctx context.Context) error { + // Emit must happen after releasing e.mu, because ResetEngineRequestEvent's + // handler re-acquires it via OnEvent. + shouldReset := false + err := func() error { + e.mu.Lock() + defer e.mu.Unlock() + if e.syncStatus != syncStatusWillStartEL { + return nil + } + synced, finalized, err := e.checkEngineAlreadySynced(ctx) + if err != nil { + return err + } + if !synced { + return nil + } + e.syncStatus = syncStatusFinishedEL + e.log.Info("Skipping EL sync on startup because the engine is already synced", "finalized", finalized.ID()) + shouldReset = true + return nil + }() + if err != nil { + return err + } + if shouldReset { + e.emitter.Emit(ctx, ResetEngineRequestEvent{}) + } + return nil +} + // SetFinalizedHead implements LocalEngineControl. func (e *EngineController) SetFinalizedHead(r eth.L2BlockRef) { e.metrics.RecordL2Ref("l2_finalized", r) @@ -571,22 +639,21 @@ func (e *EngineController) InsertUnsafePayload(ctx context.Context, envelope *et } func (e *EngineController) insertUnsafePayload(ctx context.Context, envelope *eth.ExecutionPayloadEnvelope, ref eth.L2BlockRef) error { - // Check if there is a finalized head once when doing EL sync. If so, transition to CL sync + // Check if there is a finalized head once when doing EL sync. If so, transition to CL sync. if e.syncStatus == syncStatusWillStartEL { - b, err := e.engine.L2BlockRefByLabel(ctx, eth.Finalized) - rollupGenesisIsFinalized := b.Hash == e.rollupCfg.Genesis.L2.Hash - if errors.Is(err, ethereum.NotFound) || rollupGenesisIsFinalized || e.syncCfg.SupportsPostFinalizationELSync { - e.syncStatus = syncStatusStartedEL - e.log.Info("Starting EL sync") - e.elStart = e.clock.Now() - e.SyncDeriver.OnELSyncStarted() - } else if err == nil { + synced, finalized, err := e.checkEngineAlreadySynced(ctx) + if err != nil { + return derive.NewTemporaryError(fmt.Errorf("failed to fetch finalized head: %w", err)) + } + if synced { e.syncStatus = syncStatusFinishedEL - e.log.Info("Skipping EL sync and going straight to CL sync because there is a finalized block", "id", b.ID()) + e.log.Info("Skipping EL sync and going straight to CL sync because there is a finalized block", "id", finalized.ID()) return nil - } else { - return derive.NewTemporaryError(fmt.Errorf("failed to fetch finalized head: %w", err)) } + e.syncStatus = syncStatusStartedEL + e.log.Info("Starting EL sync") + e.elStart = e.clock.Now() + e.SyncDeriver.OnELSyncStarted() } // Insert the payload & then call FCU newPayloadStart := time.Now() diff --git a/op-node/rollup/engine/engine_el_sync_startup_test.go b/op-node/rollup/engine/engine_el_sync_startup_test.go new file mode 100644 index 00000000000..636bde9c8c7 --- /dev/null +++ b/op-node/rollup/engine/engine_el_sync_startup_test.go @@ -0,0 +1,108 @@ +package engine + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/ethereum-optimism/optimism/op-node/metrics" + "github.com/ethereum-optimism/optimism/op-node/rollup" + "github.com/ethereum-optimism/optimism/op-node/rollup/sync" + "github.com/ethereum-optimism/optimism/op-service/eth" + "github.com/ethereum-optimism/optimism/op-service/testlog" + "github.com/ethereum-optimism/optimism/op-service/testutils" + "github.com/ethereum/go-ethereum/common" +) + +// TestMaybeSkipELSyncIfEngineAlreadySynced verifies the startup guard for +// #18468: when op-node starts in --syncmode=execution-layer against an +// already-synced engine, syncStatus must transition to syncStatusFinishedEL +// and a ResetEngineRequestEvent must be emitted so heads are initialized +// from the engine via FindL2Heads. Otherwise op-node stalls in +// syncStatusWillStartEL waiting for a fresh unsafe payload that never +// arrives. +func TestMaybeSkipELSyncIfEngineAlreadySynced(t *testing.T) { + genesisHash := common.HexToHash("0xdeadbeef") + genesisRef := eth.L2BlockRef{Hash: genesisHash, Number: 0} + finalizedRef := eth.L2BlockRef{Hash: common.HexToHash("0xcafe"), Number: 1000} + + cfg := &rollup.Config{ + Genesis: rollup.Genesis{ + L2: eth.BlockID{Hash: genesisHash, Number: 0}, + }, + } + + t.Run("engine synced: transitions to FinishedEL and emits reset", func(t *testing.T) { + mockEngine := &testutils.MockEngine{} + mockEngine.ExpectL2BlockRefByLabel(eth.Finalized, finalizedRef, nil) + + emitter := &testutils.MockEmitter{} + emitter.ExpectOnce(ResetEngineRequestEvent{}) + + ec := NewEngineController(context.Background(), mockEngine, testlog.Logger(t, 0), + metrics.NoopMetrics, cfg, &sync.Config{SyncMode: sync.ELSync}, false, + &testutils.MockL1Source{}, emitter, nil) + require.Equal(t, syncStatusWillStartEL, ec.syncStatus) + + require.NoError(t, ec.MaybeSkipELSyncIfEngineAlreadySynced(context.Background())) + + require.Equal(t, syncStatusFinishedEL, ec.syncStatus) + mockEngine.AssertExpectations(t) + emitter.AssertExpectations(t) + }) + + t.Run("engine at genesis finalized: stays in WillStartEL", func(t *testing.T) { + mockEngine := &testutils.MockEngine{} + mockEngine.ExpectL2BlockRefByLabel(eth.Finalized, genesisRef, nil) + + emitter := &testutils.MockEmitter{} + // No Emit expectation — any Emit call would fail AssertExpectations. + + ec := NewEngineController(context.Background(), mockEngine, testlog.Logger(t, 0), + metrics.NoopMetrics, cfg, &sync.Config{SyncMode: sync.ELSync}, false, + &testutils.MockL1Source{}, emitter, nil) + + require.NoError(t, ec.MaybeSkipELSyncIfEngineAlreadySynced(context.Background())) + + require.Equal(t, syncStatusWillStartEL, ec.syncStatus) + mockEngine.AssertExpectations(t) + emitter.AssertExpectations(t) + }) + + t.Run("SupportsPostFinalizationELSync: stays in WillStartEL", func(t *testing.T) { + mockEngine := &testutils.MockEngine{} + mockEngine.ExpectL2BlockRefByLabel(eth.Finalized, finalizedRef, nil) + + emitter := &testutils.MockEmitter{} + + ec := NewEngineController(context.Background(), mockEngine, testlog.Logger(t, 0), + metrics.NoopMetrics, cfg, + &sync.Config{SyncMode: sync.ELSync, SupportsPostFinalizationELSync: true}, false, + &testutils.MockL1Source{}, emitter, nil) + + require.NoError(t, ec.MaybeSkipELSyncIfEngineAlreadySynced(context.Background())) + + require.Equal(t, syncStatusWillStartEL, ec.syncStatus) + mockEngine.AssertExpectations(t) + emitter.AssertExpectations(t) + }) + + t.Run("not in WillStartEL: no-op, engine not queried", func(t *testing.T) { + mockEngine := &testutils.MockEngine{} + // No ExpectL2BlockRefByLabel — engine must not be queried. + + emitter := &testutils.MockEmitter{} + + ec := NewEngineController(context.Background(), mockEngine, testlog.Logger(t, 0), + metrics.NoopMetrics, cfg, &sync.Config{SyncMode: sync.CLSync}, false, + &testutils.MockL1Source{}, emitter, nil) + require.Equal(t, syncStatusCL, ec.syncStatus) + + require.NoError(t, ec.MaybeSkipELSyncIfEngineAlreadySynced(context.Background())) + + require.Equal(t, syncStatusCL, ec.syncStatus) + mockEngine.AssertExpectations(t) + emitter.AssertExpectations(t) + }) +}