clean up mempool tests

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 {

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

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)
+ }

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'

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

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)
 }