From 9e41414a53c6ddde0ecc2b37fa9c5231642a9d44 Mon Sep 17 00:00:00 2001 From: William Banfield <4561443+williambanfield@users.noreply.github.com> Date: Wed, 28 Jul 2021 12:12:11 -0400 Subject: [PATCH 1/6] light: replace homegrown mock with mockery (#6735) This pull request removes the homegrown mocks in `light/provider/mock` in favor of mockery mocks. Adds a simple benchmark only mock to avoid the overhead of `reflection` that `mockery` incurs. part of #5274 --- light/client_benchmark_test.go | 55 +++- light/client_test.go | 484 ++++++++++++++++++------------- light/detector_test.go | 386 ++++++++++++++---------- light/helpers_test.go | 37 ++- light/provider/mock/deadmock.go | 30 -- light/provider/mock/mock.go | 125 -------- light/provider/mocks/provider.go | 53 ++++ 7 files changed, 630 insertions(+), 540 deletions(-) delete mode 100644 light/provider/mock/deadmock.go delete mode 100644 light/provider/mock/mock.go create mode 100644 light/provider/mocks/provider.go diff --git a/light/client_benchmark_test.go b/light/client_benchmark_test.go index 72930928d..04ea6d1fc 100644 --- a/light/client_benchmark_test.go +++ b/light/client_benchmark_test.go @@ -10,8 +10,8 @@ import ( "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/light" "github.com/tendermint/tendermint/light/provider" - mockp "github.com/tendermint/tendermint/light/provider/mock" dbs "github.com/tendermint/tendermint/light/store/db" + "github.com/tendermint/tendermint/types" ) // NOTE: block is produced every minute. Make sure the verification time @@ -21,12 +21,50 @@ import ( // or -benchtime 100x. // // Remember that none of these benchmarks account for network latency. -var ( - benchmarkFullNode = mockp.New(genMockNode(chainID, 1000, 100, 1, bTime)) - genesisBlock, _ = benchmarkFullNode.LightBlock(context.Background(), 1) -) +var () + +type providerBenchmarkImpl struct { + currentHeight int64 + blocks map[int64]*types.LightBlock +} + +func newProviderBenchmarkImpl(headers map[int64]*types.SignedHeader, + vals map[int64]*types.ValidatorSet) provider.Provider { + impl := providerBenchmarkImpl{ + blocks: make(map[int64]*types.LightBlock, len(headers)), + } + for height, header := range headers { + if height > impl.currentHeight { + impl.currentHeight = height + } + impl.blocks[height] = &types.LightBlock{ + SignedHeader: header, + ValidatorSet: vals[height], + } + } + return &impl +} + +func (impl *providerBenchmarkImpl) LightBlock(ctx context.Context, height int64) (*types.LightBlock, error) { + if height == 0 { + return impl.blocks[impl.currentHeight], nil + } + lb, ok := impl.blocks[height] + if !ok { + return nil, provider.ErrLightBlockNotFound + } + return lb, nil +} + +func (impl *providerBenchmarkImpl) ReportEvidence(_ context.Context, _ types.Evidence) error { + panic("not implemented") +} func BenchmarkSequence(b *testing.B) { + headers, vals, _ := genLightBlocksWithKeys(chainID, 1000, 100, 1, bTime) + benchmarkFullNode := newProviderBenchmarkImpl(headers, vals) + genesisBlock, _ := benchmarkFullNode.LightBlock(context.Background(), 1) + c, err := light.NewClient( context.Background(), chainID, @@ -55,6 +93,10 @@ func BenchmarkSequence(b *testing.B) { } func BenchmarkBisection(b *testing.B) { + headers, vals, _ := genLightBlocksWithKeys(chainID, 1000, 100, 1, bTime) + benchmarkFullNode := newProviderBenchmarkImpl(headers, vals) + genesisBlock, _ := benchmarkFullNode.LightBlock(context.Background(), 1) + c, err := light.NewClient( context.Background(), chainID, @@ -82,7 +124,10 @@ func BenchmarkBisection(b *testing.B) { } func BenchmarkBackwards(b *testing.B) { + headers, vals, _ := genLightBlocksWithKeys(chainID, 1000, 100, 1, bTime) + benchmarkFullNode := newProviderBenchmarkImpl(headers, vals) trustedBlock, _ := benchmarkFullNode.LightBlock(context.Background(), 0) + c, err := light.NewClient( context.Background(), chainID, diff --git a/light/client_test.go b/light/client_test.go index 67e0525b8..e8a478a53 100644 --- a/light/client_test.go +++ b/light/client_test.go @@ -3,11 +3,13 @@ package light_test import ( "context" "errors" + "fmt" "sync" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" dbm "github.com/tendermint/tm-db" @@ -16,7 +18,7 @@ import ( "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/light" "github.com/tendermint/tendermint/light/provider" - mockp "github.com/tendermint/tendermint/light/provider/mock" + provider_mocks "github.com/tendermint/tendermint/light/provider/mocks" dbs "github.com/tendermint/tendermint/light/store/db" "github.com/tendermint/tendermint/types" ) @@ -57,14 +59,9 @@ var ( // last header (3/3 signed) 3: h3, } - l1 = &types.LightBlock{SignedHeader: h1, ValidatorSet: vals} - fullNode = mockp.New( - chainID, - headerSet, - valSet, - ) - deadNode = mockp.NewDeadMock(chainID) - largeFullNode = mockp.New(genMockNode(chainID, 10, 3, 0, bTime)) + l1 = &types.LightBlock{SignedHeader: h1, ValidatorSet: vals} + l2 = &types.LightBlock{SignedHeader: h2, ValidatorSet: vals} + l3 = &types.LightBlock{SignedHeader: h3, ValidatorSet: vals} ) func TestValidateTrustOptions(t *testing.T) { @@ -113,11 +110,6 @@ func TestValidateTrustOptions(t *testing.T) { } -func TestMock(t *testing.T) { - l, _ := fullNode.LightBlock(ctx, 3) - assert.Equal(t, int64(3), l.Height) -} - func TestClient_SequentialVerification(t *testing.T) { newKeys := genPrivKeys(4) newVals := newKeys.ToValidators(10, 1) @@ -216,28 +208,22 @@ func TestClient_SequentialVerification(t *testing.T) { } for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { + testCase := tc + t.Run(testCase.name, func(t *testing.T) { + mockNode := mockNodeFromHeadersAndVals(testCase.otherHeaders, testCase.vals) + mockNode.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) c, err := light.NewClient( ctx, chainID, trustOptions, - mockp.New( - chainID, - tc.otherHeaders, - tc.vals, - ), - []provider.Provider{mockp.New( - chainID, - tc.otherHeaders, - tc.vals, - )}, + mockNode, + []provider.Provider{mockNode}, dbs.New(dbm.NewMemDB()), light.SequentialVerification(), light.Logger(log.TestingLogger()), ) - if tc.initErr { + if testCase.initErr { require.Error(t, err) return } @@ -245,11 +231,12 @@ func TestClient_SequentialVerification(t *testing.T) { require.NoError(t, err) _, err = c.VerifyLightBlockAtHeight(ctx, 3, bTime.Add(3*time.Hour)) - if tc.verifyErr { + if testCase.verifyErr { assert.Error(t, err) } else { assert.NoError(t, err) } + mockNode.AssertExpectations(t) }) } } @@ -343,20 +330,14 @@ func TestClient_SkippingVerification(t *testing.T) { for _, tc := range testCases { tc := tc t.Run(tc.name, func(t *testing.T) { + mockNode := mockNodeFromHeadersAndVals(tc.otherHeaders, tc.vals) + mockNode.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) c, err := light.NewClient( ctx, chainID, trustOptions, - mockp.New( - chainID, - tc.otherHeaders, - tc.vals, - ), - []provider.Provider{mockp.New( - chainID, - tc.otherHeaders, - tc.vals, - )}, + mockNode, + []provider.Provider{mockNode}, dbs.New(dbm.NewMemDB()), light.SkippingVerification(light.DefaultTrustLevel), light.Logger(log.TestingLogger()), @@ -382,8 +363,23 @@ func TestClient_SkippingVerification(t *testing.T) { // start from a large light block to make sure that the pivot height doesn't select a height outside // the appropriate range func TestClientLargeBisectionVerification(t *testing.T) { - veryLargeFullNode := mockp.New(genMockNode(chainID, 100, 3, 0, bTime)) - trustedLightBlock, err := veryLargeFullNode.LightBlock(ctx, 5) + numBlocks := int64(300) + mockHeaders, mockVals, _ := genLightBlocksWithKeys(chainID, numBlocks, 101, 2, bTime) + + lastBlock := &types.LightBlock{SignedHeader: mockHeaders[numBlocks], ValidatorSet: mockVals[numBlocks]} + mockNode := &provider_mocks.Provider{} + mockNode.On("LightBlock", mock.Anything, numBlocks). + Return(lastBlock, nil) + + mockNode.On("LightBlock", mock.Anything, int64(200)). + Return(&types.LightBlock{SignedHeader: mockHeaders[200], ValidatorSet: mockVals[200]}, nil) + + mockNode.On("LightBlock", mock.Anything, int64(256)). + Return(&types.LightBlock{SignedHeader: mockHeaders[256], ValidatorSet: mockVals[256]}, nil) + + mockNode.On("LightBlock", mock.Anything, int64(0)).Return(lastBlock, nil) + + trustedLightBlock, err := mockNode.LightBlock(ctx, int64(200)) require.NoError(t, err) c, err := light.NewClient( ctx, @@ -393,20 +389,25 @@ func TestClientLargeBisectionVerification(t *testing.T) { Height: trustedLightBlock.Height, Hash: trustedLightBlock.Hash(), }, - veryLargeFullNode, - []provider.Provider{veryLargeFullNode}, + mockNode, + []provider.Provider{mockNode}, dbs.New(dbm.NewMemDB()), light.SkippingVerification(light.DefaultTrustLevel), ) require.NoError(t, err) - h, err := c.Update(ctx, bTime.Add(100*time.Minute)) + h, err := c.Update(ctx, bTime.Add(300*time.Minute)) assert.NoError(t, err) - h2, err := veryLargeFullNode.LightBlock(ctx, 100) + height, err := c.LastTrustedHeight() + require.NoError(t, err) + require.Equal(t, numBlocks, height) + h2, err := mockNode.LightBlock(ctx, numBlocks) require.NoError(t, err) assert.Equal(t, h, h2) + mockNode.AssertExpectations(t) } func TestClientBisectionBetweenTrustedHeaders(t *testing.T) { + mockFullNode := mockNodeFromHeadersAndVals(headerSet, valSet) c, err := light.NewClient( ctx, chainID, @@ -415,8 +416,8 @@ func TestClientBisectionBetweenTrustedHeaders(t *testing.T) { Height: 1, Hash: h1.Hash(), }, - fullNode, - []provider.Provider{fullNode}, + mockFullNode, + []provider.Provider{mockFullNode}, dbs.New(dbm.NewMemDB()), light.SkippingVerification(light.DefaultTrustLevel), ) @@ -432,15 +433,18 @@ func TestClientBisectionBetweenTrustedHeaders(t *testing.T) { // verify using bisection the light block between the two trusted light blocks _, err = c.VerifyLightBlockAtHeight(ctx, 2, bTime.Add(1*time.Hour)) assert.NoError(t, err) + mockFullNode.AssertExpectations(t) } func TestClient_Cleanup(t *testing.T) { + mockFullNode := &provider_mocks.Provider{} + mockFullNode.On("LightBlock", mock.Anything, int64(1)).Return(l1, nil) c, err := light.NewClient( ctx, chainID, trustOptions, - fullNode, - []provider.Provider{fullNode}, + mockFullNode, + []provider.Provider{mockFullNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) @@ -455,12 +459,14 @@ func TestClient_Cleanup(t *testing.T) { l, err := c.TrustedLightBlock(1) assert.Error(t, err) assert.Nil(t, l) + mockFullNode.AssertExpectations(t) } // trustedHeader.Height == options.Height func TestClientRestoresTrustedHeaderAfterStartup(t *testing.T) { // 1. options.Hash == trustedHeader.Hash - { + t.Run("hashes should match", func(t *testing.T) { + mockNode := &provider_mocks.Provider{} trustedStore := dbs.New(dbm.NewMemDB()) err := trustedStore.SaveLightBlock(l1) require.NoError(t, err) @@ -469,8 +475,8 @@ func TestClientRestoresTrustedHeaderAfterStartup(t *testing.T) { ctx, chainID, trustOptions, - fullNode, - []provider.Provider{fullNode}, + mockNode, + []provider.Provider{mockNode}, trustedStore, light.Logger(log.TestingLogger()), ) @@ -481,10 +487,11 @@ func TestClientRestoresTrustedHeaderAfterStartup(t *testing.T) { assert.NotNil(t, l) assert.Equal(t, l.Hash(), h1.Hash()) assert.Equal(t, l.ValidatorSet.Hash(), h1.ValidatorsHash.Bytes()) - } + mockNode.AssertExpectations(t) + }) // 2. options.Hash != trustedHeader.Hash - { + t.Run("hashes should not match", func(t *testing.T) { trustedStore := dbs.New(dbm.NewMemDB()) err := trustedStore.SaveLightBlock(l1) require.NoError(t, err) @@ -492,15 +499,7 @@ func TestClientRestoresTrustedHeaderAfterStartup(t *testing.T) { // 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)) - - primary := mockp.New( - chainID, - map[int64]*types.SignedHeader{ - // trusted header - 1: header1, - }, - valSet, - ) + mockNode := &provider_mocks.Provider{} c, err := light.NewClient( ctx, @@ -510,8 +509,8 @@ func TestClientRestoresTrustedHeaderAfterStartup(t *testing.T) { Height: 1, Hash: header1.Hash(), }, - primary, - []provider.Provider{primary}, + mockNode, + []provider.Provider{mockNode}, trustedStore, light.Logger(log.TestingLogger()), ) @@ -524,16 +523,21 @@ func TestClientRestoresTrustedHeaderAfterStartup(t *testing.T) { assert.Equal(t, l.Hash(), l1.Hash()) assert.NoError(t, l.ValidateBasic(chainID)) } - } + mockNode.AssertExpectations(t) + }) } func TestClient_Update(t *testing.T) { + mockFullNode := &provider_mocks.Provider{} + 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) c, err := light.NewClient( ctx, chainID, trustOptions, - fullNode, - []provider.Provider{fullNode}, + mockFullNode, + []provider.Provider{mockFullNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) @@ -546,15 +550,19 @@ func TestClient_Update(t *testing.T) { assert.EqualValues(t, 3, l.Height) assert.NoError(t, l.ValidateBasic(chainID)) } + mockFullNode.AssertExpectations(t) } func TestClient_Concurrency(t *testing.T) { + mockFullNode := &provider_mocks.Provider{} + mockFullNode.On("LightBlock", mock.Anything, int64(2)).Return(l2, nil) + mockFullNode.On("LightBlock", mock.Anything, int64(1)).Return(l1, nil) c, err := light.NewClient( ctx, chainID, trustOptions, - fullNode, - []provider.Provider{fullNode}, + mockFullNode, + []provider.Provider{mockFullNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) @@ -587,15 +595,20 @@ func TestClient_Concurrency(t *testing.T) { } wg.Wait() + mockFullNode.AssertExpectations(t) } func TestClient_AddProviders(t *testing.T) { + mockFullNode := mockNodeFromHeadersAndVals(map[int64]*types.SignedHeader{ + 1: h1, + 2: h2, + }, valSet) c, err := light.NewClient( ctx, chainID, trustOptions, - fullNode, - []provider.Provider{fullNode}, + mockFullNode, + []provider.Provider{mockFullNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) @@ -610,22 +623,28 @@ func TestClient_AddProviders(t *testing.T) { }() // NOTE: the light client doesn't check uniqueness of providers - c.AddProvider(fullNode) + c.AddProvider(mockFullNode) require.Len(t, c.Witnesses(), 2) select { case <-closeCh: case <-time.After(5 * time.Second): t.Fatal("concurent light block verification failed to finish in 5s") } + mockFullNode.AssertExpectations(t) } func TestClientReplacesPrimaryWithWitnessIfPrimaryIsUnavailable(t *testing.T) { + mockFullNode := &provider_mocks.Provider{} + mockFullNode.On("LightBlock", mock.Anything, mock.Anything).Return(l1, nil) + + mockDeadNode := &provider_mocks.Provider{} + mockDeadNode.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrNoResponse) c, err := light.NewClient( ctx, chainID, trustOptions, - deadNode, - []provider.Provider{fullNode, fullNode}, + mockDeadNode, + []provider.Provider{mockFullNode, mockFullNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) @@ -635,16 +654,25 @@ func TestClientReplacesPrimaryWithWitnessIfPrimaryIsUnavailable(t *testing.T) { require.NoError(t, err) // the primary should no longer be the deadNode - assert.NotEqual(t, c.Primary(), deadNode) + assert.NotEqual(t, c.Primary(), mockDeadNode) // 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) + mockFullNode.AssertExpectations(t) } func TestClient_BackwardsVerification(t *testing.T) { { - trustHeader, _ := largeFullNode.LightBlock(ctx, 6) + headers, vals, _ := genLightBlocksWithKeys(chainID, 9, 3, 0, bTime) + delete(headers, 1) + delete(headers, 2) + delete(vals, 1) + delete(vals, 2) + mockLargeFullNode := mockNodeFromHeadersAndVals(headers, vals) + trustHeader, _ := mockLargeFullNode.LightBlock(ctx, 6) + c, err := light.NewClient( ctx, chainID, @@ -653,8 +681,8 @@ func TestClient_BackwardsVerification(t *testing.T) { Height: trustHeader.Height, Hash: trustHeader.Hash(), }, - largeFullNode, - []provider.Provider{largeFullNode}, + mockLargeFullNode, + []provider.Provider{mockLargeFullNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) @@ -692,41 +720,36 @@ func TestClient_BackwardsVerification(t *testing.T) { // so expect error _, err = c.VerifyLightBlockAtHeight(ctx, 8, bTime.Add(12*time.Minute)) assert.Error(t, err) + mockLargeFullNode.AssertExpectations(t) } { testCases := []struct { - provider provider.Provider + headers map[int64]*types.SignedHeader + vals map[int64]*types.ValidatorSet }{ { // 7) provides incorrect height - mockp.New( - chainID, - map[int64]*types.SignedHeader{ - 1: h1, - 2: keys.GenSignedHeader(chainID, 1, bTime.Add(30*time.Minute), nil, vals, vals, - hash("app_hash"), hash("cons_hash"), hash("results_hash"), 0, len(keys)), - 3: h3, - }, - valSet, - ), + headers: map[int64]*types.SignedHeader{ + 2: keys.GenSignedHeader(chainID, 1, bTime.Add(30*time.Minute), nil, vals, vals, + hash("app_hash"), hash("cons_hash"), hash("results_hash"), 0, len(keys)), + 3: h3, + }, + vals: valSet, }, { // 8) provides incorrect hash - mockp.New( - chainID, - map[int64]*types.SignedHeader{ - 1: h1, - 2: keys.GenSignedHeader(chainID, 2, bTime.Add(30*time.Minute), nil, vals, vals, - hash("app_hash2"), hash("cons_hash23"), hash("results_hash30"), 0, len(keys)), - 3: h3, - }, - valSet, - ), + headers: map[int64]*types.SignedHeader{ + 2: keys.GenSignedHeader(chainID, 2, bTime.Add(30*time.Minute), nil, vals, vals, + hash("app_hash2"), hash("cons_hash23"), hash("results_hash30"), 0, len(keys)), + 3: h3, + }, + vals: valSet, }, } for idx, tc := range testCases { + mockNode := mockNodeFromHeadersAndVals(tc.headers, tc.vals) c, err := light.NewClient( ctx, chainID, @@ -735,8 +758,8 @@ func TestClient_BackwardsVerification(t *testing.T) { Height: 3, Hash: h3.Hash(), }, - tc.provider, - []provider.Provider{tc.provider}, + mockNode, + []provider.Provider{mockNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) @@ -744,6 +767,7 @@ func TestClient_BackwardsVerification(t *testing.T) { _, err = c.VerifyLightBlockAtHeight(ctx, 2, bTime.Add(1*time.Hour).Add(1*time.Second)) assert.Error(t, err, idx) + mockNode.AssertExpectations(t) } } } @@ -753,60 +777,62 @@ func TestClient_NewClientFromTrustedStore(t *testing.T) { db := dbs.New(dbm.NewMemDB()) err := db.SaveLightBlock(l1) require.NoError(t, err) + mockNode := &provider_mocks.Provider{} c, err := light.NewClientFromTrustedStore( chainID, trustPeriod, - deadNode, - []provider.Provider{deadNode}, + mockNode, + []provider.Provider{mockNode}, db, ) require.NoError(t, err) - // 2) Check light block exists (deadNode is being used to ensure we're not getting - // it from primary) + // 2) Check light block exists h, err := c.TrustedLightBlock(1) assert.NoError(t, err) assert.EqualValues(t, l1.Height, h.Height) + mockNode.AssertExpectations(t) } func TestClientRemovesWitnessIfItSendsUsIncorrectHeader(t *testing.T) { // different headers hash then primary plus less than 1/3 signed (no fork) - badProvider1 := mockp.New( - chainID, - map[int64]*types.SignedHeader{ - 1: h1, - 2: keys.GenSignedHeaderLastBlockID(chainID, 2, bTime.Add(30*time.Minute), nil, vals, vals, - hash("app_hash2"), hash("cons_hash"), hash("results_hash"), - len(keys), len(keys), types.BlockID{Hash: h1.Hash()}), - }, - map[int64]*types.ValidatorSet{ - 1: vals, - 2: vals, - }, - ) - // header is empty - badProvider2 := mockp.New( - chainID, - map[int64]*types.SignedHeader{ - 1: h1, - 2: h2, - }, - map[int64]*types.ValidatorSet{ - 1: vals, - 2: vals, - }, - ) + headers1 := map[int64]*types.SignedHeader{ + 1: h1, + 2: keys.GenSignedHeaderLastBlockID(chainID, 2, bTime.Add(30*time.Minute), nil, vals, vals, + hash("app_hash2"), hash("cons_hash"), hash("results_hash"), + len(keys), len(keys), types.BlockID{Hash: h1.Hash()}), + } + vals1 := map[int64]*types.ValidatorSet{ + 1: vals, + 2: vals, + } + mockBadNode1 := mockNodeFromHeadersAndVals(headers1, vals1) + mockBadNode1.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) - lb1, _ := badProvider1.LightBlock(ctx, 2) + // header is empty + headers2 := map[int64]*types.SignedHeader{ + 1: h1, + 2: h2, + } + vals2 := map[int64]*types.ValidatorSet{ + 1: vals, + 2: vals, + } + mockBadNode2 := mockNodeFromHeadersAndVals(headers2, vals2) + mockBadNode2.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrLightBlockNotFound) + + mockFullNode := mockNodeFromHeadersAndVals(headerSet, valSet) + + lb1, _ := mockBadNode1.LightBlock(ctx, 2) require.NotEqual(t, lb1.Hash(), l1.Hash()) c, err := light.NewClient( ctx, chainID, trustOptions, - fullNode, - []provider.Provider{badProvider1, badProvider2}, + mockFullNode, + []provider.Provider{mockBadNode1, mockBadNode2}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) @@ -828,12 +854,13 @@ func TestClientRemovesWitnessIfItSendsUsIncorrectHeader(t *testing.T) { } // witness does not have a light block -> left in the list assert.EqualValues(t, 1, len(c.Witnesses())) + mockBadNode1.AssertExpectations(t) + mockBadNode2.AssertExpectations(t) } func TestClient_TrustedValidatorSet(t *testing.T) { differentVals, _ := factory.RandValidatorSet(10, 100) - badValSetNode := mockp.New( - chainID, + mockBadValSetNode := mockNodeFromHeadersAndVals( map[int64]*types.SignedHeader{ 1: h1, // 3/3 signed, but validator set at height 2 below is invalid -> witness @@ -841,21 +868,27 @@ func TestClient_TrustedValidatorSet(t *testing.T) { 2: keys.GenSignedHeaderLastBlockID(chainID, 2, bTime.Add(30*time.Minute), nil, vals, vals, hash("app_hash2"), hash("cons_hash"), hash("results_hash"), 0, len(keys), types.BlockID{Hash: h1.Hash()}), - 3: h3, }, map[int64]*types.ValidatorSet{ 1: vals, 2: differentVals, - 3: differentVals, + }) + mockFullNode := mockNodeFromHeadersAndVals( + map[int64]*types.SignedHeader{ + 1: h1, + 2: h2, }, - ) + map[int64]*types.ValidatorSet{ + 1: vals, + 2: vals, + }) c, err := light.NewClient( ctx, chainID, trustOptions, - fullNode, - []provider.Provider{badValSetNode, fullNode}, + mockFullNode, + []provider.Provider{mockBadValSetNode, mockFullNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) @@ -865,15 +898,29 @@ func TestClient_TrustedValidatorSet(t *testing.T) { _, err = c.VerifyLightBlockAtHeight(ctx, 2, bTime.Add(2*time.Hour).Add(1*time.Second)) assert.NoError(t, err) assert.Equal(t, 1, len(c.Witnesses())) + mockBadValSetNode.AssertExpectations(t) + mockFullNode.AssertExpectations(t) } func TestClientPrunesHeadersAndValidatorSets(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, + }) + c, err := light.NewClient( ctx, chainID, trustOptions, - fullNode, - []provider.Provider{fullNode}, + mockFullNode, + []provider.Provider{mockFullNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), light.PruningSize(1), @@ -888,6 +935,7 @@ func TestClientPrunesHeadersAndValidatorSets(t *testing.T) { _, err = c.TrustedLightBlock(1) assert.Error(t, err) + mockFullNode.AssertExpectations(t) } func TestClientEnsureValidHeadersAndValSets(t *testing.T) { @@ -899,86 +947,108 @@ func TestClientEnsureValidHeadersAndValSets(t *testing.T) { testCases := []struct { headers map[int64]*types.SignedHeader vals map[int64]*types.ValidatorSet - err bool + + errorToThrow error + errorHeight int64 + + err bool }{ { - headerSet, - valSet, - false, - }, - { - headerSet, - map[int64]*types.ValidatorSet{ - 1: vals, - 2: vals, - 3: nil, - }, - true, - }, - { - map[int64]*types.SignedHeader{ + headers: map[int64]*types.SignedHeader{ 1: h1, - 2: h2, - 3: nil, + 3: h3, }, - valSet, - true, + vals: map[int64]*types.ValidatorSet{ + 1: vals, + 3: vals, + }, + err: false, }, { - headerSet, - map[int64]*types.ValidatorSet{ + headers: map[int64]*types.SignedHeader{ + 1: h1, + }, + vals: map[int64]*types.ValidatorSet{ + 1: vals, + }, + errorToThrow: provider.ErrBadLightBlock{Reason: errors.New("nil header or vals")}, + errorHeight: 3, + err: true, + }, + { + headers: map[int64]*types.SignedHeader{ + 1: h1, + }, + errorToThrow: provider.ErrBadLightBlock{Reason: errors.New("nil header or vals")}, + errorHeight: 3, + vals: valSet, + err: true, + }, + { + headers: map[int64]*types.SignedHeader{ + 1: h1, + 3: h3, + }, + vals: map[int64]*types.ValidatorSet{ 1: vals, - 2: vals, 3: emptyValSet, }, - true, + err: true, }, } - for _, tc := range testCases { - badNode := mockp.New( - chainID, - tc.headers, - tc.vals, - ) - c, err := light.NewClient( - ctx, - chainID, - trustOptions, - badNode, - []provider.Provider{badNode, badNode}, - dbs.New(dbm.NewMemDB()), - ) - require.NoError(t, err) + for i, tc := range testCases { + testCase := tc + t.Run(fmt.Sprintf("case: %d", i), func(t *testing.T) { + mockBadNode := mockNodeFromHeadersAndVals(testCase.headers, testCase.vals) + if testCase.errorToThrow != nil { + mockBadNode.On("LightBlock", mock.Anything, testCase.errorHeight).Return(nil, testCase.errorToThrow) + } - _, err = c.VerifyLightBlockAtHeight(ctx, 3, bTime.Add(2*time.Hour)) - if tc.err { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + c, err := light.NewClient( + ctx, + chainID, + trustOptions, + mockBadNode, + []provider.Provider{mockBadNode, mockBadNode}, + dbs.New(dbm.NewMemDB()), + ) + require.NoError(t, err) + + _, err = c.VerifyLightBlockAtHeight(ctx, 3, bTime.Add(2*time.Hour)) + if testCase.err { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + mockBadNode.AssertExpectations(t) + }) } } func TestClientHandlesContexts(t *testing.T) { - p := mockp.New(genMockNode(chainID, 100, 10, 1, bTime)) - genBlock, err := p.LightBlock(ctx, 1) - require.NoError(t, err) + mockNode := &provider_mocks.Provider{} + mockNode.On("LightBlock", + mock.MatchedBy(func(ctx context.Context) bool { return ctx.Err() == nil }), + int64(1)).Return(l1, nil) + mockNode.On("LightBlock", + mock.MatchedBy(func(ctx context.Context) bool { return ctx.Err() == context.DeadlineExceeded }), + mock.Anything).Return(nil, context.DeadlineExceeded) + + mockNode.On("LightBlock", + mock.MatchedBy(func(ctx context.Context) bool { return ctx.Err() == context.Canceled }), + mock.Anything).Return(nil, context.Canceled) // instantiate the light client with a timeout - ctxTimeOut, cancel := context.WithTimeout(ctx, 10*time.Millisecond) + ctxTimeOut, cancel := context.WithTimeout(ctx, 1*time.Nanosecond) defer cancel() - _, err = light.NewClient( + _, err := light.NewClient( ctxTimeOut, chainID, - light.TrustOptions{ - Period: 24 * time.Hour, - Height: 1, - Hash: genBlock.Hash(), - }, - p, - []provider.Provider{p, p}, + trustOptions, + mockNode, + []provider.Provider{mockNode, mockNode}, dbs.New(dbm.NewMemDB()), ) require.Error(t, ctxTimeOut.Err()) @@ -989,19 +1059,15 @@ func TestClientHandlesContexts(t *testing.T) { c, err := light.NewClient( ctx, chainID, - light.TrustOptions{ - Period: 24 * time.Hour, - Height: 1, - Hash: genBlock.Hash(), - }, - p, - []provider.Provider{p, p}, + trustOptions, + mockNode, + []provider.Provider{mockNode, mockNode}, dbs.New(dbm.NewMemDB()), ) require.NoError(t, err) // verify a block with a timeout - ctxTimeOutBlock, cancel := context.WithTimeout(ctx, 10*time.Millisecond) + ctxTimeOutBlock, cancel := context.WithTimeout(ctx, 1*time.Nanosecond) defer cancel() _, err = c.VerifyLightBlockAtHeight(ctxTimeOutBlock, 100, bTime.Add(100*time.Minute)) require.Error(t, ctxTimeOutBlock.Err()) @@ -1010,11 +1076,11 @@ func TestClientHandlesContexts(t *testing.T) { // verify a block with a cancel ctxCancel, cancel := context.WithCancel(ctx) - defer cancel() - time.AfterFunc(10*time.Millisecond, cancel) + cancel() _, err = c.VerifyLightBlockAtHeight(ctxCancel, 100, bTime.Add(100*time.Minute)) require.Error(t, ctxCancel.Err()) require.Error(t, err) require.True(t, errors.Is(err, context.Canceled)) + mockNode.AssertExpectations(t) } diff --git a/light/detector_test.go b/light/detector_test.go index 48efd4130..0bf96ace6 100644 --- a/light/detector_test.go +++ b/light/detector_test.go @@ -1,10 +1,12 @@ package light_test import ( + "bytes" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" dbm "github.com/tendermint/tm-db" @@ -12,7 +14,7 @@ import ( "github.com/tendermint/tendermint/libs/log" "github.com/tendermint/tendermint/light" "github.com/tendermint/tendermint/light/provider" - mockp "github.com/tendermint/tendermint/light/provider/mock" + provider_mocks "github.com/tendermint/tendermint/light/provider/mocks" dbs "github.com/tendermint/tendermint/light/store/db" "github.com/tendermint/tendermint/types" ) @@ -20,15 +22,15 @@ import ( func TestLightClientAttackEvidence_Lunatic(t *testing.T) { // primary performs a lunatic attack var ( - latestHeight = int64(10) + latestHeight = int64(3) valSize = 5 - divergenceHeight = int64(6) + divergenceHeight = int64(2) primaryHeaders = make(map[int64]*types.SignedHeader, latestHeight) primaryValidators = make(map[int64]*types.ValidatorSet, latestHeight) ) - witnessHeaders, witnessValidators, chainKeys := genMockNodeWithKeys(chainID, latestHeight, valSize, 2, bTime) - witness := mockp.New(chainID, witnessHeaders, witnessValidators) + witnessHeaders, witnessValidators, chainKeys := genLightBlocksWithKeys(chainID, latestHeight, valSize, 2, bTime) + forgedKeys := chainKeys[divergenceHeight-1].ChangeKeys(3) // we change 3 out of the 5 validators (still 2/5 remain) forgedVals := forgedKeys.ToValidators(2, 0) @@ -42,7 +44,38 @@ func TestLightClientAttackEvidence_Lunatic(t *testing.T) { nil, forgedVals, forgedVals, hash("app_hash"), hash("cons_hash"), hash("results_hash"), 0, len(forgedKeys)) primaryValidators[height] = forgedVals } - primary := mockp.New(chainID, primaryHeaders, primaryValidators) + + // never called, delete it to make mockery asserts pass + delete(witnessHeaders, 2) + delete(primaryHeaders, 2) + + mockWitness := mockNodeFromHeadersAndVals(witnessHeaders, witnessValidators) + mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryValidators) + + 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 + ConflictingBlock: &types.LightBlock{ + SignedHeader: primaryHeaders[latestHeight], + ValidatorSet: primaryValidators[latestHeight], + }, + CommonHeight: 1, + } + return bytes.Equal(evidence.Hash(), evAgainstPrimary.Hash()) + })).Return(nil) + + 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 + // hence the conflicting block is at 7 + ConflictingBlock: &types.LightBlock{ + SignedHeader: witnessHeaders[divergenceHeight+1], + ValidatorSet: witnessValidators[divergenceHeight+1], + }, + CommonHeight: divergenceHeight - 1, + } + return bytes.Equal(evidence.Hash(), evAgainstWitness.Hash()) + })).Return(nil) c, err := light.NewClient( ctx, @@ -52,121 +85,134 @@ func TestLightClientAttackEvidence_Lunatic(t *testing.T) { Height: 1, Hash: primaryHeaders[1].Hash(), }, - primary, - []provider.Provider{witness}, + mockPrimary, + []provider.Provider{mockWitness}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) require.NoError(t, err) // Check verification returns an error. - _, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour)) + _, err = c.VerifyLightBlockAtHeight(ctx, latestHeight, bTime.Add(1*time.Hour)) if assert.Error(t, err) { assert.Equal(t, light.ErrLightClientAttack, err) } - // Check evidence was sent to both full nodes. - evAgainstPrimary := &types.LightClientAttackEvidence{ - // after the divergence height the valset doesn't change so we expect the evidence to be for height 10 - ConflictingBlock: &types.LightBlock{ - SignedHeader: primaryHeaders[10], - ValidatorSet: primaryValidators[10], - }, - CommonHeight: 4, - } - assert.True(t, witness.HasEvidence(evAgainstPrimary)) - - evAgainstWitness := &types.LightClientAttackEvidence{ - // when forming evidence against witness we learn that the canonical chain continued to change validator sets - // hence the conflicting block is at 7 - ConflictingBlock: &types.LightBlock{ - SignedHeader: witnessHeaders[7], - ValidatorSet: witnessValidators[7], - }, - CommonHeight: 4, - } - assert.True(t, primary.HasEvidence(evAgainstWitness)) + mockWitness.AssertExpectations(t) + mockPrimary.AssertExpectations(t) } func TestLightClientAttackEvidence_Equivocation(t *testing.T) { - verificationOptions := map[string]light.Option{ - "sequential": light.SequentialVerification(), - "skipping": light.SkippingVerification(light.DefaultTrustLevel), + cases := []struct { + name string + lightOption light.Option + unusedWitnessBlockHeights []int64 + unusedPrimaryBlockHeights []int64 + latestHeight int64 + divergenceHeight int64 + }{ + { + name: "sequential", + lightOption: light.SequentialVerification(), + unusedWitnessBlockHeights: []int64{4, 6}, + latestHeight: int64(5), + divergenceHeight: int64(3), + }, + { + name: "skipping", + lightOption: light.SkippingVerification(light.DefaultTrustLevel), + unusedWitnessBlockHeights: []int64{2, 4, 6}, + unusedPrimaryBlockHeights: []int64{2, 4, 6}, + latestHeight: int64(5), + divergenceHeight: int64(3), + }, } - for s, verificationOption := range verificationOptions { - t.Log("==> verification", s) - - // 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] + for _, tc := range cases { + testCase := tc + t.Run(testCase.name, func(t *testing.T) { + // primary performs an equivocation attack + var ( + valSize = 5 + primaryHeaders = make(map[int64]*types.SignedHeader, testCase.latestHeight) + // validators don't change in this network (however we still use a map just for convenience) + primaryValidators = make(map[int64]*types.ValidatorSet, testCase.latestHeight) + ) + witnessHeaders, witnessValidators, chainKeys := genLightBlocksWithKeys(chainID, + testCase.latestHeight+1, valSize, 2, bTime) + for height := int64(1); height <= testCase.latestHeight; height++ { + if height < testCase.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 - // 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( - ctx, - chainID, - light.TrustOptions{ - Period: 4 * time.Hour, - Height: 1, - Hash: primaryHeaders[1].Hash(), - }, - primary, - []provider.Provider{witness}, - dbs.New(dbm.NewMemDB()), - light.Logger(log.TestingLogger()), - verificationOption, - ) - require.NoError(t, err) + for _, height := range testCase.unusedWitnessBlockHeights { + delete(witnessHeaders, height) + } + mockWitness := mockNodeFromHeadersAndVals(witnessHeaders, witnessValidators) + for _, height := range testCase.unusedPrimaryBlockHeights { + delete(primaryHeaders, height) + } + mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryValidators) - // Check verification returns an error. - _, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour)) - if assert.Error(t, err) { - assert.Equal(t, light.ErrLightClientAttack, err) - } + // Check evidence was sent to both full nodes. + // 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 + mockWitness.On("ReportEvidence", mock.Anything, mock.MatchedBy(func(evidence types.Evidence) bool { + evAgainstPrimary := &types.LightClientAttackEvidence{ + ConflictingBlock: &types.LightBlock{ + SignedHeader: primaryHeaders[testCase.divergenceHeight], + ValidatorSet: primaryValidators[testCase.divergenceHeight], + }, + CommonHeight: testCase.divergenceHeight, + } + return bytes.Equal(evidence.Hash(), evAgainstPrimary.Hash()) + })).Return(nil) + mockPrimary.On("ReportEvidence", mock.Anything, mock.MatchedBy(func(evidence types.Evidence) bool { + evAgainstWitness := &types.LightClientAttackEvidence{ + ConflictingBlock: &types.LightBlock{ + SignedHeader: witnessHeaders[testCase.divergenceHeight], + ValidatorSet: witnessValidators[testCase.divergenceHeight], + }, + CommonHeight: testCase.divergenceHeight, + } + return bytes.Equal(evidence.Hash(), evAgainstWitness.Hash()) + })).Return(nil) - // Check evidence was sent to both full nodes. - // 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 - evAgainstPrimary := &types.LightClientAttackEvidence{ - ConflictingBlock: &types.LightBlock{ - SignedHeader: primaryHeaders[divergenceHeight], - ValidatorSet: primaryValidators[divergenceHeight], - }, - CommonHeight: divergenceHeight, - } - assert.True(t, witness.HasEvidence(evAgainstPrimary)) + c, err := light.NewClient( + ctx, + chainID, + light.TrustOptions{ + Period: 4 * time.Hour, + Height: 1, + Hash: primaryHeaders[1].Hash(), + }, + mockPrimary, + []provider.Provider{mockWitness}, + dbs.New(dbm.NewMemDB()), + light.Logger(log.TestingLogger()), + testCase.lightOption, + ) + require.NoError(t, err) - evAgainstWitness := &types.LightClientAttackEvidence{ - ConflictingBlock: &types.LightBlock{ - SignedHeader: witnessHeaders[divergenceHeight], - ValidatorSet: witnessValidators[divergenceHeight], - }, - CommonHeight: divergenceHeight, - } - assert.True(t, primary.HasEvidence(evAgainstWitness)) + // Check verification returns an error. + _, err = c.VerifyLightBlockAtHeight(ctx, testCase.latestHeight, bTime.Add(300*time.Second)) + if assert.Error(t, err) { + assert.Equal(t, light.ErrLightClientAttack, err) + } + + mockWitness.AssertExpectations(t) + mockPrimary.AssertExpectations(t) + }) } } @@ -182,7 +228,10 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { primaryValidators = make(map[int64]*types.ValidatorSet, forgedHeight) ) - witnessHeaders, witnessValidators, chainKeys := genMockNodeWithKeys(chainID, latestHeight, valSize, 2, bTime) + witnessHeaders, witnessValidators, chainKeys := genLightBlocksWithKeys(chainID, latestHeight, valSize, 2, bTime) + for _, unusedHeader := range []int64{3, 5, 6, 8} { + delete(witnessHeaders, unusedHeader) + } // primary has the exact same headers except it forges one extra header in the future using keys from 2/5ths of // the validators @@ -190,6 +239,9 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { primaryHeaders[h] = witnessHeaders[h] primaryValidators[h] = witnessValidators[h] } + for _, unusedHeader := range []int64{3, 5, 6, 8} { + delete(primaryHeaders, unusedHeader) + } forgedKeys := chainKeys[latestHeight].ChangeKeys(3) // we change 3 out of the 5 validators (still 2/5 remain) primaryValidators[forgedHeight] = forgedKeys.ToValidators(2, 0) primaryHeaders[forgedHeight] = forgedKeys.GenSignedHeader( @@ -204,15 +256,36 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { hash("results_hash"), 0, len(forgedKeys), ) + mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryValidators) + 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) - witness := mockp.New(chainID, witnessHeaders, witnessValidators) - primary := mockp.New(chainID, primaryHeaders, primaryValidators) + /* + for _, unusedHeader := range []int64{3, 5, 6, 8} { + delete(witnessHeaders, unusedHeader) + } + */ + mockWitness := mockNodeFromHeadersAndVals(witnessHeaders, witnessValidators) + 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) - laggingWitness := witness.Copy("laggingWitness") + mockWitness.On("ReportEvidence", mock.Anything, mock.MatchedBy(func(evidence types.Evidence) bool { + // Check evidence was sent to the witness against the full node + evAgainstPrimary := &types.LightClientAttackEvidence{ + ConflictingBlock: &types.LightBlock{ + SignedHeader: primaryHeaders[forgedHeight], + ValidatorSet: primaryValidators[forgedHeight], + }, + CommonHeight: latestHeight, + } + return bytes.Equal(evidence.Hash(), evAgainstPrimary.Hash()) + })).Return(nil).Twice() // In order to perform the attack, the primary needs at least one accomplice as a witness to also // send the forged block - accomplice := primary + accomplice := mockPrimary c, err := light.NewClient( ctx, @@ -222,8 +295,8 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { Height: 1, Hash: primaryHeaders[1].Hash(), }, - primary, - []provider.Provider{witness, accomplice}, + mockPrimary, + []provider.Provider{mockWitness, accomplice}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), light.MaxClockDrift(1*time.Second), @@ -251,7 +324,7 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { } go func() { time.Sleep(2 * time.Second) - witness.AddLightBlock(newLb) + mockWitness.On("LightBlock", mock.Anything, int64(0)).Return(newLb, nil) }() // Now assert that verification returns an error. We craft the light clients time to be a little ahead of the chain @@ -261,26 +334,19 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { assert.Equal(t, light.ErrLightClientAttack, err) } - // Check evidence was sent to the witness against the full node - evAgainstPrimary := &types.LightClientAttackEvidence{ - ConflictingBlock: &types.LightBlock{ - SignedHeader: primaryHeaders[forgedHeight], - ValidatorSet: primaryValidators[forgedHeight], - }, - CommonHeight: latestHeight, - } - assert.True(t, witness.HasEvidence(evAgainstPrimary)) - // We attempt the same call but now the supporting witness has a block which should // immediately conflict in time with the primary _, err = c.VerifyLightBlockAtHeight(ctx, forgedHeight, bTime.Add(time.Duration(forgedHeight)*time.Minute)) if assert.Error(t, err) { assert.Equal(t, light.ErrLightClientAttack, err) } - assert.True(t, witness.HasEvidence(evAgainstPrimary)) // Lastly we test the unfortunate case where the light clients supporting witness doesn't update // in enough time + mockLaggingWitness := mockNodeFromHeadersAndVals(witnessHeaders, witnessValidators) + 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) c, err = light.NewClient( ctx, chainID, @@ -289,8 +355,8 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { Height: 1, Hash: primaryHeaders[1].Hash(), }, - primary, - []provider.Provider{laggingWitness, accomplice}, + mockPrimary, + []provider.Provider{mockLaggingWitness, accomplice}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), light.MaxClockDrift(1*time.Second), @@ -300,17 +366,20 @@ func TestLightClientAttackEvidence_ForwardLunatic(t *testing.T) { _, err = c.Update(ctx, bTime.Add(time.Duration(forgedHeight)*time.Minute)) assert.NoError(t, err) - + mockPrimary.AssertExpectations(t) + mockWitness.AssertExpectations(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) + headers, vals, _ := genLightBlocksWithKeys(chainID, 1, 5, 2, bTime) + mockPrimary := mockNodeFromHeadersAndVals(headers, vals) + firstBlock, err := mockPrimary.LightBlock(ctx, 1) require.NoError(t, err) - witness := mockp.New(genMockNode(chainID, 10, 5, 2, bTime)) + headers, vals, _ = genLightBlocksWithKeys(chainID, 1, 5, 2, bTime) + mockWitness := mockNodeFromHeadersAndVals(headers, vals) _, err = light.NewClient( ctx, @@ -320,20 +389,25 @@ func TestClientDivergentTraces1(t *testing.T) { Hash: firstBlock.Hash(), Period: 4 * time.Hour, }, - primary, - []provider.Provider{witness}, + mockPrimary, + []provider.Provider{mockWitness}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) require.Error(t, err) assert.Contains(t, err.Error(), "does not match primary") + mockWitness.AssertExpectations(t) + mockPrimary.AssertExpectations(t) } // 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) + headers, vals, _ := genLightBlocksWithKeys(chainID, 2, 5, 2, bTime) + mockPrimaryNode := mockNodeFromHeadersAndVals(headers, vals) + mockDeadNode := &provider_mocks.Provider{} + mockDeadNode.On("LightBlock", mock.Anything, mock.Anything).Return(nil, provider.ErrNoResponse) + firstBlock, err := mockPrimaryNode.LightBlock(ctx, 1) require.NoError(t, err) c, err := light.NewClient( ctx, @@ -343,31 +417,35 @@ func TestClientDivergentTraces2(t *testing.T) { Hash: firstBlock.Hash(), Period: 4 * time.Hour, }, - primary, - []provider.Provider{deadNode, deadNode, primary}, + mockPrimaryNode, + []provider.Provider{mockDeadNode, mockDeadNode, mockPrimaryNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) require.NoError(t, err) - _, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour)) + _, err = c.VerifyLightBlockAtHeight(ctx, 2, bTime.Add(1*time.Hour)) assert.NoError(t, err) assert.Equal(t, 3, len(c.Witnesses())) + mockDeadNode.AssertExpectations(t) + mockPrimaryNode.AssertExpectations(t) } // 3. witness has the same first header, but different second header // => creation should succeed, but the verification should fail +//nolint: dupl func TestClientDivergentTraces3(t *testing.T) { - _, primaryHeaders, primaryVals := genMockNode(chainID, 10, 5, 2, bTime) - primary := mockp.New(chainID, primaryHeaders, primaryVals) + // + primaryHeaders, primaryVals, _ := genLightBlocksWithKeys(chainID, 2, 5, 2, bTime) + mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryVals) - firstBlock, err := primary.LightBlock(ctx, 1) + firstBlock, err := mockPrimary.LightBlock(ctx, 1) require.NoError(t, err) - _, mockHeaders, mockVals := genMockNode(chainID, 10, 5, 2, bTime) + mockHeaders, mockVals, _ := genLightBlocksWithKeys(chainID, 2, 5, 2, bTime) mockHeaders[1] = primaryHeaders[1] mockVals[1] = primaryVals[1] - witness := mockp.New(chainID, mockHeaders, mockVals) + mockWitness := mockNodeFromHeadersAndVals(mockHeaders, mockVals) c, err := light.NewClient( ctx, @@ -377,33 +455,35 @@ func TestClientDivergentTraces3(t *testing.T) { Hash: firstBlock.Hash(), Period: 4 * time.Hour, }, - primary, - []provider.Provider{witness}, + mockPrimary, + []provider.Provider{mockWitness}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) require.NoError(t, err) - _, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour)) + _, err = c.VerifyLightBlockAtHeight(ctx, 2, bTime.Add(1*time.Hour)) assert.Error(t, err) assert.Equal(t, 1, len(c.Witnesses())) + mockWitness.AssertExpectations(t) + mockPrimary.AssertExpectations(t) } // 4. Witness has a divergent header but can not produce a valid trace to back it up. // It should be ignored +//nolint: dupl func TestClientDivergentTraces4(t *testing.T) { - _, primaryHeaders, primaryVals := genMockNode(chainID, 10, 5, 2, bTime) - primary := mockp.New(chainID, primaryHeaders, primaryVals) + // + primaryHeaders, primaryVals, _ := genLightBlocksWithKeys(chainID, 2, 5, 2, bTime) + mockPrimary := mockNodeFromHeadersAndVals(primaryHeaders, primaryVals) - firstBlock, err := primary.LightBlock(ctx, 1) + firstBlock, err := mockPrimary.LightBlock(ctx, 1) require.NoError(t, err) - _, mockHeaders, mockVals := genMockNode(chainID, 10, 5, 2, bTime) - witness := primary.Copy("witness") - witness.AddLightBlock(&types.LightBlock{ - SignedHeader: mockHeaders[10], - ValidatorSet: mockVals[10], - }) + witnessHeaders, witnessVals, _ := genLightBlocksWithKeys(chainID, 2, 5, 2, bTime) + primaryHeaders[2] = witnessHeaders[2] + primaryVals[2] = witnessVals[2] + mockWitness := mockNodeFromHeadersAndVals(primaryHeaders, primaryVals) c, err := light.NewClient( ctx, @@ -413,14 +493,16 @@ func TestClientDivergentTraces4(t *testing.T) { Hash: firstBlock.Hash(), Period: 4 * time.Hour, }, - primary, - []provider.Provider{witness}, + mockPrimary, + []provider.Provider{mockWitness}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), ) require.NoError(t, err) - _, err = c.VerifyLightBlockAtHeight(ctx, 10, bTime.Add(1*time.Hour)) + _, err = c.VerifyLightBlockAtHeight(ctx, 2, bTime.Add(1*time.Hour)) assert.Error(t, err) assert.Equal(t, 1, len(c.Witnesses())) + mockWitness.AssertExpectations(t) + mockPrimary.AssertExpectations(t) } diff --git a/light/helpers_test.go b/light/helpers_test.go index 2ca951913..1d25f9166 100644 --- a/light/helpers_test.go +++ b/light/helpers_test.go @@ -3,10 +3,12 @@ package light_test import ( "time" + "github.com/stretchr/testify/mock" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/tmhash" tmtime "github.com/tendermint/tendermint/libs/time" + provider_mocks "github.com/tendermint/tendermint/light/provider/mocks" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" "github.com/tendermint/tendermint/types" "github.com/tendermint/tendermint/version" @@ -169,12 +171,12 @@ func (pkz privKeys) ChangeKeys(delta int) privKeys { return newKeys.Extend(delta) } -// Generates the header and validator set to create a full entire mock node with blocks to height ( -// blockSize) and with variation in validator sets. BlockIntervals are in per minute. +// genLightBlocksWithKeys generates the header and validator set to create +// blocks to height. BlockIntervals are in per minute. // NOTE: Expected to have a large validator set size ~ 100 validators. -func genMockNodeWithKeys( +func genLightBlocksWithKeys( chainID string, - blockSize int64, + numBlocks int64, valSize int, valVariation float32, bTime time.Time) ( @@ -183,9 +185,9 @@ func genMockNodeWithKeys( map[int64]privKeys) { var ( - headers = make(map[int64]*types.SignedHeader, blockSize) - valset = make(map[int64]*types.ValidatorSet, blockSize+1) - keymap = make(map[int64]privKeys, blockSize+1) + headers = make(map[int64]*types.SignedHeader, numBlocks) + valset = make(map[int64]*types.ValidatorSet, numBlocks+1) + keymap = make(map[int64]privKeys, numBlocks+1) keys = genPrivKeys(valSize) totalVariation = valVariation valVariationInt int @@ -207,7 +209,7 @@ func genMockNodeWithKeys( valset[1] = keys.ToValidators(2, 0) keys = newKeys - for height := int64(2); height <= blockSize; height++ { + for height := int64(2); height <= numBlocks; height++ { totalVariation += valVariation valVariationInt = int(totalVariation) totalVariation = -float32(valVariationInt) @@ -226,17 +228,14 @@ func genMockNodeWithKeys( return headers, valset, keymap } -func genMockNode( - chainID string, - blockSize int64, - valSize int, - valVariation float32, - bTime time.Time) ( - string, - map[int64]*types.SignedHeader, - map[int64]*types.ValidatorSet) { - headers, valset, _ := genMockNodeWithKeys(chainID, blockSize, valSize, valVariation, bTime) - return chainID, headers, valset +func mockNodeFromHeadersAndVals(headers map[int64]*types.SignedHeader, + vals map[int64]*types.ValidatorSet) *provider_mocks.Provider { + mockNode := &provider_mocks.Provider{} + for i, header := range headers { + lb := &types.LightBlock{SignedHeader: header, ValidatorSet: vals[i]} + mockNode.On("LightBlock", mock.Anything, i).Return(lb, nil) + } + return mockNode } func hash(s string) []byte { diff --git a/light/provider/mock/deadmock.go b/light/provider/mock/deadmock.go deleted file mode 100644 index 6045e45f6..000000000 --- a/light/provider/mock/deadmock.go +++ /dev/null @@ -1,30 +0,0 @@ -package mock - -import ( - "context" - "fmt" - - "github.com/tendermint/tendermint/light/provider" - "github.com/tendermint/tendermint/types" -) - -type deadMock struct { - id string -} - -// NewDeadMock creates a mock provider that always errors. id is used in case of multiple providers. -func NewDeadMock(id string) provider.Provider { - return &deadMock{id: id} -} - -func (p *deadMock) String() string { - return fmt.Sprintf("DeadMock-%s", p.id) -} - -func (p *deadMock) LightBlock(_ context.Context, height int64) (*types.LightBlock, error) { - return nil, provider.ErrNoResponse -} - -func (p *deadMock) ReportEvidence(_ context.Context, ev types.Evidence) error { - return provider.ErrNoResponse -} diff --git a/light/provider/mock/mock.go b/light/provider/mock/mock.go deleted file mode 100644 index fcb8a6fa4..000000000 --- a/light/provider/mock/mock.go +++ /dev/null @@ -1,125 +0,0 @@ -package mock - -import ( - "context" - "errors" - "fmt" - "strings" - "sync" - "time" - - "github.com/tendermint/tendermint/light/provider" - "github.com/tendermint/tendermint/types" -) - -type Mock struct { - id string - - mtx sync.Mutex - headers map[int64]*types.SignedHeader - vals map[int64]*types.ValidatorSet - evidenceToReport map[string]types.Evidence // hash => evidence - latestHeight int64 -} - -var _ provider.Provider = (*Mock)(nil) - -// New creates a mock provider with the given set of headers and validator -// sets. -func New(id string, headers map[int64]*types.SignedHeader, vals map[int64]*types.ValidatorSet) *Mock { - height := int64(0) - for h := range headers { - if h > height { - height = h - } - } - return &Mock{ - id: id, - headers: headers, - vals: vals, - evidenceToReport: make(map[string]types.Evidence), - latestHeight: height, - } -} - -func (p *Mock) String() string { - var headers strings.Builder - for _, h := range p.headers { - fmt.Fprintf(&headers, " %d:%X", h.Height, h.Hash()) - } - - var vals strings.Builder - for _, v := range p.vals { - fmt.Fprintf(&vals, " %X", v.Hash()) - } - - return fmt.Sprintf("Mock{id: %s, headers: %s, vals: %v}", p.id, headers.String(), vals.String()) -} - -func (p *Mock) LightBlock(ctx context.Context, height int64) (*types.LightBlock, error) { - p.mtx.Lock() - defer p.mtx.Unlock() - - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-time.After(10 * time.Millisecond): - } - - var lb *types.LightBlock - - if height > p.latestHeight { - return nil, provider.ErrHeightTooHigh - } - - if height == 0 && len(p.headers) > 0 { - height = p.latestHeight - } - - if _, ok := p.headers[height]; ok { - sh := p.headers[height] - vals := p.vals[height] - lb = &types.LightBlock{ - SignedHeader: sh, - ValidatorSet: vals, - } - } - if lb == nil { - return nil, provider.ErrLightBlockNotFound - } - if lb.SignedHeader == nil || lb.ValidatorSet == nil { - return nil, provider.ErrBadLightBlock{Reason: errors.New("nil header or vals")} - } - if err := lb.ValidateBasic(lb.ChainID); err != nil { - return nil, provider.ErrBadLightBlock{Reason: err} - } - return lb, nil -} - -func (p *Mock) ReportEvidence(_ context.Context, ev types.Evidence) error { - p.evidenceToReport[string(ev.Hash())] = ev - return nil -} - -func (p *Mock) HasEvidence(ev types.Evidence) bool { - _, ok := p.evidenceToReport[string(ev.Hash())] - return ok -} - -func (p *Mock) AddLightBlock(lb *types.LightBlock) { - p.mtx.Lock() - defer p.mtx.Unlock() - - if err := lb.ValidateBasic(lb.ChainID); err != nil { - panic(fmt.Sprintf("unable to add light block, err: %v", err)) - } - p.headers[lb.Height] = lb.SignedHeader - p.vals[lb.Height] = lb.ValidatorSet - if lb.Height > p.latestHeight { - p.latestHeight = lb.Height - } -} - -func (p *Mock) Copy(id string) *Mock { - return New(id, p.headers, p.vals) -} diff --git a/light/provider/mocks/provider.go b/light/provider/mocks/provider.go new file mode 100644 index 000000000..5a58d6b32 --- /dev/null +++ b/light/provider/mocks/provider.go @@ -0,0 +1,53 @@ +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" + + types "github.com/tendermint/tendermint/types" +) + +// Provider is an autogenerated mock type for the Provider type +type Provider struct { + mock.Mock +} + +// LightBlock provides a mock function with given fields: ctx, height +func (_m *Provider) LightBlock(ctx context.Context, height int64) (*types.LightBlock, error) { + ret := _m.Called(ctx, height) + + var r0 *types.LightBlock + if rf, ok := ret.Get(0).(func(context.Context, int64) *types.LightBlock); ok { + r0 = rf(ctx, height) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*types.LightBlock) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, int64) error); ok { + r1 = rf(ctx, height) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// ReportEvidence provides a mock function with given fields: _a0, _a1 +func (_m *Provider) ReportEvidence(_a0 context.Context, _a1 types.Evidence) error { + ret := _m.Called(_a0, _a1) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, types.Evidence) error); ok { + r0 = rf(_a0, _a1) + } else { + r0 = ret.Error(0) + } + + return r0 +} From 6a94b55d1293ade5af361535f84bd03b708ddac3 Mon Sep 17 00:00:00 2001 From: Sam Kleinman Date: Wed, 28 Jul 2021 14:20:40 -0400 Subject: [PATCH 2/6] rpc: add documentation for genesis chunked api (#6776) --- CHANGELOG_PENDING.md | 1 + rpc/openapi/openapi.yaml | 65 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index de3a18ab6..9de5b8bcb 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -24,6 +24,7 @@ Friendly reminder: We have a [bug bounty program](https://hackerone.com/tendermi - [fastsync/rpc] \#6620 Add TotalSyncedTime & RemainingTime to SyncInfo in /status RPC (@JayT106) - [rpc/grpc] \#6725 Mark gRPC in the RPC layer as deprecated. - [blockchain/v2] \#6730 Fast Sync v2 is deprecated, please use v0 + - [rpc] Add genesis_chunked method to support paginated and parallel fetching of large genesis documents. - Apps - [ABCI] \#6408 Change the `key` and `value` fields from `[]byte` to `string` in the `EventAttribute` type. (@alexanderbez) diff --git a/rpc/openapi/openapi.yaml b/rpc/openapi/openapi.yaml index a2cbd62da..bb35d34ac 100644 --- a/rpc/openapi/openapi.yaml +++ b/rpc/openapi/openapi.yaml @@ -806,6 +806,7 @@ paths: application/json: schema: $ref: "#/components/schemas/ErrorResponse" + /genesis: get: summary: Get Genesis @@ -813,7 +814,7 @@ paths: tags: - Info description: | - Get genesis. + Get the genesis document. responses: "200": description: Genesis results. @@ -827,6 +828,39 @@ paths: application/json: schema: $ref: "#/components/schemas/ErrorResponse" + + /genesis_chunked: + get: + summary: Get Genesis in paginated chunks + operationId: genesis_chunked + tags: + - Info + description: | + Get genesis document in a paginated/chunked format to make it + easier to iterate through larger gensis structures. + parameters: + - in: query + name: chunkID + description: Sequence number of the chunk to download. + schema: + type: integer + default: 0 + example: 1 + responses: + "200": + description: Genesis results. + content: + application/json: + schema: + $ref: "#/components/schemas/GenesisChunkedResponse" + "500": + description: Error + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" + + /dump_consensus_state: get: summary: Get consensus state @@ -1894,6 +1928,35 @@ components: properties: {} type: object + GenesisChunkedResponse: + type: object + required: + - "jsonrpc" + - "id" + - "result" + properties: + jsonrpc: + type: string + example: "2.0" + id: + type: integer + example: 0 + result: + required: + - "chunk" + - "total" + - "data" + properties: + chunk: + type: integer + example: 0 + total: + type: integer + example: 1 + data: + type: string + example: "Z2VuZXNpcwo=" + DumpConsensusResponse: type: object required: From 8f06e0c9e7b76b7705c40d3a6e489276d65910f3 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Wed, 28 Jul 2021 12:38:46 -0700 Subject: [PATCH 3/6] cleanup: remove redundant error plumbing (#6778) This is a mostly-automated fixup using Comby (https://comby.dev) to remove lexically-obvious redundant error checks. No functional changes are intended. To reproduce the core change: # Collapse redundant error check conditionals % comby -in-place 'if err != nil { return err } return nil' 'return err' .go # Fold out unnecessary error temporaries % comby -in-place ':[spc~^\s*]err :[~:?]= :[any] return err' ':[spc]return :[any]' .go Fixes #6479 and related cases. --- abci/types/messages.go | 6 +----- privval/signer_listener_endpoint.go | 7 +------ state/indexer/sink/psql/psql_test.go | 7 +------ state/store.go | 7 +------ test/e2e/runner/cleanup.go | 20 +++----------------- test/e2e/runner/wait.go | 5 +---- test/fuzz/rpc/jsonrpc/server/handler.go | 3 +-- types/node_key.go | 6 +----- 8 files changed, 10 insertions(+), 51 deletions(-) diff --git a/abci/types/messages.go b/abci/types/messages.go index e0605c4e5..74f3cc75c 100644 --- a/abci/types/messages.go +++ b/abci/types/messages.go @@ -15,11 +15,7 @@ const ( func WriteMessage(msg proto.Message, w io.Writer) error { protoWriter := protoio.NewDelimitedWriter(w) _, err := protoWriter.WriteMsg(msg) - if err != nil { - return err - } - - return nil + return err } // ReadMessage reads a varint length-delimited protobuf message. diff --git a/privval/signer_listener_endpoint.go b/privval/signer_listener_endpoint.go index 16ffc5c98..292e7a476 100644 --- a/privval/signer_listener_endpoint.go +++ b/privval/signer_listener_endpoint.go @@ -142,12 +142,7 @@ func (sl *SignerListenerEndpoint) ensureConnection(maxWait time.Duration) error // block until connected or timeout sl.Logger.Info("SignerListener: Blocking for connection") sl.triggerConnect() - err := sl.WaitConnection(sl.connectionAvailableCh, maxWait) - if err != nil { - return err - } - - return nil + return sl.WaitConnection(sl.connectionAvailableCh, maxWait) } func (sl *SignerListenerEndpoint) acceptNewConnection() (net.Conn, error) { diff --git a/state/indexer/sink/psql/psql_test.go b/state/indexer/sink/psql/psql_test.go index ee1af5a5f..0df773a53 100644 --- a/state/indexer/sink/psql/psql_test.go +++ b/state/indexer/sink/psql/psql_test.go @@ -255,12 +255,7 @@ func verifyTimeStamp(tb string) error { if rows.Next() { var ts string - err = rows.Scan(&ts) - if err != nil { - return err - } - - return nil + return rows.Scan(&ts) } return errors.New("no result") diff --git a/state/store.go b/state/store.go index 84b19a685..0ecd888ca 100644 --- a/state/store.go +++ b/state/store.go @@ -661,10 +661,5 @@ func (store dbStore) saveConsensusParamsInfo( return err } - err = batch.Set(consensusParamsKey(nextHeight), bz) - if err != nil { - return err - } - - return nil + return batch.Set(consensusParamsKey(nextHeight), bz) } diff --git a/test/e2e/runner/cleanup.go b/test/e2e/runner/cleanup.go index d99ca54cf..d17c75075 100644 --- a/test/e2e/runner/cleanup.go +++ b/test/e2e/runner/cleanup.go @@ -15,11 +15,7 @@ func Cleanup(testnet *e2e.Testnet) error { if err != nil { return err } - err = cleanupDir(testnet.Dir) - if err != nil { - return err - } - return nil + return cleanupDir(testnet.Dir) } // cleanupDocker removes all E2E resources (with label e2e=True), regardless @@ -37,13 +33,8 @@ func cleanupDocker() error { return err } - err = exec("bash", "-c", fmt.Sprintf( + return exec("bash", "-c", fmt.Sprintf( "docker network ls -q --filter label=e2e | xargs %v docker network rm", xargsR)) - if err != nil { - return err - } - - return nil } // cleanupDir cleans up a testnet directory @@ -74,10 +65,5 @@ func cleanupDir(dir string) error { return err } - err = os.RemoveAll(dir) - if err != nil { - return err - } - - return nil + return os.RemoveAll(dir) } diff --git a/test/e2e/runner/wait.go b/test/e2e/runner/wait.go index 4c16fb808..9f3a4c438 100644 --- a/test/e2e/runner/wait.go +++ b/test/e2e/runner/wait.go @@ -21,10 +21,7 @@ func Wait(testnet *e2e.Testnet, blocks int64) error { func WaitUntil(testnet *e2e.Testnet, height int64) error { logger.Info(fmt.Sprintf("Waiting for all nodes to reach height %v...", height)) _, err := waitForAllNodes(testnet, height, waitingTime(len(testnet.Nodes))) - if err != nil { - return err - } - return nil + return err } // waitingTime estimates how long it should take for a node to reach the height. diff --git a/test/fuzz/rpc/jsonrpc/server/handler.go b/test/fuzz/rpc/jsonrpc/server/handler.go index eed18ceff..08f7e2b6b 100644 --- a/test/fuzz/rpc/jsonrpc/server/handler.go +++ b/test/fuzz/rpc/jsonrpc/server/handler.go @@ -59,6 +59,5 @@ func Fuzz(data []byte) int { func outputJSONIsSlice(input []byte) bool { slice := []interface{}{} - err := json.Unmarshal(input, &slice) - return err == nil + return json.Unmarshal(input, &slice) == nil } diff --git a/types/node_key.go b/types/node_key.go index b8277649a..547fa1696 100644 --- a/types/node_key.go +++ b/types/node_key.go @@ -33,11 +33,7 @@ func (nodeKey NodeKey) SaveAs(filePath string) error { if err != nil { return err } - err = ioutil.WriteFile(filePath, jsonBytes, 0600) - if err != nil { - return err - } - return nil + return ioutil.WriteFile(filePath, jsonBytes, 0600) } // LoadOrGenNodeKey attempts to load the NodeKey from the given filePath. If From 9a2a7d43073d678c4dc24747666e0508f3003bc7 Mon Sep 17 00:00:00 2001 From: JayT106 Date: Thu, 29 Jul 2021 06:52:53 -0400 Subject: [PATCH 4/6] state/privval: vote timestamp fix (#6748) --- internal/consensus/common_test.go | 29 ++++++++++++++++++++++++++++- internal/consensus/state.go | 1 + internal/consensus/state_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/internal/consensus/common_test.go b/internal/consensus/common_test.go index 26170f3bc..17ba1ce2e 100644 --- a/internal/consensus/common_test.go +++ b/internal/consensus/common_test.go @@ -88,6 +88,7 @@ type validatorStub struct { Round int32 types.PrivValidator VotingPower int64 + lastVote *types.Vote } const testMinPower int64 = 10 @@ -121,8 +122,18 @@ func (vs *validatorStub) signVote( BlockID: types.BlockID{Hash: hash, PartSetHeader: header}, } v := vote.ToProto() - err = vs.PrivValidator.SignVote(context.Background(), config.ChainID(), v) + if err := vs.PrivValidator.SignVote(context.Background(), config.ChainID(), v); err != nil { + return nil, fmt.Errorf("sign vote failed: %w", err) + } + + // ref: signVote in FilePV, the vote should use the privious vote info when the sign data is the same. + if signDataIsEqual(vs.lastVote, v) { + v.Signature = vs.lastVote.Signature + v.Timestamp = vs.lastVote.Timestamp + } + vote.Signature = v.Signature + vote.Timestamp = v.Timestamp return vote, err } @@ -139,6 +150,9 @@ func signVote( if err != nil { panic(fmt.Errorf("failed to sign vote: %v", err)) } + + vs.lastVote = v + return v } @@ -876,3 +890,16 @@ func newKVStore() abci.Application { func newPersistentKVStoreWithPath(dbDir string) abci.Application { return kvstore.NewPersistentKVStoreApplication(dbDir) } + +func signDataIsEqual(v1 *types.Vote, v2 *tmproto.Vote) bool { + if v1 == nil || v2 == nil { + return false + } + + return v1.Type == v2.Type && + bytes.Equal(v1.BlockID.Hash, v2.BlockID.GetHash()) && + v1.Height == v2.GetHeight() && + v1.Round == v2.Round && + bytes.Equal(v1.ValidatorAddress.Bytes(), v2.GetValidatorAddress()) && + v1.ValidatorIndex == v2.GetValidatorIndex() +} diff --git a/internal/consensus/state.go b/internal/consensus/state.go index a20f488e4..f7e44c600 100644 --- a/internal/consensus/state.go +++ b/internal/consensus/state.go @@ -2218,6 +2218,7 @@ func (cs *State) signVote( err := cs.privValidator.SignVote(ctx, cs.state.ChainID, v) vote.Signature = v.Signature + vote.Timestamp = v.Timestamp return vote, err } diff --git a/internal/consensus/state_test.go b/internal/consensus/state_test.go index b088e2ac7..b3b7c81a3 100644 --- a/internal/consensus/state_test.go +++ b/internal/consensus/state_test.go @@ -1939,6 +1939,30 @@ func TestStateOutputVoteStats(t *testing.T) { } +func TestSignSameVoteTwice(t *testing.T) { + config := configSetup(t) + + _, vss := randState(config, 2) + + randBytes := tmrand.Bytes(tmhash.Size) + + vote := signVote(vss[1], + config, + tmproto.PrecommitType, + randBytes, + types.PartSetHeader{Total: 10, Hash: randBytes}, + ) + + vote2 := signVote(vss[1], + config, + tmproto.PrecommitType, + randBytes, + types.PartSetHeader{Total: 10, Hash: randBytes}, + ) + + require.Equal(t, vote, vote2) +} + // subscribe subscribes test client to the given query and returns a channel with cap = 1. func subscribe(eventBus *types.EventBus, q tmpubsub.Query) <-chan tmpubsub.Message { sub, err := eventBus.Subscribe(context.Background(), testSubscriber, q) From 6dd8984fef8742ec6e5481040c159941c9d0a819 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Thu, 29 Jul 2021 19:28:32 -0700 Subject: [PATCH 5/6] Fix and clarify breaks from select cases. (#6781) Update those break statements inside case clauses that are intended to reach an enclosing for loop, so that they correctly exit the loop. The candidate files for this change were located using: % staticcheck -checks SA4011 ./... | cut -d: -f-2 This change is intended to preserve the intended semantics of the code, but since the code as-written did not have its intended effect, some behaviour may change. Specifically: Some loops may have run longer than they were supposed to, prior to this change. In one case I was not able to clearly determine the intended outcome. That case has been commented but otherwise left as-written. Fixes #6780. --- crypto/merkle/proof.go | 5 ++++- internal/p2p/peermanager.go | 2 +- state/indexer/block/kv/kv.go | 15 +++++++++------ state/indexer/tx/kv/kv.go | 18 +++++++++++------- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/crypto/merkle/proof.go b/crypto/merkle/proof.go index 2994e8048..80b289d23 100644 --- a/crypto/merkle/proof.go +++ b/crypto/merkle/proof.go @@ -204,7 +204,10 @@ func (spn *ProofNode) FlattenAunts() [][]byte { case spn.Right != nil: innerHashes = append(innerHashes, spn.Right.Hash) default: - break + // FIXME(fromberger): Per the documentation above, exactly one of + // these fields should be set. If that is true, this should probably + // be a panic since it violates the invariant. If not, when can it + // be OK to have no siblings? Does this occur at the leaves? } spn = spn.Parent } diff --git a/internal/p2p/peermanager.go b/internal/p2p/peermanager.go index fd6f96933..1e9afb38b 100644 --- a/internal/p2p/peermanager.go +++ b/internal/p2p/peermanager.go @@ -385,7 +385,7 @@ func (m *PeerManager) prunePeers() error { peerID := ranked[i].ID switch { case m.store.Size() <= int(m.options.MaxPeers): - break + return nil case m.dialing[peerID]: case m.connected[peerID]: default: diff --git a/state/indexer/block/kv/kv.go b/state/indexer/block/kv/kv.go index 1787be9ef..bc90eadf5 100644 --- a/state/indexer/block/kv/kv.go +++ b/state/indexer/block/kv/kv.go @@ -186,6 +186,7 @@ func (idx *BlockerIndexer) Search(ctx context.Context, q *query.Query) ([]int64, // fetch matching heights results = make([]int64, 0, len(filteredHeights)) +heights: for _, hBz := range filteredHeights { h := int64FromBytes(hBz) @@ -199,7 +200,7 @@ func (idx *BlockerIndexer) Search(ctx context.Context, q *query.Query) ([]int64, select { case <-ctx.Done(): - break + break heights default: } @@ -240,7 +241,7 @@ func (idx *BlockerIndexer) matchRange( } defer it.Close() -LOOP: +iter: for ; it.Valid(); it.Next() { var ( eventValue string @@ -260,7 +261,7 @@ LOOP: if _, ok := qr.AnyBound().(int64); ok { v, err := strconv.ParseInt(eventValue, 10, 64) if err != nil { - continue LOOP + continue iter } include := true @@ -279,7 +280,7 @@ LOOP: select { case <-ctx.Done(): - break + break iter default: } @@ -372,12 +373,13 @@ func (idx *BlockerIndexer) match( } defer it.Close() + iterExists: for ; it.Valid(); it.Next() { tmpHeights[string(it.Value())] = it.Value() select { case <-ctx.Done(): - break + break iterExists default: } @@ -399,6 +401,7 @@ func (idx *BlockerIndexer) match( } defer it.Close() + iterContains: for ; it.Valid(); it.Next() { eventValue, err := parseValueFromEventKey(it.Key()) if err != nil { @@ -411,7 +414,7 @@ func (idx *BlockerIndexer) match( select { case <-ctx.Done(): - break + break iterContains default: } diff --git a/state/indexer/tx/kv/kv.go b/state/indexer/tx/kv/kv.go index 5d310eea7..080dbce2c 100644 --- a/state/indexer/tx/kv/kv.go +++ b/state/indexer/tx/kv/kv.go @@ -219,6 +219,7 @@ func (txi *TxIndex) Search(ctx context.Context, q *query.Query) ([]*abci.TxResul } results := make([]*abci.TxResult, 0, len(filteredHashes)) +hashes: for _, h := range filteredHashes { res, err := txi.Get(h) if err != nil { @@ -229,7 +230,7 @@ func (txi *TxIndex) Search(ctx context.Context, q *query.Query) ([]*abci.TxResul // Potentially exit early. select { case <-ctx.Done(): - break + break hashes default: } } @@ -285,13 +286,14 @@ func (txi *TxIndex) match( } defer it.Close() + iterEqual: for ; it.Valid(); it.Next() { tmpHashes[string(it.Value())] = it.Value() // Potentially exit early. select { case <-ctx.Done(): - break + break iterEqual default: } } @@ -308,13 +310,14 @@ func (txi *TxIndex) match( } defer it.Close() + iterExists: for ; it.Valid(); it.Next() { tmpHashes[string(it.Value())] = it.Value() // Potentially exit early. select { case <-ctx.Done(): - break + break iterExists default: } } @@ -332,6 +335,7 @@ func (txi *TxIndex) match( } defer it.Close() + iterContains: for ; it.Valid(); it.Next() { value, err := parseValueFromKey(it.Key()) if err != nil { @@ -344,7 +348,7 @@ func (txi *TxIndex) match( // Potentially exit early. select { case <-ctx.Done(): - break + break iterContains default: } } @@ -412,7 +416,7 @@ func (txi *TxIndex) matchRange( } defer it.Close() -LOOP: +iter: for ; it.Valid(); it.Next() { value, err := parseValueFromKey(it.Key()) if err != nil { @@ -421,7 +425,7 @@ LOOP: if _, ok := qr.AnyBound().(int64); ok { v, err := strconv.ParseInt(value, 10, 64) if err != nil { - continue LOOP + continue iter } include := true @@ -448,7 +452,7 @@ LOOP: // Potentially exit early. select { case <-ctx.Done(): - break + break iter default: } } From 3aec71cdd4fdeab122f1ad8ca18ec076034814c1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 30 Jul 2021 09:21:27 -0400 Subject: [PATCH 6/6] build(deps): Bump styfle/cancel-workflow-action from 0.9.0 to 0.9.1 (#6786) --- .github/workflows/janitor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/janitor.yml b/.github/workflows/janitor.yml index ccacb6eeb..e6bc45ec1 100644 --- a/.github/workflows/janitor.yml +++ b/.github/workflows/janitor.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 3 steps: - - uses: styfle/cancel-workflow-action@0.9.0 + - uses: styfle/cancel-workflow-action@0.9.1 with: workflow_id: 1041851,1401230,2837803 access_token: ${{ github.token }}