From 0d9b2e23fdfb7d648bb163b8cdad8bc47c4178a1 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Mon, 14 Oct 2024 12:40:36 +0200 Subject: [PATCH] fix returned error and split tests --- sync/sync_head.go | 36 ++++--- sync/sync_head_test.go | 206 ++++++++++++++++++++++++++++------------- 2 files changed, 157 insertions(+), 85 deletions(-) diff --git a/sync/sync_head.go b/sync/sync_head.go index 50bcf6d4..f808f6c6 100644 --- a/sync/sync_head.go +++ b/sync/sync_head.go @@ -174,28 +174,26 @@ func (s *Syncer[H]) verify(ctx context.Context, newHead H) (bool, error) { } var verErr *header.VerifyError - if !errors.As(err, &verErr) { - return false, nil - } + if errors.As(err, &verErr) { + if verErr.SoftFailure { + err := s.verifySkipping(ctx, sbjHead, newHead) + var errValSet *NewValidatorSetCantBeTrustedError + return errors.As(err, &errValSet), err + } - if verErr.SoftFailure { - err := s.verifySkipping(ctx, sbjHead, newHead) - return false, err + logF := log.Warnw + if errors.Is(err, header.ErrKnownHeader) { + logF = log.Debugw + } + logF("invalid network header", + "height_of_invalid", newHead.Height(), + "hash_of_invalid", newHead.Hash(), + "height_of_subjective", sbjHead.Height(), + "hash_of_subjective", sbjHead.Hash(), + "reason", verErr.Reason) } - logF := log.Warnw - if errors.Is(err, header.ErrKnownHeader) { - logF = log.Debugw - } - logF("invalid network header", - "height_of_invalid", newHead.Height(), - "hash_of_invalid", newHead.Hash(), - "height_of_subjective", sbjHead.Height(), - "hash_of_subjective", sbjHead.Hash(), - "reason", verErr.Reason, - ) - - return verErr.SoftFailure, err + return false, err } /* diff --git a/sync/sync_head_test.go b/sync/sync_head_test.go index 1e268e0e..73749261 100644 --- a/sync/sync_head_test.go +++ b/sync/sync_head_test.go @@ -138,7 +138,7 @@ func TestSyncer_HeadWithNotEnoughValidators(t *testing.T) { require.True(t, wrappedGetter.withTrustedHead) } -func TestSyncer_verifySkipping(t *testing.T) { +func TestSyncer_verifySkippingSuccess(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) t.Cleanup(cancel) @@ -177,105 +177,179 @@ func TestSyncer_verifySkipping(t *testing.T) { require.NoError(t, err) }) - t.Run("success (with bad candidates)", func(t *testing.T) { - const iters = 4 + const iters = 4 - headers := suite.GenDummyHeaders(total) - err = remoteStore.Append(ctx, headers...) - require.NoError(t, err) + headers := suite.GenDummyHeaders(total) + err = remoteStore.Append(ctx, headers...) + require.NoError(t, err) - var verifyCounter atomic.Int32 - for i := range total { - headers[i].VerifyFn = func(hdr *headertest.DummyHeader) error { - if i >= 501 { - return nil - } + var verifyCounter atomic.Int32 + for i := range total { + headers[i].VerifyFn = func(hdr *headertest.DummyHeader) error { + if hdr.Height() != badHeaderHeight { + return nil + } - verifyCounter.Add(1) - if verifyCounter.Load() <= iters { - return &header.VerifyError{ - Reason: headertest.ErrDummyVerify, - SoftFailure: hdr.SoftFailure, - } - } + verifyCounter.Add(1) + if verifyCounter.Load() >= iters { return nil } + + return &header.VerifyError{ + Reason: headertest.ErrDummyVerify, + SoftFailure: hdr.SoftFailure, + } } + } - headers[total-1].VerifyFailure = true - headers[total-1].SoftFailure = true + headers[total-1].VerifyFailure = true + headers[total-1].SoftFailure = true - subjHead, err := syncer.subjectiveHead(ctx) - require.NoError(t, err) + subjHead, err := syncer.subjectiveHead(ctx) + require.NoError(t, err) - err = syncer.verifySkipping(ctx, subjHead, headers[total-1]) - require.NoError(t, err) - }) + err = syncer.verifySkipping(ctx, subjHead, headers[total-1]) + require.NoError(t, err) +} + +func TestSyncer_verifySkippingSuccessWithBadCandidates(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + t.Cleanup(cancel) + + const total = 1000 + const badHeaderHeight = total + 1 - t.Run("success", func(t *testing.T) { - const iters = 4 + suite := headertest.NewTestSuite(t) + head := suite.Head() - headers := suite.GenDummyHeaders(total) - err = remoteStore.Append(ctx, headers...) + localStore := newTestStore(t, ctx, head) + remoteStore := newTestStore(t, ctx, head) + + // create a wrappedGetter to track exchange interactions + wrappedGetter := newWrappedGetter(local.NewExchange(remoteStore)) + + syncer, err := NewSyncer( + wrappedGetter, + localStore, + headertest.NewDummySubscriber(), + WithBlockTime(time.Nanosecond), + WithRecencyThreshold(time.Nanosecond), // forces a request for a new sync target + // ensures that syncer's store contains a subjective head that is within + // the unbonding period so that the syncer can use a header from the network + // as a sync target + WithTrustingPeriod(time.Hour), + ) + require.NoError(t, err) + + // start the syncer which triggers a Head request that will + // load the syncer's subjective head from the store, and request + // a new sync target from the network rather than from trusted peers + err = syncer.Start(ctx) + require.NoError(t, err) + t.Cleanup(func() { + err = syncer.Stop(ctx) require.NoError(t, err) + }) - var verifyCounter atomic.Int32 - for i := range total { - headers[i].VerifyFn = func(hdr *headertest.DummyHeader) error { - if hdr.Height() != badHeaderHeight { - return nil - } + const iters = 4 - verifyCounter.Add(1) - if verifyCounter.Load() >= iters { - return nil - } + headers := suite.GenDummyHeaders(total) + err = remoteStore.Append(ctx, headers...) + require.NoError(t, err) + + var verifyCounter atomic.Int32 + for i := range total { + headers[i].VerifyFn = func(hdr *headertest.DummyHeader) error { + if i >= 501 { + return nil + } + verifyCounter.Add(1) + if verifyCounter.Load() <= iters { return &header.VerifyError{ Reason: headertest.ErrDummyVerify, SoftFailure: hdr.SoftFailure, } } + return nil } + } - headers[total-1].VerifyFailure = true - headers[total-1].SoftFailure = true + headers[total-1].VerifyFailure = true + headers[total-1].SoftFailure = true - subjHead, err := syncer.subjectiveHead(ctx) - require.NoError(t, err) + subjHead, err := syncer.subjectiveHead(ctx) + require.NoError(t, err) + + err = syncer.verifySkipping(ctx, subjHead, headers[total-1]) + require.NoError(t, err) +} + +func TestSyncer_verifySkippingCannotVerify(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + t.Cleanup(cancel) + + const total = 1000 + const badHeaderHeight = total + 1 + + suite := headertest.NewTestSuite(t) + head := suite.Head() + + localStore := newTestStore(t, ctx, head) + remoteStore := newTestStore(t, ctx, head) + + // create a wrappedGetter to track exchange interactions + wrappedGetter := newWrappedGetter(local.NewExchange(remoteStore)) - err = syncer.verifySkipping(ctx, subjHead, headers[total-1]) + syncer, err := NewSyncer( + wrappedGetter, + localStore, + headertest.NewDummySubscriber(), + WithBlockTime(time.Nanosecond), + WithRecencyThreshold(time.Nanosecond), // forces a request for a new sync target + // ensures that syncer's store contains a subjective head that is within + // the unbonding period so that the syncer can use a header from the network + // as a sync target + WithTrustingPeriod(time.Hour), + ) + require.NoError(t, err) + + // start the syncer which triggers a Head request that will + // load the syncer's subjective head from the store, and request + // a new sync target from the network rather than from trusted peers + err = syncer.Start(ctx) + require.NoError(t, err) + t.Cleanup(func() { + err = syncer.Stop(ctx) require.NoError(t, err) }) - t.Run("cannot verify", func(t *testing.T) { - headers := suite.GenDummyHeaders(total) - err = remoteStore.Append(ctx, headers...) - require.NoError(t, err) + headers := suite.GenDummyHeaders(total) + err = remoteStore.Append(ctx, headers...) + require.NoError(t, err) - for i := range total { - headers[i].VerifyFn = func(hdr *headertest.DummyHeader) error { - if hdr.Height() != badHeaderHeight { - return nil - } + for i := range total { + headers[i].VerifyFn = func(hdr *headertest.DummyHeader) error { + if hdr.Height() != badHeaderHeight { + return nil + } - return &header.VerifyError{ - Reason: headertest.ErrDummyVerify, - SoftFailure: hdr.SoftFailure, - } + return &header.VerifyError{ + Reason: headertest.ErrDummyVerify, + SoftFailure: hdr.SoftFailure, } } + } - headers[total-1].VerifyFailure = true - headers[total-1].SoftFailure = true + headers[total-1].VerifyFailure = true + headers[total-1].SoftFailure = true - subjHead, err := syncer.subjectiveHead(ctx) - require.NoError(t, err) + subjHead, err := syncer.subjectiveHead(ctx) + require.NoError(t, err) - err = syncer.verifySkipping(ctx, subjHead, headers[total-1]) - var verErr *NewValidatorSetCantBeTrustedError - assert.ErrorAs(t, err, &verErr) - }) + err = syncer.verifySkipping(ctx, subjHead, headers[total-1]) + var verErr *NewValidatorSetCantBeTrustedError + assert.ErrorAs(t, err, &verErr, "%T", err) } type wrappedGetter struct {