From a76ec163877061099fc05253cc1d78bd502dc379 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Mon, 22 Jan 2024 11:17:14 -0600 Subject: [PATCH] fix BackfillStatus init tests --- beacon-chain/db/errors.go | 3 - beacon-chain/sync/backfill/status.go | 1 - beacon-chain/sync/backfill/status_test.go | 216 +++++----------------- testing/assertions/assertions.go | 19 +- testing/require/requires.go | 5 + 5 files changed, 70 insertions(+), 174 deletions(-) diff --git a/beacon-chain/db/errors.go b/beacon-chain/db/errors.go index 29a5cb8d84ec..c9fc7351b1a2 100644 --- a/beacon-chain/db/errors.go +++ b/beacon-chain/db/errors.go @@ -22,9 +22,6 @@ var ErrNotFoundOriginBlockRoot = kv.ErrNotFoundOriginBlockRoot // ErrNotFoundBackfillBlockRoot wraps ErrNotFound for an error specific to the backfill block root. var ErrNotFoundBackfillBlockRoot = kv.ErrNotFoundBackfillBlockRoot -// ErrNotFoundGenesisBlockRoot means no genesis block root was found, indicating the db was not initialized with genesis -var ErrNotFoundGenesisBlockRoot = kv.ErrNotFoundGenesisBlockRoot - // IsNotFound allows callers to treat errors from a flat-file database, where the file record is missing, // as equivalent to db.ErrNotFound. func IsNotFound(err error) bool { diff --git a/beacon-chain/sync/backfill/status.go b/beacon-chain/sync/backfill/status.go index 410ebda35418..79c68e38e8dd 100644 --- a/beacon-chain/sync/backfill/status.go +++ b/beacon-chain/sync/backfill/status.go @@ -164,6 +164,5 @@ type BeaconDB interface { OriginCheckpointBlockRoot(context.Context) ([32]byte, error) Block(context.Context, [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) SaveROBlocks(ctx context.Context, blks []blocks.ROBlock, cache bool) error - GenesisBlockRoot(context.Context) ([32]byte, error) StateOrError(ctx context.Context, blockRoot [32]byte) (state.BeaconState, error) } diff --git a/beacon-chain/sync/backfill/status_test.go b/beacon-chain/sync/backfill/status_test.go index 36fae2044078..0ee020827557 100644 --- a/beacon-chain/sync/backfill/status_test.go +++ b/beacon-chain/sync/backfill/status_test.go @@ -1,6 +1,7 @@ package backfill import ( + "bytes" "context" "testing" @@ -12,7 +13,6 @@ import ( "github.com/prysmaticlabs/prysm/v4/proto/dbval" "github.com/pkg/errors" - "github.com/prysmaticlabs/prysm/v4/config/params" "github.com/prysmaticlabs/prysm/v4/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v4/testing/require" "github.com/prysmaticlabs/prysm/v4/testing/util" @@ -22,7 +22,6 @@ var errEmptyMockDBMethod = errors.New("uninitialized mock db method called") type mockBackfillDB struct { saveBackfillBlockRoot func(ctx context.Context, blockRoot [32]byte) error - genesisBlockRoot func(ctx context.Context) ([32]byte, error) originCheckpointBlockRoot func(ctx context.Context) ([32]byte, error) block func(ctx context.Context, blockRoot [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) saveBackfillStatus func(ctx context.Context, status *dbval.BackfillStatus) error @@ -58,13 +57,6 @@ func (d *mockBackfillDB) BackfillStatus(ctx context.Context) (*dbval.BackfillSta return d.status, nil } -func (d *mockBackfillDB) GenesisBlockRoot(ctx context.Context) ([32]byte, error) { - if d.genesisBlockRoot != nil { - return d.genesisBlockRoot(ctx) - } - return [32]byte{}, errEmptyMockDBMethod -} - func (d *mockBackfillDB) OriginCheckpointBlockRoot(ctx context.Context) ([32]byte, error) { if d.originCheckpointBlockRoot != nil { return d.originCheckpointBlockRoot(ctx) @@ -166,9 +158,8 @@ func setupTestBlock(slot primitives.Slot) (interfaces.ReadOnlySignedBeaconBlock, return blocktest.SetBlockSlot(b, slot) } -func TestReload(t *testing.T) { +func TestNewUpdater(t *testing.T) { ctx := context.Background() - derp := errors.New("derp") originSlot := primitives.Slot(100) var originRoot [32]byte @@ -178,164 +169,39 @@ func TestReload(t *testing.T) { backfillSlot := primitives.Slot(50) var backfillRoot [32]byte - copy(originRoot[:], []byte{0x02}) + copy(backfillRoot[:], []byte{0x02}) backfillBlock, err := setupTestBlock(backfillSlot) require.NoError(t, err) - + var parentRoot [32]byte + copy(parentRoot[:], []byte{0x03}) + var rootSlice = func(r [32]byte) []byte { return r[:] } + typicalBackfillStatus := &dbval.BackfillStatus{ + LowSlot: 23, + LowRoot: backfillRoot[:], + LowParentRoot: parentRoot[:], + OriginSlot: 1123, + OriginRoot: originRoot[:], + } cases := []struct { name string db BeaconDB err error expected *Store }{ - /*{ - name: "origin not found, implying genesis sync ", - db: &mockBackfillDB{ - genesisBlockRoot: goodBlockRoot(params.BeaconConfig().ZeroHash), - originCheckpointBlockRoot: func(ctx context.Context) ([32]byte, error) { - return [32]byte{}, db.ErrNotFoundOriginBlockRoot - }}, - expected: &StatusUpdater{genesisSync: true}, - }, - { - name: "genesis not found error", - err: db.ErrNotFoundGenesisBlockRoot, - db: &mockBackfillDB{ - genesisBlockRoot: func(ctx context.Context) ([32]byte, error) { - return [32]byte{}, db.ErrNotFoundGenesisBlockRoot - }, - originCheckpointBlockRoot: goodBlockRoot(originRoot), - block: func(ctx context.Context, root [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) { - switch root { - case originRoot: - return originBlock, nil - } - return nil, nil - }, - }, - }, { - name: "other genesis error", - err: derp, + name: "origin not found, implying genesis sync ", db: &mockBackfillDB{ - genesisBlockRoot: func(ctx context.Context) ([32]byte, error) { - return [32]byte{}, derp + backfillStatus: func(context.Context) (*dbval.BackfillStatus, error) { + return nil, db.ErrNotFound }, - originCheckpointBlockRoot: goodBlockRoot(originRoot), - block: func(ctx context.Context, root [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) { - switch root { - case originRoot: - return originBlock, nil - } - return nil, nil - }, - }, - }, - { - name: "origin other error", - db: &mockBackfillDB{ - genesisBlockRoot: goodBlockRoot(params.BeaconConfig().ZeroHash), originCheckpointBlockRoot: func(ctx context.Context) ([32]byte, error) { - return [32]byte{}, derp + return [32]byte{}, db.ErrNotFoundOriginBlockRoot }}, - err: derp, - }, - { - name: "origin root found, block missing", - db: &mockBackfillDB{ - genesisBlockRoot: goodBlockRoot(params.BeaconConfig().ZeroHash), - originCheckpointBlockRoot: goodBlockRoot(originRoot), - block: func(ctx context.Context, root [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) { - return nil, nil - }, - }, - err: blocks.ErrNilSignedBeaconBlock, - }, - { - name: "origin root found, block error", - db: &mockBackfillDB{ - genesisBlockRoot: goodBlockRoot(params.BeaconConfig().ZeroHash), - originCheckpointBlockRoot: goodBlockRoot(originRoot), - block: func(ctx context.Context, root [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) { - return nil, derp - }, - }, - err: derp, + expected: &Store{genesisSync: true}, }, - { - name: "origin root found, block found, backfill root not found", - db: &mockBackfillDB{ - genesisBlockRoot: goodBlockRoot(params.BeaconConfig().ZeroHash), - originCheckpointBlockRoot: goodBlockRoot(originRoot), - block: func(ctx context.Context, root [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) { - return originBlock, nil - }, - backfillBlockRoot: func(ctx context.Context) ([32]byte, error) { - return [32]byte{}, db.ErrNotFoundBackfillBlockRoot - }, - }, - err: db.ErrNotFoundBackfillBlockRoot, - }, - { - name: "origin root found, block found, random backfill root err", - db: &mockBackfillDB{ - genesisBlockRoot: goodBlockRoot(params.BeaconConfig().ZeroHash), - originCheckpointBlockRoot: goodBlockRoot(originRoot), - block: func(ctx context.Context, root [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) { - switch root { - case originRoot: - return originBlock, nil - case backfillRoot: - return nil, nil - } - return nil, derp - }, - backfillBlockRoot: func(ctx context.Context) ([32]byte, error) { - return [32]byte{}, derp - }, - }, - err: derp, - }, - { - name: "origin root found, block found, backfill root found, backfill block not found", - db: &mockBackfillDB{ - genesisBlockRoot: goodBlockRoot(params.BeaconConfig().ZeroHash), - originCheckpointBlockRoot: goodBlockRoot(originRoot), - block: func(ctx context.Context, root [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) { - switch root { - case originRoot: - return originBlock, nil - case backfillRoot: - return nil, nil - } - return nil, derp - }, - backfillBlockRoot: goodBlockRoot(backfillRoot), - }, - err: blocks.ErrNilSignedBeaconBlock, - }, - { - name: "origin root found, block found, backfill root found, backfill block random err", - db: &mockBackfillDB{ - genesisBlockRoot: goodBlockRoot(params.BeaconConfig().ZeroHash), - originCheckpointBlockRoot: goodBlockRoot(originRoot), - block: func(ctx context.Context, root [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) { - switch root { - case originRoot: - return originBlock, nil - case backfillRoot: - return nil, derp - } - return nil, errors.New("not derp") - }, - backfillBlockRoot: goodBlockRoot(backfillRoot), - }, - err: derp, - },*/ { name: "legacy recovery", db: &mockBackfillDB{ - genesisBlockRoot: goodBlockRoot(params.BeaconConfig().ZeroHash), originCheckpointBlockRoot: goodBlockRoot(originRoot), block: func(ctx context.Context, root [32]byte) (interfaces.ReadOnlySignedBeaconBlock, error) { switch root { @@ -348,22 +214,42 @@ func TestReload(t *testing.T) { }, backfillStatus: func(context.Context) (*dbval.BackfillStatus, error) { return nil, db.ErrNotFound }, }, - err: derp, - expected: &Store{genesisSync: false, bs: &dbval.BackfillStatus{LowSlot: uint64(originSlot)}}, + expected: &Store{bs: &dbval.BackfillStatus{ + LowSlot: uint64(originSlot), OriginSlot: uint64(originSlot), + LowRoot: originRoot[:], OriginRoot: originRoot[:], LowParentRoot: rootSlice(originBlock.Block().ParentRoot()), + }}, + }, + { + name: "backfill found", + db: &mockBackfillDB{backfillStatus: func(ctx context.Context) (*dbval.BackfillStatus, error) { + return typicalBackfillStatus, nil + }}, + expected: &Store{bs: typicalBackfillStatus}, }, } for _, c := range cases { - s, err := NewUpdater(ctx, c.db) - if err != nil { - require.ErrorIs(t, err, c.err) - continue - } - require.NoError(t, err) - if c.expected == nil { - continue - } - require.Equal(t, c.expected.genesisSync, s.genesisSync) - require.Equal(t, c.expected.bs.LowSlot, s.bs.LowSlot) + t.Run(c.name, func(t *testing.T) { + s, err := NewUpdater(ctx, c.db) + if c.err != nil { + require.ErrorIs(t, err, c.err) + return + } + require.NoError(t, err) + if c.expected == nil { + return + } + require.Equal(t, c.expected.genesisSync, s.genesisSync) + if c.expected.genesisSync { + require.IsNil(t, s.bs) + return + } + require.Equal(t, c.expected.bs.LowSlot, s.bs.LowSlot) + require.Equal(t, c.expected.bs.OriginSlot, s.bs.OriginSlot) + require.Equal(t, true, bytes.Equal(c.expected.bs.OriginRoot, s.bs.OriginRoot)) + require.Equal(t, true, bytes.Equal(c.expected.bs.LowRoot, s.bs.LowRoot)) + require.Equal(t, true, bytes.Equal(c.expected.bs.LowParentRoot, s.bs.LowParentRoot)) + }) } + } diff --git a/testing/assertions/assertions.go b/testing/assertions/assertions.go index f88d89031b80..6fcf998bba11 100644 --- a/testing/assertions/assertions.go +++ b/testing/assertions/assertions.go @@ -169,19 +169,28 @@ func ErrorContains(loggerFn assertionLoggerFn, want string, err error, msg ...in // NotNil asserts that passed value is not nil. func NotNil(loggerFn assertionLoggerFn, obj interface{}, msg ...interface{}) { - if isNil(obj) { + if deepNil(obj) { errMsg := parseMsg("Unexpected nil value", msg...) _, file, line, _ := runtime.Caller(2) loggerFn("%s:%d %s", filepath.Base(file), line, errMsg) } } -// isNil checks that underlying value of obj is nil. -func isNil(obj interface{}) bool { - if obj == nil { +// IsNil asserts that observed value is nil. +func IsNil(loggerFn assertionLoggerFn, got interface{}, msg ...interface{}) { + if !deepNil(got) { + errMsg := parseMsg("Value is unexpectedly not nil", msg...) + _, file, line, _ := runtime.Caller(2) + loggerFn("%s:%d %s", filepath.Base(file), line, errMsg) + } +} + +// deepNil checks that underlying value of obj is nil. +func deepNil(got interface{}) bool { + if got == nil { return true } - value := reflect.ValueOf(obj) + value := reflect.ValueOf(got) switch value.Kind() { case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Ptr, reflect.Slice, reflect.UnsafePointer: return value.IsNil() diff --git a/testing/require/requires.go b/testing/require/requires.go index 0a20d09d18b2..9dc801928f52 100644 --- a/testing/require/requires.go +++ b/testing/require/requires.go @@ -51,6 +51,11 @@ func ErrorContains(tb assertions.AssertionTestingTB, want string, err error, msg assertions.ErrorContains(tb.Fatalf, want, err, msg...) } +// IsNil asserts that the observed value is nil. +func IsNil(tb assertions.AssertionTestingTB, got interface{}, msg ...interface{}) { + assertions.IsNil(tb.Fatalf, got, msg...) +} + // NotNil asserts that passed value is not nil. func NotNil(tb assertions.AssertionTestingTB, obj interface{}, msg ...interface{}) { assertions.NotNil(tb.Fatalf, obj, msg...)