From 9c0b14aef4d74a209e42f30cbd2aa15b3cdfbd90 Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Mon, 4 Nov 2024 19:21:20 -0600 Subject: [PATCH 1/2] Add error case for parent of start of database --- op-supervisor/supervisor/backend/db/fromda/db.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/op-supervisor/supervisor/backend/db/fromda/db.go b/op-supervisor/supervisor/backend/db/fromda/db.go index 5ee609ecdfe7..c4c98624d26d 100644 --- a/op-supervisor/supervisor/backend/db/fromda/db.go +++ b/op-supervisor/supervisor/backend/db/fromda/db.go @@ -184,8 +184,13 @@ func (db *DB) PreviousDerivedFrom(derivedFrom eth.BlockID) (prevDerivedFrom type if self.derivedFrom.ID() != derivedFrom { return types.BlockSeal{}, fmt.Errorf("found %s, but expected %s: %w", self.derivedFrom, derivedFrom, types.ErrConflict) } - if selfIndex == 0 { // genesis block has a zeroed block as parent block - return types.BlockSeal{}, nil + if selfIndex == 0 { + // genesis block has a zeroed block as parent block + if self.derivedFrom.Number == 0 { + return types.BlockSeal{}, nil + } else { + return types.BlockSeal{}, fmt.Errorf("cannot find previous derived before start of database: %s", derivedFrom) + } } prev, err := db.readAt(selfIndex - 1) if err != nil { From c6927c6f0bec789356a9dcd09ae9a8cc7b603ef7 Mon Sep 17 00:00:00 2001 From: axelKingsley Date: Mon, 4 Nov 2024 20:25:37 -0600 Subject: [PATCH 2/2] fix unit tests --- .../supervisor/backend/db/fromda/db_test.go | 59 +++++++++++++------ 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/op-supervisor/supervisor/backend/db/fromda/db_test.go b/op-supervisor/supervisor/backend/db/fromda/db_test.go index fbcf82ab194e..8d6a273e1f98 100644 --- a/op-supervisor/supervisor/backend/db/fromda/db_test.go +++ b/op-supervisor/supervisor/backend/db/fromda/db_test.go @@ -131,74 +131,95 @@ func toRef(seal types.BlockSeal, parentHash common.Hash) eth.BlockRef { } func TestSingleEntryDB(t *testing.T) { - expectedDerivedFrom := mockL1(1) + expectedDerivedFrom := mockL1(0) expectedDerived := mockL2(2) runDBTest(t, func(t *testing.T, db *DB, m *stubMetrics) { require.NoError(t, db.AddDerived(toRef(expectedDerivedFrom, mockL1(0).Hash), toRef(expectedDerived, mockL2(0).Hash))) }, func(t *testing.T, db *DB, m *stubMetrics) { - derivedFrom, derived, err := db.Latest() + // First + derivedFrom, derived, err := db.First() require.NoError(t, err) require.Equal(t, expectedDerivedFrom, derivedFrom) require.Equal(t, expectedDerived, derived) - derivedFrom, derived, err = db.First() + // Latest + derivedFrom, derived, err = db.Latest() require.NoError(t, err) require.Equal(t, expectedDerivedFrom, derivedFrom) require.Equal(t, expectedDerived, derived) + // FirstAfter Latest + _, _, err = db.FirstAfter(derivedFrom.ID(), derived.ID()) + require.ErrorIs(t, err, types.ErrFuture) + + // LastDerivedAt derived, err = db.LastDerivedAt(expectedDerivedFrom.ID()) require.NoError(t, err) require.Equal(t, expectedDerived, derived) + // LastDerivedAt with a non-existent block _, err = db.LastDerivedAt(eth.BlockID{Hash: common.Hash{0xaa}, Number: expectedDerivedFrom.Number}) require.ErrorIs(t, err, types.ErrConflict) - // No block known, yet, after the given block pair - _, _, err = db.FirstAfter(derivedFrom.ID(), derived.ID()) - require.ErrorIs(t, err, types.ErrFuture) - - // Not after a non-existent block pair + // FirstAfter with a non-existent block (derived and derivedFrom) _, _, err = db.FirstAfter(eth.BlockID{Hash: common.Hash{0xaa}, Number: expectedDerivedFrom.Number}, expectedDerived.ID()) require.ErrorIs(t, err, types.ErrConflict) _, _, err = db.FirstAfter(expectedDerivedFrom.ID(), eth.BlockID{Hash: common.Hash{0xaa}, Number: expectedDerived.Number}) require.ErrorIs(t, err, types.ErrConflict) + // DerivedFrom derivedFrom, err = db.DerivedFrom(expectedDerived.ID()) require.NoError(t, err) require.Equal(t, expectedDerivedFrom, derivedFrom) + // DerivedFrom with a non-existent block _, err = db.DerivedFrom(eth.BlockID{Hash: common.Hash{0xbb}, Number: expectedDerived.Number}) require.ErrorIs(t, err, types.ErrConflict) + // PreviousDerived prev, err := db.PreviousDerived(expectedDerived.ID()) require.NoError(t, err) require.Equal(t, types.BlockSeal{}, prev, "zeroed seal before first entry") - _, _, err = db.NextDerived(expectedDerived.ID()) - require.ErrorIs(t, err, types.ErrFuture) - - // if 1 was the first inserted entry, then we skipped 0 - _, _, err = db.NextDerived(mockL2(0).ID()) - require.ErrorIs(t, err, types.ErrSkipped) - + // PreviousDerivedFrom prev, err = db.PreviousDerivedFrom(expectedDerivedFrom.ID()) require.NoError(t, err) require.Equal(t, types.BlockSeal{}, prev, "zeroed seal before first entry") - _, err = db.NextDerivedFrom(expectedDerivedFrom.ID()) + // NextDerived + _, _, err = db.NextDerived(expectedDerived.ID()) require.ErrorIs(t, err, types.ErrFuture) - // if 1 was the first inserted entry, then we skipped 0 - _, err = db.NextDerivedFrom(mockL1(0).ID()) - require.ErrorIs(t, err, types.ErrSkipped) + // NextDerivedFrom + _, err = db.NextDerivedFrom(expectedDerivedFrom.ID()) + require.ErrorIs(t, err, types.ErrFuture) + // FirstAfter _, _, err = db.FirstAfter(expectedDerivedFrom.ID(), expectedDerived.ID()) require.ErrorIs(t, err, types.ErrFuture) }) } +func TestGap(t *testing.T) { + // mockL1 starts at block 1 to produce a gap + expectedDerivedFrom := mockL1(1) + // mockL2 starts at block 2 to produce a gap + expectedDerived := mockL2(2) + runDBTest(t, + func(t *testing.T, db *DB, m *stubMetrics) { + require.NoError(t, db.AddDerived(toRef(expectedDerivedFrom, mockL1(0).Hash), toRef(expectedDerived, mockL2(0).Hash))) + }, + func(t *testing.T, db *DB, m *stubMetrics) { + _, _, err := db.NextDerived(mockL2(0).ID()) + require.ErrorIs(t, err, types.ErrSkipped) + + _, err = db.NextDerivedFrom(mockL1(0).ID()) + require.ErrorIs(t, err, types.ErrSkipped) + }) +} + func TestThreeEntryDB(t *testing.T) { l1Block0 := mockL1(0) l1Block1 := mockL1(1)