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

* 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 6080de1)

* Fix test

---------

Co-authored-by: Nikhil Vasan <97126437+nivasan1@users.noreply.github.com>
Co-authored-by: Eric <eric.warehime@gmail.com>

Author: 3 people

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, error) {
+			// always fail
+			return &cometabci.ResponseCheckTx{Code: 1}, nil
+		},
+	).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, err := 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(

abci/checktx/mempool_parity_check_tx.go

@@ -54,9 +54,11 @@ func (m MempoolParityCheckTx) CheckTx() CheckTx {
 			), nil
 		}
 
+		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,28 @@ func (m MempoolParityCheckTx) CheckTx() CheckTx {
 		}
 
 		// run the checkTxHandler
-		return m.checkTxHandler(req)
+		res, checkTxError := 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, checkTxError) && 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,
+						"check-tx-err", checkTxError,
+					)
+				}
+			}
+		}
+
+		return res, checkTxError
 	}
 }
+
+func isInvalidCheckTxExecution(resp *cmtabci.ResponseCheckTx, checkTxErr error) bool {
+	return resp == nil || resp.Code != 0 || checkTxErr != nil
+}