From 4322f7d0b94b3405c50217ac8091d7ec09d21e13 Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Tue, 21 Jun 2022 18:51:50 +0200 Subject: [PATCH] mempool: make error throwing for CheckTx consistent (#8817) --- internal/mempool/v0/clist_mempool.go | 8 ++------ internal/mempool/v0/clist_mempool_test.go | 10 +++++++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/mempool/v0/clist_mempool.go b/internal/mempool/v0/clist_mempool.go index 0a12c7000..b37e745d8 100644 --- a/internal/mempool/v0/clist_mempool.go +++ b/internal/mempool/v0/clist_mempool.go @@ -242,17 +242,13 @@ 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 types.ErrTxInCache - } } - mem.logger.Debug("tx exists already in cache", "tx_hash", tx.Hash()) - return nil + return types.ErrTxInCache } if ctx == nil { diff --git a/internal/mempool/v0/clist_mempool_test.go b/internal/mempool/v0/clist_mempool_test.go index 61ec543ef..774e32a96 100644 --- a/internal/mempool/v0/clist_mempool_test.go +++ b/internal/mempool/v0/clist_mempool_test.go @@ -200,7 +200,7 @@ 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(context.Background(), []byte{0x01}, nil, mempool.TxInfo{}) - require.NoError(t, err) + assert.Error(t, err) } // 2. Removes valid txs from the mempool @@ -305,11 +305,15 @@ func TestMempool_KeepInvalidTxsInCache(t *testing.T) { // a must be added to the cache err = mp.CheckTx(context.Background(), a, nil, mempool.TxInfo{}) - require.NoError(t, err) + if assert.Error(t, err) { + assert.Equal(t, types.ErrTxInCache, err) + } // b must remain in the cache err = mp.CheckTx(context.Background(), b, nil, mempool.TxInfo{}) - require.NoError(t, err) + if assert.Error(t, err) { + assert.Equal(t, types.ErrTxInCache, err) + } } // 2. An invalid transaction must remain in the cache