From 6dbc09ce2c3ba294fbecdc08a8d95e464add2c27 Mon Sep 17 00:00:00 2001 From: William Banfield Date: Wed, 17 Aug 2022 11:59:04 -0400 Subject: [PATCH] tests pass --- consensus/mempool_test.go | 10 +-- state/execution_test.go | 138 ++---------------------------- types/tx_test.go | 174 -------------------------------------- 3 files changed, 11 insertions(+), 311 deletions(-) diff --git a/consensus/mempool_test.go b/consensus/mempool_test.go index 87321cdda..fdb0045e3 100644 --- a/consensus/mempool_test.go +++ b/consensus/mempool_test.go @@ -259,20 +259,16 @@ func (app *CounterApplication) Commit() abci.ResponseCommit { func (app *CounterApplication) PrepareProposal( req abci.RequestPrepareProposal) abci.ResponsePrepareProposal { - - trs := make([]*abci.TxRecord, 0, len(req.Txs)) + txs := make([][]byte, 0, len(req.Txs)) var totalBytes int64 for _, tx := range req.Txs { totalBytes += int64(len(tx)) if totalBytes > req.MaxTxBytes { break } - trs = append(trs, &abci.TxRecord{ - Action: abci.TxRecord_UNMODIFIED, - Tx: tx, - }) + txs = append(txs, tx) } - return abci.ResponsePrepareProposal{TxRecords: trs} + return abci.ResponsePrepareProposal{Txs: txs} } func (app *CounterApplication) ProcessProposal( diff --git a/state/execution_test.go b/state/execution_test.go index 1e7feaed9..2a026adc4 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -614,112 +614,8 @@ func TestEmptyPrepareProposal(t *testing.T) { require.NoError(t, err) } -// TestPrepareProposalErrorOnNonExistingRemoved tests that the block creation logic returns -// an error if the ResponsePrepareProposal returned from the application marks -// a transaction as REMOVED that was not present in the original proposal. -func TestPrepareProposalErrorOnNonExistingRemoved(t *testing.T) { - const height = 2 - - state, stateDB, privVals := makeState(1, height) - stateStore := sm.NewStore(stateDB) - - evpool := &mocks.EvidencePool{} - evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0)) - - mp := &mpmocks.Mempool{} - mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs{}) - - // create an invalid ResponsePrepareProposal - rpp := abci.ResponsePrepareProposal{ - TxRecords: []*abci.TxRecord{ - { - Action: abci.TxRecord_REMOVED, - Tx: []byte("new tx"), - }, - }, - } - - app := abcimocks.NewBaseMock() - app.On("PrepareProposal", mock.Anything).Return(rpp) - - cc := proxy.NewLocalClientCreator(app) - proxyApp := proxy.NewAppConns(cc, proxy.NopMetrics()) - err := proxyApp.Start() - require.NoError(t, err) - defer proxyApp.Stop() //nolint:errcheck // ignore for tests - - blockExec := sm.NewBlockExecutor( - stateStore, - log.TestingLogger(), - proxyApp.Consensus(), - mp, - evpool, - ) - pa, _ := state.Validators.GetByIndex(0) - commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) - require.NoError(t, err) - block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil) - require.Nil(t, block) - require.ErrorContains(t, err, "new transaction incorrectly marked as removed") - - mp.AssertExpectations(t) -} - -// TestPrepareProposalRemoveTxs tests that any transactions marked as REMOVED -// are not included in the block produced by CreateProposalBlock. The test also -// ensures that any transactions removed are also removed from the mempool. -func TestPrepareProposalRemoveTxs(t *testing.T) { - const height = 2 - - state, stateDB, privVals := makeState(1, height) - stateStore := sm.NewStore(stateDB) - - evpool := &mocks.EvidencePool{} - evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, int64(0)) - - txs := factory.MakeNTxs(height, 10) - mp := &mpmocks.Mempool{} - mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs(txs)) - - trs := txsToTxRecords(types.Txs(txs)) - trs[0].Action = abci.TxRecord_REMOVED - trs[1].Action = abci.TxRecord_REMOVED - mp.On("RemoveTxByKey", mock.Anything).Return(nil).Twice() - - app := abcimocks.NewBaseMock() - app.On("PrepareProposal", mock.Anything).Return(abci.ResponsePrepareProposal{ - TxRecords: trs, - }) - cc := proxy.NewLocalClientCreator(app) - proxyApp := proxy.NewAppConns(cc, proxy.NopMetrics()) - err := proxyApp.Start() - require.NoError(t, err) - defer proxyApp.Stop() //nolint:errcheck // ignore for tests - - blockExec := sm.NewBlockExecutor( - stateStore, - log.TestingLogger(), - proxyApp.Consensus(), - mp, - evpool, - ) - pa, _ := state.Validators.GetByIndex(0) - commit, err := makeValidCommit(height, types.BlockID{}, state.Validators, privVals) - require.NoError(t, err) - block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil) - require.NoError(t, err) - require.Len(t, block.Data.Txs.ToSliceOfBytes(), len(trs)-2) - - require.Equal(t, -1, block.Data.Txs.Index(types.Tx(trs[0].Tx))) - require.Equal(t, -1, block.Data.Txs.Index(types.Tx(trs[1].Tx))) - - mp.AssertCalled(t, "RemoveTxByKey", types.Tx(trs[0].Tx).Key()) - mp.AssertCalled(t, "RemoveTxByKey", types.Tx(trs[1].Tx).Key()) - mp.AssertExpectations(t) -} - -// TestPrepareProposalAddedTxsIncluded tests that any transactions marked as ADDED -// in the prepare proposal response are included in the block. +// TestPrepareProposalAddedTxsIncluded tests that any transactions included in +// the prepare proposal response are included in the block. func TestPrepareProposalAddedTxsIncluded(t *testing.T) { const height = 2 @@ -733,13 +629,9 @@ func TestPrepareProposalAddedTxsIncluded(t *testing.T) { mp := &mpmocks.Mempool{} mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs(txs[2:])) - trs := txsToTxRecords(types.Txs(txs)) - trs[0].Action = abci.TxRecord_ADDED - trs[1].Action = abci.TxRecord_ADDED - app := abcimocks.NewBaseMock() app.On("PrepareProposal", mock.Anything).Return(abci.ResponsePrepareProposal{ - TxRecords: trs, + Txs: types.Txs(txs).ToSliceOfBytes(), }) cc := proxy.NewLocalClientCreator(app) proxyApp := proxy.NewAppConns(cc, proxy.NopMetrics()) @@ -781,13 +673,12 @@ func TestPrepareProposalReorderTxs(t *testing.T) { mp := &mpmocks.Mempool{} mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs(txs)) - trs := txsToTxRecords(types.Txs(txs)) - trs = trs[2:] - trs = append(trs[len(trs)/2:], trs[:len(trs)/2]...) + txs = txs[2:] + txs = append(txs[len(txs)/2:], txs[:len(txs)/2]...) app := abcimocks.NewBaseMock() app.On("PrepareProposal", mock.Anything).Return(abci.ResponsePrepareProposal{ - TxRecords: trs, + Txs: types.Txs(txs).ToSliceOfBytes(), }) cc := proxy.NewLocalClientCreator(app) @@ -809,7 +700,7 @@ func TestPrepareProposalReorderTxs(t *testing.T) { block, err := blockExec.CreateProposalBlock(height, state, commit, pa, nil) require.NoError(t, err) for i, tx := range block.Data.Txs { - require.Equal(t, types.Tx(trs[i].Tx), tx) + require.Equal(t, types.Tx(txs[i]), tx) } mp.AssertExpectations(t) @@ -836,11 +727,9 @@ func TestPrepareProposalErrorOnTooManyTxs(t *testing.T) { mp := &mpmocks.Mempool{} mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything).Return(types.Txs(txs)) - trs := txsToTxRecords(types.Txs(txs)) - app := abcimocks.NewBaseMock() app.On("PrepareProposal", mock.Anything).Return(abci.ResponsePrepareProposal{ - TxRecords: trs, + Txs: types.Txs(txs).ToSliceOfBytes(), }) cc := proxy.NewLocalClientCreator(app) @@ -928,14 +817,3 @@ func makeBlockID(hash []byte, partSetSize uint32, partSetHash []byte) types.Bloc }, } } - -func txsToTxRecords(txs []types.Tx) []*abci.TxRecord { - trs := make([]*abci.TxRecord, len(txs)) - for i, tx := range txs { - trs[i] = &abci.TxRecord{ - Action: abci.TxRecord_UNMODIFIED, - Tx: tx, - } - } - return trs -} diff --git a/types/tx_test.go b/types/tx_test.go index a84b0bf36..bb53e0ea5 100644 --- a/types/tx_test.go +++ b/types/tx_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - abci "github.com/tendermint/tendermint/abci/types" tmrand "github.com/tendermint/tendermint/libs/rand" ctest "github.com/tendermint/tendermint/libs/test" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" @@ -48,179 +47,6 @@ func TestTxIndexByHash(t *testing.T) { } } -func TestValidateTxRecordSet(t *testing.T) { - t.Run("should error on total transaction size exceeding max data size", func(t *testing.T) { - trs := []*abci.TxRecord{ - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{6, 7, 8, 9, 10}), - }, - } - txrSet := NewTxRecordSet(trs) - err := txrSet.Validate(9, []Tx{}) - require.Error(t, err) - }) - t.Run("should not error on removed transaction size exceeding max data size", func(t *testing.T) { - trs := []*abci.TxRecord{ - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{6, 7, 8, 9}), - }, - { - Action: abci.TxRecord_REMOVED, - Tx: Tx([]byte{10}), - }, - } - txrSet := NewTxRecordSet(trs) - err := txrSet.Validate(9, []Tx{[]byte{10}}) - require.NoError(t, err) - }) - t.Run("should error on duplicate transactions with the same action", func(t *testing.T) { - trs := []*abci.TxRecord{ - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{100}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{200}), - }, - } - txrSet := NewTxRecordSet(trs) - err := txrSet.Validate(100, []Tx{}) - require.Error(t, err) - }) - t.Run("should error on duplicate transactions with mixed actions", func(t *testing.T) { - trs := []*abci.TxRecord{ - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{100}), - }, - { - Action: abci.TxRecord_REMOVED, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{200}), - }, - } - txrSet := NewTxRecordSet(trs) - err := txrSet.Validate(100, []Tx{}) - require.Error(t, err) - }) - t.Run("should error on new transactions marked UNMODIFIED", func(t *testing.T) { - trs := []*abci.TxRecord{ - { - Action: abci.TxRecord_UNMODIFIED, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - } - txrSet := NewTxRecordSet(trs) - err := txrSet.Validate(100, []Tx{}) - require.Error(t, err) - }) - t.Run("should error on new transactions marked REMOVED", func(t *testing.T) { - trs := []*abci.TxRecord{ - { - Action: abci.TxRecord_REMOVED, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - } - txrSet := NewTxRecordSet(trs) - err := txrSet.Validate(100, []Tx{}) - require.Error(t, err) - }) - t.Run("should error on existing transaction marked as ADDED", func(t *testing.T) { - trs := []*abci.TxRecord{ - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{5, 4, 3, 2, 1}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{6}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - } - txrSet := NewTxRecordSet(trs) - err := txrSet.Validate(100, []Tx{{0}, {1, 2, 3, 4, 5}}) - require.Error(t, err) - }) - t.Run("should error if any transaction marked as UNKNOWN", func(t *testing.T) { - trs := []*abci.TxRecord{ - { - Action: abci.TxRecord_UNKNOWN, - Tx: Tx([]byte{1, 2, 3, 4, 5}), - }, - } - txrSet := NewTxRecordSet(trs) - err := txrSet.Validate(100, []Tx{}) - require.Error(t, err) - }) - t.Run("TxRecordSet preserves order", func(t *testing.T) { - trs := []*abci.TxRecord{ - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{100}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{99}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{55}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{12}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{66}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{9}), - }, - { - Action: abci.TxRecord_ADDED, - Tx: Tx([]byte{17}), - }, - } - txrSet := NewTxRecordSet(trs) - err := txrSet.Validate(100, []Tx{}) - require.NoError(t, err) - for i, tx := range txrSet.IncludedTxs() { - require.Equal(t, Tx(trs[i].Tx), tx) - } - }) -} - func TestValidTxProof(t *testing.T) { cases := []struct { txs Txs