op-node: Add flag category and improve testing (#9636)

op-node/flags/flags.go

@@ -292,10 +292,11 @@ var (
 		Category: RollupCategory,
 	}
 	SafeDBPath = &cli.StringFlag{
-		Name:    "safedb.path",
-		Usage:   "File path used to persist safe head update data. Disabled if not set.",
-		EnvVars: prefixEnvVars("SAFEDB_PATH"),
-		Hidden:  true,
+		Name:     "safedb.path",
+		Usage:    "File path used to persist safe head update data. Disabled if not set.",
+		EnvVars:  prefixEnvVars("SAFEDB_PATH"),
+		Hidden:   true,
+		Category: OperationsCategory,
 	}
 	/* Deprecated Flags */
 	L2EngineSyncEnabled = &cli.BoolFlag{

op-node/node/safedb/safedb.go

@@ -25,7 +25,7 @@ const (
 )
 
 var (
-	SafeByL1BlockNumKey = uint64Key{prefix: keyPrefixSafeByL1BlockNum}
+	safeByL1BlockNumKey = uint64Key{prefix: keyPrefixSafeByL1BlockNum}
 )
 
 type uint64Key struct {
@@ -61,7 +61,7 @@ type SafeDB struct {
 	closed bool
 }
 
-func SafeByL1BlockNumValue(l1 eth.BlockID, l2 eth.BlockID) []byte {
+func safeByL1BlockNumValue(l1 eth.BlockID, l2 eth.BlockID) []byte {
 	val := make([]byte, 0, 72)
 	val = append(val, l1.Hash.Bytes()...)
 	val = append(val, l2.Hash.Bytes()...)
@@ -69,7 +69,7 @@ func SafeByL1BlockNumValue(l1 eth.BlockID, l2 eth.BlockID) []byte {
 	return val
 }
 
-func DecodeSafeByL1BlockNum(key []byte, val []byte) (l1 eth.BlockID, l2 eth.BlockID, err error) {
+func decodeSafeByL1BlockNum(key []byte, val []byte) (l1 eth.BlockID, l2 eth.BlockID, err error) {
 	if len(key) != 9 || len(val) != 72 || key[0] != keyPrefixSafeByL1BlockNum {
 		err = ErrInvalidEntry
 		return
@@ -99,7 +99,7 @@ func (d *SafeDB) SafeHeadUpdated(safeHead eth.L2BlockRef, l1Head eth.BlockID) er
 	d.log.Info("Record safe head", "l2", safeHead.ID(), "l1", l1Head)
 	batch := d.db.NewBatch()
 	defer batch.Close()
-	if err := batch.Set(SafeByL1BlockNumKey.Of(l1Head.Number), SafeByL1BlockNumValue(l1Head, safeHead.ID()), d.writeOpts); err != nil {
+	if err := batch.Set(safeByL1BlockNumKey.Of(l1Head.Number), safeByL1BlockNumValue(l1Head, safeHead.ID()), d.writeOpts); err != nil {
 		return fmt.Errorf("failed to record safe head update: %w", err)
 	}
 	if err := batch.Commit(d.writeOpts); err != nil {
@@ -111,12 +111,12 @@ func (d *SafeDB) SafeHeadUpdated(safeHead eth.L2BlockRef, l1Head eth.BlockID) er
 func (d *SafeDB) SafeHeadReset(safeHead eth.L2BlockRef) error {
 	d.m.Lock()
 	defer d.m.Unlock()
-	iter, err := d.db.NewIter(SafeByL1BlockNumKey.IterRange())
+	iter, err := d.db.NewIter(safeByL1BlockNumKey.IterRange())
 	if err != nil {
 		return fmt.Errorf("reset failed to create iterator: %w", err)
 	}
 	defer iter.Close()
-	if valid := iter.SeekGE(SafeByL1BlockNumKey.Of(safeHead.L1Origin.Number)); !valid {
+	if valid := iter.SeekGE(safeByL1BlockNumKey.Of(safeHead.L1Origin.Number)); !valid {
 		// Reached end of column without finding any entries to delete
 		return nil
 	}
@@ -125,7 +125,7 @@ func (d *SafeDB) SafeHeadReset(safeHead eth.L2BlockRef) error {
 		if err != nil {
 			return fmt.Errorf("reset failed to read entry: %w", err)
 		}
-		l1Block, l2Block, err := DecodeSafeByL1BlockNum(iter.Key(), val)
+		l1Block, l2Block, err := decodeSafeByL1BlockNum(iter.Key(), val)
 		if err != nil {
 			return fmt.Errorf("reset encountered invalid entry: %w", err)
 		}
@@ -135,7 +135,7 @@ func (d *SafeDB) SafeHeadReset(safeHead eth.L2BlockRef) error {
 			hasPrevEntry := iter.Prev()
 			// Found the first entry that made the new safe head safe.
 			batch := d.db.NewBatch()
-			if err := batch.DeleteRange(l1HeadKey, SafeByL1BlockNumKey.Max(), d.writeOpts); err != nil {
+			if err := batch.DeleteRange(l1HeadKey, safeByL1BlockNumKey.Max(), d.writeOpts); err != nil {
 				return fmt.Errorf("reset failed to delete entries after %v: %w", l1HeadKey, err)
 			}
 
@@ -143,7 +143,7 @@ func (d *SafeDB) SafeHeadReset(safeHead eth.L2BlockRef) error {
 			// safe in that L1 block or if it was just before our records start, so don't record it as safe at the
 			// specified L1 block.
 			if hasPrevEntry {
-				if err := batch.Set(l1HeadKey, SafeByL1BlockNumValue(l1Block, safeHead.ID()), d.writeOpts); err != nil {
+				if err := batch.Set(l1HeadKey, safeByL1BlockNumValue(l1Block, safeHead.ID()), d.writeOpts); err != nil {
 					return fmt.Errorf("reset failed to record safe head update: %w", err)
 				}
 			}
@@ -153,7 +153,7 @@ func (d *SafeDB) SafeHeadReset(safeHead eth.L2BlockRef) error {
 			return nil
 		}
 		if valid := iter.Next(); !valid {
-			// Reached end of column without finding any entries to delete
+			// Reached end of column
 			return nil
 		}
 	}
@@ -162,12 +162,12 @@ func (d *SafeDB) SafeHeadReset(safeHead eth.L2BlockRef) error {
 func (d *SafeDB) SafeHeadAtL1(ctx context.Context, l1BlockNum uint64) (l1Block eth.BlockID, safeHead eth.BlockID, err error) {
 	d.m.RLock()
 	defer d.m.RUnlock()
-	iter, err := d.db.NewIterWithContext(ctx, SafeByL1BlockNumKey.IterRange())
+	iter, err := d.db.NewIterWithContext(ctx, safeByL1BlockNumKey.IterRange())
 	if err != nil {
 		return
 	}
 	defer iter.Close()
-	if valid := iter.SeekLT(SafeByL1BlockNumKey.Of(l1BlockNum + 1)); !valid {
+	if valid := iter.SeekLT(safeByL1BlockNumKey.Of(l1BlockNum + 1)); !valid {
 		err = ErrNotFound
 		return
 	}
@@ -176,7 +176,7 @@ func (d *SafeDB) SafeHeadAtL1(ctx context.Context, l1BlockNum uint64) (l1Block e
 	if err != nil {
 		return
 	}
-	l1Block, safeHead, err = DecodeSafeByL1BlockNum(iter.Key(), val)
+	l1Block, safeHead, err = decodeSafeByL1BlockNum(iter.Key(), val)
 	return
 }
 

op-node/node/safedb/safedb_test.go

@@ -214,11 +214,156 @@ func TestTruncateOnSafeHeadReset_BeforeFirstEntry(t *testing.T) {
 	require.ErrorIs(t, err, ErrNotFound)
 }
 
+func TestTruncateOnSafeHeadReset_AfterLastEntry(t *testing.T) {
+	logger := testlog.Logger(t, log.LvlInfo)
+	dir := t.TempDir()
+	db, err := NewSafeDB(logger, dir)
+	require.NoError(t, err)
+	defer db.Close()
+
+	l2a := eth.L2BlockRef{
+		Hash:   common.Hash{0x02, 0xaa},
+		Number: 20,
+		L1Origin: eth.BlockID{
+			Number: 60,
+		},
+	}
+	l2b := eth.L2BlockRef{
+		Hash:   common.Hash{0x02, 0xbb},
+		Number: 22,
+		L1Origin: eth.BlockID{
+			Number: 90,
+		},
+	}
+	l2c := eth.L2BlockRef{
+		Hash:   common.Hash{0x02, 0xcc},
+		Number: 25,
+		L1Origin: eth.BlockID{
+			Number: 110,
+		},
+	}
+	l1a := eth.BlockID{
+		Hash:   common.Hash{0x01, 0xaa},
+		Number: 100,
+	}
+	l1b := eth.BlockID{
+		Hash:   common.Hash{0x01, 0xbb},
+		Number: 150,
+	}
+	l1c := eth.BlockID{
+		Hash:   common.Hash{0x01, 0xcc},
+		Number: 160,
+	}
+
+	// Add some entries
+	require.NoError(t, db.SafeHeadUpdated(l2a, l1a))
+	require.NoError(t, db.SafeHeadUpdated(l2b, l1b))
+	require.NoError(t, db.SafeHeadUpdated(l2c, l1c))
+
+	verifySafeHeads := func() {
+		// Everything is still safe
+		actualL1, actualL2, err := db.SafeHeadAtL1(context.Background(), l1a.Number)
+		require.NoError(t, err)
+		require.Equal(t, l1a, actualL1)
+		require.Equal(t, l2a.ID(), actualL2)
+
+		// Everything is still safe
+		actualL1, actualL2, err = db.SafeHeadAtL1(context.Background(), l1b.Number)
+		require.NoError(t, err)
+		require.Equal(t, l1b, actualL1)
+		require.Equal(t, l2b.ID(), actualL2)
+
+		// Everything is still safe
+		actualL1, actualL2, err = db.SafeHeadAtL1(context.Background(), l1c.Number)
+		require.NoError(t, err)
+		require.Equal(t, l1c, actualL1)
+		require.Equal(t, l2c.ID(), actualL2)
+	}
+	verifySafeHeads()
+
+	// Then reset to an L2 block after all entries with an origin after all L1 entries
+	require.NoError(t, db.SafeHeadReset(eth.L2BlockRef{
+		Hash:   common.Hash{0x02, 0xdd},
+		Number: 30,
+		L1Origin: eth.BlockID{
+			Number: l1c.Number + 1,
+		},
+	}))
+	verifySafeHeads()
+
+	// Then reset to an L2 block after all entries with an origin before some L1 entries
+	require.NoError(t, db.SafeHeadReset(eth.L2BlockRef{
+		Hash:   common.Hash{0x02, 0xdd},
+		Number: 30,
+		L1Origin: eth.BlockID{
+			Number: l1b.Number - 1,
+		},
+	}))
+	verifySafeHeads()
+}
+
 func TestKeysFollowNaturalByteOrdering(t *testing.T) {
 	vals := []uint64{0, 1, math.MaxUint32 - 1, math.MaxUint32, math.MaxUint32 + 1, math.MaxUint64 - 1, math.MaxUint64}
 	for i := 1; i < len(vals); i++ {
-		prev := SafeByL1BlockNumKey.Of(vals[i-1])
-		cur := SafeByL1BlockNumKey.Of(vals[i])
+		prev := safeByL1BlockNumKey.Of(vals[i-1])
+		cur := safeByL1BlockNumKey.Of(vals[i])
 		require.True(t, slices.Compare(prev, cur) < 0, "Expected %v key %x to be less than %v key %x", vals[i-1], prev, vals[i], cur)
 	}
 }
+
+func TestDecodeSafeByL1BlockNum(t *testing.T) {
+	l1 := eth.BlockID{
+		Hash:   common.Hash{0x01},
+		Number: 84298,
+	}
+	l2 := eth.BlockID{
+		Hash:   common.Hash{0x02},
+		Number: 3224,
+	}
+	validKey := safeByL1BlockNumKey.Of(l1.Number)
+	validValue := safeByL1BlockNumValue(l1, l2)
+
+	t.Run("Roundtrip", func(t *testing.T) {
+		actualL1, actualL2, err := decodeSafeByL1BlockNum(validKey, validValue)
+		require.NoError(t, err)
+		require.Equal(t, l1, actualL1)
+		require.Equal(t, l2, actualL2)
+	})
+
+	t.Run("ErrorOnEmptyKey", func(t *testing.T) {
+		_, _, err := decodeSafeByL1BlockNum([]byte{}, validValue)
+		require.ErrorIs(t, err, ErrInvalidEntry)
+	})
+
+	t.Run("ErrorOnTooShortKey", func(t *testing.T) {
+		_, _, err := decodeSafeByL1BlockNum([]byte{1, 2, 3, 4}, validValue)
+		require.ErrorIs(t, err, ErrInvalidEntry)
+	})
+
+	t.Run("ErrorOnTooLongKey", func(t *testing.T) {
+		_, _, err := decodeSafeByL1BlockNum(append(validKey, 2), validValue)
+		require.ErrorIs(t, err, ErrInvalidEntry)
+	})
+
+	t.Run("ErrorOnWrongKeyPrefix", func(t *testing.T) {
+		invalidKey := slices.Clone(validKey)
+		invalidKey[0] = 49
+		_, _, err := decodeSafeByL1BlockNum(invalidKey, validValue)
+		require.ErrorIs(t, err, ErrInvalidEntry)
+	})
+
+	t.Run("ErrorOnEmptyValue", func(t *testing.T) {
+		_, _, err := decodeSafeByL1BlockNum(validKey, []byte{})
+		require.ErrorIs(t, err, ErrInvalidEntry)
+	})
+
+	t.Run("ErrorOnTooShortValue", func(t *testing.T) {
+		_, _, err := decodeSafeByL1BlockNum(validKey, []byte{1, 2, 3, 4})
+		require.ErrorIs(t, err, ErrInvalidEntry)
+	})
+
+	t.Run("ErrorOnTooLongValue", func(t *testing.T) {
+		_, _, err := decodeSafeByL1BlockNum(validKey, append(validKey, 2))
+		require.ErrorIs(t, err, ErrInvalidEntry)
+	})
+}