light: run detector for sequentially validating light client (#5538) (#5601)

Closes #5445

Backport of #5538

light/client.go

@@ -123,7 +123,7 @@ type Client struct {
 	providerMutex tmsync.Mutex
 	// Primary provider of new headers.
 	primary provider.Provider
-	// See Witnesses option
+	// Providers used to "witness" new headers.
 	witnesses []provider.Provider
 
 	// Where trusted light blocks are stored.
@@ -218,7 +218,7 @@ func NewClientFromTrustedStore(
 	}
 
 	// Validate the number of witnesses.
-	if len(c.witnesses) < 1 && c.verificationMode == skipping {
+	if len(c.witnesses) < 1 {
 		return nil, errNoWitnesses{}
 	}
 
@@ -363,10 +363,8 @@ func (c *Client) initializeWithTrustOptions(ctx context.Context, options TrustOp
 	}
 
 	// 3) Cross-verify with witnesses to ensure everybody has the same state.
-	if len(c.witnesses) > 0 {
+	if err := c.compareFirstHeaderWithWitnesses(ctx, l.SignedHeader); err != nil {
-		if err := c.compareFirstHeaderWithWitnesses(ctx, l.SignedHeader); err != nil {
+		return err
-			return err
-		}
 	}
 
 	// 4) Persist both of them and continue.
@@ -443,7 +441,7 @@ func (c *Client) Update(ctx context.Context, now time.Time) (*types.LightBlock,
 }
 
 // VerifyLightBlockAtHeight fetches the light block at the given height
-// and calls verifyLightBlock. It returns the block immediately if it exists in
+// and verifies it. It returns the block immediately if it exists in
 // the trustedStore (no verification is needed).
 //
 // height must be > 0.
@@ -600,6 +598,7 @@ func (c *Client) verifySequential(
 		verifiedBlock = trustedBlock
 		interimBlock  *types.LightBlock
 		err           error
+		trace         = []*types.LightBlock{trustedBlock}
 	)
 
 	for height := trustedBlock.Height + 1; height <= newLightBlock.Height; height++ {
@@ -669,9 +668,17 @@ func (c *Client) verifySequential(
 
 		// 3) Update verifiedBlock
 		verifiedBlock = interimBlock
+
+		// 4) Add verifiedBlock to trace
+		trace = append(trace, verifiedBlock)
 	}
 
-	return nil
+	// Compare header with the witnesses to ensure it's not a fork.
+	// More witnesses we have, more chance to notice one.
+	//
+	// CORRECTNESS ASSUMPTION: there's at least 1 correct full node
+	// (primary or one of the witnesses).
+	return c.detectDivergence(ctx, trace, now)
 }
 
 // see VerifyHeader
@@ -995,6 +1002,10 @@ func (c *Client) compareFirstHeaderWithWitnesses(ctx context.Context, h *types.S
 	compareCtx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
+	if len(c.witnesses) < 1 {
+		return errNoWitnesses{}
+	}
+
 	errc := make(chan error, len(c.witnesses))
 	for i, witness := range c.witnesses {
 		go c.compareNewHeaderWithWitness(compareCtx, errc, h, witness, i)

light/detector.go

@@ -15,8 +15,7 @@ import (
 // More info here:
 // tendermint/docs/architecture/adr-047-handling-evidence-from-light-client.md
 
-// detectDivergence is a second wall of defense for the light client and is used
+// detectDivergence is a second wall of defense for the light client.
-// only in the case of skipping verification which employs the trust level mechanism.
 //
 // It takes the target verified header and compares it with the headers of a set of
 // witness providers that the light client is connected to. If a conflicting header

light/detector_test.go

@@ -90,76 +90,86 @@ func TestLightClientAttackEvidence_Lunatic(t *testing.T) {
 }
 
 func TestLightClientAttackEvidence_Equivocation(t *testing.T) {
-	// primary performs an equivocation attack
+	verificationOptions := map[string]light.Option{
-	var (
+		"sequential": light.SequentialVerification(),
-		latestHeight      = int64(10)
+		"skipping":   light.SkippingVerification(light.DefaultTrustLevel),
-		valSize           = 5
+	}
-		divergenceHeight  = int64(6)
-		primaryHeaders    = make(map[int64]*types.SignedHeader, latestHeight)
-		primaryValidators = make(map[int64]*types.ValidatorSet, latestHeight)
-	)
-	// validators don't change in this network (however we still use a map just for convenience)
-	witnessHeaders, witnessValidators, chainKeys := genMockNodeWithKeys(chainID, latestHeight+2, valSize, 2, bTime)
-	witness := mockp.New(chainID, witnessHeaders, witnessValidators)
 
-	for height := int64(1); height <= latestHeight; height++ {
+	for s, verificationOption := range verificationOptions {
-		if height < divergenceHeight {
+		t.Log("==> verification", s)
-			primaryHeaders[height] = witnessHeaders[height]
+
+		// primary performs an equivocation attack
+		var (
+			latestHeight      = int64(10)
+			valSize           = 5
+			divergenceHeight  = int64(6)
+			primaryHeaders    = make(map[int64]*types.SignedHeader, latestHeight)
+			primaryValidators = make(map[int64]*types.ValidatorSet, latestHeight)
+		)
+		// validators don't change in this network (however we still use a map just for convenience)
+		witnessHeaders, witnessValidators, chainKeys := genMockNodeWithKeys(chainID, latestHeight+2, valSize, 2, bTime)
+		witness := mockp.New(chainID, witnessHeaders, witnessValidators)
+
+		for height := int64(1); height <= latestHeight; height++ {
+			if height < divergenceHeight {
+				primaryHeaders[height] = witnessHeaders[height]
+				primaryValidators[height] = witnessValidators[height]
+				continue
+			}
+			// we don't have a network partition so we will make 4/5 (greater than 2/3) malicious and vote again for
+			// a different block (which we do by adding txs)
+			primaryHeaders[height] = chainKeys[height].GenSignedHeader(chainID, height,
+				bTime.Add(time.Duration(height)*time.Minute), []types.Tx{[]byte("abcd")},
+				witnessValidators[height], witnessValidators[height+1], hash("app_hash"),
+				hash("cons_hash"), hash("results_hash"), 0, len(chainKeys[height])-1)
 			primaryValidators[height] = witnessValidators[height]
-			continue
 		}
-		// we don't have a network partition so we will make 4/5 (greater than 2/3) malicious and vote again for
+		primary := mockp.New(chainID, primaryHeaders, primaryValidators)
-		// a different block (which we do by adding txs)
-		primaryHeaders[height] = chainKeys[height].GenSignedHeader(chainID, height,
-			bTime.Add(time.Duration(height)*time.Minute), []types.Tx{[]byte("abcd")},
-			witnessValidators[height], witnessValidators[height+1], hash("app_hash"),
-			hash("cons_hash"), hash("results_hash"), 0, len(chainKeys[height])-1)
-		primaryValidators[height] = witnessValidators[height]
-	}
-	primary := mockp.New(chainID, primaryHeaders, primaryValidators)
 
-	c, err := light.NewClient(
+		c, err := light.NewClient(
-		ctx,
+			ctx,
-		chainID,
+			chainID,
-		light.TrustOptions{
+			light.TrustOptions{
-			Period: 4 * time.Hour,
+				Period: 4 * time.Hour,
-			Height: 1,
+				Height: 1,
-			Hash:   primaryHeaders[1].Hash(),
+				Hash:   primaryHeaders[1].Hash(),
-		},
+			},
-		primary,
+			primary,
-		[]provider.Provider{witness},
+			[]provider.Provider{witness},
-		dbs.New(dbm.NewMemDB(), chainID),
+			dbs.New(dbm.NewMemDB(), chainID),
-		light.Logger(log.TestingLogger()),
+			light.Logger(log.TestingLogger()),
-		light.MaxRetryAttempts(1),
+			light.MaxRetryAttempts(1),
-	)
+			verificationOption,
-	require.NoError(t, err)
+		)
+		require.NoError(t, err)
 
-	// Check verification returns an error.
+		// Check verification returns an error.
-	_, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour))
+		_, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour))
-	if assert.Error(t, err) {
+		if assert.Error(t, err) {
-		assert.Contains(t, err.Error(), "does not match primary")
+			assert.Contains(t, err.Error(), "does not match primary")
-	}
+		}
 
-	// Check evidence was sent to both full nodes.
+		// Check evidence was sent to both full nodes.
-	// Common height should be set to the height of the divergent header in the instance
+		// Common height should be set to the height of the divergent header in the instance
-	// of an equivocation attack and the validator sets are the same as what the witness has
+		// of an equivocation attack and the validator sets are the same as what the witness has
-	evAgainstPrimary := &types.LightClientAttackEvidence{
+		evAgainstPrimary := &types.LightClientAttackEvidence{
-		ConflictingBlock: &types.LightBlock{
+			ConflictingBlock: &types.LightBlock{
-			SignedHeader: primaryHeaders[divergenceHeight],
+				SignedHeader: primaryHeaders[divergenceHeight],
-			ValidatorSet: primaryValidators[divergenceHeight],
+				ValidatorSet: primaryValidators[divergenceHeight],
-		},
+			},
-		CommonHeight: divergenceHeight,
+			CommonHeight: divergenceHeight,
-	}
+		}
-	assert.True(t, witness.HasEvidence(evAgainstPrimary))
+		assert.True(t, witness.HasEvidence(evAgainstPrimary))
 
-	evAgainstWitness := &types.LightClientAttackEvidence{
+		evAgainstWitness := &types.LightClientAttackEvidence{
-		ConflictingBlock: &types.LightBlock{
+			ConflictingBlock: &types.LightBlock{
-			SignedHeader: witnessHeaders[divergenceHeight],
+				SignedHeader: witnessHeaders[divergenceHeight],
-			ValidatorSet: witnessValidators[divergenceHeight],
+				ValidatorSet: witnessValidators[divergenceHeight],
-		},
+			},
-		CommonHeight: divergenceHeight,
+			CommonHeight: divergenceHeight,
+		}
+		assert.True(t, primary.HasEvidence(evAgainstWitness))
 	}
-	assert.True(t, primary.HasEvidence(evAgainstWitness))
 }
 
 // 1. Different nodes therefore a divergent header is produced.

light/errors.go

@@ -60,9 +60,7 @@ func (e ErrVerificationFailed) Unwrap() error {
 }
 
 func (e ErrVerificationFailed) Error() string {
-	return fmt.Sprintf(
+	return fmt.Sprintf("verify from #%d to #%d failed: %v", e.From, e.To, e.Reason)
-		"verify from #%d to #%d failed: %v",
-		e.From, e.To, e.Reason)
 }
 
 // ----------------------------- INTERNAL ERRORS ---------------------------------