mempool: introduce KeepInvalidTxsInCache config option (#5813)

When set to true, an invalid transaction will be kept in the cache (this may help some applications to protect against spam).

NOTE: this is a temporary config option. The more correct solution would be to add a TTL to each transaction (i.e. CheckTx may return a TTL in ResponseCheckTx).

Closes: #5751

CHANGELOG_PENDING.md

@@ -22,10 +22,10 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 
 ### FEATURES
 
-
-
 ### IMPROVEMENTS
 
+- [mempool] \#5813 Add `keep-invalid-txs-in-cache` config option. When set to true, mempool will keep invalid transactions in the cache (@p4u)
+
 ### BUG FIXES
 
 - [crypto] \#5707 Fix infinite recursion in string formatting of Secp256k1 keys (@erikgrinaker)

config/config.go

@@ -656,6 +656,10 @@ type MempoolConfig struct {
 	MaxTxsBytes int64 `mapstructure:"max_txs_bytes"`
 	// Size of the cache (used to filter transactions we saw earlier) in transactions
 	CacheSize int `mapstructure:"cache_size"`
+	// Do not remove invalid transactions from the cache (default: false)
+	// Set to true if it's not possible for any invalid transaction to become
+	// valid again in the future.
+	KeepInvalidTxsInCache bool `mapstructure:"keep-invalid-txs-in-cache"`
 	// Maximum size of a single transaction
 	// NOTE: the max size of a tx transmitted over the network is {max_tx_bytes}.
 	MaxTxBytes int `mapstructure:"max_tx_bytes"`

config/toml.go

@@ -329,6 +329,11 @@ max_txs_bytes = {{ .Mempool.MaxTxsBytes }}
 # Size of the cache (used to filter transactions we saw earlier) in transactions
 cache_size = {{ .Mempool.CacheSize }}
 
+# Do not remove invalid transactions from the cache (default: false)
+# Set to true if it's not possible for any invalid transaction to become valid
+# again in the future.
+keep-invalid-txs-in-cache = {{ .Mempool.KeepInvalidTxsInCache }}
+
 # Maximum size of a single transaction.
 # NOTE: the max size of a tx transmitted over the network is {max_tx_bytes}.
 max_tx_bytes = {{ .Mempool.MaxTxBytes }}

docs/tendermint-core/configuration.md

@@ -278,6 +278,11 @@ max_txs_bytes = 1073741824
 # Size of the cache (used to filter transactions we saw earlier) in transactions
 cache_size = 10000
 
+# Do not remove invalid transactions from the cache (default: false)
+# Set to true if it's not possible for any invalid transaction to become valid
+# again in the future.
+keep-invalid-txs-in-cache = false
+
 # Maximum size of a single transaction.
 # NOTE: the max size of a tx transmitted over the network is {max_tx_bytes}.
 max_tx_bytes = 1048576

mempool/clist_mempool.go

@@ -439,8 +439,10 @@ func (mem *CListMempool) resCbFirstTime(
 			mem.logger.Info("Rejected bad transaction",
 				"tx", txID(tx), "peerID", peerP2PID, "res", r, "err", postCheckErr)
 			mem.metrics.FailedTxs.Add(1)
-			// remove from cache (it might be good later)
+			if !mem.config.KeepInvalidTxsInCache {
-			mem.cache.Remove(tx)
+				// remove from cache (it might be good later)
+				mem.cache.Remove(tx)
+			}
 		}
 	default:
 		// ignore other messages
@@ -472,7 +474,7 @@ func (mem *CListMempool) resCbRecheck(req *abci.Request, res *abci.Response) {
 			// Tx became invalidated due to newly committed block.
 			mem.logger.Info("Tx is no longer valid", "tx", txID(tx), "res", r, "err", postCheckErr)
 			// NOTE: we remove tx from the cache because it might be good later
-			mem.removeTx(tx, mem.recheckCursor, true)
+			mem.removeTx(tx, mem.recheckCursor, !mem.config.KeepInvalidTxsInCache)
 		}
 		if mem.recheckCursor == mem.recheckEnd {
 			mem.recheckCursor = nil
@@ -586,7 +588,7 @@ func (mem *CListMempool) Update(
 		if deliverTxResponses[i].Code == abci.CodeTypeOK {
 			// Add valid committed tx to the cache (if missing).
 			_ = mem.cache.Push(tx)
-		} else {
+		} else if !mem.config.KeepInvalidTxsInCache {
 			// Allow invalid transactions to be resubmitted.
 			mem.cache.Remove(tx)
 		}

mempool/clist_mempool_test.go

@@ -216,6 +216,63 @@ func TestMempoolUpdate(t *testing.T) {
 	}
 }
 
+func TestMempool_KeepInvalidTxsInCache(t *testing.T) {
+	app := counter.NewApplication(true)
+	cc := proxy.NewLocalClientCreator(app)
+	wcfg := cfg.DefaultConfig()
+	wcfg.Mempool.KeepInvalidTxsInCache = true
+	mempool, cleanup := newMempoolWithAppAndConfig(cc, wcfg)
+	defer cleanup()
+
+	// 1. An invalid transaction must remain in the cache after Update
+	{
+		a := make([]byte, 8)
+		binary.BigEndian.PutUint64(a, 0)
+
+		b := make([]byte, 8)
+		binary.BigEndian.PutUint64(b, 1)
+
+		err := mempool.CheckTx(b, nil, TxInfo{})
+		require.NoError(t, err)
+
+		// simulate new block
+		_ = app.DeliverTx(abci.RequestDeliverTx{Tx: a})
+		_ = app.DeliverTx(abci.RequestDeliverTx{Tx: b})
+		err = mempool.Update(1, []types.Tx{a, b},
+			[]*abci.ResponseDeliverTx{{Code: abci.CodeTypeOK}, {Code: 2}}, nil, nil)
+		require.NoError(t, err)
+
+		// a must be added to the cache
+		err = mempool.CheckTx(a, nil, TxInfo{})
+		if assert.Error(t, err) {
+			assert.Equal(t, ErrTxInCache, err)
+		}
+
+		// b must remain in the cache
+		err = mempool.CheckTx(b, nil, TxInfo{})
+		if assert.Error(t, err) {
+			assert.Equal(t, ErrTxInCache, err)
+		}
+	}
+
+	// 2. An invalid transaction must remain in the cache
+	{
+		a := make([]byte, 8)
+		binary.BigEndian.PutUint64(a, 0)
+
+		// remove a from the cache to test (2)
+		mempool.cache.Remove(a)
+
+		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)
+		}
+	}
+}
+
 func TestTxsAvailable(t *testing.T) {
 	app := kvstore.NewApplication()
 	cc := proxy.NewLocalClientCreator(app)