diff --git a/CHANGELOG.md b/CHANGELOG.md index acbf7ee2e7e..fac1a8a8c4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +## State Machine Breaking + +* (baseapp, x/auth/posthandler) [#13940](https://github.com/cosmos/cosmos-sdk/pull/13940) Update `PostHandler` to receive the `runTx` success boolean. + ## [v0.47.0-rc1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.0-rc1) - 2022-01-09 ### Features diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index de8f9634b3a..b114337975f 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 b45a7198573..69449fdd994 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -164,7 +164,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 b7ad57e58a8..4d8b2d18943 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 efcbd1e7f87..c3a59d0362b 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 87762744ee8..1a86c166055 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 d656360a4ac..8c2510719c8 100644 --- a/types/handler_test.go +++ b/types/handler_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/cosmos/cosmos-sdk/testutil/mock" @@ -45,3 +46,34 @@ func (s *handlerTestSuite) TestChainAnteDecorators() { mockAnteDecorator2)(ctx, tx, true) s.Require().NoError(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 8d3fb7776c6..40310cad5ae 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 }