lite2: make maxClockDrift an option (#4616)

Closes #4607

CHANGELOG_PENDING.md

@@ -14,6 +14,9 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 
 - Go API
 
+  - [lite2] [\#4616](https://github.com/tendermint/tendermint/pull/4616) Make `maxClockDrift` an option (@melekes).
+    `Verify/VerifyAdjacent/VerifyNonAdjacent` now accept `maxClockDrift time.Duration`.
+
 ### FEATURES:
 
 - [rpc] [\#4611](https://github.com/tendermint/tendermint/pull/4611) Add `codespace` to `ResultBroadcastTx` (@whylee259)

docs/architecture/adr-046-light-client-implementation.md

@@ -4,6 +4,7 @@
 * 13-02-2020: Initial draft
 * 26-02-2020: Cross-checking the first header
 * 28-02-2020: Bisection algorithm details
+* 31-03-2020: Verify signature got changed
 
 ## Context
 
@@ -100,6 +101,10 @@ type Store interface {
 	FirstSignedHeaderHeight() (int64, error)
 
 	SignedHeaderAfter(height int64) (*types.SignedHeader, error)
+
+	Prune(size uint16) error
+
+	Size() uint16
 }
 ```
 
@@ -110,12 +115,13 @@ database, used in Tendermint). In the future, remote adapters are possible
 ```go
 func Verify(
 	chainID string,
-	h1 *types.SignedHeader,
-	h1NextVals *types.ValidatorSet,
-	h2 *types.SignedHeader,
-	h2Vals *types.ValidatorSet,
+	trustedHeader *types.SignedHeader, // height=X
+	trustedVals *types.ValidatorSet, // height=X or height=X+1
+	untrustedHeader *types.SignedHeader, // height=Y
+	untrustedVals *types.ValidatorSet, // height=Y
 	trustingPeriod time.Duration,
 	now time.Time,
+	maxClockDrift time.Duration,
 	trustLevel tmmath.Fraction) error {
 ```
 
@@ -124,6 +130,9 @@ cases of adjacent and non-adjacent headers. In the former case, it compares the
 hashes directly (2/3+ signed transition). Otherwise, it verifies 1/3+
 (`trustLevel`) of trusted validators are still present in new validators.
 
+While `Verify` function is certainly handy, `VerifyAdjacent` and
+`VerifyNonAdjacent` should be used most often to avoid logic errors.
+
 ### Bisection algorithm details
 
 Non-recursive bisection algorithm was implemented despite the spec containing

lite2/client.go

@@ -29,6 +29,12 @@ const (
 	// find something in between the range, 9/16 is used.
 	bisectionNumerator   = 9
 	bisectionDenominator = 16
+
+	// 10s should cover most of the clients.
+	// References:
+	// - http://vancouver-webpages.com/time/web.html
+	// - https://blog.codinghorror.com/keeping-time-on-the-pc/
+	defaultMaxClockDrift = 10 * time.Second
 )
 
 // Option sets a parameter for the light client.
@@ -94,6 +100,14 @@ func MaxRetryAttempts(max uint16) Option {
 	}
 }
 
+// MaxClockDrift defines how much new (untrusted) header's Time can drift into
+// the future. Default: 10s.
+func MaxClockDrift(d time.Duration) Option {
+	return func(c *Client) {
+		c.maxClockDrift = d
+	}
+}
+
 // Client represents a light client, connected to a single chain, which gets
 // headers from a primary provider, verifies them either sequentially or by
 // skipping some and stores them in a trusted store (usually, a local FS).
@@ -105,6 +119,7 @@ type Client struct {
 	verificationMode mode
 	trustLevel       tmmath.Fraction
 	maxRetryAttempts uint16 // see MaxRetryAttempts option
+	maxClockDrift    time.Duration
 
 	// Mutex for locking during changes of the lite clients providers
 	providerMutex sync.Mutex
@@ -191,6 +206,7 @@ func NewClientFromTrustedStore(
 		verificationMode: skipping,
 		trustLevel:       DefaultTrustLevel,
 		maxRetryAttempts: defaultMaxRetryAttempts,
+		maxClockDrift:    defaultMaxClockDrift,
 		primary:          primary,
 		witnesses:        witnesses,
 		trustedStore:     trustedStore,
@@ -636,7 +652,7 @@ func (c *Client) sequence(
 			"newHash", hash2str(interimHeader.Hash()))
 
 		err = VerifyAdjacent(c.chainID, trustedHeader, interimHeader, interimVals,
-			c.trustingPeriod, now)
+			c.trustingPeriod, now, c.maxClockDrift)
 		if err != nil {
 			err = fmt.Errorf("verify adjacent from #%d to #%d failed: %w",
 				trustedHeader.Height, interimHeader.Height, err)
@@ -697,7 +713,7 @@ func (c *Client) bisection(
 			"newHash", hash2str(headerCache[depth].sh.Hash()))
 
 		err := Verify(c.chainID, trustedHeader, trustedVals, headerCache[depth].sh, headerCache[depth].valSet,
-			c.trustingPeriod, now, c.trustLevel)
+			c.trustingPeriod, now, c.maxClockDrift, c.trustLevel)
 		switch err.(type) {
 		case nil:
 			// Have we verified the last header

lite2/verifier.go

@@ -10,10 +10,6 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
-const (
-	maxClockDrift = 10 * time.Second
-)
-
 var (
 	// DefaultTrustLevel - new header can be trusted if at least one correct
 	// validator signed it.
@@ -30,6 +26,9 @@ var (
 //	d) more than 2/3 of untrustedVals have signed h2
 //    (otherwise, ErrInvalidHeader is returned)
 //  e) headers are non-adjacent.
+//
+// maxClockDrift defines how much untrustedHeader.Time can drift into the
+// future.
 func VerifyNonAdjacent(
 	chainID string,
 	trustedHeader *types.SignedHeader, // height=X
@@ -38,6 +37,7 @@ func VerifyNonAdjacent(
 	untrustedVals *types.ValidatorSet, // height=Y
 	trustingPeriod time.Duration,
 	now time.Time,
+	maxClockDrift time.Duration,
 	trustLevel tmmath.Fraction) error {
 
 	if untrustedHeader.Height == trustedHeader.Height+1 {
@@ -48,7 +48,11 @@ func VerifyNonAdjacent(
 		return ErrOldHeaderExpired{trustedHeader.Time.Add(trustingPeriod), now}
 	}
 
-	if err := verifyNewHeaderAndVals(chainID, untrustedHeader, untrustedVals, trustedHeader, now); err != nil {
+	if err := verifyNewHeaderAndVals(
+		chainID,
+		untrustedHeader, untrustedVals,
+		trustedHeader,
+		now, maxClockDrift); err != nil {
 		return ErrInvalidHeader{err}
 	}
 
@@ -86,13 +90,17 @@ func VerifyNonAdjacent(
 //  d) more than 2/3 of new validators (untrustedVals) have signed h2
 //    (otherwise, ErrInvalidHeader is returned)
 //  e) headers are adjacent.
+//
+// maxClockDrift defines how much untrustedHeader.Time can drift into the
+// future.
 func VerifyAdjacent(
 	chainID string,
 	trustedHeader *types.SignedHeader, // height=X
 	untrustedHeader *types.SignedHeader, // height=X+1
 	untrustedVals *types.ValidatorSet, // height=X+1
 	trustingPeriod time.Duration,
-	now time.Time) error {
+	now time.Time,
+	maxClockDrift time.Duration) error {
 
 	if untrustedHeader.Height != trustedHeader.Height+1 {
 		return errors.New("headers must be adjacent in height")
@@ -102,7 +110,11 @@ func VerifyAdjacent(
 		return ErrOldHeaderExpired{trustedHeader.Time.Add(trustingPeriod), now}
 	}
 
-	if err := verifyNewHeaderAndVals(chainID, untrustedHeader, untrustedVals, trustedHeader, now); err != nil {
+	if err := verifyNewHeaderAndVals(
+		chainID,
+		untrustedHeader, untrustedVals,
+		trustedHeader,
+		now, maxClockDrift); err != nil {
 		return ErrInvalidHeader{err}
 	}
 
@@ -133,14 +145,15 @@ func Verify(
 	untrustedVals *types.ValidatorSet, // height=Y
 	trustingPeriod time.Duration,
 	now time.Time,
+	maxClockDrift time.Duration,
 	trustLevel tmmath.Fraction) error {
 
 	if untrustedHeader.Height != trustedHeader.Height+1 {
 		return VerifyNonAdjacent(chainID, trustedHeader, trustedVals, untrustedHeader, untrustedVals,
-			trustingPeriod, now, trustLevel)
+			trustingPeriod, now, maxClockDrift, trustLevel)
 	}
 
-	return VerifyAdjacent(chainID, trustedHeader, untrustedHeader, untrustedVals, trustingPeriod, now)
+	return VerifyAdjacent(chainID, trustedHeader, untrustedHeader, untrustedVals, trustingPeriod, now, maxClockDrift)
 }
 
 func verifyNewHeaderAndVals(
@@ -148,7 +161,8 @@ func verifyNewHeaderAndVals(
 	untrustedHeader *types.SignedHeader,
 	untrustedVals *types.ValidatorSet,
 	trustedHeader *types.SignedHeader,
-	now time.Time) error {
+	now time.Time,
+	maxClockDrift time.Duration) error {
 
 	if err := untrustedHeader.ValidateBasic(chainID); err != nil {
 		return errors.Wrap(err, "untrustedHeader.ValidateBasic failed")

lite2/verifier_test.go

@@ -11,6 +11,10 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
+const (
+	maxClockDrift = 10 * time.Second
+)
+
 func TestVerifyAdjacentHeaders(t *testing.T) {
 	const (
 		chainID    = "TestVerifyAdjacentHeaders"
@@ -151,7 +155,7 @@ func TestVerifyAdjacentHeaders(t *testing.T) {
 	for i, tc := range testCases {
 		tc := tc
 		t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) {
-			err := VerifyAdjacent(chainID, header, tc.newHeader, tc.newVals, tc.trustingPeriod, tc.now)
+			err := VerifyAdjacent(chainID, header, tc.newHeader, tc.newVals, tc.trustingPeriod, tc.now, maxClockDrift)
 			switch {
 			case tc.expErr != nil && assert.Error(t, err):
 				assert.Equal(t, tc.expErr, err)
@@ -265,7 +269,8 @@ func TestVerifyNonAdjacentHeaders(t *testing.T) {
 	for i, tc := range testCases {
 		tc := tc
 		t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) {
-			err := VerifyNonAdjacent(chainID, header, vals, tc.newHeader, tc.newVals, tc.trustingPeriod, tc.now,
+			err := VerifyNonAdjacent(chainID, header, vals, tc.newHeader, tc.newVals, tc.trustingPeriod,
+				tc.now, maxClockDrift,
 				DefaultTrustLevel)
 
 			switch {
@@ -295,7 +300,7 @@ func TestVerifyReturnsErrorIfTrustLevelIsInvalid(t *testing.T) {
 			[]byte("app_hash"), []byte("cons_hash"), []byte("results_hash"), 0, len(keys))
 	)
 
-	err := Verify(chainID, header, vals, header, vals, 2*time.Hour, time.Now(),
+	err := Verify(chainID, header, vals, header, vals, 2*time.Hour, time.Now(), maxClockDrift,
 		tmmath.Fraction{Numerator: 2, Denominator: 1})
 	assert.Error(t, err)
 }