diff --git a/op-e2e/actions/helpers/l2_verifier.go b/op-e2e/actions/helpers/l2_verifier.go index de2a28afc39f8..82a125932ec56 100644 --- a/op-e2e/actions/helpers/l2_verifier.go +++ b/op-e2e/actions/helpers/l2_verifier.go @@ -160,9 +160,9 @@ func NewL2Verifier(t Testing, log log.Logger, l1 derive.L1Fetcher, var finalizer driver.Finalizer if cfg.AltDAEnabled() { - finalizer = finality.NewAltDAFinalizer(ctx, log, cfg, l1, altDASrc) + finalizer = finality.NewAltDAFinalizer(ctx, log, cfg, l1, altDASrc, ec) } else { - finalizer = finality.NewFinalizer(ctx, log, cfg, l1) + finalizer = finality.NewFinalizer(ctx, log, cfg, l1, ec) } sys.Register("finalizer", finalizer, opts) diff --git a/op-node/rollup/driver/driver.go b/op-node/rollup/driver/driver.go index 8d77bb3fec4eb..70582a49343f4 100644 --- a/op-node/rollup/driver/driver.go +++ b/op-node/rollup/driver/driver.go @@ -203,9 +203,9 @@ func NewDriver( var finalizer Finalizer if cfg.AltDAEnabled() { - finalizer = finality.NewAltDAFinalizer(driverCtx, log, cfg, l1, altDA) + finalizer = finality.NewAltDAFinalizer(driverCtx, log, cfg, l1, altDA, ec) } else { - finalizer = finality.NewFinalizer(driverCtx, log, cfg, l1) + finalizer = finality.NewFinalizer(driverCtx, log, cfg, l1, ec) } sys.Register("finalizer", finalizer) diff --git a/op-node/rollup/engine/engine_controller.go b/op-node/rollup/engine/engine_controller.go index 27396aea67015..34e824610fb90 100644 --- a/op-node/rollup/engine/engine_controller.go +++ b/op-node/rollup/engine/engine_controller.go @@ -698,19 +698,6 @@ func (d *EngineController) OnEvent(ctx context.Context, ev event.Event) bool { if !d.rollupCfg.IsInterop(x.Ref.Time) { d.PromoteSafe(ctx, x.Ref, x.Source) } - case PromoteFinalizedEvent: - if x.Ref.Number < d.Finalized().Number { - d.log.Error("Cannot rewind finality,", "ref", x.Ref, "finalized", d.Finalized()) - return true - } - if x.Ref.Number > d.SafeL2Head().Number { - d.log.Error("Block must be safe before it can be finalized", "ref", x.Ref, "safe", d.SafeL2Head()) - return true - } - d.SetFinalizedHead(x.Ref) - d.emitter.Emit(ctx, FinalizedUpdateEvent(x)) - // Try to apply the forkchoice changes - d.TryUpdateEngine(ctx) case InteropInvalidateBlockEvent: d.emitter.Emit(ctx, BuildStartEvent{Attributes: x.Attributes}) case BuildStartEvent: @@ -793,6 +780,21 @@ func (e *EngineController) PromoteSafe(ctx context.Context, ref eth.L2BlockRef, e.TryUpdateEngine(ctx) } +func (e *EngineController) PromoteFinalized(ctx context.Context, ref eth.L2BlockRef) { + if ref.Number < e.Finalized().Number { + e.log.Error("Cannot rewind finality,", "ref", ref, "finalized", e.Finalized()) + return + } + if ref.Number > e.SafeL2Head().Number { + e.log.Error("Block must be safe before it can be finalized", "ref", ref, "safe", e.SafeL2Head()) + return + } + e.SetFinalizedHead(ref) + e.emitter.Emit(ctx, FinalizedUpdateEvent{Ref: ref}) + // Try to apply the forkchoice changes + e.TryUpdateEngine(ctx) +} + // SetAttributesResetter sets the attributes component that needs force reset notifications func (e *EngineController) SetAttributesResetter(resetter AttributesForceResetter) { e.attributesResetter = resetter diff --git a/op-node/rollup/engine/events.go b/op-node/rollup/engine/events.go index 0d4a195477b6e..c59a54d063e9f 100644 --- a/op-node/rollup/engine/events.go +++ b/op-node/rollup/engine/events.go @@ -96,15 +96,6 @@ func (ev EngineResetConfirmedEvent) String() string { return "engine-reset-confirmed" } -// PromoteFinalizedEvent signals that a block can be marked as finalized. -type PromoteFinalizedEvent struct { - Ref eth.L2BlockRef -} - -func (ev PromoteFinalizedEvent) String() string { - return "promote-finalized" -} - // FinalizedUpdateEvent signals that a block has been marked as finalized. type FinalizedUpdateEvent struct { Ref eth.L2BlockRef diff --git a/op-node/rollup/finality/altda.go b/op-node/rollup/finality/altda.go index 1d87d1f0f5316..ae32725c70ef3 100644 --- a/op-node/rollup/finality/altda.go +++ b/op-node/rollup/finality/altda.go @@ -29,9 +29,9 @@ type AltDAFinalizer struct { func NewAltDAFinalizer(ctx context.Context, log log.Logger, cfg *rollup.Config, l1Fetcher FinalizerL1Interface, - backend AltDABackend) *AltDAFinalizer { + backend AltDABackend, ec EngineController) *AltDAFinalizer { - inner := NewFinalizer(ctx, log, cfg, l1Fetcher) + inner := NewFinalizer(ctx, log, cfg, l1Fetcher, ec) // In alt-da mode, the finalization signal is proxied through the AltDA manager. // Finality signal will come from the DA contract or L1 finality whichever is last. diff --git a/op-node/rollup/finality/altda_test.go b/op-node/rollup/finality/altda_test.go index a33f7053207aa..a3c1a3843867d 100644 --- a/op-node/rollup/finality/altda_test.go +++ b/op-node/rollup/finality/altda_test.go @@ -35,6 +35,16 @@ func (b *fakeAltDABackend) OnFinalizedHeadSignal(f altda.HeadSignalFn) { var _ AltDABackend = (*fakeAltDABackend)(nil) +type fakeEngineController struct { + finalizedL2 eth.L2BlockRef +} + +var _ EngineController = (*fakeEngineController)(nil) + +func (f *fakeEngineController) PromoteFinalized(_ context.Context, ref eth.L2BlockRef) { + f.finalizedL2 = ref +} + func TestAltDAFinalityData(t *testing.T) { logger := testlog.Logger(t, log.LevelInfo) l1F := &testutils.MockL1Source{} @@ -97,7 +107,8 @@ func TestAltDAFinalityData(t *testing.T) { } emitter := &testutils.MockEmitter{} - fi := NewAltDAFinalizer(context.Background(), logger, cfg, l1F, altDABackend) + ec := new(fakeEngineController) + fi := NewAltDAFinalizer(context.Background(), logger, cfg, l1F, altDABackend, ec) fi.AttachEmitter(emitter) require.NotNil(t, altDABackend.forwardTo, "altda backend must have access to underlying standard finalizer") @@ -167,20 +178,12 @@ func TestAltDAFinalityData(t *testing.T) { // of the safe block matches that of the finalized L1 block. l1F.ExpectL1BlockRefByNumber(commitmentInclusionFinalized.Number, commitmentInclusionFinalized, nil) l1F.ExpectL1BlockRefByNumber(commitmentInclusionFinalized.Number, commitmentInclusionFinalized, nil) - var finalizedL2 eth.L2BlockRef - emitter.ExpectOnceRun(func(ev event.Event) { - if x, ok := ev.(engine.PromoteFinalizedEvent); ok { - finalizedL2 = x.Ref - } else { - t.Fatalf("expected L2 finalization, but got: %s", ev) - } - }) fi.OnEvent(context.Background(), TryFinalizeEvent{}) l1F.AssertExpectations(t) emitter.AssertExpectations(t) - require.Equal(t, commitmentInclusionFinalized.Number, finalizedL2.L1Origin.Number+1) + require.Equal(t, commitmentInclusionFinalized.Number, ec.finalizedL2.L1Origin.Number+1) // Confirm finalization, so there will be no repeats of the PromoteFinalizedEvent - fi.OnEvent(context.Background(), engine.ForkchoiceUpdateEvent{FinalizedL2Head: finalizedL2}) + fi.OnEvent(context.Background(), engine.ForkchoiceUpdateEvent{FinalizedL2Head: ec.finalizedL2}) emitter.AssertExpectations(t) } } diff --git a/op-node/rollup/finality/finalizer.go b/op-node/rollup/finality/finalizer.go index 4d7e44cecf36a..4d520696d9440 100644 --- a/op-node/rollup/finality/finalizer.go +++ b/op-node/rollup/finality/finalizer.go @@ -66,6 +66,10 @@ type FinalizerL1Interface interface { L1BlockRefByNumber(context.Context, uint64) (eth.L1BlockRef, error) } +type EngineController interface { + PromoteFinalized(context.Context, eth.L2BlockRef) +} + type Finalizer struct { mu sync.Mutex @@ -77,6 +81,8 @@ type Finalizer struct { emitter event.Emitter + engineController EngineController + // finalizedL1 is the currently perceived finalized L1 block. // This may be ahead of the current traversed origin when syncing. finalizedL1 eth.L1BlockRef @@ -96,13 +102,14 @@ type Finalizer struct { l1Fetcher FinalizerL1Interface } -func NewFinalizer(ctx context.Context, log log.Logger, cfg *rollup.Config, l1Fetcher FinalizerL1Interface) *Finalizer { +func NewFinalizer(ctx context.Context, log log.Logger, cfg *rollup.Config, l1Fetcher FinalizerL1Interface, ec EngineController) *Finalizer { lookback := calcFinalityLookback(cfg) return &Finalizer{ ctx: ctx, cfg: cfg, log: log, finalizedL1: eth.L1BlockRef{}, + engineController: ec, triedFinalizeAt: 0, finalityData: make([]FinalityData, 0, lookback), finalityLookback: lookback, @@ -247,7 +254,7 @@ func (fi *Finalizer) tryFinalize() { }) return } - fi.emitter.Emit(fi.ctx, engine.PromoteFinalizedEvent{Ref: finalizedL2}) + fi.engineController.PromoteFinalized(ctx, finalizedL2) } } diff --git a/op-node/rollup/finality/finalizer_test.go b/op-node/rollup/finality/finalizer_test.go index 5bb5d78924e45..4ab1cf492c19d 100644 --- a/op-node/rollup/finality/finalizer_test.go +++ b/op-node/rollup/finality/finalizer_test.go @@ -193,7 +193,8 @@ func TestEngineQueue_Finalize(t *testing.T) { l1F.ExpectL1BlockRefByNumber(refD.Number, refD, nil) emitter := &testutils.MockEmitter{} - fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F) + ec := new(fakeEngineController) + fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F, ec) fi.AttachEmitter(emitter) // now say C1 was included in D and became the new safe head @@ -212,9 +213,9 @@ func TestEngineQueue_Finalize(t *testing.T) { fi.OnL1Finalized(refD) // C1 was included in finalized D, and should now be finalized - emitter.ExpectOnce(engine.PromoteFinalizedEvent{Ref: refC1}) fi.OnEvent(ctx, TryFinalizeEvent{}) emitter.AssertExpectations(t) + require.Equal(t, refC1, ec.finalizedL2) }) // Finality signal is received, but couldn't immediately be checked @@ -227,7 +228,8 @@ func TestEngineQueue_Finalize(t *testing.T) { l1F.ExpectL1BlockRefByNumber(refD.Number, refD, nil) // to check what was derived from (same in this case) emitter := &testutils.MockEmitter{} - fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F) + ec := new(fakeEngineController) + fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F, ec) fi.AttachEmitter(emitter) // now say C1 was included in D and became the new safe head @@ -254,9 +256,8 @@ func TestEngineQueue_Finalize(t *testing.T) { emitter.AssertExpectations(t) // C1 was included in finalized D, and should now be finalized, as check can succeed when revisited - emitter.ExpectOnce(engine.PromoteFinalizedEvent{Ref: refC1}) fi.OnEvent(ctx, TryFinalizeEvent{}) - emitter.AssertExpectations(t) + require.Equal(t, refC1, ec.finalizedL2) }) // Test that finality progression can repeat a few times. @@ -266,7 +267,8 @@ func TestEngineQueue_Finalize(t *testing.T) { defer l1F.AssertExpectations(t) emitter := &testutils.MockEmitter{} - fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F) + ec := new(fakeEngineController) + fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F, ec) fi.AttachEmitter(emitter) fi.OnEvent(ctx, engine.SafeDerivedEvent{Safe: refC1, Source: refD}) @@ -282,10 +284,10 @@ func TestEngineQueue_Finalize(t *testing.T) { fi.OnL1Finalized(refD) // C1 was included in D, and should be finalized now - emitter.ExpectOnce(engine.PromoteFinalizedEvent{Ref: refC1}) l1F.ExpectL1BlockRefByNumber(refD.Number, refD, nil) l1F.ExpectL1BlockRefByNumber(refD.Number, refD, nil) fi.OnEvent(ctx, TryFinalizeEvent{}) + require.Equal(t, refC1, ec.finalizedL2) emitter.AssertExpectations(t) l1F.AssertExpectations(t) @@ -294,10 +296,10 @@ func TestEngineQueue_Finalize(t *testing.T) { fi.OnL1Finalized(refE) // D0 was included in E, and should be finalized now - emitter.ExpectOnce(engine.PromoteFinalizedEvent{Ref: refD0}) l1F.ExpectL1BlockRefByNumber(refE.Number, refE, nil) l1F.ExpectL1BlockRefByNumber(refE.Number, refE, nil) fi.OnEvent(ctx, TryFinalizeEvent{}) + require.Equal(t, refD0, ec.finalizedL2) emitter.AssertExpectations(t) l1F.AssertExpectations(t) @@ -331,10 +333,10 @@ func TestEngineQueue_Finalize(t *testing.T) { fi.OnL1Finalized(refH) // F1 should be finalized now, since it was included in H - emitter.ExpectOnce(engine.PromoteFinalizedEvent{Ref: refF1}) l1F.ExpectL1BlockRefByNumber(refH.Number, refH, nil) l1F.ExpectL1BlockRefByNumber(refH.Number, refH, nil) fi.OnEvent(ctx, TryFinalizeEvent{}) + require.Equal(t, refF1, ec.finalizedL2) emitter.AssertExpectations(t) l1F.AssertExpectations(t) }) @@ -349,7 +351,8 @@ func TestEngineQueue_Finalize(t *testing.T) { l1F.ExpectL1BlockRefByNumber(refC.Number, refC, nil) // check what we derived the L2 block from emitter := &testutils.MockEmitter{} - fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F) + ec := new(fakeEngineController) + fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F, ec) fi.AttachEmitter(emitter) // now say B1 was included in C and became the new safe head @@ -367,8 +370,8 @@ func TestEngineQueue_Finalize(t *testing.T) { fi.OnL1Finalized(refD) // B1 was included in finalized D, and should now be finalized - emitter.ExpectOnce(engine.PromoteFinalizedEvent{Ref: refB1}) fi.OnEvent(ctx, TryFinalizeEvent{}) + require.Equal(t, refB1, ec.finalizedL2) emitter.AssertExpectations(t) }) @@ -385,7 +388,8 @@ func TestEngineQueue_Finalize(t *testing.T) { l1F.ExpectL1BlockRefByNumber(refE.Number, refE, nil) // post-reorg emitter := &testutils.MockEmitter{} - fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F) + ec := new(fakeEngineController) + fi := NewFinalizer(context.Background(), logger, &rollup.Config{}, l1F, ec) fi.AttachEmitter(emitter) // now say B1 was included in C and became the new safe head @@ -464,8 +468,8 @@ func TestEngineQueue_Finalize(t *testing.T) { emitter.ExpectOnce(TryFinalizeEvent{}) fi.OnEvent(ctx, derive.DeriverIdleEvent{Origin: refE}) emitter.AssertExpectations(t) - emitter.ExpectOnce(engine.PromoteFinalizedEvent{Ref: refC0}) fi.OnEvent(ctx, TryFinalizeEvent{}) + require.Equal(t, refC0, ec.finalizedL2) emitter.AssertExpectations(t) }) @@ -479,9 +483,10 @@ func TestEngineQueue_Finalize(t *testing.T) { l1F.ExpectL1BlockRefByNumber(refD.Number, refD, nil) emitter := &testutils.MockEmitter{} + ec := new(fakeEngineController) fi := NewFinalizer(context.Background(), logger, &rollup.Config{ InteropTime: &refC1.Time, - }, l1F) + }, l1F, ec) fi.AttachEmitter(emitter) // now say C0 and C1 were included in D and became the new safe head @@ -494,8 +499,8 @@ func TestEngineQueue_Finalize(t *testing.T) { fi.OnL1Finalized(refD) // C1 was Interop, C0 was not yet interop and can be finalized - emitter.ExpectOnce(engine.PromoteFinalizedEvent{Ref: refC0}) fi.OnEvent(ctx, TryFinalizeEvent{}) + require.Equal(t, refC0, ec.finalizedL2) emitter.AssertExpectations(t) }) } diff --git a/op-node/rollup/interop/indexing/system.go b/op-node/rollup/interop/indexing/system.go index 4c6dc489e22a1..61a7d9c519465 100644 --- a/op-node/rollup/interop/indexing/system.go +++ b/op-node/rollup/interop/indexing/system.go @@ -48,6 +48,7 @@ type L1Source interface { type EngineController interface { ForceReset(ctx context.Context, localUnsafe, crossUnsafe, localSafe, crossSafe, finalized eth.L2BlockRef) PromoteSafe(ctx context.Context, ref eth.L2BlockRef, source eth.L1BlockRef) + PromoteFinalized(ctx context.Context, ref eth.L2BlockRef) } // IndexingMode makes the op-node managed by an op-supervisor, @@ -307,9 +308,7 @@ func (m *IndexingMode) UpdateFinalized(ctx context.Context, id eth.BlockID) erro if err != nil { return fmt.Errorf("failed to get L2BlockRef: %w", err) } - m.emitter.Emit(m.ctx, engine.PromoteFinalizedEvent{Ref: l2Ref}) - // We return early: there is no point waiting for the finalized engine-update synchronously. - // All error-feedback comes to the supervisor by aborting derivation tasks with an error. + m.engineController.PromoteFinalized(ctx, l2Ref) return nil }