From 27ccf3b590928fc4c5b2a21165e79146691314f0 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Tue, 8 Feb 2022 22:09:10 +0100 Subject: [PATCH] Don't allow duplicates for light client providers (#7781) * Allow for zero witness providers * Verify provider duplicates, fix tests * Add duplicate provider ID to the error * Return error on attempt to remove last witness * Verify duplicates when restoring from store --- cmd/tendermint/commands/light.go | 12 +++- light/client.go | 46 ++++++++----- light/client_benchmark_test.go | 6 +- light/client_test.go | 111 ++++++++++++++++++++++--------- light/detector.go | 7 +- light/detector_test.go | 48 +++++++++---- light/example_test.go | 3 +- light/light_test.go | 23 ++----- light/setup.go | 29 -------- 9 files changed, 165 insertions(+), 120 deletions(-) diff --git a/cmd/tendermint/commands/light.go b/cmd/tendermint/commands/light.go index 0ee774835..8ea9860a5 100644 --- a/cmd/tendermint/commands/light.go +++ b/cmd/tendermint/commands/light.go @@ -83,9 +83,11 @@ will be verified before passing them back to the caller. Other than that, it will present the same interface as a full Tendermint node. Furthermore to the chainID, a fresh instance of a light client will -need a primary RPC address, a trusted hash and height and witness RPC addresses -(if not using sequential verification). To restart the node, thereafter -only the chainID is required. +need a primary RPC address and a trusted hash and height. It is also highly +recommended to provide additional witness RPC addresses, especially if +not using sequential verification. + +To restart the node, thereafter only the chainID is required. When /abci_query is called, the Merkle key path format is: @@ -127,6 +129,10 @@ for applications built w/ Cosmos SDK). } } + if len(witnessesAddrs) < 1 && !sequential { + logger.Info("In skipping verification mode it is highly recommended to provide at least one witness") + } + trustLevel, err := tmmath.ParseFraction(trustLevelStr) if err != nil { return fmt.Errorf("can't parse trust level: %w", err) diff --git a/light/client.go b/light/client.go index 2b84805b0..92f713b0d 100644 --- a/light/client.go +++ b/light/client.go @@ -142,14 +142,29 @@ type Client struct { logger log.Logger } +func validatePrimaryAndWitnesses(primary provider.Provider, witnesses []provider.Provider) error { + witnessMap := make(map[string]struct{}) + for _, w := range witnesses { + if w.ID() == primary.ID() { + return fmt.Errorf("primary (%s) cannot be also configured as witness", primary.ID()) + } + if _, duplicate := witnessMap[w.ID()]; duplicate { + return fmt.Errorf("witness list must not contain duplicates; duplicate found: %s", w.ID()) + } + witnessMap[w.ID()] = struct{}{} + } + return nil +} + // NewClient returns a new light client. It returns an error if it fails to -// obtain the light block from the primary or they are invalid (e.g. trust +// obtain the light block from the primary, or they are invalid (e.g. trust // hash does not match with the one from the headers). // // Witnesses are providers, which will be used for cross-checking the primary -// provider. At least one witness must be given when skipping verification is -// used (default). A witness can become a primary iff the current primary is -// unavailable. +// provider. At least one witness should be given when skipping verification is +// used (default). A verified header is compared with the headers at same height +// obtained from the specified witnesses. A witness can become a primary iff the +// current primary is unavailable. // // See all Option(s) for the additional configuration. func NewClient( @@ -174,16 +189,16 @@ func NewClient( ) } + // Check that the witness list does not include duplicates or the primary + if err := validatePrimaryAndWitnesses(primary, witnesses); err != nil { + return nil, err + } + // Validate trust options if err := trustOptions.ValidateBasic(); err != nil { return nil, fmt.Errorf("invalid TrustOptions: %w", err) } - // Validate the number of witnesses. - if len(witnesses) < 1 { - return nil, ErrNoWitnesses - } - c := &Client{ chainID: chainID, trustingPeriod: trustOptions.Period, @@ -226,6 +241,11 @@ func NewClientFromTrustedStore( trustedStore store.Store, options ...Option) (*Client, error) { + // Check that the witness list does not include duplicates or the primary + if err := validatePrimaryAndWitnesses(primary, witnesses); err != nil { + return nil, err + } + c := &Client{ chainID: chainID, trustingPeriod: trustingPeriod, @@ -244,11 +264,6 @@ func NewClientFromTrustedStore( o(c) } - // Validate the number of witnesses. - if len(c.witnesses) < 1 { - return nil, ErrNoWitnesses - } - // Validate trust level. if err := ValidateTrustLevel(c.trustLevel); err != nil { return nil, err @@ -960,7 +975,6 @@ func (c *Client) getLightBlock(ctx context.Context, p provider.Provider, height // NOTE: requires a providerMutex lock func (c *Client) removeWitnesses(indexes []int) error { - // check that we will still have witnesses remaining if len(c.witnesses) <= len(indexes) { return ErrNoWitnesses } @@ -1082,7 +1096,7 @@ func (c *Client) compareFirstHeaderWithWitnesses(ctx context.Context, h *types.S defer c.providerMutex.Unlock() if len(c.witnesses) < 1 { - return ErrNoWitnesses + return nil } errc := make(chan error, len(c.witnesses)) diff --git a/light/client_benchmark_test.go b/light/client_benchmark_test.go index ca0e402a0..4a54e2b54 100644 --- a/light/client_benchmark_test.go +++ b/light/client_benchmark_test.go @@ -84,7 +84,7 @@ func BenchmarkSequence(b *testing.B) { Hash: genesisBlock.Hash(), }, benchmarkFullNode, - []provider.Provider{benchmarkFullNode}, + nil, dbs.New(dbm.NewMemDB()), light.Logger(logger), light.SequentialVerification(), @@ -121,7 +121,7 @@ func BenchmarkBisection(b *testing.B) { Hash: genesisBlock.Hash(), }, benchmarkFullNode, - []provider.Provider{benchmarkFullNode}, + nil, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -157,7 +157,7 @@ func BenchmarkBackwards(b *testing.B) { Hash: trustedBlock.Hash(), }, benchmarkFullNode, - []provider.Provider{benchmarkFullNode}, + nil, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) diff --git a/light/client_test.go b/light/client_test.go index bc585f7b8..d2f5a137c 100644 --- a/light/client_test.go +++ b/light/client_test.go @@ -233,7 +233,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockNode, - []provider.Provider{mockNode}, + nil, dbs.New(dbm.NewMemDB()), light.SequentialVerification(), light.Logger(logger), @@ -360,7 +360,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockNode, - []provider.Provider{mockNode}, + nil, dbs.New(dbm.NewMemDB()), light.SkippingVerification(light.DefaultTrustLevel), light.Logger(logger), @@ -416,7 +416,7 @@ func TestClient(t *testing.T) { Hash: trustedLightBlock.Hash(), }, mockNode, - []provider.Provider{mockNode}, + nil, dbs.New(dbm.NewMemDB()), light.SkippingVerification(light.DefaultTrustLevel), ) @@ -445,7 +445,7 @@ func TestClient(t *testing.T) { Hash: h1.Hash(), }, mockFullNode, - []provider.Provider{mockFullNode}, + nil, dbs.New(dbm.NewMemDB()), light.SkippingVerification(light.DefaultTrustLevel), ) @@ -475,7 +475,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockFullNode, - []provider.Provider{mockFullNode}, + nil, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -515,7 +515,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockNode, - []provider.Provider{mockNode}, + nil, trustedStore, light.Logger(logger), ) @@ -554,7 +554,7 @@ func TestClient(t *testing.T) { Hash: header1.Hash(), }, mockNode, - []provider.Provider{mockNode}, + nil, trustedStore, light.Logger(logger), ) @@ -575,9 +575,14 @@ func TestClient(t *testing.T) { defer cancel() mockFullNode := &provider_mocks.Provider{} + mockFullNode.On("ID").Return("mockFullNode") mockFullNode.On("LightBlock", mock.Anything, int64(0)).Return(l3, nil) mockFullNode.On("LightBlock", mock.Anything, int64(1)).Return(l1, nil) - mockFullNode.On("LightBlock", mock.Anything, int64(3)).Return(l3, nil) + + mockWitnessNode := &provider_mocks.Provider{} + mockWitnessNode.On("ID").Return("mockWitnessNode") + mockWitnessNode.On("LightBlock", mock.Anything, int64(1)).Return(l1, nil) + mockWitnessNode.On("LightBlock", mock.Anything, int64(3)).Return(l3, nil) logger := log.NewTestingLogger(t) @@ -586,7 +591,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockFullNode, - []provider.Provider{mockFullNode}, + []provider.Provider{mockWitnessNode}, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -600,6 +605,7 @@ func TestClient(t *testing.T) { assert.NoError(t, l.ValidateBasic(chainID)) } mockFullNode.AssertExpectations(t) + mockWitnessNode.AssertExpectations(t) }) t.Run("Concurrency", func(t *testing.T) { @@ -615,7 +621,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockFullNode, - []provider.Provider{mockFullNode}, + nil, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -665,7 +671,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockFullNode, - []provider.Provider{mockFullNode}, + nil, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -679,9 +685,8 @@ func TestClient(t *testing.T) { close(closeCh) }() - // NOTE: the light client doesn't check uniqueness of providers c.AddProvider(mockFullNode) - require.Len(t, c.Witnesses(), 2) + require.Len(t, c.Witnesses(), 1) select { case <-closeCh: case <-time.After(5 * time.Second): @@ -695,9 +700,10 @@ func TestClient(t *testing.T) { mockFullNode := &provider_mocks.Provider{} mockFullNode.On("LightBlock", mock.Anything, mock.Anything).Return(l1, nil) - + mockFullNode.On("ID").Return("mockFullNode") mockDeadNode := &provider_mocks.Provider{} mockDeadNode.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrNoResponse) + mockDeadNode.On("ID").Return("mockDeadNode") logger := log.NewTestingLogger(t) @@ -706,7 +712,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockDeadNode, - []provider.Provider{mockDeadNode, mockFullNode}, + []provider.Provider{mockFullNode}, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -720,7 +726,7 @@ func TestClient(t *testing.T) { // we should still have the dead node as a witness because it // hasn't repeatedly been unresponsive yet - assert.Equal(t, 2, len(c.Witnesses())) + assert.Equal(t, 1, len(c.Witnesses())) mockDeadNode.AssertExpectations(t) mockFullNode.AssertExpectations(t) }) @@ -730,17 +736,24 @@ func TestClient(t *testing.T) { mockFullNode := &provider_mocks.Provider{} mockFullNode.On("LightBlock", mock.Anything, mock.Anything).Return(l1, nil) + mockFullNode.On("ID").Return("mockFullNode") logger := log.NewTestingLogger(t) - mockDeadNode := &provider_mocks.Provider{} - mockDeadNode.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) + mockDeadNode1 := &provider_mocks.Provider{} + mockDeadNode1.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) + mockDeadNode1.On("ID").Return("mockDeadNode1") + + mockDeadNode2 := &provider_mocks.Provider{} + mockDeadNode2.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) + mockDeadNode2.On("ID").Return("mockDeadNode2") + c, err := light.NewClient( ctx, chainID, trustOptions, - mockDeadNode, - []provider.Provider{mockDeadNode, mockFullNode}, + mockDeadNode1, + []provider.Provider{mockFullNode, mockDeadNode2}, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -751,7 +764,7 @@ func TestClient(t *testing.T) { // we should still have the dead node as a witness because it // hasn't repeatedly been unresponsive yet assert.Equal(t, 2, len(c.Witnesses())) - mockDeadNode.AssertExpectations(t) + mockDeadNode1.AssertExpectations(t) mockFullNode.AssertExpectations(t) }) t.Run("BackwardsVerification", func(t *testing.T) { @@ -777,7 +790,7 @@ func TestClient(t *testing.T) { Hash: trustHeader.Hash(), }, mockLargeFullNode, - []provider.Provider{mockLargeFullNode}, + nil, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -836,7 +849,7 @@ func TestClient(t *testing.T) { Hash: h3.Hash(), }, mockNode, - []provider.Provider{mockNode}, + nil, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -852,13 +865,15 @@ func TestClient(t *testing.T) { db := dbs.New(dbm.NewMemDB()) err := db.SaveLightBlock(l1) require.NoError(t, err) - mockNode := &provider_mocks.Provider{} - + mockPrimary := &provider_mocks.Provider{} + mockPrimary.On("ID").Return("mockPrimary") + mockWitness := &provider_mocks.Provider{} + mockWitness.On("ID").Return("mockWitness") c, err := light.NewClientFromTrustedStore( chainID, trustPeriod, - mockNode, - []provider.Provider{mockNode}, + mockPrimary, + []provider.Provider{mockWitness}, db, ) require.NoError(t, err) @@ -867,7 +882,8 @@ func TestClient(t *testing.T) { h, err := c.TrustedLightBlock(1) assert.NoError(t, err) assert.EqualValues(t, l1.Height, h.Height) - mockNode.AssertExpectations(t) + mockPrimary.AssertExpectations(t) + mockWitness.AssertExpectations(t) }) t.Run("RemovesWitnessIfItSendsUsIncorrectHeader", func(t *testing.T) { logger := log.NewTestingLogger(t) @@ -885,6 +901,7 @@ func TestClient(t *testing.T) { } mockBadNode1 := mockNodeFromHeadersAndVals(headers1, vals1) mockBadNode1.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) + mockBadNode1.On("ID").Return("mockBadNode1") // header is empty headers2 := map[int64]*types.SignedHeader{ @@ -897,8 +914,10 @@ func TestClient(t *testing.T) { } mockBadNode2 := mockNodeFromHeadersAndVals(headers2, vals2) mockBadNode2.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) + mockBadNode2.On("ID").Return("mockBadNode2") mockFullNode := mockNodeFromHeadersAndVals(headerSet, valSet) + mockFullNode.On("ID").Return("mockFullNode") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -956,6 +975,8 @@ func TestClient(t *testing.T) { 1: vals, 2: differentVals, }) + mockBadValSetNode.On("ID").Return("mockBadValSetNode") + mockFullNode := mockNodeFromHeadersAndVals( map[int64]*types.SignedHeader{ 1: h1, @@ -965,13 +986,25 @@ func TestClient(t *testing.T) { 1: vals, 2: vals, }) + mockFullNode.On("ID").Return("mockFullNode") + + mockGoodWitness := mockNodeFromHeadersAndVals( + map[int64]*types.SignedHeader{ + 1: h1, + 2: h2, + }, + map[int64]*types.ValidatorSet{ + 1: vals, + 2: vals, + }) + mockGoodWitness.On("ID").Return("mockGoodWitness") c, err := light.NewClient( ctx, chainID, trustOptions, mockFullNode, - []provider.Provider{mockBadValSetNode, mockFullNode}, + []provider.Provider{mockBadValSetNode, mockGoodWitness}, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -988,15 +1021,26 @@ func TestClient(t *testing.T) { mockFullNode := mockNodeFromHeadersAndVals( map[int64]*types.SignedHeader{ 1: h1, - 3: h3, 0: h3, }, map[int64]*types.ValidatorSet{ 1: vals, - 3: vals, 0: vals, }) + mockFullNode.On("ID").Return("mockFullNode") + + mockGoodWitness := mockNodeFromHeadersAndVals( + map[int64]*types.SignedHeader{ + 1: h1, + 3: h3, + }, + map[int64]*types.ValidatorSet{ + 1: vals, + 3: vals, + }) + mockGoodWitness.On("ID").Return("mockGoodWitness") + ctx, cancel := context.WithCancel(context.Background()) defer cancel() logger := log.NewTestingLogger(t) @@ -1006,7 +1050,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockFullNode, - []provider.Provider{mockFullNode}, + []provider.Provider{mockGoodWitness}, dbs.New(dbm.NewMemDB()), light.Logger(logger), light.PruningSize(1), @@ -1022,6 +1066,7 @@ func TestClient(t *testing.T) { _, err = c.TrustedLightBlock(1) assert.Error(t, err) mockFullNode.AssertExpectations(t) + mockGoodWitness.AssertExpectations(t) }) t.Run("EnsureValidHeadersAndValSets", func(t *testing.T) { emptyValSet := &types.ValidatorSet{ @@ -1098,7 +1143,7 @@ func TestClient(t *testing.T) { chainID, trustOptions, mockBadNode, - []provider.Provider{mockBadNode, mockBadNode}, + nil, dbs.New(dbm.NewMemDB()), ) require.NoError(t, err) diff --git a/light/detector.go b/light/detector.go index 0e5303acb..c0a5bd2c1 100644 --- a/light/detector.go +++ b/light/detector.go @@ -26,6 +26,9 @@ import ( // If there are no conflictinge headers, the light client deems the verified target header // trusted and saves it to the trusted store. func (c *Client) detectDivergence(ctx context.Context, primaryTrace []*types.LightBlock, now time.Time) error { + if len(c.witnesses) < 1 { + return nil + } if primaryTrace == nil || len(primaryTrace) < 2 { return errors.New("nil or single block primary trace") } @@ -40,10 +43,6 @@ func (c *Client) detectDivergence(ctx context.Context, primaryTrace []*types.Lig c.providerMutex.Lock() defer c.providerMutex.Unlock() - if len(c.witnesses) == 0 { - return ErrNoWitnesses - } - // launch one goroutine per witness to retrieve the light block of the target height // and compare it with the header from the primary errc := make(chan error, len(c.witnesses)) diff --git a/light/detector_test.go b/light/detector_test.go index 84b6f210c..2e5304822 100644 --- a/light/detector_test.go +++ b/light/detector_test.go @@ -56,8 +56,7 @@ func TestLightClientAttackEvidence_Lunatic(t *testing.T) { delete(primaryHeaders, 2) mockWitness := mockNodeFromHeadersAndVals(witnessHeaders, witnessValidators) - mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryValidators) - + mockWitness.On("ID").Return("mockWitness") mockWitness.On("ReportEvidence", mock.Anything, mock.MatchedBy(func(evidence types.Evidence) bool { evAgainstPrimary := &types.LightClientAttackEvidence{ // after the divergence height the valset doesn't change so we expect the evidence to be for the latest height @@ -70,6 +69,8 @@ func TestLightClientAttackEvidence_Lunatic(t *testing.T) { return bytes.Equal(evidence.Hash(), evAgainstPrimary.Hash()) })).Return(nil) + mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryValidators) + mockPrimary.On("ID").Return("mockPrimary") mockPrimary.On("ReportEvidence", mock.Anything, mock.MatchedBy(func(evidence types.Evidence) bool { evAgainstWitness := &types.LightClientAttackEvidence{ // when forming evidence against witness we learn that the canonical chain continued to change validator sets @@ -173,10 +174,13 @@ func TestLightClientAttackEvidence_Equivocation(t *testing.T) { delete(witnessHeaders, height) } mockWitness := mockNodeFromHeadersAndVals(witnessHeaders, witnessValidators) + mockWitness.On("ID").Return("mockWitness") + for _, height := range testCase.unusedPrimaryBlockHeights { delete(primaryHeaders, height) } mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryValidators) + mockPrimary.On("ID").Return("mockPrimary") // Check evidence was sent to both full nodes. // Common height should be set to the height of the divergent header in the instance @@ -275,16 +279,13 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { 0, len(forgedKeys), ) mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryValidators) + mockPrimary.On("ID").Return("mockPrimary") lastBlock, _ := mockPrimary.LightBlock(ctx, forgedHeight) mockPrimary.On("LightBlock", mock.Anything, int64(0)).Return(lastBlock, nil) mockPrimary.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) - /* - for _, unusedHeader := range []int64{3, 5, 6, 8} { - delete(witnessHeaders, unusedHeader) - } - */ mockWitness := mockNodeFromHeadersAndVals(witnessHeaders, witnessValidators) + mockWitness.On("ID").Return("mockWitness") lastBlock, _ = mockWitness.LightBlock(ctx, latestHeight) mockWitness.On("LightBlock", mock.Anything, int64(0)).Return(lastBlock, nil).Once() mockWitness.On("LightBlock", mock.Anything, int64(12)).Return(nil, provider.ErrHeightTooHigh) @@ -303,7 +304,11 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { // In order to perform the attack, the primary needs at least one accomplice as a witness to also // send the forged block - accomplice := mockPrimary + accomplice := mockNodeFromHeadersAndVals(primaryHeaders, primaryValidators) + accomplice.On("ID").Return("accomplice") + lastBlock, _ = accomplice.LightBlock(ctx, forgedHeight) + accomplice.On("LightBlock", mock.Anything, int64(0)).Return(lastBlock, nil) + accomplice.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) c, err := light.NewClient( ctx, @@ -362,6 +367,7 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { // Lastly we test the unfortunate case where the light clients supporting witness doesn't update // in enough time mockLaggingWitness := mockNodeFromHeadersAndVals(witnessHeaders, witnessValidators) + mockLaggingWitness.On("ID").Return("mockLaggingWitness") mockLaggingWitness.On("LightBlock", mock.Anything, int64(12)).Return(nil, provider.ErrHeightTooHigh) lastBlock, _ = mockLaggingWitness.LightBlock(ctx, latestHeight) mockLaggingWitness.On("LightBlock", mock.Anything, int64(0)).Return(lastBlock, nil) @@ -397,10 +403,13 @@ func TestClientDivergentTraces1(t *testing.T) { headers, vals, _ := genLightBlocksWithKeys(t, chainID, 1, 5, 2, bTime) mockPrimary := mockNodeFromHeadersAndVals(headers, vals) + mockPrimary.On("ID").Return("mockPrimary") + firstBlock, err := mockPrimary.LightBlock(ctx, 1) require.NoError(t, err) headers, vals, _ = genLightBlocksWithKeys(t, chainID, 1, 5, 2, bTime) mockWitness := mockNodeFromHeadersAndVals(headers, vals) + mockWitness.On("ID").Return("mockWitness") logger := log.NewTestingLogger(t) @@ -432,8 +441,19 @@ func TestClientDivergentTraces2(t *testing.T) { headers, vals, _ := genLightBlocksWithKeys(t, chainID, 2, 5, 2, bTime) mockPrimaryNode := mockNodeFromHeadersAndVals(headers, vals) - mockDeadNode := &provider_mocks.Provider{} - mockDeadNode.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrNoResponse) + mockPrimaryNode.On("ID").Return("mockPrimaryNode") + + mockGoodWitness := mockNodeFromHeadersAndVals(headers, vals) + mockGoodWitness.On("ID").Return("mockGoodWitness") + + mockDeadNode1 := &provider_mocks.Provider{} + mockDeadNode1.On("ID").Return("mockDeadNode1") + mockDeadNode1.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrNoResponse) + + mockDeadNode2 := &provider_mocks.Provider{} + mockDeadNode2.On("ID").Return("mockDeadNode2") + mockDeadNode2.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrNoResponse) + firstBlock, err := mockPrimaryNode.LightBlock(ctx, 1) require.NoError(t, err) c, err := light.NewClient( @@ -445,7 +465,7 @@ func TestClientDivergentTraces2(t *testing.T) { Period: 4 * time.Hour, }, mockPrimaryNode, - []provider.Provider{mockDeadNode, mockDeadNode, mockPrimaryNode}, + []provider.Provider{mockDeadNode1, mockDeadNode2, mockGoodWitness}, dbs.New(dbm.NewMemDB()), light.Logger(logger), ) @@ -454,7 +474,7 @@ func TestClientDivergentTraces2(t *testing.T) { _, err = c.VerifyLightBlockAtHeight(ctx, 2, bTime.Add(1*time.Hour)) assert.NoError(t, err) assert.Equal(t, 3, len(c.Witnesses())) - mockDeadNode.AssertExpectations(t) + mockDeadNode1.AssertExpectations(t) mockPrimaryNode.AssertExpectations(t) } @@ -467,6 +487,7 @@ func TestClientDivergentTraces3(t *testing.T) { // primaryHeaders, primaryVals, _ := genLightBlocksWithKeys(t, chainID, 2, 5, 2, bTime) mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryVals) + mockPrimary.On("ID").Return("mockPrimary") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -478,6 +499,7 @@ func TestClientDivergentTraces3(t *testing.T) { mockHeaders[1] = primaryHeaders[1] mockVals[1] = primaryVals[1] mockWitness := mockNodeFromHeadersAndVals(mockHeaders, mockVals) + mockWitness.On("ID").Return("mockWitness") c, err := light.NewClient( ctx, @@ -510,6 +532,7 @@ func TestClientDivergentTraces4(t *testing.T) { // primaryHeaders, primaryVals, _ := genLightBlocksWithKeys(t, chainID, 2, 5, 2, bTime) mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryVals) + mockPrimary.On("ID").Return("mockPrimary") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -521,6 +544,7 @@ func TestClientDivergentTraces4(t *testing.T) { primaryHeaders[2] = witnessHeaders[2] primaryVals[2] = witnessVals[2] mockWitness := mockNodeFromHeadersAndVals(primaryHeaders, primaryVals) + mockWitness.On("ID").Return("mockWitness") c, err := light.NewClient( ctx, diff --git a/light/example_test.go b/light/example_test.go index d22525b30..7362ca06b 100644 --- a/light/example_test.go +++ b/light/example_test.go @@ -11,7 +11,6 @@ import ( "github.com/tendermint/tendermint/abci/example/kvstore" "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/light" - "github.com/tendermint/tendermint/light/provider" httpp "github.com/tendermint/tendermint/light/provider/http" dbs "github.com/tendermint/tendermint/light/store/db" rpctest "github.com/tendermint/tendermint/rpc/test" @@ -74,7 +73,7 @@ func ExampleClient() { Hash: block.Hash(), }, primary, - []provider.Provider{primary}, // NOTE: primary should not be used here + nil, dbs.New(db), light.Logger(logger), ) diff --git a/light/light_test.go b/light/light_test.go index 00d0741ce..7e8977de9 100644 --- a/light/light_test.go +++ b/light/light_test.go @@ -67,7 +67,7 @@ func TestClientIntegration_Update(t *testing.T) { Hash: block.Hash(), }, primary, - []provider.Provider{primary}, // NOTE: primary should not be used here + nil, dbs.New(db), light.Logger(logger), ) @@ -127,7 +127,7 @@ func TestClientIntegration_VerifyLightBlockAtHeight(t *testing.T) { Hash: block.Hash(), }, primary, - []provider.Provider{primary}, // NOTE: primary should not be used here + nil, dbs.New(db), light.Logger(logger), ) @@ -197,10 +197,9 @@ func TestClientStatusRPC(t *testing.T) { db, err := dbm.NewGoLevelDB("light-client-db", dbDir) require.NoError(t, err) - // In order to not create a full testnet to verify whether we get the correct IPs - // if we have more than one witness, we add the primary multiple times - // TODO This should be buggy behavior, we should not be allowed to add the same nodes as witnesses - witnesses := []provider.Provider{primary, primary, primary} + // In order to not create a full testnet we create the light client with no witnesses + // and only verify the primary IP address. + witnesses := []provider.Provider{} c, err := light.NewClient(ctx, chainID, @@ -223,9 +222,6 @@ func TestClientStatusRPC(t *testing.T) { // Verify primary IP require.True(t, lightStatus.PrimaryID == primary.ID()) - // Verify IPs of witnesses - require.ElementsMatch(t, mapProviderArrayToIP(witnesses), lightStatus.WitnessesID) - // Verify that number of peers is equal to number of witnesses (+ 1 if the primary is not a witness) require.Equal(t, len(witnesses)+1*primaryNotInWitnessList(witnesses, primary), lightStatus.NumPeers) @@ -238,15 +234,6 @@ func TestClientStatusRPC(t *testing.T) { } -// Extract the IP address of all the providers within an array -func mapProviderArrayToIP(el []provider.Provider) []string { - ips := make([]string, len(el)) - for i, v := range el { - ips[i] = v.ID() - } - return ips -} - // If the primary is not in the witness list, we will return 1 // Otherwise, return 0 func primaryNotInWitnessList(witnesses []provider.Provider, primary provider.Provider) int { diff --git a/light/setup.go b/light/setup.go index af72301b0..40b181230 100644 --- a/light/setup.go +++ b/light/setup.go @@ -2,7 +2,6 @@ package light import ( "context" - "time" "github.com/tendermint/tendermint/light/provider" "github.com/tendermint/tendermint/light/provider/http" @@ -39,34 +38,6 @@ func NewHTTPClient( options...) } -// NewHTTPClientFromTrustedStore initiates an instance of a light client using -// HTTP addresses for both the primary provider and witnesses and uses a -// trusted store as the root of trust. -// -// See all Option(s) for the additional configuration. -// See NewClientFromTrustedStore. -func NewHTTPClientFromTrustedStore( - chainID string, - trustingPeriod time.Duration, - primaryAddress string, - witnessesAddresses []string, - trustedStore store.Store, - options ...Option) (*Client, error) { - - providers, err := providersFromAddresses(append(witnessesAddresses, primaryAddress), chainID) - if err != nil { - return nil, err - } - - return NewClientFromTrustedStore( - chainID, - trustingPeriod, - providers[len(providers)-1], - providers[:len(providers)-1], - trustedStore, - options...) -} - func providersFromAddresses(addrs []string, chainID string) ([]provider.Provider, error) { providers := make([]provider.Provider, len(addrs)) for idx, address := range addrs {