Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 21 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f4ef9d6
feat(app): Update post handlers
fedekunze Nov 18, 2022
f98a21f
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Nov 21, 2022
73a85e7
docs
fedekunze Nov 21, 2022
2c29db4
Merge branch 'fedekunze/update-posthandlers' of github.com:cosmos/cos…
fedekunze Nov 21, 2022
70e44d0
c++
fedekunze Nov 21, 2022
5145420
add post decorator mocks and tests (#13945)
MalteHerrmann Nov 21, 2022
1ed6c98
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Nov 21, 2022
99b69d5
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Nov 21, 2022
b12c0b3
fix: Added godoc for the PostHandler and reference comment for follow…
Vvaradinov Dec 12, 2022
4fa8327
conflicts
fedekunze Dec 13, 2022
4846d62
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Dec 13, 2022
364d6d8
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Dec 14, 2022
5131903
refactor: Use only MsgResponses from Result in PostHandler (#14299)
Vvaradinov Dec 15, 2022
f8841ee
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Dec 15, 2022
325f613
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Jan 3, 2023
ff1cdfc
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Jan 7, 2023
6277794
fix: remove MsgResponses from PostHandler (#14522)
Vvaradinov Jan 9, 2023
1e3745f
Update CHANGELOG.md
fedekunze Jan 11, 2023
5332380
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Jan 11, 2023
f51f2af
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Jan 11, 2023
1ded094
Merge branch 'main' into fedekunze/update-posthandlers
fedekunze Jan 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,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` call `Result` and success boolean.
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
* (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.
Expand Down
11 changes: 4 additions & 7 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not a regression of #13983 to remove this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@Vvaradinov Vvaradinov Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this was deleted by mistake, will address in another PR. #14613


newCtx, err := app.postHandler(postCtx, tx, mode == runTxModeSimulate)
// Follow-up Ref: https://github.com/cosmos/cosmos-sdk/pull/13941
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
newCtx, err := app.postHandler(runMsgCtx, tx, result.MsgResponses, mode == runTxModeSimulate, err == nil)
if err != nil {
return gInfo, nil, anteEvents, priority, err
}
Expand Down
2 changes: 1 addition & 1 deletion baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,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")
}
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/core/00-baseapp.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
43 changes: 41 additions & 2 deletions testutil/mock/types_handler.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 41 additions & 2 deletions types/handler.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
package types

import codectypes "github.com/cosmos/cosmos-sdk/codec/types"

// Handler defines the core of the state transition function of an application.
type Handler func(ctx Context, msg Msg) (*Result, error)

// AnteHandler authenticates transactions, before their internal messages are handled.
// 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, msgResponses []*codectypes.Any, 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)
}

// PostDecorator wraps the next PostHandler to perform custom post-processing.
type PostDecorator interface {
PostHandle(ctx Context, tx Tx, msgResponses []*codectypes.Any, simulate, success bool, next PostHandler) (newCtx Context, err error)
}

// ChainDecorator chains AnteDecorators together with each AnteDecorator
// wrapping over the decorators further along chain and returns a single AnteHandler.
//
Expand Down Expand Up @@ -41,6 +52,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, msgResponses []*codectypes.Any, simulate, success bool) (Context, error) {
return chain[0].PostHandle(ctx, tx, msgResponses, 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
//
Expand All @@ -61,7 +95,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
}

// PostHandler returns the provided Context and nil error
func (t Terminator) PostHandle(ctx Context, _ Tx, _ []*codectypes.Any, _, _ bool, _ PostHandler) (Context, error) {
return ctx, nil
}
32 changes: 32 additions & 0 deletions types/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,35 @@ func TestChainAnteDecorators(t *testing.T) {
mockAnteDecorator2)(ctx, tx, true)
require.NoError(t, err)
}

func (s *handlerTestSuite) TestChainPostDecorators() {
// test panic when passing an empty sclice of PostDecorators
s.Require().Nil(sdk.ChainPostDecorators([]sdk.PostDecorator{}...))

// Create empty context as well as transaction
ctx := sdk.Context{}
tx := sdk.Tx(nil)
res := &sdk.Result{}

// Create mocks
mockCtrl := gomock.NewController(s.T())
mockPostDecorator1 := mock.NewMockPostDecorator(mockCtrl)
mockPostDecorator2 := mock.NewMockPostDecorator(mockCtrl)

// Test chaining only one post decorator
mockPostDecorator1.EXPECT().PostHandle(gomock.Eq(ctx), gomock.Eq(tx), gomock.Eq(res.MsgResponses), true, gomock.Eq(true), gomock.Any()).Times(1)
_, err := sdk.ChainPostDecorators(mockPostDecorator1)(ctx, tx, res.MsgResponses, true, true)
s.Require().NoError(err)

// Tests chaining multiple post decorators
mockPostDecorator1.EXPECT().PostHandle(gomock.Eq(ctx), gomock.Eq(tx), gomock.Eq(res.MsgResponses), true, gomock.Eq(true), gomock.Any()).Times(1)
mockPostDecorator2.EXPECT().PostHandle(gomock.Eq(ctx), gomock.Eq(tx), gomock.Eq(res.MsgResponses), 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, res.MsgResponses, true, true)
s.Require().NoError(err)
}
8 changes: 4 additions & 4 deletions x/auth/posthandler/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}