Refactor RunTx for stateful PostHandler logic on failure

[fedekunze]

Summary

Refactor BaseApps's RunTx so that PostHandler is run in the same way as the AnteHandler (even on failure).

Problem Definition

In order to fully support stateful precompiled contracts on Evmos (and other EVM chains using Etherming), we need to migrate the leftover gas refund logic to the PostHandler. However, the current structure of the BaseApp (more specifically, runTx) doesn't support running post handler logic when the transaction fails and is only run when runMsgs is successful.

Proposal

Refactor the PostHandlers so that RunTx is always run even on failure. This would make PostHandler stateful in the same way that the AnteHandler currently works.

The main change involved is moving the postHandler logic outside the if err == nil check as shown below

cosmos-sdk/baseapp/baseapp.go

Lines 735 to 761 in 385bff5

	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, mode == runTxModeSimulate)
	if err != nil {
	return gInfo, nil, nil, priority, err
	}
	result.Events = append(result.Events, newCtx.EventManager().ABCIEvents()...)
	}
	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
	result.Events = append(anteEvents, result.Events...)
	}
	}

[amaury1093]

ACK in general.

Transaction tips was a use case where the postHandler only ran when msg execution was successful, how would one achieve this with your proposal?

I also saw #13940, is it the case that if msg execution fails, then the new result arg is nil?

[alexanderbez]

Closing via #13942