improve error handling

celestiaorg · Aug 23, 2023 · 950e424 · 950e424
1 parent 1ffb292
commit 950e424
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 29 deletions.
diff --git 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
@@ -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,15 +33,15 @@ func TestVerify(t *testing.T) {
 		},
 		{
 			prepare: func() *headertest.DummyHeader {
-				untrusted := suite.NextHeader()
+				untrusted := next()
 				untrusted.VerifyFailure = true
 				return untrusted
 			},
 			err: true,
 		},
 		{
 			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)
 		})
 	}
 }