From e9ce605aa35e2b1f650ea6663130bdfa50ba9ed1 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Thu, 29 Sep 2022 11:48:39 +0200 Subject: [PATCH] clean up mempool tests --- abci/example/kvstore/helpers.go | 13 +++- abci/example/kvstore/kvstore.go | 6 +- abci/example/kvstore/kvstore_test.go | 11 ++++ mempool/v0/cache_test.go | 18 ++++-- mempool/v0/clist_mempool_test.go | 94 +++++++++++++--------------- mempool/v0/reactor_test.go | 13 ++-- 6 files changed, 88 insertions(+), 67 deletions(-) diff --git a/abci/example/kvstore/helpers.go b/abci/example/kvstore/helpers.go index 91c68ca5a..e521072b0 100644 --- a/abci/example/kvstore/helpers.go +++ b/abci/example/kvstore/helpers.go @@ -49,18 +49,25 @@ func NewTx(key, value string) []byte { return []byte(strings.Join([]string{key, value}, "=")) } -func NewRandomTx() []byte { - return NewTx(rand.Str(5), rand.Str(10)) +func NewRandomTx(size int) []byte { + if size < 4 { + panic("random tx size must be greater than 3") + } + return NewTx(rand.Str(2), rand.Str(size - 3)) } func NewRandomTxs(n int) [][]byte { txs := make([][]byte, n) for i := 0; i < n; i++ { - txs[i] = NewRandomTx() + txs[i] = NewRandomTx(10) } return txs } +func NewTxFromId(i int) []byte { + return []byte(fmt.Sprintf("%d=%d", i)) +} + // Create a transaction to add/remove/update a validator // To remove, set power to 0. func MakeValSetChangeTx(pubkey crypto.PublicKey, power int64) []byte { diff --git a/abci/example/kvstore/kvstore.go b/abci/example/kvstore/kvstore.go index 58f891e84..cb37cd914 100644 --- a/abci/example/kvstore/kvstore.go +++ b/abci/example/kvstore/kvstore.go @@ -119,15 +119,13 @@ func (app *Application) InitChain(_ context.Context, req *types.RequestInitChain // As this is called frequently, it's preferably to keep the check as stateless and as quick as possible. // Here we check that the transaction has the correctly key=value format. func (app *Application) CheckTx(_ context.Context, req *types.RequestCheckTx) (*types.ResponseCheckTx, error) { - if !isValidTx(req.Tx) { - return &types.ResponseCheckTx{Code: CodeTypeInvalidTxFormat}, nil - } - // If it is a validator update transaction, check that it is correctly formatted if isValidatorTx(req.Tx) { if _, _, err := parseValidatorTx(req.Tx); err != nil { return &types.ResponseCheckTx{Code: CodeTypeInvalidTxFormat}, nil } + } else if !isValidTx(req.Tx) { + return &types.ResponseCheckTx{Code: CodeTypeInvalidTxFormat}, nil } return &types.ResponseCheckTx{Code: CodeTypeOK, GasWanted: 1}, nil diff --git a/abci/example/kvstore/kvstore_test.go b/abci/example/kvstore/kvstore_test.go index 8245333b7..82a4948db 100644 --- a/abci/example/kvstore/kvstore_test.go +++ b/abci/example/kvstore/kvstore_test.go @@ -188,6 +188,8 @@ func TestCheckTx(t *testing.T) { defer cancel() kvstore := NewInMemoryApplication() + val := RandVal(1) + testCases := []struct { expCode uint32 tx []byte @@ -198,11 +200,15 @@ func TestCheckTx(t *testing.T) { {CodeTypeInvalidTxFormat, []byte("=hello")}, {CodeTypeInvalidTxFormat, []byte("hello=")}, {CodeTypeOK, []byte("a=b")}, + {CodeTypeInvalidTxFormat, []byte("val=hello")}, + {CodeTypeInvalidTxFormat, []byte("val=hi!5")}, + {CodeTypeOK, MakeValSetChangeTx(val.PubKey, 10)}, } for idx, tc := range testCases { resp, err := kvstore.CheckTx(ctx, &types.RequestCheckTx{Tx: tc.tx}) require.NoError(t, err, idx) + fmt.Println(string(tc.tx)) require.Equal(t, tc.expCode, resp.Code, idx) } } @@ -336,3 +342,8 @@ func runClientTests(ctx context.Context, t *testing.T, client abcicli.Client) { tx = []byte(testKey + "=" + testValue) testKVStore(ctx, t, client, tx, testKey, testValue) } + + func TestTxGeneration(t *testing.T) { + require.Len(t, NewRandomTx(20), 20) + require.Len(t, NewRandomTxs(10), 10) + } \ No newline at end of file diff --git a/mempool/v0/cache_test.go b/mempool/v0/cache_test.go index f7f475ee1..a2d27699e 100644 --- a/mempool/v0/cache_test.go +++ b/mempool/v0/cache_test.go @@ -2,6 +2,7 @@ package v0 import ( "crypto/sha256" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -35,22 +36,26 @@ func TestCacheAfterUpdate(t *testing.T) { } for tcIndex, tc := range tests { for i := 0; i < tc.numTxsToCreate; i++ { - tx := types.Tx{byte(i)} - err := mp.CheckTx(tx, nil, mempool.TxInfo{}) + tx := kvstore.NewTx(fmt.Sprintf("%d", i), "value") + err := mp.CheckTx(tx, func (resp *abci.ResponseCheckTx) { + require.False(t, resp.IsErr()) + }, mempool.TxInfo{}) require.NoError(t, err) } updateTxs := []types.Tx{} for _, v := range tc.updateIndices { - tx := types.Tx{byte(v)} + tx := kvstore.NewTx(fmt.Sprintf("%d", v), "value") updateTxs = append(updateTxs, tx) } err := mp.Update(int64(tcIndex), updateTxs, abciResponses(len(updateTxs), abci.CodeTypeOK), nil, nil) require.NoError(t, err) for _, v := range tc.reAddIndices { - tx := types.Tx{byte(v)} - _ = mp.CheckTx(tx, nil, mempool.TxInfo{}) + tx := kvstore.NewTx(fmt.Sprintf("%d", v), "value") + _ = mp.CheckTx(tx, func (resp *abci.ResponseCheckTx) { + require.False(t, resp.IsErr()) + }, mempool.TxInfo{}) } cache := mp.cache.(*mempool.LRUTxCache) @@ -61,7 +66,8 @@ func TestCacheAfterUpdate(t *testing.T) { "cache larger than expected on testcase %d", tcIndex) nodeVal := node.Value.(types.TxKey) - expectedBz := sha256.Sum256([]byte{byte(tc.txsInCache[len(tc.txsInCache)-counter-1])}) + expTx := kvstore.NewTx(fmt.Sprintf("%d", tc.txsInCache[len(tc.txsInCache)-counter-1]), "value") + expectedBz := sha256.Sum256(expTx) // Reference for reading the errors: // >>> sha256('\x00').hexdigest() // '6e340b9cffb37a989ca544e6bb780a2c78901d3fb33738768511a30617afa01d' diff --git a/mempool/v0/clist_mempool_test.go b/mempool/v0/clist_mempool_test.go index 854aa9d22..fbe28d4da 100644 --- a/mempool/v0/clist_mempool_test.go +++ b/mempool/v0/clist_mempool_test.go @@ -2,7 +2,6 @@ package v0 import ( "context" - "crypto/rand" "encoding/binary" "fmt" mrand "math/rand" @@ -22,7 +21,6 @@ import ( abciserver "github.com/tendermint/tendermint/abci/server" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/config" - "github.com/tendermint/tendermint/internal/test" "github.com/tendermint/tendermint/libs/log" tmrand "github.com/tendermint/tendermint/libs/rand" "github.com/tendermint/tendermint/libs/service" @@ -83,7 +81,7 @@ func ensureNoFire(t *testing.T, ch <-chan struct{}, timeoutMS int) { timer := time.NewTimer(time.Duration(timeoutMS) * time.Millisecond) select { case <-ch: - t.Fatal("Expected not to fire") + panic("Expected not to fire") case <-timer.C: } } @@ -101,12 +99,8 @@ func checkTxs(t *testing.T, mp mempool.Mempool, count int, peerID uint16) types. txs := make(types.Txs, count) txInfo := mempool.TxInfo{SenderID: peerID} for i := 0; i < count; i++ { - txBytes := make([]byte, 20) + txBytes := kvstore.NewRandomTx(20) txs[i] = txBytes - _, err := rand.Read(txBytes) - if err != nil { - t.Error(err) - } if err := mp.CheckTx(txBytes, nil, txInfo); err != nil { // Skip invalid txs. // TestMempoolFilters will fail otherwise. It asserts a number of txs @@ -214,9 +208,10 @@ func TestMempoolUpdate(t *testing.T) { // 1. Adds valid txs to the cache { - err := mp.Update(1, []types.Tx{[]byte{0x01}}, abciResponses(1, abci.CodeTypeOK), nil, nil) + tx1 := kvstore.NewTxFromId(1) + err := mp.Update(1, []types.Tx{tx1}, abciResponses(1, abci.CodeTypeOK), nil, nil) require.NoError(t, err) - err = mp.CheckTx([]byte{0x01}, nil, mempool.TxInfo{}) + err = mp.CheckTx(tx1, nil, mempool.TxInfo{}) if assert.Error(t, err) { assert.Equal(t, mempool.ErrTxInCache, err) } @@ -224,22 +219,24 @@ func TestMempoolUpdate(t *testing.T) { // 2. Removes valid txs from the mempool { - err := mp.CheckTx([]byte{0x02}, nil, mempool.TxInfo{}) + tx2 := kvstore.NewTxFromId(2) + err := mp.CheckTx(tx2, nil, mempool.TxInfo{}) require.NoError(t, err) - err = mp.Update(1, []types.Tx{[]byte{0x02}}, abciResponses(1, abci.CodeTypeOK), nil, nil) + err = mp.Update(1, []types.Tx{tx2}, abciResponses(1, abci.CodeTypeOK), nil, nil) require.NoError(t, err) assert.Zero(t, mp.Size()) } // 3. Removes invalid transactions from the cache and the mempool (if present) { - err := mp.CheckTx([]byte{0x03}, nil, mempool.TxInfo{}) + tx3 := kvstore.NewTxFromId(3) + err := mp.CheckTx(tx3, nil, mempool.TxInfo{}) require.NoError(t, err) - err = mp.Update(1, []types.Tx{[]byte{0x03}}, abciResponses(1, 1), nil, nil) + err = mp.Update(1, []types.Tx{tx3}, abciResponses(1, 1), nil, nil) require.NoError(t, err) assert.Zero(t, mp.Size()) - err = mp.CheckTx([]byte{0x03}, nil, mempool.TxInfo{}) + err = mp.CheckTx(tx3, nil, mempool.TxInfo{}) require.NoError(t, err) } } @@ -352,7 +349,7 @@ func TestTxsAvailable(t *testing.T) { // call update with half the txs. // it should fire once now for the new height // since there are still txs left - committedTxs, txs := txs[:50], txs[50:] + committedTxs, remainingTxs := txs[:50], txs[50:] if err := mp.Update(1, committedTxs, abciResponses(len(committedTxs), abci.CodeTypeOK), nil, nil); err != nil { t.Error(err) } @@ -364,7 +361,7 @@ func TestTxsAvailable(t *testing.T) { ensureNoFire(t, mp.TxsAvailable(), timeoutMS) // now call update with all the txs. it should not fire as there are no txs left - committedTxs = append(txs, moreTxs...) + committedTxs = append(remainingTxs, moreTxs...) if err := mp.Update(2, committedTxs, abciResponses(len(committedTxs), abci.CodeTypeOK), nil, nil); err != nil { t.Error(err) } @@ -392,10 +389,7 @@ func TestSerialReap(t *testing.T) { deliverTxsRange := func(start, end int) { // Deliver some txs. for i := start; i < end; i++ { - - // This will succeed - txBytes := make([]byte, 8) - binary.BigEndian.PutUint64(txBytes, uint64(i)) + txBytes := kvstore.NewTx(fmt.Sprintf("%d", i), "true") err := mp.CheckTx(txBytes, nil, mempool.TxInfo{}) _, cached := cacheMap[string(txBytes)] if cached { @@ -417,11 +411,9 @@ func TestSerialReap(t *testing.T) { } updateRange := func(start, end int) { - txs := make([]types.Tx, 0) + txs := make(types.Txs, end - start) for i := start; i < end; i++ { - txBytes := make([]byte, 8) - binary.BigEndian.PutUint64(txBytes, uint64(i)) - txs = append(txs, txBytes) + txs[i - start] = kvstore.NewTx(fmt.Sprintf("%d", i), "true") } if err := mp.Update(0, txs, abciResponses(len(txs), abci.CodeTypeOK), nil, nil); err != nil { t.Error(err) @@ -430,7 +422,12 @@ func TestSerialReap(t *testing.T) { commitRange := func(start, end int) { // Deliver some txs in a block - res, err := appConnCon.FinalizeBlock(context.Background(), &abci.RequestFinalizeBlock{Txs: test.MakeNTxs(10, 8).ToSliceOfBytes()}) + txs := make([][]byte, end - start) + for i := start; i < end; i++ { + txs[i - start] = kvstore.NewTx(fmt.Sprintf("%d", i), "true") + } + + res, err := appConnCon.FinalizeBlock(context.Background(), &abci.RequestFinalizeBlock{Txs: txs}) if err != nil { t.Errorf("client error committing tx: %v", err) } @@ -537,7 +534,7 @@ func TestMempoolTxsBytes(t *testing.T) { cfg := config.ResetTestRoot("mempool_test") - cfg.Mempool.MaxTxsBytes = 10 + cfg.Mempool.MaxTxsBytes = 100 mp, cleanup := newMempoolWithAppAndConfig(cc, cfg) defer cleanup() @@ -545,32 +542,32 @@ func TestMempoolTxsBytes(t *testing.T) { assert.EqualValues(t, 0, mp.SizeBytes()) // 2. len(tx) after CheckTx - err := mp.CheckTx([]byte{0x01}, nil, mempool.TxInfo{}) + tx1 := kvstore.NewRandomTx(10) + err := mp.CheckTx(tx1, nil, mempool.TxInfo{}) require.NoError(t, err) - assert.EqualValues(t, 1, mp.SizeBytes()) + assert.EqualValues(t, 10, mp.SizeBytes()) // 3. zero again after tx is removed by Update - err = mp.Update(1, []types.Tx{[]byte{0x01}}, abciResponses(1, abci.CodeTypeOK), nil, nil) + err = mp.Update(1, []types.Tx{tx1}, abciResponses(1, abci.CodeTypeOK), nil, nil) require.NoError(t, err) assert.EqualValues(t, 0, mp.SizeBytes()) // 4. zero after Flush - err = mp.CheckTx([]byte{0x02, 0x03}, nil, mempool.TxInfo{}) + tx2 := kvstore.NewRandomTx(20) + err = mp.CheckTx(tx2, nil, mempool.TxInfo{}) require.NoError(t, err) - assert.EqualValues(t, 2, mp.SizeBytes()) + assert.EqualValues(t, 20, mp.SizeBytes()) mp.Flush() assert.EqualValues(t, 0, mp.SizeBytes()) - + // 5. ErrMempoolIsFull is returned when/if MaxTxsBytes limit is reached. - err = mp.CheckTx( - []byte{0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04}, - nil, - mempool.TxInfo{}, - ) + tx3 := kvstore.NewRandomTx(100) + err = mp.CheckTx(tx3, nil, mempool.TxInfo{}) require.NoError(t, err) - err = mp.CheckTx([]byte{0x05}, nil, mempool.TxInfo{}) + tx4 := kvstore.NewRandomTx(10) + err = mp.CheckTx(tx4, nil, mempool.TxInfo{}) if assert.Error(t, err) { assert.IsType(t, mempool.ErrMempoolIsFull{}, err) } @@ -582,12 +579,11 @@ func TestMempoolTxsBytes(t *testing.T) { mp, cleanup = newMempoolWithApp(cc) defer cleanup() - txBytes := make([]byte, 8) - binary.BigEndian.PutUint64(txBytes, uint64(0)) + txBytes := kvstore.NewRandomTx(10) err = mp.CheckTx(txBytes, nil, mempool.TxInfo{}) require.NoError(t, err) - assert.EqualValues(t, 8, mp.SizeBytes()) + assert.EqualValues(t, 10, mp.SizeBytes()) appConnCon, _ := cc.NewABCIClient() appConnCon.SetLogger(log.TestingLogger().With("module", "abci-client", "connection", "consensus")) @@ -610,16 +606,16 @@ func TestMempoolTxsBytes(t *testing.T) { // Pretend like we committed nothing so txBytes gets rechecked and removed. err = mp.Update(1, []types.Tx{}, abciResponses(0, abci.CodeTypeOK), nil, nil) require.NoError(t, err) - assert.EqualValues(t, 8, mp.SizeBytes()) + assert.EqualValues(t, 10, mp.SizeBytes()) // 7. Test RemoveTxByKey function - err = mp.CheckTx([]byte{0x06}, nil, mempool.TxInfo{}) + err = mp.CheckTx(tx1, nil, mempool.TxInfo{}) require.NoError(t, err) - assert.EqualValues(t, 9, mp.SizeBytes()) + assert.EqualValues(t, 20, mp.SizeBytes()) assert.Error(t, mp.RemoveTxByKey(types.Tx([]byte{0x07}).Key())) - assert.EqualValues(t, 9, mp.SizeBytes()) - assert.NoError(t, mp.RemoveTxByKey(types.Tx([]byte{0x06}).Key())) - assert.EqualValues(t, 8, mp.SizeBytes()) + assert.EqualValues(t, 20, mp.SizeBytes()) + assert.NoError(t, mp.RemoveTxByKey(types.Tx(tx1).Key())) + assert.EqualValues(t, 10, mp.SizeBytes()) } @@ -647,7 +643,7 @@ func TestMempoolRemoteAppConcurrency(t *testing.T) { txLen := 200 txs := make([]types.Tx, nTxs) for i := 0; i < nTxs; i++ { - txs[i] = tmrand.Bytes(txLen) + txs[i] = kvstore.NewRandomTx(txLen) } // simulate a group of peers sending them over and over diff --git a/mempool/v0/reactor_test.go b/mempool/v0/reactor_test.go index c42935137..55b9f1550 100644 --- a/mempool/v0/reactor_test.go +++ b/mempool/v0/reactor_test.go @@ -17,7 +17,6 @@ import ( abci "github.com/tendermint/tendermint/abci/types" cfg "github.com/tendermint/tendermint/config" "github.com/tendermint/tendermint/libs/log" - tmrand "github.com/tendermint/tendermint/libs/rand" "github.com/tendermint/tendermint/mempool" "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/p2p/mock" @@ -170,8 +169,10 @@ func TestReactor_MaxTxBytes(t *testing.T) { // Broadcast a tx, which has the max size // => ensure it's received by the second reactor. - tx1 := tmrand.Bytes(config.Mempool.MaxTxBytes) - err := reactors[0].mempool.CheckTx(tx1, nil, mempool.TxInfo{SenderID: mempool.UnknownPeerID}) + tx1 := kvstore.NewRandomTx(config.Mempool.MaxTxBytes) + err := reactors[0].mempool.CheckTx(tx1, func (resp *abci.ResponseCheckTx) { + require.False(t, resp.IsErr()) + }, mempool.TxInfo{SenderID: mempool.UnknownPeerID}) require.NoError(t, err) waitForTxsOnReactors(t, []types.Tx{tx1}, reactors) @@ -180,8 +181,10 @@ func TestReactor_MaxTxBytes(t *testing.T) { // Broadcast a tx, which is beyond the max size // => ensure it's not sent - tx2 := tmrand.Bytes(config.Mempool.MaxTxBytes + 1) - err = reactors[0].mempool.CheckTx(tx2, nil, mempool.TxInfo{SenderID: mempool.UnknownPeerID}) + tx2 := kvstore.NewRandomTx(config.Mempool.MaxTxBytes + 1) + err = reactors[0].mempool.CheckTx(tx2, func (resp *abci.ResponseCheckTx) { + require.False(t, resp.IsErr()) + }, mempool.TxInfo{SenderID: mempool.UnknownPeerID}) require.Error(t, err) }