op-node: Prefer following seq drift instead of conf depth (#3861)

* op-node: Prefer following seq drift instead of conf depth

We found a log on a testnet where a batch was rejected because it's
timestamp was greater than the origin timestamp + sequencer drift.
I looked into origin selection and determined we produce empty blocks
if the block time is ahead of the origin + seq drift; however if there
is a next origin, it is not enough to produce an empty block, we must
change the origin.

I hypothesize that this occurred because the L1 head that the node was
looking at was lagging. That would cause the node to keep using an
old origin even if the timestamp was past the max drift. I added
a unit test which shows this case & would fail without this fix
being applied.

* op-node: Don't immediately attempt to advance origin

If the current origin is past the sequencer drift, it needs to
ignore the conf depth staying, but does still need to check next
L2 block time against the next origin time.

I've also added a test for this case.

op-node/rollup/driver/origin_selector.go

@@ -47,7 +47,18 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l1Head eth.L1Bloc
 		return eth.L1BlockRef{}, err
 	}
 
-	if currentOrigin.Number+1+los.sequencingConfDepth > l1Head.Number {
+	// If we are past the sequencer depth, we may want to advance the origin, but need to still
+	// check the time of the next origin.
+	pastSeqDrift := l2Head.Time+los.cfg.BlockTime > currentOrigin.Time+los.cfg.MaxSequencerDrift
+	if pastSeqDrift {
+		log.Info("Next L2 block time is past the sequencer drift + current origin time",
+			"current", currentOrigin, "current_time", currentOrigin.Time,
+			"l1_head", l1Head, "l1_head_time", l1Head.Time,
+			"l2_head", l2Head, "l2_head_time", l2Head.Time,
+			"depth", los.sequencingConfDepth)
+	}
+
+	if !pastSeqDrift && currentOrigin.Number+1+los.sequencingConfDepth > l1Head.Number {
 		// TODO: we can decide to ignore confirmation depth if we would be forced
 		//  to make an empty block (only deposits) by staying on the current origin.
 		log.Info("sequencing with old origin to preserve conf depth",
@@ -62,6 +73,8 @@ func (los *L1OriginSelector) FindL1Origin(ctx context.Context, l1Head eth.L1Bloc
 	// the current origin block.
 	nextOrigin, err := los.l1.L1BlockRefByNumber(ctx, currentOrigin.Number+1)
 	if err != nil {
+		// TODO: this could result in a bad origin being selected if we are past the seq
+		// drift & should instead advance to the next origin.
 		log.Error("Failed to get next origin. Falling back to current origin", "err", err)
 		return currentOrigin, nil
 	}

op-node/rollup/driver/origin_selector_test.go

@@ -133,3 +133,86 @@ func TestOriginSelectorRespectsConfDepth(t *testing.T) {
 	require.Nil(t, err)
 	require.Equal(t, a, next)
 }
+
+// TestOriginSelectorRespectsMaxSeqDrift ensures that the origin selector
+// will advance if the time delta between the current L1 origin and the next
+// L2 block is greater than the sequencer drift. This needs to occur even
+// if conf depth needs to be ignored
+//
+// There are 2 L1 blocks at time 20 & 25. The L2 Head is at time 27.
+// The next L2 time is 29. The sequencer drift is 8 so the L2 head is
+// valid with origin `a`, but the next L2 block is not valid with origin `b.`
+// This is because 29 (next L2 time) > 20 (origin) + 8 (seq drift) => invalid block.
+// Even though the LOS would normally refuse to advance because block `b` does not
+// have enough confirmations, it should in this instance.
+func TestOriginSelectorRespectsMaxSeqDrift(t *testing.T) {
+	log := testlog.Logger(t, log.LvlCrit)
+	cfg := &rollup.Config{
+		MaxSequencerDrift: 8,
+		BlockTime:         2,
+	}
+	l1 := &testutils.MockL1Source{}
+	a := eth.L1BlockRef{
+		Hash:   common.Hash{'a'},
+		Number: 10,
+		Time:   20,
+	}
+	b := eth.L1BlockRef{
+		Hash:       common.Hash{'b'},
+		Number:     11,
+		Time:       25,
+		ParentHash: a.Hash,
+	}
+	l2Head := eth.L2BlockRef{
+		L1Origin: a.ID(),
+		Time:     27,
+	}
+
+	l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
+	l1.ExpectL1BlockRefByNumber(b.Number, b, nil)
+
+	s := NewL1OriginSelector(log, cfg, l1, 10)
+
+	next, err := s.FindL1Origin(context.Background(), b, l2Head)
+	require.Nil(t, err)
+	require.Equal(t, b, next)
+}
+
+// TestOriginSelectorSeqDriftRespectsNextOriginTime
+//
+// There are 2 L1 blocks at time 20 & 100. The L2 Head is at time 27.
+// The next L2 time is 29. Even though the next L2 time is past the seq
+// drift, the origin should remain on block `a` because the next origin's
+// time is greater than the next L2 time.
+func TestOriginSelectorSeqDriftRespectsNextOriginTime(t *testing.T) {
+	log := testlog.Logger(t, log.LvlCrit)
+	cfg := &rollup.Config{
+		MaxSequencerDrift: 8,
+		BlockTime:         2,
+	}
+	l1 := &testutils.MockL1Source{}
+	a := eth.L1BlockRef{
+		Hash:   common.Hash{'a'},
+		Number: 10,
+		Time:   20,
+	}
+	b := eth.L1BlockRef{
+		Hash:       common.Hash{'b'},
+		Number:     11,
+		Time:       100,
+		ParentHash: a.Hash,
+	}
+	l2Head := eth.L2BlockRef{
+		L1Origin: a.ID(),
+		Time:     27,
+	}
+
+	l1.ExpectL1BlockRefByHash(a.Hash, a, nil)
+	l1.ExpectL1BlockRefByNumber(b.Number, b, nil)
+
+	s := NewL1OriginSelector(log, cfg, l1, 10)
+
+	next, err := s.FindL1Origin(context.Background(), b, l2Head)
+	require.Nil(t, err)
+	require.Equal(t, a, next)
+}