diff --git a/op-node/rollup/attributes/attributes.go b/op-node/rollup/attributes/attributes.go index fef5f5ddb38..12fa0cd4dd7 100644 --- a/op-node/rollup/attributes/attributes.go +++ b/op-node/rollup/attributes/attributes.go @@ -26,6 +26,7 @@ type EngineController interface { TryUpdateLocalSafe(ctx context.Context, ref eth.L2BlockRef, concluding bool, source eth.L1BlockRef) RequestForkchoiceUpdate(ctx context.Context) RequestPendingSafeUpdate(ctx context.Context) + IsEngineInitialELSyncing() bool } type L2 interface { @@ -203,6 +204,17 @@ func (eq *AttributesHandler) consolidateNextSafeAttributes(attributes *derive.At envelope, err := eq.l2.PayloadByNumber(ctx, attributes.Parent.Number+1) if err != nil { if errors.Is(err, ethereum.NotFound) { + if eq.engineController.IsEngineInitialELSyncing() { + // The EL has accepted a future EL-sync target but hasn't filled in + // pending_safe+1 yet. Resetting now would issue a forkchoice update + // that re-targets the EL away from its sync target and prevents EL + // sync from ever completing. Stall consolidation and retry once + // the EL catches up. + eq.emitter.Emit(eq.ctx, rollup.EngineTemporaryErrorEvent{ + Err: fmt.Errorf("waiting for EL sync to fill in block %d for consolidation: %w", attributes.Parent.Number+1, err), + }) + return + } // engine may have restarted, or inconsistent safe head. We need to reset eq.emitter.Emit(eq.ctx, rollup.ResetEvent{ Err: fmt.Errorf("expected engine was synced and had unsafe block to reconcile, but cannot find the block: %w", err), diff --git a/op-node/rollup/attributes/attributes_test.go b/op-node/rollup/attributes/attributes_test.go index 168255dbc61..08cb3455204 100644 --- a/op-node/rollup/attributes/attributes_test.go +++ b/op-node/rollup/attributes/attributes_test.go @@ -9,6 +9,7 @@ import ( "github.com/holiman/uint256" "github.com/stretchr/testify/require" + "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" @@ -339,6 +340,63 @@ func TestAttributesHandler(t *testing.T) { fn(t, false) }) }) + t.Run("not found during EL sync stalls without reset", func(t *testing.T) { + logger := testlog.Logger(t, log.LevelInfo) + l2 := &testutils.MockL2Client{} + emitter := &testutils.MockEmitter{} + engDeriver := &MockEngineController{} + ah := NewAttributesHandler(logger, cfg, context.Background(), l2, engDeriver) + ah.AttachEmitter(emitter) + + emitter.ExpectOnce(derive.ConfirmReceivedAttributesEvent{}) + engDeriver.On("RequestPendingSafeUpdate", context.Background()).Once() + ah.OnEvent(context.Background(), derive.DerivedAttributesEvent{Attributes: attrA1}) + engDeriver.AssertExpectations(t) + emitter.AssertExpectations(t) + require.NotNil(t, ah.attributes, "queued up derived attributes") + + // EL has accepted a future EL-sync target but hasn't filled in + // block refA1.Number yet, so the consolidate lookup returns NotFound. + l2.ExpectPayloadByNumber(refA1.Number, nil, ethereum.NotFound) + engDeriver.On("IsEngineInitialELSyncing").Return(true).Once() + + emitter.ExpectOnceType("EngineTemporaryErrorEvent") + ah.OnEvent(context.Background(), engine.PendingSafeUpdateEvent{ + PendingSafe: refA0, + Unsafe: refA1, + }) + engDeriver.AssertExpectations(t) + l2.AssertExpectations(t) + emitter.AssertExpectations(t) + require.NotNil(t, ah.attributes, + "attributes stay queued so consolidation retries once EL sync fills in the block") + }) + t.Run("not found without EL sync resets", func(t *testing.T) { + logger := testlog.Logger(t, log.LevelInfo) + l2 := &testutils.MockL2Client{} + emitter := &testutils.MockEmitter{} + engDeriver := &MockEngineController{} + ah := NewAttributesHandler(logger, cfg, context.Background(), l2, engDeriver) + ah.AttachEmitter(emitter) + + emitter.ExpectOnce(derive.ConfirmReceivedAttributesEvent{}) + engDeriver.On("RequestPendingSafeUpdate", context.Background()).Once() + ah.OnEvent(context.Background(), derive.DerivedAttributesEvent{Attributes: attrA1}) + engDeriver.AssertExpectations(t) + emitter.AssertExpectations(t) + + l2.ExpectPayloadByNumber(refA1.Number, nil, ethereum.NotFound) + engDeriver.On("IsEngineInitialELSyncing").Return(false).Once() + + emitter.ExpectOnceType("ResetEvent") + ah.OnEvent(context.Background(), engine.PendingSafeUpdateEvent{ + PendingSafe: refA0, + Unsafe: refA1, + }) + engDeriver.AssertExpectations(t) + l2.AssertExpectations(t) + emitter.AssertExpectations(t) + }) }) t.Run("pending equals unsafe", func(t *testing.T) { diff --git a/op-node/rollup/attributes/testutils.go b/op-node/rollup/attributes/testutils.go index bd6542c70df..9e52f3644e5 100644 --- a/op-node/rollup/attributes/testutils.go +++ b/op-node/rollup/attributes/testutils.go @@ -28,3 +28,8 @@ func (m *MockEngineController) RequestForkchoiceUpdate(ctx context.Context) { func (m *MockEngineController) RequestPendingSafeUpdate(ctx context.Context) { m.Mock.MethodCalled("RequestPendingSafeUpdate", ctx) } + +func (m *MockEngineController) IsEngineInitialELSyncing() bool { + out := m.Mock.MethodCalled("IsEngineInitialELSyncing") + return out.Bool(0) +} diff --git a/op-node/rollup/sequencing/sequencer_test.go b/op-node/rollup/sequencing/sequencer_test.go index 9fd1f774db7..47843023dd9 100644 --- a/op-node/rollup/sequencing/sequencer_test.go +++ b/op-node/rollup/sequencing/sequencer_test.go @@ -173,6 +173,8 @@ func (fakeEngController) TryUpdateLocalSafe(ctx context.Context, ref eth.L2Block func (fakeEngController) RequestPendingSafeUpdate(ctx context.Context) {} +func (fakeEngController) IsEngineInitialELSyncing() bool { return false } + // TestSequencer_StartStop runs through start/stop state back and forth to test state changes. func TestSequencer_StartStop(t *testing.T) { logger := testlog.Logger(t, log.LevelError)