fix: prevent updating check state before mempool insert

CHANGELOG.md

@@ -76,7 +76,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
-
+* (baseapp) [#25550](https://github.com/cosmos/cosmos-sdk/pull/25550) Prevent updating check state before mempool insert
 * (mempool) [#25563](https://github.com/cosmos/cosmos-sdk/pull/25563) Cleanup sender indices in case of tx replacement.
 * (x/epochs) [#25425](https://github.com/cosmos/cosmos-sdk/pull/25425) Fix `InvokeSetHooks` being called with a nil keeper and `AppModule` containing a copy instead of a pointer (hooks set post creating the `AppModule` like with depinject didn't apply because it's a different instance).
 * (client, client/rpc, x/auth/tx) [#24551](https://github.com/cosmos/cosmos-sdk/pull/24551) Handle cancellation properly when supplying context to client methods.

baseapp/abci_test.go

@@ -696,6 +696,46 @@ func TestABCI_CheckTx(t *testing.T) {
 	require.Nil(t, storedBytes)
 }
 
+func TestABCI_CheckTx_DoesNotCorruptStateOnMempoolFailure(t *testing.T) {
+	counterKey := []byte("counter-key")
+	anteOpt := func(bapp *baseapp.BaseApp) {
+		bapp.SetAnteHandler(anteHandlerTxTest(t, capKey1, counterKey))
+	}
+
+	cfg := mempool.DefaultPriorityNonceMempoolConfig()
+	cfg.MaxTx = 1
+	pool := mempool.NewPriorityMempool(cfg)
+
+	suite := NewBaseAppSuite(t, anteOpt, baseapp.SetMempool(pool))
+	baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImpl{t, capKey1, counterKey})
+
+	_, err := suite.baseApp.InitChain(&abci.RequestInitChain{
+		ConsensusParams: &cmtproto.ConsensusParams{},
+	})
+	require.NoError(t, err)
+
+	tx := newTxCounter(t, suite.txConfig, 0, 0)
+	txBytes, err := suite.txConfig.TxEncoder()(tx)
+	require.NoError(t, err)
+
+	res, err := suite.baseApp.CheckTx(&abci.RequestCheckTx{Tx: txBytes})
+	require.NoError(t, err)
+	require.True(t, res.IsOK(), fmt.Sprintf("%v", res))
+
+	failTx := newTxCounter(t, suite.txConfig, 1, 0)
+	failTxBytes, err := suite.txConfig.TxEncoder()(failTx)
+	require.NoError(t, err)
+
+	failRes, err := suite.baseApp.CheckTx(&abci.RequestCheckTx{Tx: failTxBytes})
+	require.NoError(t, err)
+	require.False(t, failRes.IsOK())
+	require.Contains(t, failRes.Log, mempool.ErrMempoolTxMaxCapacity.Error())
+
+	checkStateStore := getCheckStateCtx(suite.baseApp).KVStore(capKey1)
+	require.Equal(t, int64(1), getIntFromStore(t, checkStateStore, counterKey))
+	require.Equal(t, 1, pool.CountTx())
+}
+
 func TestABCI_FinalizeBlock_DeliverTx(t *testing.T) {
 	anteKey := []byte("ante-key")
 	anteOpt := func(bapp *baseapp.BaseApp) { bapp.SetAnteHandler(anteHandlerTxTest(t, capKey1, anteKey)) }

baseapp/baseapp.go

@@ -846,6 +846,9 @@ func (app *BaseApp) RunTx(mode sdk.ExecMode, txBytes []byte, tx sdk.Tx, txIndex
 		}
 	}
 
+	mempoolCtx := ctx
+	var commitAnteCache func()
+
 	if app.anteHandler != nil {
 		var (
 			anteCtx sdk.Context
@@ -888,16 +891,30 @@ func (app *BaseApp) RunTx(mode sdk.ExecMode, txBytes []byte, tx sdk.Tx, txIndex
 			return gInfo, nil, nil, err
 		}
 
-		msCache.Write()
-		anteEvents = events.ToABCIEvents()
+		commitAnteCache = func() {
+			if msCache != nil {
+				msCache.Write()
+			}
+			anteEvents = events.ToABCIEvents()
+		}
+
+		if mode == execModeCheck {
+			mempoolCtx = ctx.WithMultiStore(msCache)
+		} else {
+			commitAnteCache()
+			commitAnteCache = nil
+		}
 	}
 
 	switch mode {
 	case execModeCheck:
-		err = app.mempool.Insert(ctx, tx)
-		if err != nil {
+		if err := app.mempool.Insert(mempoolCtx, tx); err != nil {
 			return gInfo, nil, anteEvents, err
 		}
+
+		if commitAnteCache != nil {
+			commitAnteCache()
+		}
 	case execModeFinalize:
 		err = app.mempool.Remove(tx)
 		if err != nil && !errors.Is(err, mempool.ErrTxNotFound) {