From 1ffb292dda81b4005b9130dd8b940636cbe98ce4 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Mon, 31 Jul 2023 12:21:13 +0200 Subject: [PATCH 1/7] refactor: harden verification --- errors.go | 20 ------ headertest/dummy_header.go | 15 +--- headertest/dummy_suite.go | 2 + p2p/exchange_test.go | 7 +- p2p/subscriber.go | 5 +- store/store.go | 3 +- sync/sync_head.go | 68 +++++++++--------- sync/sync_test.go | 7 +- sync/verify/verify.go | 112 ++++++++++++++++++++++++++++++ sync/verify/verify_test.go | 139 +++++++++++++++++++++++++++++++++++++ 10 files changed, 300 insertions(+), 78 deletions(-) delete mode 100644 errors.go create mode 100644 sync/verify/verify.go create mode 100644 sync/verify/verify_test.go diff --git a/errors.go b/errors.go deleted file mode 100644 index 127499c7..00000000 --- a/errors.go +++ /dev/null @@ -1,20 +0,0 @@ -package header - -import "fmt" - -// VerifyError is thrown during for Headers failed verification. -type VerifyError struct { - // Reason why verification failed as inner error. - Reason error - // Uncertain signals that the was not enough information to conclude a Header is correct or not. - // May happen with recent Headers during unfinished historical sync or because of local errors. - Uncertain bool -} - -func (vr *VerifyError) Error() string { - return fmt.Sprintf("header: verify: %s", vr.Reason.Error()) -} - -func (vr *VerifyError) Unwrap() error { - return vr.Reason -} diff --git a/headertest/dummy_header.go b/headertest/dummy_header.go index 39f932f0..20fda64d 100644 --- a/headertest/dummy_header.go +++ b/headertest/dummy_header.go @@ -61,7 +61,7 @@ func (d *DummyHeader) IsZero() bool { } func (d *DummyHeader) ChainID() string { - return "private" + return d.Raw.ChainID } func (d *DummyHeader) Hash() header.Hash { @@ -109,19 +109,6 @@ func (d *DummyHeader) Verify(header header.Header) error { return fmt.Errorf("header at height %d failed verification", header.Height()) } - epsilon := 10 * time.Second - if header.Time().After(time.Now().Add(epsilon)) { - return fmt.Errorf("header Time too far in the future") - } - - if header.Height() <= d.Height() { - return fmt.Errorf("expected new header Height %d to be larger than old header Height %d", header.Height(), d.Height()) - } - - if header.Time().Before(d.Time()) { - return fmt.Errorf("expected new header Time %v to be after old header Time %v", header.Time(), d.Time()) - } - return nil } diff --git a/headertest/dummy_suite.go b/headertest/dummy_suite.go index f5a81bee..1b0ad9f7 100644 --- a/headertest/dummy_suite.go +++ b/headertest/dummy_suite.go @@ -45,6 +45,7 @@ func (s *DummySuite) NextHeader() *DummyHeader { dh.Raw.Time = s.head.Time().Add(time.Nanosecond) dh.Raw.Height = s.head.Height() + 1 dh.Raw.PreviousHash = s.head.Hash() + dh.Raw.ChainID = s.head.ChainID() _ = dh.rehash() s.head = dh return s.head @@ -57,6 +58,7 @@ func (s *DummySuite) genesis() *DummyHeader { PreviousHash: nil, Height: 1, Time: time.Now().Add(-10 * time.Second).UTC(), + ChainID: "test", }, } } diff --git a/p2p/exchange_test.go b/p2p/exchange_test.go index 02f563df..71cee81e 100644 --- a/p2p/exchange_test.go +++ b/p2p/exchange_test.go @@ -19,14 +19,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/celestiaorg/go-libp2p-messenger/serde" - "github.com/celestiaorg/go-header" "github.com/celestiaorg/go-header/headertest" p2p_pb "github.com/celestiaorg/go-header/p2p/pb" + "github.com/celestiaorg/go-libp2p-messenger/serde" ) -const networkID = "private" +const networkID = "test" // must match the chain-id in test suite func TestExchange_RequestHead(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) @@ -393,7 +392,7 @@ func TestExchange_RequestByHashFails(t *testing.T) { func TestExchange_HandleHeaderWithDifferentChainID(t *testing.T) { hosts := createMocknet(t, 2) exchg, store := createP2PExAndServer(t, hosts[0], hosts[1]) - exchg.Params.chainID = "test" + exchg.Params.chainID = "test1" _, err := exchg.Head(context.Background()) require.Error(t, err) diff --git a/p2p/subscriber.go b/p2p/subscriber.go index d9ad88f6..8f89caa6 100644 --- a/p2p/subscriber.go +++ b/p2p/subscriber.go @@ -9,6 +9,7 @@ import ( "github.com/libp2p/go-libp2p/core/peer" "github.com/celestiaorg/go-header" + "github.com/celestiaorg/go-header/sync/verify" ) // Subscriber manages the lifecycle and relationship of header Module @@ -77,12 +78,12 @@ func (p *Subscriber[H]) SetVerifier(val func(context.Context, H) error) error { // additional unmarhalling msg.ValidatorData = hdr - var verErr *header.VerifyError + var verErr *verify.VerifyError err = val(ctx, hdr) switch { case err == nil: return pubsub.ValidationAccept - case errors.As(err, &verErr) && verErr.Uncertain: + case errors.As(err, &verErr) && verErr.SoftFailure: return pubsub.ValidationIgnore default: return pubsub.ValidationReject diff --git a/store/store.go b/store/store.go index ae389087..afaa61ec 100644 --- a/store/store.go +++ b/store/store.go @@ -12,6 +12,7 @@ import ( logging "github.com/ipfs/go-log/v2" "github.com/celestiaorg/go-header" + "github.com/celestiaorg/go-header/sync/verify" ) var log = logging.Logger("header/store") @@ -332,7 +333,7 @@ func (s *Store[H]) Append(ctx context.Context, headers ...H) error { err = head.Verify(h) if err != nil { - var verErr *header.VerifyError + var verErr *verify.VerifyError if errors.As(err, &verErr) { log.Errorw("invalid header", "height_of_head", head.Height(), diff --git a/sync/sync_head.go b/sync/sync_head.go index 1dc43639..707cdfa0 100644 --- a/sync/sync_head.go +++ b/sync/sync_head.go @@ -6,6 +6,7 @@ import ( "time" "github.com/celestiaorg/go-header" + "github.com/celestiaorg/go-header/sync/verify" ) // Head returns the Network Head. @@ -40,8 +41,8 @@ func (s *Syncer[H]) Head(ctx context.Context, _ ...header.HeadOption) (H, error) defer s.getter.Unlock() netHead, err := s.getter.Head(ctx, header.WithTrustedHead(sbjHead)) if err != nil { - log.Warnw("failed to return head from trusted peer, returning subjective head which may not be recent", "sbjHead", sbjHead.Height(), "err", err) - return sbjHead, nil + log.Warnw("failed to get recent head, returning current subjective", "sbjHead", sbjHead.Height(), "err", err) + return s.subjectiveHead(ctx) } // process and validate netHead fetched from trusted peers // NOTE: We could trust the netHead like we do during 'automatic subjective initialization' @@ -134,54 +135,53 @@ func (s *Syncer[H]) setSubjectiveHead(ctx context.Context, netHead H) { // incomingNetworkHead processes new potential network headers. // If the header valid, sets as new subjective header. -func (s *Syncer[H]) incomingNetworkHead(ctx context.Context, netHead H) error { +func (s *Syncer[H]) incomingNetworkHead(ctx context.Context, head H) error { // ensure there is no racing between network head candidates s.incomingMu.Lock() defer s.incomingMu.Unlock() - // first of all, check the validity of the netHead - err := s.validateHead(ctx, netHead) - if err != nil { + + softFailure, err := s.verify(ctx, head) + if err != nil && !softFailure { return err } - // and set it if valid - s.setSubjectiveHead(ctx, netHead) - return nil + + // TODO(@Wondertan): + // Implement setSyncTarget and use it for soft failures + s.setSubjectiveHead(ctx, head) + return err } -// validateHead checks validity of the given header against the subjective head. -func (s *Syncer[H]) validateHead(ctx context.Context, new H) error { +// verify verifies given network head candidate. +func (s *Syncer[H]) verify(ctx context.Context, newHead H) (bool, error) { sbjHead, err := s.subjectiveHead(ctx) if err != nil { log.Errorw("getting subjective head during validation", "err", err) - // local error, so uncertain - return &header.VerifyError{Reason: err, Uncertain: true} - } - if new.Height() <= sbjHead.Height() { - log.Warnw("received known network header", - "current_height", sbjHead.Height(), - "header_height", new.Height(), - "header_hash", new.Hash()) - // set uncertain, if it's from the past - return &header.VerifyError{Reason: err, Uncertain: true} - } - // perform verification - err = sbjHead.Verify(new) - var verErr *header.VerifyError - if errors.As(err, &verErr) { + // local error, so soft + return true, &verify.VerifyError{Reason: err, SoftFailure: true} + } + + var heightThreshold int64 + if s.Params.TrustingPeriod != 0 && s.Params.blockTime != 0 { + heightThreshold = int64(s.Params.TrustingPeriod / s.Params.blockTime) + } + + err = verify.Verify(sbjHead, newHead, heightThreshold) + if err == nil { + return false, nil + } + + var verErr *verify.VerifyError + if errors.As(err, &verErr) && !verErr.SoftFailure { log.Errorw("invalid network header", - "height_of_invalid", new.Height(), - "hash_of_invalid", new.Hash(), + "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 } - // and accept if the header is good - return nil -} -// TODO(@Wondertan): We should request TrustingPeriod from the network's state params or -// listen for network params changes to always have a topical value. + return verErr.SoftFailure, err +} // isExpired checks if header is expired against trusting period. func isExpired(header header.Header, period time.Duration) bool { diff --git a/sync/sync_test.go b/sync/sync_test.go index ba8adb9e..470b8367 100644 --- a/sync/sync_test.go +++ b/sync/sync_test.go @@ -12,6 +12,7 @@ import ( "github.com/celestiaorg/go-header/headertest" "github.com/celestiaorg/go-header/local" "github.com/celestiaorg/go-header/store" + "github.com/celestiaorg/go-header/sync/verify" ) func TestSyncSimpleRequestingHead(t *testing.T) { @@ -277,10 +278,9 @@ func TestSyncerIncomingDuplicate(t *testing.T) { time.Sleep(time.Millisecond * 10) - var verErr *header.VerifyError + var verErr *verify.VerifyError err = syncer.incomingNetworkHead(ctx, range1[len(range1)-1]) assert.ErrorAs(t, err, &verErr) - assert.True(t, verErr.Uncertain) err = syncer.SyncWait(ctx) require.NoError(t, err) @@ -356,7 +356,8 @@ func TestSync_InvalidSyncTarget(t *testing.T) { // a new sync job to a good sync target expectedHead, err := remoteStore.Head(ctx) require.NoError(t, err) - syncer.incomingNetworkHead(ctx, expectedHead) + err = syncer.incomingNetworkHead(ctx, expectedHead) + require.NoError(t, err) // wait for syncer to finish (give it a bit of time to register // new job with new sync target) diff --git a/sync/verify/verify.go b/sync/verify/verify.go new file mode 100644 index 00000000..32255def --- /dev/null +++ b/sync/verify/verify.go @@ -0,0 +1,112 @@ +// TODO(@Wondertan): Should be just part of sync pkg and not subpkg +// +// Fix after adjacency requirement is removed from the Store. +package verify + +import ( + "errors" + "fmt" + "time" + + "github.com/celestiaorg/go-header" +) + +// DefaultHeightThreshold defines default height threshold beyond which headers are rejected +// NOTE: Compared against subjective head which is guaranteed to be non-expired +const DefaultHeightThreshold int64 = 40000 // ~ 7 days of 15 second headers + +// VerifyError is thrown during for Headers failed verification. +type VerifyError struct { + // Reason why verification failed as inner error. + Reason error + // SoftFailure means verification did not have enough information to definitively conclude a + // Header was correct or not. + // May happen with recent Headers during unfinished historical sync or because of local errors. + // TODO(@Wondertan): Better be part of signature Header.Verify() (bool, error), but kept here + // not to break + SoftFailure bool +} + +func (vr *VerifyError) Error() string { + return fmt.Sprintf("header: verify: %s", vr.Reason.Error()) +} + +func (vr *VerifyError) Unwrap() error { + return vr.Reason +} + +// Verify verifies untrusted Header against trusted following general Header checks and +// custom user-specific checks defined in Header.Verify +// +// If heightThreshold is zero, uses DefaultHeightThreshold. +// Always returns VerifyError. +func Verify[H header.Header](trstd, untrstd H, heightThreshold int64) error { + // general mandatory verification + err := verify[H](trstd, untrstd, heightThreshold) + if err != nil { + return &VerifyError{Reason: err} + } + // user defined verification + err = trstd.Verify(untrstd) + if err == nil { + return nil + } + // if that's an error, ensure we always return VerifyError + var verErr *VerifyError + if !errors.As(err, &verErr) { + verErr = &VerifyError{Reason: err} + } + // check adjacency of failed verification + adjacent := untrstd.Height() == trstd.Height()+1 + if !adjacent { + // if non-adjacent, we don't know if the header is *really* wrong + // so set as soft + verErr.SoftFailure = true + } + // we trust adjacent verification to it's fullest + // if verification fails - the header is *really* wrong + return verErr +} + +// verify is a little bro of Verify yet performs mandatory Header checks +// for any Header implementation. +func verify[H header.Header](trstd, untrstd H, heightThreshold int64) error { + if heightThreshold == 0 { + heightThreshold = DefaultHeightThreshold + } + + if untrstd.IsZero() { + return fmt.Errorf("zero header") + } + + if untrstd.ChainID() != trstd.ChainID() { + return fmt.Errorf("wrong header chain id %s, not %s", untrstd.ChainID(), trstd.ChainID()) + } + + if !untrstd.Time().After(trstd.Time()) { + return fmt.Errorf("unordered header timestamp %v is before %v", untrstd.Time(), trstd.Time()) + } + + now := time.Now() + if !untrstd.Time().Before(now.Add(clockDrift)) { + return fmt.Errorf("header timestamp %v is from future (now: %v, clock_drift: %v)", untrstd.Time(), now, clockDrift) + } + + known := untrstd.Height() <= trstd.Height() + if known { + return fmt.Errorf("known header height %d, current %d", untrstd.Height(), trstd.Height()) + } + // reject headers with height too far from the future + // this is essential for headers failed non-adjacent verification + // yet taken as sync target + adequateHeight := untrstd.Height()-trstd.Height() < heightThreshold + if !adequateHeight { + return fmt.Errorf("header height %d is far from future (current: %d, threshold: %d)", untrstd.Height(), trstd.Height(), heightThreshold) + } + + return nil +} + +// clockDrift defines how much new header's time can drift into +// the future relative to the now time during verification. +var clockDrift = 10 * time.Second diff --git a/sync/verify/verify_test.go b/sync/verify/verify_test.go new file mode 100644 index 00000000..954fd73c --- /dev/null +++ b/sync/verify/verify_test.go @@ -0,0 +1,139 @@ +package verify + +import ( + "errors" + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/celestiaorg/go-header/headertest" +) + +func TestVerify(t *testing.T) { + suite := headertest.NewTestSuite(t) + trusted := suite.GenDummyHeaders(1)[0] + + tests := []struct { + prepare func() *headertest.DummyHeader + err bool + soft bool + }{ + { + prepare: func() *headertest.DummyHeader { + return nil + }, + err: true, + }, + { + prepare: func() *headertest.DummyHeader { + untrusted := suite.NextHeader() + untrusted.VerifyFailure = true + return untrusted + }, + err: true, + }, + { + prepare: func() *headertest.DummyHeader { + untrusted := suite.NextHeader() + untrusted.VerifyFailure = true + return untrusted + }, + err: true, + soft: true, // soft because non-adjacent + }, + { + prepare: func() *headertest.DummyHeader { + return suite.NextHeader() + }, + }, + } + + for i, test := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + err := Verify(trusted, test.prepare(), 0) + if test.err { + var verErr *VerifyError + assert.ErrorAs(t, err, &verErr) + assert.NotNil(t, errors.Unwrap(verErr)) + assert.Equal(t, test.soft, verErr.SoftFailure) + } else { + assert.NoError(t, err) + } + }) + } +} + +func Test_verify(t *testing.T) { + suite := headertest.NewTestSuite(t) + trusted := suite.GenDummyHeaders(1)[0] + + tests := []struct { + prepare func() *headertest.DummyHeader + err bool + }{ + { + prepare: func() *headertest.DummyHeader { + return suite.NextHeader() + }, + }, + { + prepare: func() *headertest.DummyHeader { + return nil + }, + err: true, + }, + { + prepare: func() *headertest.DummyHeader { + untrusted := suite.NextHeader() + untrusted.Raw.ChainID = "gtmb" + return untrusted + }, + err: true, + }, + { + prepare: func() *headertest.DummyHeader { + untrusted := suite.NextHeader() + untrusted.Raw.Time = untrusted.Raw.Time.Truncate(time.Minute * 10) + return untrusted + }, + err: true, + }, + { + prepare: func() *headertest.DummyHeader { + untrusted := suite.NextHeader() + untrusted.Raw.Time = untrusted.Raw.Time.Add(time.Minute) + return untrusted + }, + err: true, + }, + { + prepare: func() *headertest.DummyHeader { + untrusted := suite.NextHeader() + untrusted.Raw.Height = trusted.Height() + return untrusted + }, + err: true, + }, + { + prepare: func() *headertest.DummyHeader { + untrusted := suite.NextHeader() + untrusted.Raw.Height += 100000 + return untrusted + }, + err: true, + }, + } + + for i, test := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + err := verify(trusted, test.prepare(), 0) + if test.err { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} From 950e4247068eeaa2419754de68e914fa666724f9 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Wed, 23 Aug 2023 19:59:09 +0200 Subject: [PATCH 2/7] improve error handling --- sync/verify/verify.go | 30 ++++++++++++++++------- sync/verify/verify_test.go | 49 ++++++++++++++++++++++---------------- 2 files changed, 50 insertions(+), 29 deletions(-) diff --git a/sync/verify/verify.go b/sync/verify/verify.go index 32255def..bdc5031b 100644 --- a/sync/verify/verify.go +++ b/sync/verify/verify.go @@ -28,7 +28,7 @@ type VerifyError struct { } func (vr *VerifyError) Error() string { - return fmt.Sprintf("header: verify: %s", vr.Reason.Error()) + return fmt.Sprintf("header verification failed: %s", vr.Reason.Error()) } func (vr *VerifyError) Unwrap() error { @@ -76,37 +76,51 @@ func verify[H header.Header](trstd, untrstd H, heightThreshold int64) error { } if untrstd.IsZero() { - return fmt.Errorf("zero header") + return errZero } if untrstd.ChainID() != trstd.ChainID() { - return fmt.Errorf("wrong header chain id %s, not %s", untrstd.ChainID(), trstd.ChainID()) + return fmt.Errorf("%w: '%s' != '%s'", errWrongChain, untrstd.ChainID(), trstd.ChainID()) } if !untrstd.Time().After(trstd.Time()) { - return fmt.Errorf("unordered header timestamp %v is before %v", untrstd.Time(), trstd.Time()) + return fmt.Errorf("%w: timestamp '%s' < current '%s'", errUnordered, formatTime(untrstd.Time()), formatTime(trstd.Time())) } now := time.Now() - if !untrstd.Time().Before(now.Add(clockDrift)) { - return fmt.Errorf("header timestamp %v is from future (now: %v, clock_drift: %v)", untrstd.Time(), now, clockDrift) + if untrstd.Time().After(now.Add(clockDrift)) { + return fmt.Errorf("%w: timestamp '%s' > now '%s', clock_drift '%v'", errFromFuture, formatTime(untrstd.Time()), formatTime(now), clockDrift) } known := untrstd.Height() <= trstd.Height() if known { - return fmt.Errorf("known header height %d, current %d", untrstd.Height(), trstd.Height()) + return fmt.Errorf("%w: '%d' <= current '%d'", errKnown, untrstd.Height(), trstd.Height()) } // reject headers with height too far from the future // this is essential for headers failed non-adjacent verification // yet taken as sync target adequateHeight := untrstd.Height()-trstd.Height() < heightThreshold if !adequateHeight { - return fmt.Errorf("header height %d is far from future (current: %d, threshold: %d)", untrstd.Height(), trstd.Height(), heightThreshold) + return fmt.Errorf("%w: '%d' - current '%d' >= threshold '%d'", errHeightFromFuture, untrstd.Height(), trstd.Height(), heightThreshold) } return nil } +func formatTime(t time.Time) string { + return t.UTC().Format(time.DateTime) +} + +// unexported for testing, but can be exported if needed +var ( + errZero = errors.New("zero header") + errWrongChain = errors.New("wrong chain id") + errUnordered = errors.New("unordered headers") + errFromFuture = errors.New("header is from the future") + errKnown = errors.New("known header") + errHeightFromFuture = errors.New("header height is far from future") +) + // clockDrift defines how much new header's time can drift into // the future relative to the now time during verification. var clockDrift = 10 * time.Second diff --git a/sync/verify/verify_test.go b/sync/verify/verify_test.go index 954fd73c..ae7d6ecf 100644 --- a/sync/verify/verify_test.go +++ b/sync/verify/verify_test.go @@ -15,6 +15,11 @@ func TestVerify(t *testing.T) { suite := headertest.NewTestSuite(t) trusted := suite.GenDummyHeaders(1)[0] + next := func() *headertest.DummyHeader { + next := *suite.NextHeader() + return &next + } + tests := []struct { prepare func() *headertest.DummyHeader err bool @@ -28,7 +33,7 @@ func TestVerify(t *testing.T) { }, { prepare: func() *headertest.DummyHeader { - untrusted := suite.NextHeader() + untrusted := next() untrusted.VerifyFailure = true return untrusted }, @@ -36,7 +41,7 @@ func TestVerify(t *testing.T) { }, { prepare: func() *headertest.DummyHeader { - untrusted := suite.NextHeader() + untrusted := next() untrusted.VerifyFailure = true return untrusted }, @@ -45,7 +50,7 @@ func TestVerify(t *testing.T) { }, { prepare: func() *headertest.DummyHeader { - return suite.NextHeader() + return next() }, }, } @@ -69,71 +74,73 @@ func Test_verify(t *testing.T) { suite := headertest.NewTestSuite(t) trusted := suite.GenDummyHeaders(1)[0] + next := func() *headertest.DummyHeader { + next := *suite.NextHeader() // copy is required + return &next + } + tests := []struct { prepare func() *headertest.DummyHeader - err bool + err error }{ { prepare: func() *headertest.DummyHeader { - return suite.NextHeader() + return next() }, }, { prepare: func() *headertest.DummyHeader { return nil }, - err: true, + err: errZero, }, { prepare: func() *headertest.DummyHeader { - untrusted := suite.NextHeader() + untrusted := next() untrusted.Raw.ChainID = "gtmb" return untrusted }, - err: true, + err: errWrongChain, }, { prepare: func() *headertest.DummyHeader { - untrusted := suite.NextHeader() + untrusted := next() untrusted.Raw.Time = untrusted.Raw.Time.Truncate(time.Minute * 10) return untrusted }, - err: true, + err: errUnordered, }, { prepare: func() *headertest.DummyHeader { - untrusted := suite.NextHeader() + untrusted := next() untrusted.Raw.Time = untrusted.Raw.Time.Add(time.Minute) return untrusted }, - err: true, + err: errFromFuture, }, { prepare: func() *headertest.DummyHeader { - untrusted := suite.NextHeader() + untrusted := next() untrusted.Raw.Height = trusted.Height() return untrusted }, - err: true, + err: errKnown, }, { prepare: func() *headertest.DummyHeader { - untrusted := suite.NextHeader() + untrusted := next() untrusted.Raw.Height += 100000 return untrusted }, - err: true, + err: errHeightFromFuture, }, } for i, test := range tests { t.Run(strconv.Itoa(i), func(t *testing.T) { err := verify(trusted, test.prepare(), 0) - if test.err { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + assert.ErrorIs(t, err, test.err) + t.Log(err) }) } } From 4e0f2dd7d37a782a3842cc227fa14a8378fb12d5 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Wed, 23 Aug 2023 20:31:01 +0200 Subject: [PATCH 3/7] move verify to root and cleanups --- headertest/dummy_header.go | 55 ++++------- headertest/dummy_suite.go | 18 ++-- headertest/verify_test.go | 110 ++++++++++++++++++++++ p2p/subscriber.go | 3 +- store/store.go | 3 +- sync/sync_head.go | 7 +- sync/sync_test.go | 3 +- sync/verify/verify_test.go | 146 ----------------------------- sync/verify/verify.go => verify.go | 84 ++++++++--------- 9 files changed, 179 insertions(+), 250 deletions(-) create mode 100644 headertest/verify_test.go delete mode 100644 sync/verify/verify_test.go rename sync/verify/verify.go => verify.go (69%) diff --git a/headertest/dummy_header.go b/headertest/dummy_header.go index 20fda64d..c3dfe33b 100644 --- a/headertest/dummy_header.go +++ b/headertest/dummy_header.go @@ -1,11 +1,10 @@ package headertest import ( - "bytes" "crypto/rand" "encoding/binary" - "encoding/gob" - "fmt" + "encoding/json" + "errors" "math" "testing" "time" @@ -15,16 +14,13 @@ import ( "github.com/celestiaorg/go-header" ) -type Raw struct { - ChainID string - PreviousHash header.Hash - - Height int64 - Time time.Time -} +var ErrDummyVerify = errors.New("dummy verify error") type DummyHeader struct { - Raw + Chainid string + PreviousHash header.Hash + HeightI int64 + Timestamp time.Time hash header.Hash @@ -37,13 +33,9 @@ func RandDummyHeader(t *testing.T) *DummyHeader { t.Helper() dh := &DummyHeader{ - Raw{ - PreviousHash: RandBytes(32), - Height: randInt63(), - Time: time.Now().UTC(), - }, - nil, - false, + PreviousHash: RandBytes(32), + HeightI: randInt63(), + Timestamp: time.Now().UTC(), } err := dh.rehash() if err != nil { @@ -61,7 +53,7 @@ func (d *DummyHeader) IsZero() bool { } func (d *DummyHeader) ChainID() string { - return d.Raw.ChainID + return d.Chainid } func (d *DummyHeader) Hash() header.Hash { @@ -84,15 +76,15 @@ func (d *DummyHeader) rehash() error { } func (d *DummyHeader) Height() int64 { - return d.Raw.Height + return d.HeightI } func (d *DummyHeader) LastHeader() header.Hash { - return d.Raw.PreviousHash + return d.PreviousHash } func (d *DummyHeader) Time() time.Time { - return d.Raw.Time + return d.Timestamp } func (d *DummyHeader) IsRecent(blockTime time.Duration) bool { @@ -106,7 +98,7 @@ func (d *DummyHeader) IsExpired(period time.Duration) bool { func (d *DummyHeader) Verify(header header.Header) error { if dummy, _ := header.(*DummyHeader); dummy.VerifyFailure { - return fmt.Errorf("header at height %d failed verification", header.Height()) + return ErrDummyVerify } return nil @@ -117,24 +109,11 @@ func (d *DummyHeader) Validate() error { } func (d *DummyHeader) MarshalBinary() ([]byte, error) { - var buf bytes.Buffer - enc := gob.NewEncoder(&buf) - err := enc.Encode(d.Raw) - return buf.Bytes(), err + return json.Marshal(d) } func (d *DummyHeader) UnmarshalBinary(data []byte) error { - dec := gob.NewDecoder(bytes.NewReader(data)) - err := dec.Decode(&d.Raw) - if err != nil { - return err - } - err = d.rehash() - if err != nil { - return err - } - - return nil + return json.Unmarshal(data, d) } // RandBytes returns slice of n-bytes, or nil in case of error diff --git a/headertest/dummy_suite.go b/headertest/dummy_suite.go index 1b0ad9f7..27a78c5e 100644 --- a/headertest/dummy_suite.go +++ b/headertest/dummy_suite.go @@ -42,10 +42,10 @@ func (s *DummySuite) NextHeader() *DummyHeader { } dh := RandDummyHeader(s.t) - dh.Raw.Time = s.head.Time().Add(time.Nanosecond) - dh.Raw.Height = s.head.Height() + 1 - dh.Raw.PreviousHash = s.head.Hash() - dh.Raw.ChainID = s.head.ChainID() + dh.Timestamp = s.head.Time().Add(time.Nanosecond) + dh.HeightI = s.head.Height() + 1 + dh.PreviousHash = s.head.Hash() + dh.Chainid = s.head.ChainID() _ = dh.rehash() s.head = dh return s.head @@ -53,12 +53,8 @@ func (s *DummySuite) NextHeader() *DummyHeader { func (s *DummySuite) genesis() *DummyHeader { return &DummyHeader{ - hash: nil, - Raw: Raw{ - PreviousHash: nil, - Height: 1, - Time: time.Now().Add(-10 * time.Second).UTC(), - ChainID: "test", - }, + HeightI: 1, + Timestamp: time.Now().Add(-10 * time.Second).UTC(), + Chainid: "test", } } diff --git a/headertest/verify_test.go b/headertest/verify_test.go new file mode 100644 index 00000000..62099d85 --- /dev/null +++ b/headertest/verify_test.go @@ -0,0 +1,110 @@ +package headertest + +import ( + "errors" + "strconv" + "testing" + "time" + + "github.com/celestiaorg/go-header" + "github.com/stretchr/testify/assert" +) + +func TestVerify(t *testing.T) { + suite := NewTestSuite(t) + trusted := suite.GenDummyHeaders(1)[0] + + next := func() *DummyHeader { + next := *suite.NextHeader() + return &next + } + + tests := []struct { + prepare func() *DummyHeader + err error + soft bool + }{ + { + prepare: func() *DummyHeader { + untrusted := next() + untrusted.VerifyFailure = true + return untrusted + }, + err: ErrDummyVerify, + }, + { + prepare: func() *DummyHeader { + untrusted := next() + untrusted.VerifyFailure = true + return untrusted + }, + err: ErrDummyVerify, + soft: true, // soft because non-adjacent + }, + { + prepare: func() *DummyHeader { + return next() + }, + }, + { + prepare: func() *DummyHeader { + return nil + }, + err: header.ErrZeroHeader, + }, + { + prepare: func() *DummyHeader { + untrusted := next() + untrusted.Chainid = "gtmb" + return untrusted + }, + err: header.ErrWrongChainID, + }, + { + prepare: func() *DummyHeader { + untrusted := next() + untrusted.Timestamp = untrusted.Timestamp.Truncate(time.Minute * 10) + return untrusted + }, + err: header.ErrUnorderedTime, + }, + { + prepare: func() *DummyHeader { + untrusted := next() + untrusted.Timestamp = untrusted.Timestamp.Add(time.Minute) + return untrusted + }, + err: header.ErrFromFuture, + }, + { + prepare: func() *DummyHeader { + untrusted := next() + untrusted.HeightI = trusted.Height() + return untrusted + }, + err: header.ErrKnownHeader, + }, + { + prepare: func() *DummyHeader { + untrusted := next() + untrusted.HeightI += 100000 + return untrusted + }, + err: header.ErrHeightFromFuture, + }, + } + + for i, test := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + err := header.Verify(trusted, test.prepare(), 0) + if test.err != nil { + var verErr *header.VerifyError + assert.ErrorAs(t, err, &verErr) + assert.ErrorIs(t, errors.Unwrap(verErr), test.err) + assert.Equal(t, test.soft, verErr.SoftFailure) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/p2p/subscriber.go b/p2p/subscriber.go index 8f89caa6..30341333 100644 --- a/p2p/subscriber.go +++ b/p2p/subscriber.go @@ -9,7 +9,6 @@ import ( "github.com/libp2p/go-libp2p/core/peer" "github.com/celestiaorg/go-header" - "github.com/celestiaorg/go-header/sync/verify" ) // Subscriber manages the lifecycle and relationship of header Module @@ -78,7 +77,7 @@ func (p *Subscriber[H]) SetVerifier(val func(context.Context, H) error) error { // additional unmarhalling msg.ValidatorData = hdr - var verErr *verify.VerifyError + var verErr *header.VerifyError err = val(ctx, hdr) switch { case err == nil: diff --git a/store/store.go b/store/store.go index afaa61ec..ae389087 100644 --- a/store/store.go +++ b/store/store.go @@ -12,7 +12,6 @@ import ( logging "github.com/ipfs/go-log/v2" "github.com/celestiaorg/go-header" - "github.com/celestiaorg/go-header/sync/verify" ) var log = logging.Logger("header/store") @@ -333,7 +332,7 @@ func (s *Store[H]) Append(ctx context.Context, headers ...H) error { err = head.Verify(h) if err != nil { - var verErr *verify.VerifyError + var verErr *header.VerifyError if errors.As(err, &verErr) { log.Errorw("invalid header", "height_of_head", head.Height(), diff --git a/sync/sync_head.go b/sync/sync_head.go index 707cdfa0..5db7382a 100644 --- a/sync/sync_head.go +++ b/sync/sync_head.go @@ -6,7 +6,6 @@ import ( "time" "github.com/celestiaorg/go-header" - "github.com/celestiaorg/go-header/sync/verify" ) // Head returns the Network Head. @@ -157,7 +156,7 @@ func (s *Syncer[H]) verify(ctx context.Context, newHead H) (bool, error) { if err != nil { log.Errorw("getting subjective head during validation", "err", err) // local error, so soft - return true, &verify.VerifyError{Reason: err, SoftFailure: true} + return true, &header.VerifyError{Reason: err, SoftFailure: true} } var heightThreshold int64 @@ -165,12 +164,12 @@ func (s *Syncer[H]) verify(ctx context.Context, newHead H) (bool, error) { heightThreshold = int64(s.Params.TrustingPeriod / s.Params.blockTime) } - err = verify.Verify(sbjHead, newHead, heightThreshold) + err = header.Verify(sbjHead, newHead, heightThreshold) if err == nil { return false, nil } - var verErr *verify.VerifyError + var verErr *header.VerifyError if errors.As(err, &verErr) && !verErr.SoftFailure { log.Errorw("invalid network header", "height_of_invalid", newHead.Height(), diff --git a/sync/sync_test.go b/sync/sync_test.go index 470b8367..1cc45ab3 100644 --- a/sync/sync_test.go +++ b/sync/sync_test.go @@ -12,7 +12,6 @@ import ( "github.com/celestiaorg/go-header/headertest" "github.com/celestiaorg/go-header/local" "github.com/celestiaorg/go-header/store" - "github.com/celestiaorg/go-header/sync/verify" ) func TestSyncSimpleRequestingHead(t *testing.T) { @@ -278,7 +277,7 @@ func TestSyncerIncomingDuplicate(t *testing.T) { time.Sleep(time.Millisecond * 10) - var verErr *verify.VerifyError + var verErr *header.VerifyError err = syncer.incomingNetworkHead(ctx, range1[len(range1)-1]) assert.ErrorAs(t, err, &verErr) diff --git a/sync/verify/verify_test.go b/sync/verify/verify_test.go deleted file mode 100644 index ae7d6ecf..00000000 --- a/sync/verify/verify_test.go +++ /dev/null @@ -1,146 +0,0 @@ -package verify - -import ( - "errors" - "strconv" - "testing" - "time" - - "github.com/stretchr/testify/assert" - - "github.com/celestiaorg/go-header/headertest" -) - -func TestVerify(t *testing.T) { - suite := headertest.NewTestSuite(t) - trusted := suite.GenDummyHeaders(1)[0] - - next := func() *headertest.DummyHeader { - next := *suite.NextHeader() - return &next - } - - tests := []struct { - prepare func() *headertest.DummyHeader - err bool - soft bool - }{ - { - prepare: func() *headertest.DummyHeader { - return nil - }, - err: true, - }, - { - prepare: func() *headertest.DummyHeader { - untrusted := next() - untrusted.VerifyFailure = true - return untrusted - }, - err: true, - }, - { - prepare: func() *headertest.DummyHeader { - untrusted := next() - untrusted.VerifyFailure = true - return untrusted - }, - err: true, - soft: true, // soft because non-adjacent - }, - { - prepare: func() *headertest.DummyHeader { - return next() - }, - }, - } - - for i, test := range tests { - t.Run(strconv.Itoa(i), func(t *testing.T) { - err := Verify(trusted, test.prepare(), 0) - if test.err { - var verErr *VerifyError - assert.ErrorAs(t, err, &verErr) - assert.NotNil(t, errors.Unwrap(verErr)) - assert.Equal(t, test.soft, verErr.SoftFailure) - } else { - assert.NoError(t, err) - } - }) - } -} - -func Test_verify(t *testing.T) { - suite := headertest.NewTestSuite(t) - trusted := suite.GenDummyHeaders(1)[0] - - next := func() *headertest.DummyHeader { - next := *suite.NextHeader() // copy is required - return &next - } - - tests := []struct { - prepare func() *headertest.DummyHeader - err error - }{ - { - prepare: func() *headertest.DummyHeader { - return next() - }, - }, - { - prepare: func() *headertest.DummyHeader { - return nil - }, - err: errZero, - }, - { - prepare: func() *headertest.DummyHeader { - untrusted := next() - untrusted.Raw.ChainID = "gtmb" - return untrusted - }, - err: errWrongChain, - }, - { - prepare: func() *headertest.DummyHeader { - untrusted := next() - untrusted.Raw.Time = untrusted.Raw.Time.Truncate(time.Minute * 10) - return untrusted - }, - err: errUnordered, - }, - { - prepare: func() *headertest.DummyHeader { - untrusted := next() - untrusted.Raw.Time = untrusted.Raw.Time.Add(time.Minute) - return untrusted - }, - err: errFromFuture, - }, - { - prepare: func() *headertest.DummyHeader { - untrusted := next() - untrusted.Raw.Height = trusted.Height() - return untrusted - }, - err: errKnown, - }, - { - prepare: func() *headertest.DummyHeader { - untrusted := next() - untrusted.Raw.Height += 100000 - return untrusted - }, - err: errHeightFromFuture, - }, - } - - for i, test := range tests { - t.Run(strconv.Itoa(i), func(t *testing.T) { - err := verify(trusted, test.prepare(), 0) - assert.ErrorIs(t, err, test.err) - t.Log(err) - }) - } -} diff --git a/sync/verify/verify.go b/verify.go similarity index 69% rename from sync/verify/verify.go rename to verify.go index bdc5031b..9a64cf85 100644 --- a/sync/verify/verify.go +++ b/verify.go @@ -1,46 +1,21 @@ -// TODO(@Wondertan): Should be just part of sync pkg and not subpkg -// -// Fix after adjacency requirement is removed from the Store. -package verify +package header import ( "errors" "fmt" "time" - - "github.com/celestiaorg/go-header" ) // DefaultHeightThreshold defines default height threshold beyond which headers are rejected // NOTE: Compared against subjective head which is guaranteed to be non-expired const DefaultHeightThreshold int64 = 40000 // ~ 7 days of 15 second headers -// VerifyError is thrown during for Headers failed verification. -type VerifyError struct { - // Reason why verification failed as inner error. - Reason error - // SoftFailure means verification did not have enough information to definitively conclude a - // Header was correct or not. - // May happen with recent Headers during unfinished historical sync or because of local errors. - // TODO(@Wondertan): Better be part of signature Header.Verify() (bool, error), but kept here - // not to break - SoftFailure bool -} - -func (vr *VerifyError) Error() string { - return fmt.Sprintf("header verification failed: %s", vr.Reason.Error()) -} - -func (vr *VerifyError) Unwrap() error { - return vr.Reason -} - // Verify verifies untrusted Header against trusted following general Header checks and // custom user-specific checks defined in Header.Verify // // If heightThreshold is zero, uses DefaultHeightThreshold. // Always returns VerifyError. -func Verify[H header.Header](trstd, untrstd H, heightThreshold int64) error { +func Verify[H Header](trstd, untrstd H, heightThreshold int64) error { // general mandatory verification err := verify[H](trstd, untrstd, heightThreshold) if err != nil { @@ -70,57 +45,76 @@ func Verify[H header.Header](trstd, untrstd H, heightThreshold int64) error { // verify is a little bro of Verify yet performs mandatory Header checks // for any Header implementation. -func verify[H header.Header](trstd, untrstd H, heightThreshold int64) error { +func verify[H Header](trstd, untrstd H, heightThreshold int64) error { if heightThreshold == 0 { heightThreshold = DefaultHeightThreshold } if untrstd.IsZero() { - return errZero + return ErrZeroHeader } if untrstd.ChainID() != trstd.ChainID() { - return fmt.Errorf("%w: '%s' != '%s'", errWrongChain, untrstd.ChainID(), trstd.ChainID()) + return fmt.Errorf("%w: '%s' != '%s'", ErrWrongChainID, untrstd.ChainID(), trstd.ChainID()) } if !untrstd.Time().After(trstd.Time()) { - return fmt.Errorf("%w: timestamp '%s' < current '%s'", errUnordered, formatTime(untrstd.Time()), formatTime(trstd.Time())) + return fmt.Errorf("%w: timestamp '%s' < current '%s'", ErrUnorderedTime, formatTime(untrstd.Time()), formatTime(trstd.Time())) } now := time.Now() if untrstd.Time().After(now.Add(clockDrift)) { - return fmt.Errorf("%w: timestamp '%s' > now '%s', clock_drift '%v'", errFromFuture, formatTime(untrstd.Time()), formatTime(now), clockDrift) + return fmt.Errorf("%w: timestamp '%s' > now '%s', clock_drift '%v'", ErrFromFuture, formatTime(untrstd.Time()), formatTime(now), clockDrift) } known := untrstd.Height() <= trstd.Height() if known { - return fmt.Errorf("%w: '%d' <= current '%d'", errKnown, untrstd.Height(), trstd.Height()) + return fmt.Errorf("%w: '%d' <= current '%d'", ErrKnownHeader, untrstd.Height(), trstd.Height()) } // reject headers with height too far from the future // this is essential for headers failed non-adjacent verification // yet taken as sync target adequateHeight := untrstd.Height()-trstd.Height() < heightThreshold if !adequateHeight { - return fmt.Errorf("%w: '%d' - current '%d' >= threshold '%d'", errHeightFromFuture, untrstd.Height(), trstd.Height(), heightThreshold) + return fmt.Errorf("%w: '%d' - current '%d' >= threshold '%d'", ErrHeightFromFuture, untrstd.Height(), trstd.Height(), heightThreshold) } return nil } -func formatTime(t time.Time) string { - return t.UTC().Format(time.DateTime) -} - -// unexported for testing, but can be exported if needed var ( - errZero = errors.New("zero header") - errWrongChain = errors.New("wrong chain id") - errUnordered = errors.New("unordered headers") - errFromFuture = errors.New("header is from the future") - errKnown = errors.New("known header") - errHeightFromFuture = errors.New("header height is far from future") + ErrZeroHeader = errors.New("zero header") + ErrWrongChainID = errors.New("wrong chain id") + ErrUnorderedTime = errors.New("unordered headers") + ErrFromFuture = errors.New("header is from the future") + ErrKnownHeader = errors.New("known header") + ErrHeightFromFuture = errors.New("header height is far from future") ) +// VerifyError is thrown during for Headers failed verification. +type VerifyError struct { + // Reason why verification failed as inner error. + Reason error + // SoftFailure means verification did not have enough information to definitively conclude a + // Header was correct or not. + // May happen with recent Headers during unfinished historical sync or because of local errors. + // TODO(@Wondertan): Better be part of signature Header.Verify() (bool, error), but kept here + // not to break + SoftFailure bool +} + +func (vr *VerifyError) Error() string { + return fmt.Sprintf("header verification failed: %s", vr.Reason) +} + +func (vr *VerifyError) Unwrap() error { + return vr.Reason +} + // clockDrift defines how much new header's time can drift into // the future relative to the now time during verification. var clockDrift = 10 * time.Second + +func formatTime(t time.Time) string { + return t.UTC().Format(time.RFC3339) +} \ No newline at end of file From 6deee3ade92a28226fbab10e9e7a71de99a5be0e Mon Sep 17 00:00:00 2001 From: Wondertan Date: Wed, 23 Aug 2023 20:35:34 +0200 Subject: [PATCH 4/7] update comment --- verify.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/verify.go b/verify.go index 9a64cf85..783db18e 100644 --- a/verify.go +++ b/verify.go @@ -91,7 +91,7 @@ var ( ErrHeightFromFuture = errors.New("header height is far from future") ) -// VerifyError is thrown during for Headers failed verification. +// VerifyError is thrown if a Header failed verification. type VerifyError struct { // Reason why verification failed as inner error. Reason error From 8db8d5499dca876dd0be6a04e9bd9c62dc9bd136 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Wed, 23 Aug 2023 20:48:02 +0200 Subject: [PATCH 5/7] fmt --- Makefile | 5 ++++- headertest/verify_test.go | 3 ++- p2p/exchange_test.go | 3 ++- p2p/helpers.go | 3 ++- p2p/server.go | 3 ++- store/heightsub_test.go | 6 +++--- verify.go | 8 ++++---- 7 files changed, 19 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 000af090..cce9d629 100644 --- a/Makefile +++ b/Makefile @@ -60,7 +60,10 @@ lint-imports: .PHONY: lint-imports sort-imports: - @goimports-reviser -company-prefixes "github.com/celestiaorg" -project-name "github.com/celestiaorg/"$(PROJECTNAME)"" -output stdout . + @for file in `find . -type f -name '*.go'`; \ + do goimports-reviser -company-prefixes "github.com/celestiaorg" -project-name "github.com/celestiaorg/"$(PROJECTNAME)"" $$file \ + || exit 1; \ + done; .PHONY: sort-imports pb-gen: diff --git a/headertest/verify_test.go b/headertest/verify_test.go index 62099d85..2774c056 100644 --- a/headertest/verify_test.go +++ b/headertest/verify_test.go @@ -6,8 +6,9 @@ import ( "testing" "time" - "github.com/celestiaorg/go-header" "github.com/stretchr/testify/assert" + + "github.com/celestiaorg/go-header" ) func TestVerify(t *testing.T) { diff --git a/p2p/exchange_test.go b/p2p/exchange_test.go index 71cee81e..47d6e60f 100644 --- a/p2p/exchange_test.go +++ b/p2p/exchange_test.go @@ -19,10 +19,11 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/celestiaorg/go-libp2p-messenger/serde" + "github.com/celestiaorg/go-header" "github.com/celestiaorg/go-header/headertest" p2p_pb "github.com/celestiaorg/go-header/p2p/pb" - "github.com/celestiaorg/go-libp2p-messenger/serde" ) const networkID = "test" // must match the chain-id in test suite diff --git a/p2p/helpers.go b/p2p/helpers.go index 5b943b58..fd85a417 100644 --- a/p2p/helpers.go +++ b/p2p/helpers.go @@ -11,9 +11,10 @@ import ( "github.com/libp2p/go-libp2p/core/peer" "github.com/libp2p/go-libp2p/core/protocol" + "github.com/celestiaorg/go-libp2p-messenger/serde" + "github.com/celestiaorg/go-header" p2p_pb "github.com/celestiaorg/go-header/p2p/pb" - "github.com/celestiaorg/go-libp2p-messenger/serde" ) func protocolID(networkID string) protocol.ID { diff --git a/p2p/server.go b/p2p/server.go index 8ad89984..a33a08f7 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -13,9 +13,10 @@ import ( "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" + "github.com/celestiaorg/go-libp2p-messenger/serde" + "github.com/celestiaorg/go-header" p2p_pb "github.com/celestiaorg/go-header/p2p/pb" - "github.com/celestiaorg/go-libp2p-messenger/serde" ) var ( diff --git a/store/heightsub_test.go b/store/heightsub_test.go index 2145403a..34981037 100644 --- a/store/heightsub_test.go +++ b/store/heightsub_test.go @@ -19,7 +19,7 @@ func TestHeightSub(t *testing.T) { // assert subscription returns nil for past heights { h := headertest.RandDummyHeader(t) - h.Raw.Height = 100 + h.HeightI = 100 hs.SetHeight(99) hs.Pub(h) @@ -35,9 +35,9 @@ func TestHeightSub(t *testing.T) { time.Sleep(time.Millisecond) h1 := headertest.RandDummyHeader(t) - h1.Raw.Height = 101 + h1.HeightI = 101 h2 := headertest.RandDummyHeader(t) - h2.Raw.Height = 102 + h2.HeightI = 102 hs.Pub(h1, h2) }() diff --git a/verify.go b/verify.go index 783db18e..071d51a7 100644 --- a/verify.go +++ b/verify.go @@ -83,9 +83,9 @@ func verify[H Header](trstd, untrstd H, heightThreshold int64) error { } var ( - ErrZeroHeader = errors.New("zero header") - ErrWrongChainID = errors.New("wrong chain id") - ErrUnorderedTime = errors.New("unordered headers") + ErrZeroHeader = errors.New("zero header") + ErrWrongChainID = errors.New("wrong chain id") + ErrUnorderedTime = errors.New("unordered headers") ErrFromFuture = errors.New("header is from the future") ErrKnownHeader = errors.New("known header") ErrHeightFromFuture = errors.New("header height is far from future") @@ -117,4 +117,4 @@ var clockDrift = 10 * time.Second func formatTime(t time.Time) string { return t.UTC().Format(time.RFC3339) -} \ No newline at end of file +} From 84b76c358699b627d2dcbd8cefbcaac7c2b4907a Mon Sep 17 00:00:00 2001 From: Wondertan Date: Thu, 24 Aug 2023 15:30:19 +0200 Subject: [PATCH 6/7] add small buffer for header from future accounting --- sync/sync_head.go | 3 ++- verify.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sync/sync_head.go b/sync/sync_head.go index 5db7382a..8d79b715 100644 --- a/sync/sync_head.go +++ b/sync/sync_head.go @@ -161,7 +161,8 @@ func (s *Syncer[H]) verify(ctx context.Context, newHead H) (bool, error) { var heightThreshold int64 if s.Params.TrustingPeriod != 0 && s.Params.blockTime != 0 { - heightThreshold = int64(s.Params.TrustingPeriod / s.Params.blockTime) + buffer := time.Hour * 6 / s.Params.blockTime // small buffer to account for network delays + heightThreshold = int64(s.Params.TrustingPeriod / s.Params.blockTime + buffer) } err = header.Verify(sbjHead, newHead, heightThreshold) diff --git a/verify.go b/verify.go index 071d51a7..22a86ff0 100644 --- a/verify.go +++ b/verify.go @@ -8,7 +8,7 @@ import ( // DefaultHeightThreshold defines default height threshold beyond which headers are rejected // NOTE: Compared against subjective head which is guaranteed to be non-expired -const DefaultHeightThreshold int64 = 40000 // ~ 7 days of 15 second headers +const DefaultHeightThreshold int64 = 80000 // ~ 14 days of 15 second headers // Verify verifies untrusted Header against trusted following general Header checks and // custom user-specific checks defined in Header.Verify From efe87e00b922ad15af275fe33369a1909d6ac07f Mon Sep 17 00:00:00 2001 From: Wondertan Date: Thu, 24 Aug 2023 15:33:16 +0200 Subject: [PATCH 7/7] use Before and not !After --- verify.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/verify.go b/verify.go index 22a86ff0..abe8c263 100644 --- a/verify.go +++ b/verify.go @@ -58,7 +58,7 @@ func verify[H Header](trstd, untrstd H, heightThreshold int64) error { return fmt.Errorf("%w: '%s' != '%s'", ErrWrongChainID, untrstd.ChainID(), trstd.ChainID()) } - if !untrstd.Time().After(trstd.Time()) { + if untrstd.Time().Before(trstd.Time()) { return fmt.Errorf("%w: timestamp '%s' < current '%s'", ErrUnorderedTime, formatTime(untrstd.Time()), formatTime(trstd.Time())) }