refactor!: harden verification

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:

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 "private"
+	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,20 +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())
-	}
-
-	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 ErrDummyVerify
 	}
 
 	return nil
@@ -130,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

headertest/dummy_suite.go

@@ -42,21 +42,19 @@ 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.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
 }
 
 func (s *DummySuite) genesis() *DummyHeader {
 	return &DummyHeader{
-		hash: nil,
-		Raw: Raw{
-			PreviousHash: nil,
-			Height:       1,
-			Time:         time.Now().Add(-10 * time.Second).UTC(),
-		},
+		HeightI:   1,
+		Timestamp: time.Now().Add(-10 * time.Second).UTC(),
+		Chainid:   "test",
 	}
 }

headertest/verify_test.go

@@ -0,0 +1,111 @@
+package headertest
+
+import (
+	"errors"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/celestiaorg/go-header"
+)
+
+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)
+			}
+		})
+	}
+}

p2p/exchange_test.go

@@ -26,7 +26,7 @@ import (
 	p2p_pb "github.com/celestiaorg/go-header/p2p/pb"
 )
 
-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 +393,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)

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 {

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 (

p2p/subscriber.go

@@ -82,7 +82,7 @@ func (p *Subscriber[H]) SetVerifier(val func(context.Context, H) error) error {
 		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

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)
 		}()