From 12347fca7bd9001f9c1996f52a55c140f19408e9 Mon Sep 17 00:00:00 2001 From: Arham Jain Date: Fri, 1 Mar 2024 11:30:43 +0530 Subject: [PATCH 1/4] test(ut): added unit tests to check the existing behaviour for manager_test.go refactor(manager): update test cases and refactored da submission method fix(revert): da changes to observe breaking test cases --- block/manager_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/block/manager_test.go b/block/manager_test.go index 7c9721fda8..14eeab4cf4 100644 --- a/block/manager_test.go +++ b/block/manager_test.go @@ -240,6 +240,48 @@ func TestSubmitBlocksToDA(t *testing.T) { isErrExpected: true, expectedPendingBlocksLength: 1, }, + { + name: "A and B are fair blocks, but C has a marshalling error. So C never gets submitted", + blocks: func() []*types.Block { + numBlocks, numTxs := 3, 5 + blocks := make([]*types.Block, numBlocks) + blocks[0] = types.GetRandomBlock(uint64(1), numTxs) + blocks[1] = types.GetRandomBlock(uint64(2), numTxs) + blocks[2] = types.GetRandomBlock(uint64(3), numTxs) + invalidateBlockHeader(blocks[2]) + return blocks + }(), + isErrExpected: true, + expectedPendingBlocksLength: 1, + }, + { + name: "A is too big on its own. So A, B and C never get submitted", + blocks: func() []*types.Block { + numBlocks, numTxs := 3, 5 + blocks := make([]*types.Block, numBlocks) + blocks[0], err = getBlockBiggerThan(1, maxDABlobSizeLimit) + require.NoError(err) + blocks[1] = types.GetRandomBlock(uint64(2), numTxs) + blocks[2] = types.GetRandomBlock(uint64(3), numTxs) + return blocks + }(), + isErrExpected: true, + expectedPendingBlocksLength: 3, + }, + { + name: "A itself has a marshalling error. So A, B and C never get submitted", + blocks: func() []*types.Block { + numBlocks, numTxs := 3, 5 + blocks := make([]*types.Block, numBlocks) + blocks[0] = types.GetRandomBlock(uint64(1), numTxs) + invalidateBlockHeader(blocks[0]) + blocks[1] = types.GetRandomBlock(uint64(2), numTxs) + blocks[2] = types.GetRandomBlock(uint64(3), numTxs) + return blocks + }(), + isErrExpected: true, + expectedPendingBlocksLength: 3, + }, } for _, tc := range testCases { @@ -282,6 +324,18 @@ func getTempKVStore(t *testing.T) ds.TxnDatastore { return kvStore } +// invalidateBlockHeader results in a block header that produces a marshalling error +func invalidateBlockHeader(block *types.Block) { + for i := range block.SignedHeader.Validators.Validators { + block.SignedHeader.Validators.Validators[i] = &cmtypes.Validator{ + Address: []byte(""), + PubKey: nil, + VotingPower: -1, + ProposerPriority: 0, + } + } +} + func Test_isProposer(t *testing.T) { require := require.New(t) From e74cc160e4811ddcce6a57aeb7d529f9fa2df7df Mon Sep 17 00:00:00 2001 From: Arham Jain Date: Mon, 11 Mar 2024 11:41:36 +0530 Subject: [PATCH 2/4] fix(ut): block_manager submit blocks to DA --- block/manager_test.go | 32 ++------------------------------ 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/block/manager_test.go b/block/manager_test.go index 14eeab4cf4..34dac8d9bb 100644 --- a/block/manager_test.go +++ b/block/manager_test.go @@ -241,41 +241,13 @@ func TestSubmitBlocksToDA(t *testing.T) { expectedPendingBlocksLength: 1, }, { - name: "A and B are fair blocks, but C has a marshalling error. So C never gets submitted", + name: "B is too big on its own. So A gets submitted but, B and C never get submitted", blocks: func() []*types.Block { numBlocks, numTxs := 3, 5 blocks := make([]*types.Block, numBlocks) blocks[0] = types.GetRandomBlock(uint64(1), numTxs) - blocks[1] = types.GetRandomBlock(uint64(2), numTxs) - blocks[2] = types.GetRandomBlock(uint64(3), numTxs) - invalidateBlockHeader(blocks[2]) - return blocks - }(), - isErrExpected: true, - expectedPendingBlocksLength: 1, - }, - { - name: "A is too big on its own. So A, B and C never get submitted", - blocks: func() []*types.Block { - numBlocks, numTxs := 3, 5 - blocks := make([]*types.Block, numBlocks) - blocks[0], err = getBlockBiggerThan(1, maxDABlobSizeLimit) + blocks[1], err = getBlockBiggerThan(2, maxDABlobSizeLimit) require.NoError(err) - blocks[1] = types.GetRandomBlock(uint64(2), numTxs) - blocks[2] = types.GetRandomBlock(uint64(3), numTxs) - return blocks - }(), - isErrExpected: true, - expectedPendingBlocksLength: 3, - }, - { - name: "A itself has a marshalling error. So A, B and C never get submitted", - blocks: func() []*types.Block { - numBlocks, numTxs := 3, 5 - blocks := make([]*types.Block, numBlocks) - blocks[0] = types.GetRandomBlock(uint64(1), numTxs) - invalidateBlockHeader(blocks[0]) - blocks[1] = types.GetRandomBlock(uint64(2), numTxs) blocks[2] = types.GetRandomBlock(uint64(3), numTxs) return blocks }(), From 672bef3d49197e62d5fade861adc2258ff4608f5 Mon Sep 17 00:00:00 2001 From: Arham Jain Date: Mon, 11 Mar 2024 11:48:18 +0530 Subject: [PATCH 3/4] fix(ut): fix expected result --- block/manager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/manager_test.go b/block/manager_test.go index 34dac8d9bb..619d3a64c1 100644 --- a/block/manager_test.go +++ b/block/manager_test.go @@ -252,7 +252,7 @@ func TestSubmitBlocksToDA(t *testing.T) { return blocks }(), isErrExpected: true, - expectedPendingBlocksLength: 3, + expectedPendingBlocksLength: 2, }, } From 6d8c3071908bc2ea162245d31896f415985cbf7d Mon Sep 17 00:00:00 2001 From: Arham Jain Date: Wed, 13 Mar 2024 16:48:03 +0530 Subject: [PATCH 4/4] feat(ut): added serialisation err UT back to manager_test --- Makefile | 1 + block/manager_test.go | 67 ++++++++ test/mocks/Store.go | 355 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 423 insertions(+) create mode 100644 test/mocks/Store.go diff --git a/Makefile b/Makefile index 69d461a1cd..e174bafe35 100644 --- a/Makefile +++ b/Makefile @@ -84,6 +84,7 @@ mock-gen: mockery --output test/mocks --srcpkg github.com/cometbft/cometbft/rpc/client --name Client mockery --output test/mocks --srcpkg github.com/cometbft/cometbft/abci/types --name Application mockery --output test/mocks --srcpkg github.com/rollkit/go-da --name DA + mockery --output test/mocks --srcpkg github.com/rollkit/rollkit/store --name Store .PHONY: mock-gen diff --git a/block/manager_test.go b/block/manager_test.go index 619d3a64c1..bfe291ec40 100644 --- a/block/manager_test.go +++ b/block/manager_test.go @@ -22,6 +22,7 @@ import ( "github.com/rollkit/rollkit/da/mock" "github.com/rollkit/rollkit/store" test "github.com/rollkit/rollkit/test/log" + "github.com/rollkit/rollkit/test/mocks" "github.com/rollkit/rollkit/types" ) @@ -296,6 +297,72 @@ func getTempKVStore(t *testing.T) ds.TxnDatastore { return kvStore } +// Test_submitBlocksToDA_BlockMarshalErrorCase1: A itself has a marshalling error. So A, B and C never get submitted. +func Test_submitBlocksToDA_BlockMarshalErrorCase1(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + ctx := context.Background() + + m := getManager(t, goDATest.NewDummyDA()) + + block1 := types.GetRandomBlock(uint64(0), 5) + block2 := types.GetRandomBlock(uint64(1), 5) + block3 := types.GetRandomBlock(uint64(2), 5) + + store := mocks.NewStore(t) + invalidateBlockHeader(block1) + store.On("GetMetadata", ctx, LastSubmittedHeightKey).Return(nil, ds.ErrNotFound) + store.On("GetBlock", ctx, uint64(1)).Return(block1, nil) + store.On("GetBlock", ctx, uint64(2)).Return(block2, nil) + store.On("GetBlock", ctx, uint64(3)).Return(block3, nil) + store.On("Height").Return(uint64(3)) + + m.store = store + + var err error + m.pendingBlocks, err = NewPendingBlocks(store, m.logger) + require.NoError(err) + + err = m.submitBlocksToDA(ctx) + assert.ErrorContains(err, "failed to submit all blocks to DA layer") + blocks, err := m.pendingBlocks.getPendingBlocks(ctx) + assert.NoError(err) + assert.Equal(3, len(blocks)) +} + +// Test_submitBlocksToDA_BlockMarshalErrorCase2: A and B are fair blocks, but C has a marshalling error. None of the blocks get submitted to DA layer. +func Test_submitBlocksToDA_BlockMarshalErrorCase2(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + ctx := context.Background() + + m := getManager(t, goDATest.NewDummyDA()) + + block1 := types.GetRandomBlock(uint64(0), 5) + block2 := types.GetRandomBlock(uint64(1), 5) + block3 := types.GetRandomBlock(uint64(2), 5) + + store := mocks.NewStore(t) + invalidateBlockHeader(block3) + store.On("GetMetadata", ctx, LastSubmittedHeightKey).Return(nil, ds.ErrNotFound) + store.On("GetBlock", ctx, uint64(1)).Return(block1, nil) + store.On("GetBlock", ctx, uint64(2)).Return(block2, nil) + store.On("GetBlock", ctx, uint64(3)).Return(block3, nil) + store.On("Height").Return(uint64(3)) + + m.store = store + + var err error + m.pendingBlocks, err = NewPendingBlocks(store, m.logger) + require.NoError(err) + + err = m.submitBlocksToDA(ctx) + assert.ErrorContains(err, "failed to submit all blocks to DA layer") + blocks, err := m.pendingBlocks.getPendingBlocks(ctx) + assert.NoError(err) + assert.Equal(3, len(blocks)) +} + // invalidateBlockHeader results in a block header that produces a marshalling error func invalidateBlockHeader(block *types.Block) { for i := range block.SignedHeader.Validators.Validators { diff --git a/test/mocks/Store.go b/test/mocks/Store.go new file mode 100644 index 0000000000..8a645b54f6 --- /dev/null +++ b/test/mocks/Store.go @@ -0,0 +1,355 @@ +// Code generated by mockery v2.42.0. DO NOT EDIT. + +package mocks + +import ( + context "context" + + abcitypes "github.com/cometbft/cometbft/abci/types" + + header "github.com/celestiaorg/go-header" + + mock "github.com/stretchr/testify/mock" + + types "github.com/rollkit/rollkit/types" +) + +// Store is an autogenerated mock type for the Store type +type Store struct { + mock.Mock +} + +// Close provides a mock function with given fields: +func (_m *Store) Close() error { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for Close") + } + + var r0 error + if rf, ok := ret.Get(0).(func() error); ok { + r0 = rf() + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// GetBlock provides a mock function with given fields: ctx, height +func (_m *Store) GetBlock(ctx context.Context, height uint64) (*types.Block, error) { + ret := _m.Called(ctx, height) + + if len(ret) == 0 { + panic("no return value specified for GetBlock") + } + + var r0 *types.Block + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, uint64) (*types.Block, error)); ok { + return rf(ctx, height) + } + if rf, ok := ret.Get(0).(func(context.Context, uint64) *types.Block); ok { + r0 = rf(ctx, height) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*types.Block) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, uint64) error); ok { + r1 = rf(ctx, height) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetBlockByHash provides a mock function with given fields: ctx, hash +func (_m *Store) GetBlockByHash(ctx context.Context, hash header.Hash) (*types.Block, error) { + ret := _m.Called(ctx, hash) + + if len(ret) == 0 { + panic("no return value specified for GetBlockByHash") + } + + var r0 *types.Block + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, header.Hash) (*types.Block, error)); ok { + return rf(ctx, hash) + } + if rf, ok := ret.Get(0).(func(context.Context, header.Hash) *types.Block); ok { + r0 = rf(ctx, hash) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*types.Block) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, header.Hash) error); ok { + r1 = rf(ctx, hash) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetBlockResponses provides a mock function with given fields: ctx, height +func (_m *Store) GetBlockResponses(ctx context.Context, height uint64) (*abcitypes.ResponseFinalizeBlock, error) { + ret := _m.Called(ctx, height) + + if len(ret) == 0 { + panic("no return value specified for GetBlockResponses") + } + + var r0 *abcitypes.ResponseFinalizeBlock + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, uint64) (*abcitypes.ResponseFinalizeBlock, error)); ok { + return rf(ctx, height) + } + if rf, ok := ret.Get(0).(func(context.Context, uint64) *abcitypes.ResponseFinalizeBlock); ok { + r0 = rf(ctx, height) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*abcitypes.ResponseFinalizeBlock) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, uint64) error); ok { + r1 = rf(ctx, height) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetCommit provides a mock function with given fields: ctx, height +func (_m *Store) GetCommit(ctx context.Context, height uint64) (*types.Commit, error) { + ret := _m.Called(ctx, height) + + if len(ret) == 0 { + panic("no return value specified for GetCommit") + } + + var r0 *types.Commit + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, uint64) (*types.Commit, error)); ok { + return rf(ctx, height) + } + if rf, ok := ret.Get(0).(func(context.Context, uint64) *types.Commit); ok { + r0 = rf(ctx, height) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*types.Commit) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, uint64) error); ok { + r1 = rf(ctx, height) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetCommitByHash provides a mock function with given fields: ctx, hash +func (_m *Store) GetCommitByHash(ctx context.Context, hash header.Hash) (*types.Commit, error) { + ret := _m.Called(ctx, hash) + + if len(ret) == 0 { + panic("no return value specified for GetCommitByHash") + } + + var r0 *types.Commit + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, header.Hash) (*types.Commit, error)); ok { + return rf(ctx, hash) + } + if rf, ok := ret.Get(0).(func(context.Context, header.Hash) *types.Commit); ok { + r0 = rf(ctx, hash) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*types.Commit) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, header.Hash) error); ok { + r1 = rf(ctx, hash) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetMetadata provides a mock function with given fields: ctx, key +func (_m *Store) GetMetadata(ctx context.Context, key string) ([]byte, error) { + ret := _m.Called(ctx, key) + + if len(ret) == 0 { + panic("no return value specified for GetMetadata") + } + + var r0 []byte + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) ([]byte, error)); ok { + return rf(ctx, key) + } + if rf, ok := ret.Get(0).(func(context.Context, string) []byte); ok { + r0 = rf(ctx, key) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]byte) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, key) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetState provides a mock function with given fields: ctx +func (_m *Store) GetState(ctx context.Context) (types.State, error) { + ret := _m.Called(ctx) + + if len(ret) == 0 { + panic("no return value specified for GetState") + } + + var r0 types.State + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) (types.State, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(context.Context) types.State); ok { + r0 = rf(ctx) + } else { + r0 = ret.Get(0).(types.State) + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Height provides a mock function with given fields: +func (_m *Store) Height() uint64 { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for Height") + } + + var r0 uint64 + if rf, ok := ret.Get(0).(func() uint64); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint64) + } + + return r0 +} + +// SaveBlock provides a mock function with given fields: ctx, block, commit +func (_m *Store) SaveBlock(ctx context.Context, block *types.Block, commit *types.Commit) error { + ret := _m.Called(ctx, block, commit) + + if len(ret) == 0 { + panic("no return value specified for SaveBlock") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *types.Block, *types.Commit) error); ok { + r0 = rf(ctx, block, commit) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SaveBlockResponses provides a mock function with given fields: ctx, height, responses +func (_m *Store) SaveBlockResponses(ctx context.Context, height uint64, responses *abcitypes.ResponseFinalizeBlock) error { + ret := _m.Called(ctx, height, responses) + + if len(ret) == 0 { + panic("no return value specified for SaveBlockResponses") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, uint64, *abcitypes.ResponseFinalizeBlock) error); ok { + r0 = rf(ctx, height, responses) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SetHeight provides a mock function with given fields: ctx, height +func (_m *Store) SetHeight(ctx context.Context, height uint64) { + _m.Called(ctx, height) +} + +// SetMetadata provides a mock function with given fields: ctx, key, value +func (_m *Store) SetMetadata(ctx context.Context, key string, value []byte) error { + ret := _m.Called(ctx, key, value) + + if len(ret) == 0 { + panic("no return value specified for SetMetadata") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, []byte) error); ok { + r0 = rf(ctx, key, value) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// UpdateState provides a mock function with given fields: ctx, state +func (_m *Store) UpdateState(ctx context.Context, state types.State) error { + ret := _m.Called(ctx, state) + + if len(ret) == 0 { + panic("no return value specified for UpdateState") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, types.State) error); ok { + r0 = rf(ctx, state) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// NewStore creates a new instance of Store. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewStore(t interface { + mock.TestingT + Cleanup(func()) +}) *Store { + mock := &Store{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +}