From 90fe178b5290f35ce1ebdfa1466527d7c716f256 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Wed, 3 Mar 2021 21:02:45 +0100 Subject: [PATCH] mempool: don't return an error on checktx with the same tx (#6199) --- mempool/clist_mempool.go | 8 ++++++-- mempool/clist_mempool_test.go | 17 +++-------------- test/app/counter_test.sh | 9 +-------- 3 files changed, 10 insertions(+), 24 deletions(-) diff --git a/mempool/clist_mempool.go b/mempool/clist_mempool.go index f9ef9ae23..6751338c5 100644 --- a/mempool/clist_mempool.go +++ b/mempool/clist_mempool.go @@ -277,13 +277,17 @@ func (mem *CListMempool) CheckTx(tx types.Tx, cb func(*abci.Response), txInfo Tx // so we only record the sender for txs still in the mempool. if e, ok := mem.txsMap.Load(TxKey(tx)); ok { memTx := e.(*clist.CElement).Value.(*mempoolTx) - memTx.senders.LoadOrStore(txInfo.SenderID, true) + _, loaded := 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 ErrTxInCache + } } - return ErrTxInCache + mem.logger.Debug("tx exists already in cache", "tx_hash", tx.Hash()) + return nil } ctx := context.Background() diff --git a/mempool/clist_mempool_test.go b/mempool/clist_mempool_test.go index 8e20b4e4c..19ca5b9a1 100644 --- a/mempool/clist_mempool_test.go +++ b/mempool/clist_mempool_test.go @@ -190,9 +190,7 @@ func TestMempoolUpdate(t *testing.T) { err := mempool.Update(1, []types.Tx{[]byte{0x01}}, abciResponses(1, abci.CodeTypeOK), nil, nil) require.NoError(t, err) err = mempool.CheckTx([]byte{0x01}, nil, TxInfo{}) - if assert.Error(t, err) { - assert.Equal(t, ErrTxInCache, err) - } + require.NoError(t, err) } // 2. Removes valid txs from the mempool @@ -245,15 +243,11 @@ func TestMempool_KeepInvalidTxsInCache(t *testing.T) { // a must be added to the cache err = mempool.CheckTx(a, nil, TxInfo{}) - if assert.Error(t, err) { - assert.Equal(t, ErrTxInCache, err) - } + require.NoError(t, err) // b must remain in the cache err = mempool.CheckTx(b, nil, TxInfo{}) - if assert.Error(t, err) { - assert.Equal(t, ErrTxInCache, err) - } + require.NoError(t, err) } // 2. An invalid transaction must remain in the cache @@ -266,11 +260,6 @@ func TestMempool_KeepInvalidTxsInCache(t *testing.T) { err := mempool.CheckTx(a, nil, TxInfo{}) require.NoError(t, err) - - err = mempool.CheckTx(a, nil, TxInfo{}) - if assert.Error(t, err) { - assert.Equal(t, ErrTxInCache, err) - } } } diff --git a/test/app/counter_test.sh b/test/app/counter_test.sh index e5c45a410..4dc7474d8 100755 --- a/test/app/counter_test.sh +++ b/test/app/counter_test.sh @@ -109,14 +109,7 @@ if [[ $APPEND_TX_CODE != 0 ]]; then exit 1 fi - -echo "... sending tx. expect error" - -# second time should get rejected by the mempool (return error and non-zero code) -sendTx $TX true - - -echo "... sending tx. expect no error" +echo "... sending new tx. expect no error" # now, TX=01 should pass, with no error TX=01