From 406dd74220a6f356dcb8d827b61715c4b1fbb35d Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 9 Oct 2020 14:29:22 +0400 Subject: [PATCH] light: cross-check the very first header (#5429) Closes #5428 --- cmd/tendermint/commands/lite.go | 21 ++++++++++- light/client.go | 53 ++++++++++++++++++++++++++-- light/client_test.go | 2 +- light/detector_test.go | 62 ++++++++++++++++++++++++++------- 4 files changed, 122 insertions(+), 16 deletions(-) diff --git a/cmd/tendermint/commands/lite.go b/cmd/tendermint/commands/lite.go index 59d922f1c..50ce7c9fb 100644 --- a/cmd/tendermint/commands/lite.go +++ b/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()) diff --git a/light/client.go b/light/client.go index 49858c2bb..78b01f94a 100644 --- a/light/client.go +++ b/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) } diff --git a/light/client_test.go b/light/client_test.go index a41315dc3..13d550de7 100644 --- a/light/client_test.go +++ b/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)) diff --git a/light/detector_test.go b/light/detector_test.go index 4777d0c2d..dc0823476 100644 --- a/light/detector_test.go +++ b/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())) }