From f005beff72b1b82c83efa8119de9528c67724db4 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Sat, 16 Aug 2025 12:07:17 -0700 Subject: [PATCH 1/7] refactor: remove CrossUpdateRequestEvent --- op-node/rollup/driver/state.go | 2 +- op-node/rollup/engine/engine_controller.go | 28 ++++++++++++---------- op-node/rollup/engine/events.go | 10 -------- 3 files changed, 16 insertions(+), 24 deletions(-) diff --git a/op-node/rollup/driver/state.go b/op-node/rollup/driver/state.go index bf2cd65eb141f..0f30d53c00156 100644 --- a/op-node/rollup/driver/state.go +++ b/op-node/rollup/driver/state.go @@ -430,7 +430,7 @@ func (s *SyncDeriver) SyncStep() { // If interop is configured, we have to run the engine events, // to ensure cross-L2 safety is continuously verified against the interop-backend. if s.Config.InteropTime != nil && !s.ManagedBySupervisor { - s.Emitter.Emit(s.Ctx, engine.CrossUpdateRequestEvent{}) + s.Engine.OnCrossUpdate(s.Ctx, false, false) } } diff --git a/op-node/rollup/engine/engine_controller.go b/op-node/rollup/engine/engine_controller.go index 078294e2d1118..5bbc49dbd6104 100644 --- a/op-node/rollup/engine/engine_controller.go +++ b/op-node/rollup/engine/engine_controller.go @@ -224,6 +224,21 @@ func (e *EngineController) SetBackupUnsafeL2Head(r eth.L2BlockRef, triggerReorg e.needFCUCallForBackupUnsafeReorg = triggerReorg } +func (e *EngineController) OnCrossUpdate(ctx context.Context, crossUnsafe bool, crossSafe bool) { + if crossUnsafe { + e.emitter.Emit(ctx, CrossUnsafeUpdateEvent{ + CrossUnsafe: e.CrossUnsafeL2Head(), + LocalUnsafe: e.UnsafeL2Head(), + }) + } + if crossSafe { + e.emitter.Emit(ctx, CrossSafeUpdateEvent{ + CrossSafe: e.SafeL2Head(), + LocalSafe: e.LocalSafeL2Head(), + }) + } +} + // logSyncProgressMaybe helps log forkchoice state-changes when applicable. // First, the pre-state is registered. // A callback is returned to then log the changes to the pre-state, if any. @@ -718,19 +733,6 @@ func (d *EngineController) OnEvent(ctx context.Context, ev event.Event) bool { d.emitter.Emit(ctx, FinalizedUpdateEvent(x)) // Try to apply the forkchoice changes d.TryUpdateEngine(ctx) - case CrossUpdateRequestEvent: - if x.CrossUnsafe { - d.emitter.Emit(ctx, CrossUnsafeUpdateEvent{ - CrossUnsafe: d.CrossUnsafeL2Head(), - LocalUnsafe: d.UnsafeL2Head(), - }) - } - if x.CrossSafe { - d.emitter.Emit(ctx, CrossSafeUpdateEvent{ - CrossSafe: d.SafeL2Head(), - LocalSafe: d.LocalSafeL2Head(), - }) - } case InteropInvalidateBlockEvent: d.emitter.Emit(ctx, BuildStartEvent{Attributes: x.Attributes}) case BuildStartEvent: diff --git a/op-node/rollup/engine/events.go b/op-node/rollup/engine/events.go index f84c53c55803c..776508f5105f5 100644 --- a/op-node/rollup/engine/events.go +++ b/op-node/rollup/engine/events.go @@ -151,16 +151,6 @@ func (ev FinalizedUpdateEvent) String() string { return "finalized-update" } -// CrossUpdateRequestEvent triggers update events to be emitted, repeating the current state. -type CrossUpdateRequestEvent struct { - CrossUnsafe bool - CrossSafe bool -} - -func (ev CrossUpdateRequestEvent) String() string { - return "cross-update-request" -} - // InteropInvalidateBlockEvent is emitted when a block needs to be invalidated, and a replacement is needed. type InteropInvalidateBlockEvent struct { Invalidated eth.BlockRef From e88dc6726b6ee8cc0ddf223dea17abc96c970b50 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Sun, 17 Aug 2025 12:24:29 -0700 Subject: [PATCH 2/7] refactor: remove CrossUnsafeUpdateEvent and CrossSafeUpdateEvent --- op-e2e/actions/helpers/l2_verifier.go | 3 + op-node/rollup/driver/driver.go | 2 + op-node/rollup/engine/engine_controller.go | 64 ++++++++++++++-------- op-node/rollup/engine/events.go | 19 ------- op-node/rollup/status/status.go | 30 +++++++--- 5 files changed, 68 insertions(+), 50 deletions(-) diff --git a/op-e2e/actions/helpers/l2_verifier.go b/op-e2e/actions/helpers/l2_verifier.go index c27583963346a..29fd6d70c2383 100644 --- a/op-e2e/actions/helpers/l2_verifier.go +++ b/op-e2e/actions/helpers/l2_verifier.go @@ -173,6 +173,9 @@ func NewL2Verifier(t Testing, log log.Logger, l1 derive.L1Fetcher, syncStatusTracker := status.NewStatusTracker(log, metrics) sys.Register("status", syncStatusTracker, opts) + ec.SetCrossUnsafeUpdateHandler(syncStatusTracker) + ec.SetCrossSafeUpdateHandler(syncStatusTracker) + stepDeriver := NewTestingStepSchedulingDeriver() stepDeriver.AttachEmitter(testActionEmitter) diff --git a/op-node/rollup/driver/driver.go b/op-node/rollup/driver/driver.go index 5f921dc6328f3..7825e865d4bb6 100644 --- a/op-node/rollup/driver/driver.go +++ b/op-node/rollup/driver/driver.go @@ -191,6 +191,8 @@ func NewDriver( verifConfDepth := confdepth.NewConfDepth(driverCfg.VerifierConfDepth, statusTracker.L1Head, l1) ec := engine.NewEngineController(driverCtx, l2, log, metrics, cfg, syncCfg, sys.Register("engine-controller", nil)) + ec.SetCrossUnsafeUpdateHandler(statusTracker) + ec.SetCrossSafeUpdateHandler(statusTracker) sys.Register("engine-reset", engine.NewEngineResetDeriver(driverCtx, log, cfg, l1, l2, syncCfg)) diff --git a/op-node/rollup/engine/engine_controller.go b/op-node/rollup/engine/engine_controller.go index 5bbc49dbd6104..f2de925acf359 100644 --- a/op-node/rollup/engine/engine_controller.go +++ b/op-node/rollup/engine/engine_controller.go @@ -50,6 +50,14 @@ type SyncDeriver interface { OnELSyncStarted() } +type CrossUnsafeUpdateHandler interface { + OnCrossUnsafeUpdate(ctx context.Context, crossUnsafe eth.L2BlockRef, localUnsafe eth.L2BlockRef) +} + +type CrossSafeUpdateHandler interface { + OnCrossSafeUpdate(ctx context.Context, crossSafe eth.L2BlockRef, localSafe eth.L2BlockRef) +} + type EngineController struct { engine ExecEngine // Underlying execution engine RPC log log.Logger @@ -102,6 +110,10 @@ type EngineController struct { // EngineController is first initialized and used to initialize SyncDeriver. // Embed SyncDeriver into EngineController after initializing SyncDeriver SyncDeriver SyncDeriver + + // Handlers for cross-unsafe updates + crossUnsafeUpdateHandler CrossUnsafeUpdateHandler + crossSafeUpdateHandler CrossSafeUpdateHandler } func NewEngineController(ctx context.Context, engine ExecEngine, log log.Logger, m opmetrics.Metricer, @@ -224,18 +236,21 @@ func (e *EngineController) SetBackupUnsafeL2Head(r eth.L2BlockRef, triggerReorg e.needFCUCallForBackupUnsafeReorg = triggerReorg } +func (e *EngineController) SetCrossUnsafeUpdateHandler(handler CrossUnsafeUpdateHandler) { + e.crossUnsafeUpdateHandler = handler +} + +func (e *EngineController) SetCrossSafeUpdateHandler(handler CrossSafeUpdateHandler) { + e.crossSafeUpdateHandler = handler +} + func (e *EngineController) OnCrossUpdate(ctx context.Context, crossUnsafe bool, crossSafe bool) { - if crossUnsafe { - e.emitter.Emit(ctx, CrossUnsafeUpdateEvent{ - CrossUnsafe: e.CrossUnsafeL2Head(), - LocalUnsafe: e.UnsafeL2Head(), - }) + // Nil checks required because op-program omits these handlers + if crossUnsafe && e.crossUnsafeUpdateHandler != nil { + e.crossUnsafeUpdateHandler.OnCrossUnsafeUpdate(ctx, e.CrossUnsafeL2Head(), e.UnsafeL2Head()) } - if crossSafe { - e.emitter.Emit(ctx, CrossSafeUpdateEvent{ - CrossSafe: e.SafeL2Head(), - LocalSafe: e.LocalSafeL2Head(), - }) + if crossSafe && e.crossSafeUpdateHandler != nil { + e.crossSafeUpdateHandler.OnCrossSafeUpdate(ctx, e.SafeL2Head(), e.LocalSafeL2Head()) } } @@ -464,7 +479,10 @@ func (e *EngineController) InsertUnsafePayload(ctx context.Context, envelope *et e.emitter.Emit(ctx, UnsafeUpdateEvent{Ref: ref}) e.SetLocalSafeHead(ref) e.SetSafeHead(ref) - e.emitter.Emit(ctx, CrossSafeUpdateEvent{LocalSafe: ref, CrossSafe: ref}) + // Nil check required because op-program omits handler + if e.crossSafeUpdateHandler != nil { + e.crossSafeUpdateHandler.OnCrossSafeUpdate(ctx, ref, ref) + } e.SetFinalizedHead(ref) } logFn := e.logSyncProgressMaybe() @@ -686,10 +704,10 @@ func (d *EngineController) OnEvent(ctx context.Context, ev event.Event) bool { d.TryUpdateEngine(ctx) case PromoteCrossUnsafeEvent: d.SetCrossUnsafeHead(x.Ref) - d.emitter.Emit(ctx, CrossUnsafeUpdateEvent{ - CrossUnsafe: x.Ref, - LocalUnsafe: d.UnsafeL2Head(), - }) + // Nil check required because op-program omits handler + if d.crossUnsafeUpdateHandler != nil { + d.crossUnsafeUpdateHandler.OnCrossUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) + } case PendingSafeRequestEvent: d.emitter.Emit(ctx, PendingSafeUpdateEvent{ PendingSafe: d.PendingSafeL2Head(), @@ -706,17 +724,17 @@ func (d *EngineController) OnEvent(ctx context.Context, ev event.Event) bool { d.SetSafeHead(x.Ref) // Finalizer can pick up this safe cross-block now d.emitter.Emit(ctx, SafeDerivedEvent{Safe: x.Ref, Source: x.Source}) - d.emitter.Emit(ctx, CrossSafeUpdateEvent{ - CrossSafe: d.SafeL2Head(), - LocalSafe: d.LocalSafeL2Head(), - }) + // Nil check required because op-program omits handler + if d.crossSafeUpdateHandler != nil { + d.crossSafeUpdateHandler.OnCrossSafeUpdate(ctx, d.SafeL2Head(), d.LocalSafeL2Head()) + } if x.Ref.Number > d.crossUnsafeHead.Number { d.log.Debug("Cross Unsafe Head is stale, updating to match cross safe", "cross_unsafe", d.crossUnsafeHead, "cross_safe", x.Ref) d.SetCrossUnsafeHead(x.Ref) - d.emitter.Emit(ctx, CrossUnsafeUpdateEvent{ - CrossUnsafe: x.Ref, - LocalUnsafe: d.UnsafeL2Head(), - }) + // Nil check required because op-program omits handler + if d.crossUnsafeUpdateHandler != nil { + d.crossUnsafeUpdateHandler.OnCrossUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) + } } // Try to apply the forkchoice changes d.TryUpdateEngine(ctx) diff --git a/op-node/rollup/engine/events.go b/op-node/rollup/engine/events.go index 776508f5105f5..1bac262ce70b1 100644 --- a/op-node/rollup/engine/events.go +++ b/op-node/rollup/engine/events.go @@ -47,16 +47,6 @@ func (ev PromoteCrossUnsafeEvent) String() string { return "promote-cross-unsafe" } -// CrossUnsafeUpdateEvent signals that the given block is now considered cross-unsafe. -type CrossUnsafeUpdateEvent struct { - CrossUnsafe eth.L2BlockRef - LocalUnsafe eth.L2BlockRef -} - -func (ev CrossUnsafeUpdateEvent) String() string { - return "cross-unsafe-update" -} - type PendingSafeUpdateEvent struct { PendingSafe eth.L2BlockRef Unsafe eth.L2BlockRef // tip, added to the signal, to determine if there are existing blocks to consolidate @@ -66,15 +56,6 @@ func (ev PendingSafeUpdateEvent) String() string { return "pending-safe-update" } -type CrossSafeUpdateEvent struct { - CrossSafe eth.L2BlockRef - LocalSafe eth.L2BlockRef -} - -func (ev CrossSafeUpdateEvent) String() string { - return "cross-safe-update" -} - // LocalSafeUpdateEvent signals that a block is now considered to be local-safe. type LocalSafeUpdateEvent struct { Ref eth.L2BlockRef diff --git a/op-node/rollup/status/status.go b/op-node/rollup/status/status.go index f0f95f98e9398..042e306c99d67 100644 --- a/op-node/rollup/status/status.go +++ b/op-node/rollup/status/status.go @@ -60,17 +60,9 @@ func (st *StatusTracker) OnEvent(ctx context.Context, ev event.Event) bool { case engine.PendingSafeUpdateEvent: st.data.UnsafeL2 = x.Unsafe st.data.PendingSafeL2 = x.PendingSafe - case engine.CrossUnsafeUpdateEvent: - st.log.Debug("Cross unsafe head updated", "cross_unsafe", x.CrossUnsafe, "local_unsafe", x.LocalUnsafe) - st.data.CrossUnsafeL2 = x.CrossUnsafe - st.data.UnsafeL2 = x.LocalUnsafe case engine.LocalSafeUpdateEvent: st.log.Debug("Local safe head updated", "local_safe", x.Ref) st.data.LocalSafeL2 = x.Ref - case engine.CrossSafeUpdateEvent: - st.log.Debug("Cross safe head updated", "cross_safe", x.CrossSafe, "local_safe", x.LocalSafe) - st.data.SafeL2 = x.CrossSafe - st.data.LocalSafeL2 = x.LocalSafe case derive.DeriverL1StatusEvent: st.data.CurrentL1 = x.Origin case rollup.ResetEvent: @@ -151,3 +143,25 @@ func (st *StatusTracker) SyncStatus() *eth.SyncStatus { func (st *StatusTracker) L1Head() eth.L1BlockRef { return st.SyncStatus().HeadL1 } + +func (st *StatusTracker) OnCrossUnsafeUpdate(ctx context.Context, crossUnsafe eth.L2BlockRef, localUnsafe eth.L2BlockRef) { + st.mu.Lock() + defer st.mu.Unlock() + + st.log.Debug("Cross unsafe head updated", "cross_unsafe", crossUnsafe, "local_unsafe", localUnsafe) + st.data.CrossUnsafeL2 = crossUnsafe + st.data.UnsafeL2 = localUnsafe + + st.UpdateSyncStatus() +} + +func (st *StatusTracker) OnCrossSafeUpdate(ctx context.Context, crossSafe eth.L2BlockRef, localSafe eth.L2BlockRef) { + st.mu.Lock() + defer st.mu.Unlock() + + st.log.Debug("Cross safe head updated", "cross_safe", crossSafe, "local_safe", localSafe) + st.data.SafeL2 = crossSafe + st.data.LocalSafeL2 = localSafe + + st.UpdateSyncStatus() +} From 185acaeabf3ca20bf164d476867e2e8172a039b8 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Mon, 18 Aug 2025 07:00:15 -0700 Subject: [PATCH 3/7] build: compile time check that statusTracker implements the handlers --- op-node/rollup/status/status.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/op-node/rollup/status/status.go b/op-node/rollup/status/status.go index 042e306c99d67..0e9f42b42d31f 100644 --- a/op-node/rollup/status/status.go +++ b/op-node/rollup/status/status.go @@ -14,6 +14,10 @@ import ( "github.com/ethereum-optimism/optimism/op-service/event" ) +// Compile-time interface compliance checks +var _ engine.CrossUnsafeUpdateHandler = (*StatusTracker)(nil) +var _ engine.CrossSafeUpdateHandler = (*StatusTracker)(nil) + type Metrics interface { RecordL1ReorgDepth(d uint64) RecordL1Ref(name string, ref eth.L1BlockRef) From d9890899eaa53518a210b3e8ff9b191d34554e0a Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Mon, 18 Aug 2025 11:30:24 -0700 Subject: [PATCH 4/7] doc: add todo comments --- op-e2e/actions/helpers/l2_verifier.go | 1 + op-node/rollup/driver/driver.go | 1 + 2 files changed, 2 insertions(+) diff --git a/op-e2e/actions/helpers/l2_verifier.go b/op-e2e/actions/helpers/l2_verifier.go index 29fd6d70c2383..3abfd503af99b 100644 --- a/op-e2e/actions/helpers/l2_verifier.go +++ b/op-e2e/actions/helpers/l2_verifier.go @@ -173,6 +173,7 @@ func NewL2Verifier(t Testing, log log.Logger, l1 derive.L1Fetcher, syncStatusTracker := status.NewStatusTracker(log, metrics) sys.Register("status", syncStatusTracker, opts) + // TODO(#17115): Refactor dependency cycles ec.SetCrossUnsafeUpdateHandler(syncStatusTracker) ec.SetCrossSafeUpdateHandler(syncStatusTracker) diff --git a/op-node/rollup/driver/driver.go b/op-node/rollup/driver/driver.go index 7825e865d4bb6..72ccb8387bc20 100644 --- a/op-node/rollup/driver/driver.go +++ b/op-node/rollup/driver/driver.go @@ -191,6 +191,7 @@ func NewDriver( verifConfDepth := confdepth.NewConfDepth(driverCfg.VerifierConfDepth, statusTracker.L1Head, l1) ec := engine.NewEngineController(driverCtx, l2, log, metrics, cfg, syncCfg, sys.Register("engine-controller", nil)) + // TODO(#17115): Refactor dependency cycles ec.SetCrossUnsafeUpdateHandler(statusTracker) ec.SetCrossSafeUpdateHandler(statusTracker) From 223545584584110c47610a92fda551f4ef656e05 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Mon, 18 Aug 2025 11:43:15 -0700 Subject: [PATCH 5/7] refactor: remove no-op code --- op-node/rollup/driver/state.go | 5 ----- op-node/rollup/engine/engine_controller.go | 9 --------- 2 files changed, 14 deletions(-) diff --git a/op-node/rollup/driver/state.go b/op-node/rollup/driver/state.go index 0f30d53c00156..8bd4d9d6fdaa3 100644 --- a/op-node/rollup/driver/state.go +++ b/op-node/rollup/driver/state.go @@ -427,11 +427,6 @@ func (s *SyncDeriver) SyncStep() { // to generate new attributes, if no attributes are known already. s.Emitter.Emit(s.Ctx, engine.PendingSafeRequestEvent{}) - // If interop is configured, we have to run the engine events, - // to ensure cross-L2 safety is continuously verified against the interop-backend. - if s.Config.InteropTime != nil && !s.ManagedBySupervisor { - s.Engine.OnCrossUpdate(s.Ctx, false, false) - } } // ResetDerivationPipeline forces a reset of the derivation pipeline. diff --git a/op-node/rollup/engine/engine_controller.go b/op-node/rollup/engine/engine_controller.go index f2de925acf359..2c087269b19f0 100644 --- a/op-node/rollup/engine/engine_controller.go +++ b/op-node/rollup/engine/engine_controller.go @@ -244,15 +244,6 @@ func (e *EngineController) SetCrossSafeUpdateHandler(handler CrossSafeUpdateHand e.crossSafeUpdateHandler = handler } -func (e *EngineController) OnCrossUpdate(ctx context.Context, crossUnsafe bool, crossSafe bool) { - // Nil checks required because op-program omits these handlers - if crossUnsafe && e.crossUnsafeUpdateHandler != nil { - e.crossUnsafeUpdateHandler.OnCrossUnsafeUpdate(ctx, e.CrossUnsafeL2Head(), e.UnsafeL2Head()) - } - if crossSafe && e.crossSafeUpdateHandler != nil { - e.crossSafeUpdateHandler.OnCrossSafeUpdate(ctx, e.SafeL2Head(), e.LocalSafeL2Head()) - } -} // logSyncProgressMaybe helps log forkchoice state-changes when applicable. // First, the pre-state is registered. From ef45f7a4f59d1d8c18a3cc7afb633701eff1495f Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Mon, 18 Aug 2025 12:06:11 -0700 Subject: [PATCH 6/7] refactor: unify into a single CrossUpdateHandler --- op-e2e/actions/helpers/l2_verifier.go | 3 +- op-node/rollup/driver/driver.go | 3 +- op-node/rollup/engine/engine_controller.go | 37 +++++++++------------- op-node/rollup/status/status.go | 5 ++- 4 files changed, 19 insertions(+), 29 deletions(-) diff --git a/op-e2e/actions/helpers/l2_verifier.go b/op-e2e/actions/helpers/l2_verifier.go index 3abfd503af99b..f5eb7872adb6e 100644 --- a/op-e2e/actions/helpers/l2_verifier.go +++ b/op-e2e/actions/helpers/l2_verifier.go @@ -174,8 +174,7 @@ func NewL2Verifier(t Testing, log log.Logger, l1 derive.L1Fetcher, sys.Register("status", syncStatusTracker, opts) // TODO(#17115): Refactor dependency cycles - ec.SetCrossUnsafeUpdateHandler(syncStatusTracker) - ec.SetCrossSafeUpdateHandler(syncStatusTracker) + ec.SetCrossUpdateHandler(syncStatusTracker) stepDeriver := NewTestingStepSchedulingDeriver() stepDeriver.AttachEmitter(testActionEmitter) diff --git a/op-node/rollup/driver/driver.go b/op-node/rollup/driver/driver.go index 72ccb8387bc20..a3aeb9bc350df 100644 --- a/op-node/rollup/driver/driver.go +++ b/op-node/rollup/driver/driver.go @@ -192,8 +192,7 @@ func NewDriver( ec := engine.NewEngineController(driverCtx, l2, log, metrics, cfg, syncCfg, sys.Register("engine-controller", nil)) // TODO(#17115): Refactor dependency cycles - ec.SetCrossUnsafeUpdateHandler(statusTracker) - ec.SetCrossSafeUpdateHandler(statusTracker) + ec.SetCrossUpdateHandler(statusTracker) sys.Register("engine-reset", engine.NewEngineResetDeriver(driverCtx, log, cfg, l1, l2, syncCfg)) diff --git a/op-node/rollup/engine/engine_controller.go b/op-node/rollup/engine/engine_controller.go index 2c087269b19f0..27fae80495a44 100644 --- a/op-node/rollup/engine/engine_controller.go +++ b/op-node/rollup/engine/engine_controller.go @@ -50,11 +50,10 @@ type SyncDeriver interface { OnELSyncStarted() } -type CrossUnsafeUpdateHandler interface { +// CrossUpdateHandler handles both cross-unsafe and cross-safe L2 head changes. +// Nil check required because op-program omits this handler. +type CrossUpdateHandler interface { OnCrossUnsafeUpdate(ctx context.Context, crossUnsafe eth.L2BlockRef, localUnsafe eth.L2BlockRef) -} - -type CrossSafeUpdateHandler interface { OnCrossSafeUpdate(ctx context.Context, crossSafe eth.L2BlockRef, localSafe eth.L2BlockRef) } @@ -111,9 +110,8 @@ type EngineController struct { // Embed SyncDeriver into EngineController after initializing SyncDeriver SyncDeriver SyncDeriver - // Handlers for cross-unsafe updates - crossUnsafeUpdateHandler CrossUnsafeUpdateHandler - crossSafeUpdateHandler CrossSafeUpdateHandler + // Handler for cross-unsafe and cross-safe updates + crossUpdateHandler CrossUpdateHandler } func NewEngineController(ctx context.Context, engine ExecEngine, log log.Logger, m opmetrics.Metricer, @@ -236,15 +234,10 @@ func (e *EngineController) SetBackupUnsafeL2Head(r eth.L2BlockRef, triggerReorg e.needFCUCallForBackupUnsafeReorg = triggerReorg } -func (e *EngineController) SetCrossUnsafeUpdateHandler(handler CrossUnsafeUpdateHandler) { - e.crossUnsafeUpdateHandler = handler +func (e *EngineController) SetCrossUpdateHandler(handler CrossUpdateHandler) { + e.crossUpdateHandler = handler } -func (e *EngineController) SetCrossSafeUpdateHandler(handler CrossSafeUpdateHandler) { - e.crossSafeUpdateHandler = handler -} - - // logSyncProgressMaybe helps log forkchoice state-changes when applicable. // First, the pre-state is registered. // A callback is returned to then log the changes to the pre-state, if any. @@ -471,8 +464,8 @@ func (e *EngineController) InsertUnsafePayload(ctx context.Context, envelope *et e.SetLocalSafeHead(ref) e.SetSafeHead(ref) // Nil check required because op-program omits handler - if e.crossSafeUpdateHandler != nil { - e.crossSafeUpdateHandler.OnCrossSafeUpdate(ctx, ref, ref) + if e.crossUpdateHandler != nil { + e.crossUpdateHandler.OnCrossSafeUpdate(ctx, ref, ref) } e.SetFinalizedHead(ref) } @@ -696,8 +689,8 @@ func (d *EngineController) OnEvent(ctx context.Context, ev event.Event) bool { case PromoteCrossUnsafeEvent: d.SetCrossUnsafeHead(x.Ref) // Nil check required because op-program omits handler - if d.crossUnsafeUpdateHandler != nil { - d.crossUnsafeUpdateHandler.OnCrossUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) + if d.crossUpdateHandler != nil { + d.crossUpdateHandler.OnCrossUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) } case PendingSafeRequestEvent: d.emitter.Emit(ctx, PendingSafeUpdateEvent{ @@ -716,15 +709,15 @@ func (d *EngineController) OnEvent(ctx context.Context, ev event.Event) bool { // Finalizer can pick up this safe cross-block now d.emitter.Emit(ctx, SafeDerivedEvent{Safe: x.Ref, Source: x.Source}) // Nil check required because op-program omits handler - if d.crossSafeUpdateHandler != nil { - d.crossSafeUpdateHandler.OnCrossSafeUpdate(ctx, d.SafeL2Head(), d.LocalSafeL2Head()) + if d.crossUpdateHandler != nil { + d.crossUpdateHandler.OnCrossSafeUpdate(ctx, d.SafeL2Head(), d.LocalSafeL2Head()) } if x.Ref.Number > d.crossUnsafeHead.Number { d.log.Debug("Cross Unsafe Head is stale, updating to match cross safe", "cross_unsafe", d.crossUnsafeHead, "cross_safe", x.Ref) d.SetCrossUnsafeHead(x.Ref) // Nil check required because op-program omits handler - if d.crossUnsafeUpdateHandler != nil { - d.crossUnsafeUpdateHandler.OnCrossUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) + if d.crossUpdateHandler != nil { + d.crossUpdateHandler.OnCrossUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) } } // Try to apply the forkchoice changes diff --git a/op-node/rollup/status/status.go b/op-node/rollup/status/status.go index 0e9f42b42d31f..b6419f26f2c6b 100644 --- a/op-node/rollup/status/status.go +++ b/op-node/rollup/status/status.go @@ -14,9 +14,8 @@ import ( "github.com/ethereum-optimism/optimism/op-service/event" ) -// Compile-time interface compliance checks -var _ engine.CrossUnsafeUpdateHandler = (*StatusTracker)(nil) -var _ engine.CrossSafeUpdateHandler = (*StatusTracker)(nil) +// Compile-time interface compliance check +var _ engine.CrossUpdateHandler = (*StatusTracker)(nil) type Metrics interface { RecordL1ReorgDepth(d uint64) From baf92d761504a7261cfcbe38cf821e6cd565ebf5 Mon Sep 17 00:00:00 2001 From: Matt Solomon Date: Mon, 18 Aug 2025 12:26:19 -0700 Subject: [PATCH 7/7] refactor: extract nil checks to helper method --- op-node/rollup/engine/engine_controller.go | 34 ++++++++++++---------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/op-node/rollup/engine/engine_controller.go b/op-node/rollup/engine/engine_controller.go index 27fae80495a44..bcfa8f3d696b1 100644 --- a/op-node/rollup/engine/engine_controller.go +++ b/op-node/rollup/engine/engine_controller.go @@ -238,6 +238,20 @@ func (e *EngineController) SetCrossUpdateHandler(handler CrossUpdateHandler) { e.crossUpdateHandler = handler } +func (e *EngineController) onUnsafeUpdate(ctx context.Context, crossUnsafe, localUnsafe eth.L2BlockRef) { + // Nil check required because op-program omits this handler. + if e.crossUpdateHandler != nil { + e.crossUpdateHandler.OnCrossUnsafeUpdate(ctx, crossUnsafe, localUnsafe) + } +} + +func (e *EngineController) onSafeUpdate(ctx context.Context, crossSafe, localSafe eth.L2BlockRef) { + // Nil check required because op-program omits this handler. + if e.crossUpdateHandler != nil { + e.crossUpdateHandler.OnCrossSafeUpdate(ctx, crossSafe, localSafe) + } +} + // logSyncProgressMaybe helps log forkchoice state-changes when applicable. // First, the pre-state is registered. // A callback is returned to then log the changes to the pre-state, if any. @@ -463,10 +477,7 @@ func (e *EngineController) InsertUnsafePayload(ctx context.Context, envelope *et e.emitter.Emit(ctx, UnsafeUpdateEvent{Ref: ref}) e.SetLocalSafeHead(ref) e.SetSafeHead(ref) - // Nil check required because op-program omits handler - if e.crossUpdateHandler != nil { - e.crossUpdateHandler.OnCrossSafeUpdate(ctx, ref, ref) - } + e.onSafeUpdate(ctx, ref, ref) e.SetFinalizedHead(ref) } logFn := e.logSyncProgressMaybe() @@ -688,10 +699,7 @@ func (d *EngineController) OnEvent(ctx context.Context, ev event.Event) bool { d.TryUpdateEngine(ctx) case PromoteCrossUnsafeEvent: d.SetCrossUnsafeHead(x.Ref) - // Nil check required because op-program omits handler - if d.crossUpdateHandler != nil { - d.crossUpdateHandler.OnCrossUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) - } + d.onUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) case PendingSafeRequestEvent: d.emitter.Emit(ctx, PendingSafeUpdateEvent{ PendingSafe: d.PendingSafeL2Head(), @@ -708,17 +716,11 @@ func (d *EngineController) OnEvent(ctx context.Context, ev event.Event) bool { d.SetSafeHead(x.Ref) // Finalizer can pick up this safe cross-block now d.emitter.Emit(ctx, SafeDerivedEvent{Safe: x.Ref, Source: x.Source}) - // Nil check required because op-program omits handler - if d.crossUpdateHandler != nil { - d.crossUpdateHandler.OnCrossSafeUpdate(ctx, d.SafeL2Head(), d.LocalSafeL2Head()) - } + d.onSafeUpdate(ctx, d.SafeL2Head(), d.LocalSafeL2Head()) if x.Ref.Number > d.crossUnsafeHead.Number { d.log.Debug("Cross Unsafe Head is stale, updating to match cross safe", "cross_unsafe", d.crossUnsafeHead, "cross_safe", x.Ref) d.SetCrossUnsafeHead(x.Ref) - // Nil check required because op-program omits handler - if d.crossUpdateHandler != nil { - d.crossUpdateHandler.OnCrossUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) - } + d.onUnsafeUpdate(ctx, x.Ref, d.UnsafeL2Head()) } // Try to apply the forkchoice changes d.TryUpdateEngine(ctx)