From 76471f4ce515b5e87a852592b96f676fc823e608 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Mon, 22 Apr 2024 15:43:42 -0700
Subject: [PATCH] fix(check-tx): remove txs failing recheck from app-side
 mempool (backport #476) (#477)

* fix(check-tx): remove txs failing recheck from app-side mempool (#476)

* remove txs failing recheck from app-side mempool

* linting

(cherry picked from commit 6080de11e4b1951bc9bffdd5630d60b2b5b60df5)

* fix backport

---------

Co-authored-by: Nikhil Vasan <97126437+nivasan1@users.noreply.github.com>
Co-authored-by: Nikhil Vasan <nikhil@skip.money>
---
 abci/checktx/check_tx_test.go           | 46 +++++++++++++++++++++++++
 abci/checktx/mempool_parity_check_tx.go | 27 +++++++++++++--
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/abci/checktx/check_tx_test.go b/abci/checktx/check_tx_test.go
index 8e3ed65d..75bd4083 100644
--- a/abci/checktx/check_tx_test.go
+++ b/abci/checktx/check_tx_test.go
@@ -111,6 +111,52 @@ func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
 	})
 }
 
+func (s *CheckTxTestSuite) TestRemovalOnRecheckTx() {
+	// create a tx that should not be inserted in the mev-lane
+	tx, _, err := testutils.CreateAuctionTx(
+		s.EncCfg.TxConfig,
+		s.Accounts[0],
+		sdk.NewCoin(s.GasTokenDenom, math.NewInt(100)),
+		1,
+		0,
+		nil,
+		100,
+	)
+	s.Require().NoError(err)
+
+	mevLane := s.InitLane(math.LegacyOneDec(), nil)
+	mempool, err := block.NewLanedMempool(s.Ctx.Logger(), []block.Lane{mevLane})
+	s.Require().NoError(err)
+
+	handler := checktx.NewMempoolParityCheckTx(
+		s.Ctx.Logger(),
+		mempool,
+		s.EncCfg.TxConfig.TxDecoder(),
+		func(cometabci.RequestCheckTx) cometabci.ResponseCheckTx {
+			// always fail
+			return cometabci.ResponseCheckTx{Code: 1}
+		},
+	).CheckTx()
+
+	s.Run("tx is removed on check-tx failure when re-check", func() {
+		// check that tx exists in mempool
+		txBz, err := s.EncCfg.TxConfig.TxEncoder()(tx)
+		s.Require().NoError(err)
+
+		s.Require().NoError(mempool.Insert(s.Ctx, tx))
+		s.Require().True(mempool.Contains(tx))
+
+		// check tx
+		res := handler(cometabci.RequestCheckTx{Tx: txBz, Type: cometabci.CheckTxType_Recheck})
+		s.Require().NoError(err)
+
+		s.Require().Equal(uint32(1), res.Code)
+
+		// check that tx is removed from mempool
+		s.Require().False(mempool.Contains(tx))
+	})
+}
+
 func (s *CheckTxTestSuite) TestMempoolParityCheckTx() {
 	s.Run("tx fails tx-decoding", func() {
 		handler := checktx.NewMempoolParityCheckTx(
diff --git a/abci/checktx/mempool_parity_check_tx.go b/abci/checktx/mempool_parity_check_tx.go
index 35960189..9df630af 100644
--- a/abci/checktx/mempool_parity_check_tx.go
+++ b/abci/checktx/mempool_parity_check_tx.go
@@ -54,9 +54,11 @@ func (m MempoolParityCheckTx) CheckTx() CheckTx {
 			)
 		}
 
+		isReCheck := req.Type == cmtabci.CheckTxType_Recheck
+
 		// if the mode is ReCheck and the app's mempool does not contain the given tx, we fail
 		// immediately, to purge the tx from the comet mempool.
-		if req.Type == cmtabci.CheckTxType_Recheck && !m.mempl.Contains(tx) {
+		if isReCheck && !m.mempl.Contains(tx) {
 			m.logger.Debug(
 				"tx from comet mempool not found in app-side mempool",
 				"tx", tx,
@@ -72,6 +74,27 @@ func (m MempoolParityCheckTx) CheckTx() CheckTx {
 		}
 
 		// run the checkTxHandler
-		return m.checkTxHandler(req)
+		res := m.checkTxHandler(req)
+
+		// if re-check fails for a transaction, we'll need to explicitly purge the tx from
+		// the app-side mempool
+		if isInvalidCheckTxExecution(res) && isReCheck {
+			// check if the tx exists first
+			if m.mempl.Contains(tx) {
+				// remove the tx
+				if err := m.mempl.Remove(tx); err != nil {
+					m.logger.Debug(
+						"failed to remove tx from app-side mempool when purging for re-check failure",
+						"removal-err", err,
+					)
+				}
+			}
+		}
+
+		return res
 	}
 }
+
+func isInvalidCheckTxExecution(resp cmtabci.ResponseCheckTx) bool {
+	return resp.Code != 0
+}