Merge pull request #5047 from ethereum-optimism/aj/verify-unsafe-origin

op-node/rollup/derive: Check origin of L2 unsafe head match new L1 origin

op-e2e/actions/l2_sequencer_test.go

@@ -4,8 +4,6 @@ import (
 	"math/big"
 	"testing"
 
-	"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
-
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/params"
 
@@ -144,24 +142,19 @@ func TestL2Sequencer_SequencerOnlyReorg(gt *testing.T) {
 	// so it'll keep the L2 block with the old L1 origin, since no conflict is detected.
 	sequencer.ActL1HeadSignal(t)
 	sequencer.ActL2PipelineFull(t)
-	// TODO: CLI-3405 we can detect the inconsistency of the L1 origin of the unsafe L2 head:
-	//  as verifier, there is no need to wait for sequencer to recognize it.
+	// Verifier should detect the inconsistency of the L1 origin and reset the pipeline to follow the reorg
 	newStatus := sequencer.SyncStatus()
-	require.Equal(t, status.HeadL1.Hash, newStatus.UnsafeL2.L1Origin.Hash, "still have old bad L1 origin")
+	require.Zero(t, newStatus.UnsafeL2.L1Origin.Number, "back to genesis block with good L1 origin, drop old unsafe L2 chain with bad L1 origins")
 	require.NotEqual(t, status.HeadL1.Hash, newStatus.HeadL1.Hash, "did see the new L1 head change")
 	require.Equal(t, newStatus.HeadL1.Hash, newStatus.CurrentL1.Hash, "did sync the new L1 head as verifier")
 
 	// the block N+1 cannot build on the old N which still refers to the now orphaned L1 origin
 	require.Equal(t, status.UnsafeL2.L1Origin.Number, newStatus.HeadL1.Number-1, "seeing N+1 to attempt to build on N")
 	require.NotEqual(t, status.UnsafeL2.L1Origin.Hash, newStatus.HeadL1.ParentHash, "but N+1 cannot fit on N")
-	sequencer.ActL1HeadSignal(t)
-	// sequence more L2 blocks, until we actually need the next L1 origin
-	sequencer.ActBuildToL1HeadExclUnsafe(t)
-	// We expect block building to fail when the next L1 block is not consistent with the existing L1 origin
-	sequencer.ActL2StartBlockCheckErr(t, derive.ErrReset)
-	// After hitting a reset error, it reset derivation, and drops the old L1 chain
+
+	// After hitting a reset error, it resets derivation, and drops the old L1 chain
 	sequencer.ActL2PipelineFull(t)
-	require.Zero(t, sequencer.SyncStatus().UnsafeL2.L1Origin.Number, "back to genesis block with good L1 origin, drop old unsafe L2 chain with bad L1 origins")
+
 	// Can build new L2 blocks with good L1 origin
 	sequencer.ActBuildToL1HeadUnsafe(t)
 	require.Equal(t, newStatus.HeadL1.Hash, sequencer.SyncStatus().UnsafeL2.L1Origin.Hash, "build L2 chain with new correct L1 origins")

op-node/rollup/derive/engine_queue.go

@@ -224,7 +224,12 @@ func (eq *EngineQueue) Step(ctx context.Context) error {
 	}
 	outOfData := false
 	if len(eq.safeAttributes) == 0 {
-		eq.origin = eq.prev.Origin()
+		newOrigin := eq.prev.Origin()
+		// Check if the L2 unsafe head origin is consistent with the new origin
+		if err := eq.verifyNewL1Origin(ctx, newOrigin); err != nil {
+			return err
+		}
+		eq.origin = newOrigin
 		eq.postProcessSafeL2() // make sure we track the last L2 safe head for every new L1 block
 		if next, err := eq.prev.NextAttributes(ctx, eq.safeHead); err == io.EOF {
 			outOfData = true
@@ -246,6 +251,38 @@ func (eq *EngineQueue) Step(ctx context.Context) error {
 	}
 }
 
+// verifyNewL1Origin checks that the L2 unsafe head still has a L1 origin that is on the canonical chain.
+// If the unsafe head origin is after the new L1 origin it is assumed to still be canonical.
+// The check is only required when moving to a new L1 origin.
+func (eq *EngineQueue) verifyNewL1Origin(ctx context.Context, newOrigin eth.L1BlockRef) error {
+	if newOrigin == eq.origin {
+		return nil
+	}
+	unsafeOrigin := eq.unsafeHead.L1Origin
+	if newOrigin.Number == unsafeOrigin.Number && newOrigin.ID() != unsafeOrigin {
+		return NewResetError(fmt.Errorf("l1 origin was inconsistent with l2 unsafe head origin, need reset to resolve: l1 origin: %v; unsafe origin: %v",
+			newOrigin.ID(), unsafeOrigin))
+	}
+	// Avoid requesting an older block by checking against the parent hash
+	if newOrigin.Number == unsafeOrigin.Number+1 && newOrigin.ParentHash != unsafeOrigin.Hash {
+		return NewResetError(fmt.Errorf("l2 unsafe head origin is no longer canonical, need reset to resolve: canonical hash: %v; unsafe origin hash: %v",
+			newOrigin.ParentHash, unsafeOrigin.Hash))
+	}
+	if newOrigin.Number > unsafeOrigin.Number+1 {
+		// If unsafe origin is further behind new origin, check it's still on the canonical chain.
+		canonical, err := eq.l1Fetcher.L1BlockRefByNumber(ctx, unsafeOrigin.Number)
+		if err != nil {
+			return NewTemporaryError(fmt.Errorf("failed to fetch canonical L1 block at slot: %v; err: %w", unsafeOrigin.Number, err))
+		}
+		if canonical.ID() != unsafeOrigin {
+			eq.log.Error("Resetting due to origin mismatch")
+			return NewResetError(fmt.Errorf("l2 unsafe head origin is no longer canonical, need reset to resolve: canonical: %v; unsafe origin: %v",
+				canonical, unsafeOrigin))
+		}
+	}
+	return nil
+}
+
 // tryFinalizeL2 traverses the past L1 blocks, checks if any has been finalized,
 // and then marks the latest fully derived L2 block from this as finalized,
 // or defaults to the current finalized L2 block.