From 5eb677dec18b9590abd41326701e50f338eeef91 Mon Sep 17 00:00:00 2001
From: Jasmina Malicevic <jasmina.dustinac@gmail.com>
Date: Tue, 21 Jun 2022 13:03:43 +0200
Subject: [PATCH] Fixed fifo mempool tests

---
 mempool/v0/clist_mempool.go      |  9 ++------
 mempool/v0/clist_mempool_test.go | 39 ++++++++++++++++++++++++++------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/mempool/v0/clist_mempool.go b/mempool/v0/clist_mempool.go
index 5bdf96187..4695eed17 100644
--- a/mempool/v0/clist_mempool.go
+++ b/mempool/v0/clist_mempool.go
@@ -241,17 +241,12 @@ func (mem *CListMempool) CheckTx(
 		// so we only record the sender for txs still in the mempool.
 		if e, ok := mem.txsMap.Load(tx.Key()); ok {
 			memTx := e.(*clist.CElement).Value.(*mempoolTx)
-			_, loaded := memTx.senders.LoadOrStore(txInfo.SenderID, true)
+			memTx.senders.LoadOrStore(txInfo.SenderID, true)
 			// TODO: consider punishing peer for dups,
 			// its non-trivial since invalid txs can become valid,
 			// but they can spam the same tx with little cost to them atm.
-			if loaded {
-				return mempool.ErrTxInCache
-			}
 		}
-
-		mem.logger.Debug("tx exists already in cache", "tx_hash", tx.Hash())
-		return nil
+		return mempool.ErrTxInCache
 	}
 
 	reqRes := mem.proxyAppConn.CheckTxAsync(abci.RequestCheckTx{Tx: tx})
diff --git a/mempool/v0/clist_mempool_test.go b/mempool/v0/clist_mempool_test.go
index e569867ec..00292d52f 100644
--- a/mempool/v0/clist_mempool_test.go
+++ b/mempool/v0/clist_mempool_test.go
@@ -33,6 +33,27 @@ import (
 // test.
 type cleanupFunc func()
 
+func newMempoolWithAppMock(cc proxy.ClientCreator, client abciclient.Client) (*CListMempool, cleanupFunc, error) {
+	conf := config.ResetTestRoot("mempool_test")
+
+	mp, cu := newMempoolWithAppAndConfigMock(cc, conf, client)
+	return mp, cu, nil
+}
+
+func newMempoolWithAppAndConfigMock(cc proxy.ClientCreator, cfg *config.Config, client abciclient.Client) (*CListMempool, cleanupFunc) {
+	appConnMem := client
+	appConnMem.SetLogger(log.TestingLogger().With("module", "abci-client", "connection", "mempool"))
+	err := appConnMem.Start()
+	if err != nil {
+		panic(err)
+	}
+
+	mp := NewCListMempool(cfg.Mempool, appConnMem, 0)
+	mp.SetLogger(log.TestingLogger())
+
+	return mp, func() { os.RemoveAll(cfg.RootDir) }
+}
+
 func newMempoolWithApp(cc proxy.ClientCreator) (*CListMempool, cleanupFunc, error) {
 	conf := config.ResetTestRoot("mempool_test")
 
@@ -197,7 +218,9 @@ func TestMempoolUpdate(t *testing.T) {
 		err := mp.Update(1, []types.Tx{[]byte{0x01}}, abciResponses(1, abci.CodeTypeOK), nil, nil)
 		require.NoError(t, err)
 		err = mp.CheckTx([]byte{0x01}, nil, mempool.TxInfo{})
-		require.NoError(t, err)
+		if assert.Error(t, err) {
+			assert.Equal(t, mempool.ErrTxInCache, err)
+		}
 	}
 
 	// 2. Removes valid txs from the mempool
@@ -232,11 +255,9 @@ func TestMempoolUpdateDoesNotPanicWhenApplicationMissedTx(t *testing.T) {
 	mockClient.On("FlushAsync", mock.Anything).Return(abciclient.NewReqRes(abci.ToRequestFlush()), nil)
 	mockClient.On("SetResponseCallback", mock.MatchedBy(func(cb abciclient.Callback) bool { callback = cb; return true }))
 
-	// cc := func() (abciclient.Client, error) {
-	// 	return mockClient, nil
-	// }
 	app := kvstore.NewApplication()
-	mp, cleanup, err := newMempoolWithApp(proxy.NewLocalClientCreator(app))
+	cc := proxy.NewLocalClientCreator(app)
+	mp, cleanup, err := newMempoolWithAppMock(cc, mockClient)
 	require.NoError(t, err)
 	defer cleanup()
 
@@ -302,11 +323,15 @@ func TestMempool_KeepInvalidTxsInCache(t *testing.T) {
 
 		// a must be added to the cache
 		err = mp.CheckTx(a, nil, mempool.TxInfo{})
-		require.NoError(t, err)
+		if assert.Error(t, err) {
+			assert.Equal(t, mempool.ErrTxInCache, err)
+		}
 
 		// b must remain in the cache
 		err = mp.CheckTx(b, nil, mempool.TxInfo{})
-		require.NoError(t, err)
+		if assert.Error(t, err) {
+			assert.Equal(t, mempool.ErrTxInCache, err)
+		}
 	}
 
 	// 2. An invalid transaction must remain in the cache