feat(app): Update post handlers to incorporate the runMsg success bool (#13940)

Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com>
Co-authored-by: Vladislav Varadinov <vladislav.varadinov@gmail.com>
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

CHANGELOG.md

@@ -128,6 +128,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### State Machine Breaking
 
+* (baseapp, x/auth/posthandler) [#13940](https://github.com/cosmos/cosmos-sdk/pull/13940) Update `PostHandler` to receive the `runTx` success boolean.
 * (x/group) [#13742](https://github.com/cosmos/cosmos-sdk/pull/13742) Migrate group policy account from module accounts to base account.
 * (codec) [#13307](https://github.com/cosmos/cosmos-sdk/pull/13307) Register all modules' `Msg`s with group's ModuleCdc so that Amino sign bytes are correctly generated.
 * (codec) [#13196](https://github.com/cosmos/cosmos-sdk/pull/13196) Register all modules' `Msg`s with gov's ModuleCdc so that Amino sign bytes are correctly generated.

baseapp/baseapp.go

@@ -63,7 +63,7 @@ type BaseApp struct { //nolint: maligned
 
 	mempool         mempool.Mempool            // application side mempool
 	anteHandler     sdk.AnteHandler            // ante handler for fee and auth
-	postHandler     sdk.AnteHandler            // post handler, optional, e.g. for tips
+	postHandler     sdk.PostHandler            // post handler, optional, e.g. for tips
 	initChainer     sdk.InitChainer            // initialize state with validators and state blob
 	beginBlocker    sdk.BeginBlocker           // logic to run before any txs
 	processProposal sdk.ProcessProposalHandler // the handler which runs on ABCI ProcessProposal
@@ -717,18 +717,15 @@ 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 {
-			// The runMsgCtx context currently contains events emitted by the ante handler.
-			// We clear this to correctly order events without duplicates.
-			// Note that the state is still preserved.
-			postCtx := runMsgCtx.WithEventManager(sdk.NewEventManager())
-
-			newCtx, err := app.postHandler(postCtx, tx, mode == runTxModeSimulate)
+			// Follow-up Ref: https://github.com/cosmos/cosmos-sdk/pull/13941
+			newCtx, err := app.postHandler(runMsgCtx, tx, mode == runTxModeSimulate, err == nil)
 			if err != nil {
 				return gInfo, nil, anteEvents, priority, err
 			}

baseapp/options.go

@@ -166,7 +166,7 @@ func (app *BaseApp) SetAnteHandler(ah sdk.AnteHandler) {
 	app.anteHandler = ah
 }
 
-func (app *BaseApp) SetPostHandler(ph sdk.AnteHandler) {
+func (app *BaseApp) SetPostHandler(ph sdk.PostHandler) {
 	if app.sealed {
 		panic("SetPostHandler() on sealed BaseApp")
 	}

docs/docs/core/00-baseapp.md

@@ -363,7 +363,7 @@ First, it retrieves the `sdk.Msg`'s fully-qualified type name, by checking the `
 
 ### PostHandler
 
-_PostHandler_ are like `AnteHandler` (they share the same signature), but they execute after [`RunMsgs`](#runmsgs).
+`PostHandler` is similar to `AnteHandler`, but it, as the name suggests, executes custom post tx processing logic after [`RunMsgs`](#runmsgs) is called. `PostHandler` receives the `Result` of the the `RunMsgs` in order to enable this customizable behavior.
 
 Like `AnteHandler`s, `PostHandler`s are theoretically optional, one use case for `PostHandler`s is transaction tips (enabled by default in simapp).
 Other use cases like unused gas refund can also be enabled by `PostHandler`s.

testutil/mock/types_handler.go

@@ -1,7 +1,8 @@
 // Code generated by MockGen.
 // Source: types/handler.go
-// Chanes:
-// + AnteHandler(...): calling `next` at the end of the function to run all anthe handler chain.
+// Manual changes:
+// + AnteHandler(...): calling `next` at the end of the function to run all ante handler chain.
+// + PostHandler(...): calling `next` at the end of the function to run all post handler chain.
 
 // Package mock is a generated GoMock package.
 package mock
@@ -50,3 +51,40 @@ func (mr *MockAnteDecoratorMockRecorder) AnteHandle(ctx, tx, simulate, next inte
 	mr.mock.ctrl.T.Helper()
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AnteHandle", reflect.TypeOf((*MockAnteDecorator)(nil).AnteHandle), ctx, tx, simulate, next)
 }
+
+// MockPostDecorator is a mock of PostDecorator interface.
+type MockPostDecorator struct {
+	ctrl     *gomock.Controller
+	recorder *MockPostDecoratorMockRecorder
+}
+
+// MockPostDecoratorMockRecorder is the mock recorder for MockPostDecorator.
+type MockPostDecoratorMockRecorder struct {
+	mock *MockPostDecorator
+}
+
+// NewMockPostDecorator creates a new mock instance.
+func NewMockPostDecorator(ctrl *gomock.Controller) *MockPostDecorator {
+	mock := &MockPostDecorator{ctrl: ctrl}
+	mock.recorder = &MockPostDecoratorMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockPostDecorator) EXPECT() *MockPostDecoratorMockRecorder {
+	return m.recorder
+}
+
+// PostHandle mocks base method.
+func (m *MockPostDecorator) PostHandle(ctx types.Context, tx types.Tx, simulate, success bool, next types.PostHandler) (types.Context, error) {
+	m.ctrl.T.Helper()
+	m.ctrl.Call(m, "PostHandle", ctx, tx, simulate, success, next)
+	// NOTE: we need to edit the generated code to call the "next handler"
+	return next(ctx, tx, simulate, success)
+}
+
+// PostHandle indicates an expected call of PostHandle.
+func (mr *MockPostDecoratorMockRecorder) PostHandle(ctx, tx, simulate, success, next interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PostHandle", reflect.TypeOf((*MockPostDecorator)(nil).PostHandle), ctx, tx, simulate, success, next)
+}

types/handler.go

@@ -7,12 +7,21 @@ type Handler func(ctx Context, msg Msg) (*Result, error)
 // If newCtx.IsZero(), ctx is used instead.
 type AnteHandler func(ctx Context, tx Tx, simulate bool) (newCtx Context, err error)
 
-// AnteDecorator wraps the next AnteHandler to perform custom pre- and post-processing.
+// PostHandler like AnteHandler but it executes after RunMsgs. Runs on success
+// or failure and enables use cases like gas refunding.
+type PostHandler func(ctx Context, tx Tx, simulate, success bool) (newCtx Context, err error)
+
+// AnteDecorator wraps the next AnteHandler to perform custom pre-processing.
 type AnteDecorator interface {
 	AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error)
 }
 
-// ChainDecorator chains AnteDecorators together with each AnteDecorator
+// PostDecorator wraps the next PostHandler to perform custom post-processing.
+type PostDecorator interface {
+	PostHandle(ctx Context, tx Tx, simulate, success bool, next PostHandler) (newCtx Context, err error)
+}
+
+// ChainAnteDecorators ChainDecorator chains AnteDecorators together with each AnteDecorator
 // wrapping over the decorators further along chain and returns a single AnteHandler.
 //
 // NOTE: The first element is outermost decorator, while the last element is innermost
@@ -41,6 +50,29 @@ func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler {
 	}
 }
 
+// ChainPostDecorators chains PostDecorators together with each PostDecorator
+// wrapping over the decorators further along chain and returns a single PostHandler.
+//
+// NOTE: The first element is outermost decorator, while the last element is innermost
+// decorator. Decorator ordering is critical since some decorators will expect
+// certain checks and updates to be performed (e.g. the Context) before the decorator
+// is run. These expectations should be documented clearly in a CONTRACT docline
+// in the decorator's godoc.
+func ChainPostDecorators(chain ...PostDecorator) PostHandler {
+	if len(chain) == 0 {
+		return nil
+	}
+
+	// handle non-terminated decorators chain
+	if (chain[len(chain)-1] != Terminator{}) {
+		chain = append(chain, Terminator{})
+	}
+
+	return func(ctx Context, tx Tx, simulate, success bool) (Context, error) {
+		return chain[0].PostHandle(ctx, tx, simulate, success, ChainPostDecorators(chain[1:]...))
+	}
+}
+
 // Terminator AnteDecorator will get added to the chain to simplify decorator code
 // Don't need to check if next == nil further up the chain
 //
@@ -61,7 +93,12 @@ func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler {
 //	 snd    \  \      \        /
 type Terminator struct{}
 
-// Simply return provided Context and nil error
+// AnteHandle returns the provided Context and nil error
 func (t Terminator) AnteHandle(ctx Context, _ Tx, _ bool, _ AnteHandler) (Context, error) {
 	return ctx, nil
 }
+
+// PostHandle returns the provided Context and nil error
+func (t Terminator) PostHandle(ctx Context, _ Tx, _, _ bool, _ PostHandler) (Context, error) {
+	return ctx, nil
+}

types/handler_test.go

@@ -33,3 +33,34 @@ func TestChainAnteDecorators(t *testing.T) {
 		mockAnteDecorator2)(ctx, tx, true)
 	require.NoError(t, err)
 }
+
+func TestChainPostDecorators(t *testing.T) {
+	// test panic when passing an empty sclice of PostDecorators
+	require.Nil(t, sdk.ChainPostDecorators([]sdk.PostDecorator{}...))
+
+	// Create empty context as well as transaction
+	ctx := sdk.Context{}
+	tx := sdk.Tx(nil)
+
+	// Create mocks
+	mockCtrl := gomock.NewController(t)
+	mockPostDecorator1 := mock.NewMockPostDecorator(mockCtrl)
+	mockPostDecorator2 := mock.NewMockPostDecorator(mockCtrl)
+
+	// Test chaining only one post decorator
+	mockPostDecorator1.EXPECT().PostHandle(gomock.Eq(ctx), gomock.Eq(tx), true, gomock.Eq(true), gomock.Any()).Times(1)
+	_, err := sdk.ChainPostDecorators(mockPostDecorator1)(ctx, tx, true, true)
+	require.NoError(t, err)
+
+	// Tests chaining multiple post decorators
+	mockPostDecorator1.EXPECT().PostHandle(gomock.Eq(ctx), gomock.Eq(tx), true, gomock.Eq(true), gomock.Any()).Times(1)
+	mockPostDecorator2.EXPECT().PostHandle(gomock.Eq(ctx), gomock.Eq(tx), true, gomock.Eq(true), gomock.Any()).Times(1)
+	// NOTE: we can't check that mockAnteDecorator2 is passed as the last argument because
+	// ChainAnteDecorators wraps the decorators into closures, so each decorator is
+	// receiving a closure.
+	_, err = sdk.ChainPostDecorators(
+		mockPostDecorator1,
+		mockPostDecorator2,
+	)(ctx, tx, true, true)
+	require.NoError(t, err)
+}

x/auth/posthandler/post.go

@@ -7,9 +7,9 @@ import (
 // HandlerOptions are the options required for constructing a default SDK PostHandler.
 type HandlerOptions struct{}
 
-// NewPostHandler returns an empty posthandler chain.
-func NewPostHandler(options HandlerOptions) (sdk.AnteHandler, error) {
-	postDecorators := []sdk.AnteDecorator{}
+// NewPostHandler returns an empty PostHandler chain.
+func NewPostHandler(_ HandlerOptions) (sdk.PostHandler, error) {
+	postDecorators := []sdk.PostDecorator{}
 
-	return sdk.ChainAnteDecorators(postDecorators...), nil
+	return sdk.ChainPostDecorators(postDecorators...), nil
 }