From 45cf4ed514ba1bb7e7ed37ea1fb3f94744decae9 Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Mon, 9 Jan 2023 12:20:15 +0200 Subject: [PATCH] cherry-pick update-posthandler commit --- baseapp/baseapp.go | 20 +++++++--------- testutil/mock/types_handler.go | 42 +++++++++++++++++++++++++++++++-- types/handler.go | 43 +++++++++++++++++++++++++++++++--- types/handler_test.go | 31 ++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 16 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 1e95dac964ee..95e0f7386c4d 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -6,16 +6,17 @@ import ( "sort" "strings" + dbm "github.com/cosmos/cosmos-db" "github.com/cosmos/gogoproto/proto" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/tmhash" "github.com/tendermint/tendermint/libs/log" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" - dbm "github.com/tendermint/tm-db" "golang.org/x/exp/maps" codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/store" + storemetrics "github.com/cosmos/cosmos-sdk/store/metrics" "github.com/cosmos/cosmos-sdk/store/snapshots" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -62,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 @@ -141,7 +142,7 @@ type BaseApp struct { //nolint: maligned // abciListeners for hooking into the ABCI message processing of the BaseApp // and exposing the requests and responses to external consumers - abciListeners []ABCIListener + abciListeners []storetypes.ABCIListener } // NewBaseApp returns a reference to an initialized BaseApp. It accepts a @@ -156,7 +157,7 @@ func NewBaseApp( logger: logger, name: name, db: db, - cms: store.NewCommitMultiStore(db), + cms: store.NewCommitMultiStore(db, logger, storemetrics.NewNoOpMetrics()), // by default we use a no-op metric gather in store storeLoader: DefaultStoreLoader, grpcQueryRouter: NewGRPCQueryRouter(), msgServiceRouter: NewMsgServiceRouter(), @@ -716,20 +717,17 @@ 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, nil, priority, err + return gInfo, nil, anteEvents, priority, err } result.Events = append(result.Events, newCtx.EventManager().ABCIEvents()...) 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) +}