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 (backport #13940) #14586

Merged
merged 2 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
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

@julienrbrt julienrbrt Jan 13, 2023

Choose a reason for hiding this comment

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

I think it's 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.

If we merge this we need to backport #14613 too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, let's backport both


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
}
Expand Down
2 changes: 1 addition & 1 deletion baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
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
42 changes: 40 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: 40 additions & 3 deletions types/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
//
Expand All @@ -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
}
32 changes: 32 additions & 0 deletions types/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
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
}