[DNM] feat(baseapp): refactor runTx

baseapp/abci.go

@@ -303,9 +303,9 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx {
 		panic(fmt.Sprintf("unknown RequestCheckTx type: %s", req.Type))
 	}
 
-	gInfo, result, anteEvents, priority, err := app.runTx(mode, req.Tx)
+	gInfo, result, events, priority, err := app.runTx(mode, req.Tx)
 	if err != nil {
-		return sdkerrors.ResponseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, app.trace)
+		return sdkerrors.ResponseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, events, app.trace)
 	}
 
 	return abci.ResponseCheckTx{
@@ -342,10 +342,10 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliv
 		telemetry.SetGauge(float32(gInfo.GasWanted), "tx", "gas", "wanted")
 	}()
 
-	gInfo, result, anteEvents, _, err := app.runTx(runTxModeDeliver, req.Tx)
+	gInfo, result, events, _, err := app.runTx(runTxModeDeliver, req.Tx)
 	if err != nil {
 		resultStr = "failed"
-		return sdkerrors.ResponseDeliverTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, sdk.MarkEventsToIndex(anteEvents, app.indexEvents), app.trace)
+		return sdkerrors.ResponseDeliverTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, sdk.MarkEventsToIndex(events, app.indexEvents), app.trace)
 	}
 
 	return abci.ResponseDeliverTx{

baseapp/baseapp.go

@@ -616,12 +616,17 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context
 // Note, gas execution info is always returned. A reference to a Result is
 // returned if the tx does not run out of gas and if all the messages are valid
 // and execute successfully. An error is returned otherwise.
-func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, priority int64, err error) {
+func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, events []abci.Event, priority int64, err error) {
 	// NOTE: GasWanted should be returned by the AnteHandler. GasUsed is
 	// determined by the GasMeter. We need access to the context to get the gas
 	// meter, so we initialize upfront.
 	var gasWanted uint64
 
+	var (
+		anteEvents []abci.Event
+		postEvents []abci.Event
+	)
+
 	ctx := app.getContextForTx(mode, txBytes)
 	ms := ctx.MultiStore()
 
@@ -686,6 +691,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re
 		anteCtx, msCache = app.cacheTxContext(ctx, txBytes)
 		anteCtx = anteCtx.WithEventManager(sdk.NewEventManager())
 		newCtx, err := app.anteHandler(anteCtx, tx, mode == runTxModeSimulate)
+		if err != nil {
+			return gInfo, nil, nil, 0, err
+		}
 
 		if !newCtx.IsZero() {
 			// At this point, newCtx.MultiStore() is a store branch, or something else
@@ -697,33 +705,30 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re
 			ctx = newCtx.WithMultiStore(ms)
 		}
 
-		events := ctx.EventManager().Events()
-
 		// GasMeter expected to be set in AnteHandler
 		gasWanted = ctx.GasMeter().Limit()
-
-		if err != nil {
-			return gInfo, nil, nil, 0, err
-		}
-
 		priority = ctx.Priority()
-		msCache.Write()
+		events := ctx.EventManager().Events()
 		anteEvents = events.ToABCIEvents()
+
+		msCache.Write()
 	}
 
-	if mode == runTxModeCheck {
+	// insert or remove the transaction from the mempool
+	switch mode {
+	case runTxModeCheck:
 		err = app.mempool.Insert(ctx, tx)
-		if err != nil {
-			return gInfo, nil, anteEvents, priority, err
-		}
-	} else if mode == runTxModeDeliver {
+	case runTxModeDeliver:
 		err = app.mempool.Remove(tx)
 		if err != nil && !errors.Is(err, mempool.ErrTxNotFound) {
-			return gInfo, nil, anteEvents, priority,
-				fmt.Errorf("failed to remove tx from mempool: %w", err)
+			err = fmt.Errorf("failed to remove tx from mempool: %w", err)
 		}
 	}
 
+	if err != nil {
+		return gInfo, nil, events, priority, err
+	}
+
 	// Create a new Context based off of the existing Context with a MultiStore branch
 	// in case message processing fails. At this point, the MultiStore
 	// is a branch of a branch.
@@ -733,34 +738,81 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re
 	// and we're in DeliverTx. Note, runMsgs will never return a reference to a
 	// Result if any single message fails or does not have a registered Handler.
 	result, err = app.runMsgs(runMsgCtx, msgs, mode)
-	if err == nil {
 
-		// Run optional postHandlers.
-		//
-		// Note: If the postHandler fails, we also revert the runMsgs state.
-		if app.postHandler != nil {
-			newCtx, err := app.postHandler(runMsgCtx, tx, result, mode == runTxModeSimulate, err == nil)
-			if err != nil {
-				return gInfo, nil, nil, priority, err
-			}
+	// Case 1: the msg errors and the post handler is not set.
+	if err != nil && app.postHandler == nil {
+		return gInfo, nil, events, priority, err
+	}
+
+	// Case 2: Run PostHandler and only
+	if err != nil && app.postHandler != nil {
+		// Run optional postHandlers with a context branched off the ante handler ctx
 
-			result.Events = append(result.Events, newCtx.EventManager().ABCIEvents()...)
+		postCtx, postCache := app.cacheTxContext(ctx, txBytes)
+
+		newCtx, err := app.postHandler(postCtx, tx, result, mode == runTxModeSimulate, err == nil)
+		if err != nil {
+			// return result in case the pointer has been modified by the post decorators
+			return gInfo, result, events, priority, err
 		}
 
 		if mode == runTxModeDeliver {
 			// When block gas exceeds, it'll panic and won't commit the cached store.
 			consumeBlockGas()
 
-			msCache.Write()
+			postCache.Write()
 		}
 
-		if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) {
+		if len(events) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) {
 			// append the events in the order of occurrence
-			result.Events = append(anteEvents, result.Events...)
+			postEvents = newCtx.EventManager().ABCIEvents()
+			events = append(events, postEvents...)
+			// TODO: copy slice
+			result.Events = append(result.Events, events...)
+		}
+
+		return gInfo, result, events, priority, err
+	}
+
+	// Case 3: Run Post Handler with runMsgCtx so that the state from runMsgs is persisted
+
+	if app.postHandler != nil {
+		newCtx, err := app.postHandler(runMsgCtx, tx, result, mode == runTxModeSimulate, err == nil)
+		if err != nil {
+			return gInfo, nil, nil, priority, err
 		}
+
+		postEvents = newCtx.EventManager().Events().ToABCIEvents()
+		result.Events = append(result.Events, postEvents...)
+
+		if len(anteEvents) > 0 {
+			events = make([]abci.Event, len(anteEvents)+len(postEvents))
+			copy(events[:len(anteEvents)], anteEvents)
+			copy(events[len(anteEvents):], postEvents)
+		} else {
+			events = make([]abci.Event, len(postEvents))
+			copy(events, postEvents)
+		}
+	}
+
+	if mode == runTxModeDeliver {
+		// When block gas exceeds, it'll panic and won't commit the cached store.
+		consumeBlockGas()
+
+		msCache.Write()
+	}
+
+	if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) {
+		// append the events in the order of occurrence:
+		// 	1. AnteHandler events
+		// 	2. Transaction Result events
+		// 	3. PostHandler events
+		result.Events = append(anteEvents, result.Events...)
+
+		copy(events, result.Events)
 	}
 
-	return gInfo, result, anteEvents, priority, err
+	return gInfo, result, events, priority, err
 }
 
 // runMsgs iterates through a list of messages and executes them with the provided