light: cross-check the very first header (#5429)

Closes #5428

cmd/tendermint/commands/lite.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"bufio"
 	"context"
 	"errors"
 	"fmt"
@@ -140,7 +141,25 @@ func runProxy(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("can't parse trust level: %w", err)
 	}
 
-	options := []light.Option{light.Logger(logger)}
+	options := []light.Option{
+		light.Logger(logger),
+		light.ConfirmationFunction(func(action string) bool {
+			fmt.Println(action)
+			scanner := bufio.NewScanner(os.Stdin)
+			for {
+				scanner.Scan()
+				response := scanner.Text()
+				switch response {
+				case "y", "Y":
+					return true
+				case "n", "N":
+					return false
+				default:
+					fmt.Println("please input 'Y' or 'n' and press ENTER")
+				}
+			}
+		}),
+	}
 
 	if sequential {
 		options = append(options, light.SequentialVerification())

light/client.go

@@ -356,13 +356,20 @@ func (c *Client) initializeWithTrustOptions(ctx context.Context, options TrustOp
 		return fmt.Errorf("expected header's hash %X, but got %X", options.Hash, l.Hash())
 	}
 
-	// Ensure that +2/3 of validators signed correctly.
+	// 2) Ensure that +2/3 of validators signed correctly.
 	err = l.ValidatorSet.VerifyCommitLight(c.chainID, l.Commit.BlockID, l.Height, l.Commit)
 	if err != nil {
 		return fmt.Errorf("invalid commit: %w", err)
 	}
 
-	// 3) Persist both of them and continue.
+	// 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 {
+			return err
+		}
+	}
+
+	// 4) Persist both of them and continue.
 	return c.updateTrustedLightBlock(l)
 }
 
@@ -982,6 +989,48 @@ func (c *Client) lightBlockFromPrimary(ctx context.Context, height int64) (*type
 	return l, err
 }
 
+// compareFirstHeaderWithWitnesses compares h with all witnesses. If any
+// witness reports a different header than h, the function returns an error.
+func (c *Client) compareFirstHeaderWithWitnesses(ctx context.Context, h *types.SignedHeader) error {
+	compareCtx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
+	errc := make(chan error, len(c.witnesses))
+	for i, witness := range c.witnesses {
+		go c.compareNewHeaderWithWitness(compareCtx, errc, h, witness, i)
+	}
+
+	witnessesToRemove := make([]int, 0, len(c.witnesses))
+
+	// handle errors from the header comparisons as they come in
+	for i := 0; i < cap(errc); i++ {
+		err := <-errc
+
+		switch e := err.(type) {
+		case nil:
+			continue
+		case errConflictingHeaders:
+			c.logger.Error(fmt.Sprintf(`Witness #%d has a different header. Please check primary is correct
+and remove witness. Otherwise, use the different primary`, e.WitnessIndex), "witness", c.witnesses[e.WitnessIndex])
+			return err
+		case errBadWitness:
+			// If witness sent us an invalid header, then remove it. If it didn't
+			// respond or couldn't find the block, then we ignore it and move on to
+			// the next witness.
+			if _, ok := e.Reason.(provider.ErrBadLightBlock); ok {
+				c.logger.Info("Witness sent us invalid header / vals -> removing it", "witness", c.witnesses[e.WitnessIndex])
+				witnessesToRemove = append(witnessesToRemove, e.WitnessIndex)
+			}
+		}
+	}
+
+	for _, idx := range witnessesToRemove {
+		c.removeWitness(idx)
+	}
+
+	return nil
+}
+
 func hash2str(hash []byte) string {
 	return fmt.Sprintf("%X", hash)
 }

light/client_test.go

@@ -488,7 +488,7 @@ func TestClientRestoresTrustedHeaderAfterStartup1(t *testing.T) {
 		err := trustedStore.SaveLightBlock(l1)
 		require.NoError(t, err)
 
-		// header1 != header
+		// header1 != h1
 		header1 := keys.GenSignedHeader(chainID, 1, bTime.Add(1*time.Hour), nil, vals, vals,
 			hash("app_hash"), hash("cons_hash"), hash("results_hash"), 0, len(keys))
 

light/detector_test.go

@@ -162,13 +162,16 @@ func TestLightClientAttackEvidence_Equivocation(t *testing.T) {
 	assert.True(t, primary.HasEvidence(evAgainstWitness))
 }
 
-func TestClientDivergentTraces(t *testing.T) {
+// 1. Different nodes therefore a divergent header is produced.
+// => light client returns an error upon creation because primary and witness
+// have a different view.
+func TestClientDivergentTraces1(t *testing.T) {
 	primary := mockp.New(genMockNode(chainID, 10, 5, 2, bTime))
 	firstBlock, err := primary.LightBlock(ctx, 1)
 	require.NoError(t, err)
 	witness := mockp.New(genMockNode(chainID, 10, 5, 2, bTime))
 
-	c, err := light.NewClient(
+	_, err = light.NewClient(
 		ctx,
 		chainID,
 		light.TrustOptions{
@@ -182,18 +185,52 @@ func TestClientDivergentTraces(t *testing.T) {
 		light.Logger(log.TestingLogger()),
 		light.MaxRetryAttempts(1),
 	)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "does not match primary")
+}
+
+// 2. Two out of three nodes don't respond but the third has a header that matches
+// => verification should be successful and all the witnesses should remain
+func TestClientDivergentTraces2(t *testing.T) {
+	primary := mockp.New(genMockNode(chainID, 10, 5, 2, bTime))
+	firstBlock, err := primary.LightBlock(ctx, 1)
+	require.NoError(t, err)
+	c, err := light.NewClient(
+		ctx,
+		chainID,
+		light.TrustOptions{
+			Height: 1,
+			Hash:   firstBlock.Hash(),
+			Period: 4 * time.Hour,
+		},
+		primary,
+		[]provider.Provider{deadNode, deadNode, primary},
+		dbs.New(dbm.NewMemDB(), chainID),
+		light.Logger(log.TestingLogger()),
+		light.MaxRetryAttempts(1),
+	)
 	require.NoError(t, err)
 
-	// 1. Different nodes therefore a divergent header is produced but the
-	// light client can't verify it because it has a different trusted header.
 	_, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour))
-	assert.Error(t, err)
-	assert.Equal(t, 0, len(c.Witnesses()))
+	assert.NoError(t, err)
+	assert.Equal(t, 3, len(c.Witnesses()))
+}
+
+// 3. witness has the same first header, but different second header
+// => creation should succeed, but the verification should fail
+func TestClientDivergentTraces3(t *testing.T) {
+	_, primaryHeaders, primaryVals := genMockNode(chainID, 10, 5, 2, bTime)
+	primary := mockp.New(chainID, primaryHeaders, primaryVals)
+
+	firstBlock, err := primary.LightBlock(ctx, 1)
+	require.NoError(t, err)
 
-	// 2. Two out of three nodes don't respond but the third has a header that matches
-	// verification should be successful and all the witnesses should remain
+	_, mockHeaders, mockVals := genMockNode(chainID, 10, 5, 2, bTime)
+	mockHeaders[1] = primaryHeaders[1]
+	mockVals[1] = primaryVals[1]
+	witness := mockp.New(chainID, mockHeaders, mockVals)
 
-	c, err = light.NewClient(
+	c, err := light.NewClient(
 		ctx,
 		chainID,
 		light.TrustOptions{
@@ -202,13 +239,14 @@ func TestClientDivergentTraces(t *testing.T) {
 			Period: 4 * time.Hour,
 		},
 		primary,
-		[]provider.Provider{deadNode, deadNode, primary},
+		[]provider.Provider{witness},
 		dbs.New(dbm.NewMemDB(), chainID),
 		light.Logger(log.TestingLogger()),
 		light.MaxRetryAttempts(1),
 	)
 	require.NoError(t, err)
+
 	_, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour))
-	assert.NoError(t, err)
-	assert.Equal(t, 3, len(c.Witnesses()))
+	assert.Error(t, err)
+	assert.Equal(t, 0, len(c.Witnesses()))
 }