diff --git a/CHANGELOG.md b/CHANGELOG.md index e5fc8483d9e0..cf3820959c19 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index d8a6feef91e3..0fa09db8e1b0 100644 --- a/baseapp/baseapp.go +++ b/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 } diff --git a/baseapp/options.go b/baseapp/options.go index 22d3aab19485..e6e7c5486481 100644 --- a/baseapp/options.go +++ b/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") } diff --git a/docs/docs/core/00-baseapp.md b/docs/docs/core/00-baseapp.md index b7ad57e58a80..4d8b2d18943f 100644 --- a/docs/docs/core/00-baseapp.md +++ b/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. diff --git a/testutil/mock/types_handler.go b/testutil/mock/types_handler.go index efcbd1e7f873..c3a59d0362b2 100644 --- a/testutil/mock/types_handler.go +++ b/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) +} diff --git a/types/handler.go b/types/handler.go index 87762744ee86..1a86c1660551 100644 --- a/types/handler.go +++ b/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 +} diff --git a/types/handler_test.go b/types/handler_test.go index 14c38d282412..3d20d3023d2d 100644 --- a/types/handler_test.go +++ b/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) +} diff --git a/x/auth/posthandler/post.go b/x/auth/posthandler/post.go index 8d3fb7776c63..40310cad5ae7 100644 --- a/x/auth/posthandler/post.go +++ b/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 }