diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d201c57aa84..49afe1181821 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,6 +126,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/abci.go b/baseapp/abci.go index aacfcb77ed64..17a15e6e5d85 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -349,9 +349,9 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx { panic(fmt.Sprintf("unknown RequestCheckTx type: %s", req.Type)) } - gInfo, result, anteEvents, priority, err := app.runTx(mode, req.Tx) + gInfo, result, events, priority, err := app.runTx(mode, req.Tx) if err != nil { - return sdkerrors.ResponseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, app.trace) + return sdkerrors.ResponseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, events, app.trace) } return abci.ResponseCheckTx{ @@ -388,10 +388,10 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) (res abci.ResponseDeliv telemetry.SetGauge(float32(gInfo.GasWanted), "tx", "gas", "wanted") }() - gInfo, result, anteEvents, _, err := app.runTx(runTxModeDeliver, req.Tx) + gInfo, result, events, _, err := app.runTx(runTxModeDeliver, req.Tx) if err != nil { resultStr = "failed" - return sdkerrors.ResponseDeliverTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, sdk.MarkEventsToIndex(anteEvents, app.indexEvents), app.trace) + return sdkerrors.ResponseDeliverTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, sdk.MarkEventsToIndex(events, app.indexEvents), app.trace) } return abci.ResponseDeliverTx{ diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 95e0f7386c4d..7329d59f9515 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -600,12 +600,17 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context // Note, gas execution info is always returned. A reference to a Result is // returned if the tx does not run out of gas and if all the messages are valid // and execute successfully. An error is returned otherwise. -func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, priority int64, err error) { +func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, events []abci.Event, priority int64, err error) { // NOTE: GasWanted should be returned by the AnteHandler. GasUsed is // determined by the GasMeter. We need access to the context to get the gas // meter, so we initialize upfront. var gasWanted uint64 + var ( + anteEvents []abci.Event + postEvents []abci.Event + ) + ctx := app.getContextForTx(mode, txBytes) ms := ctx.MultiStore() @@ -670,6 +675,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re anteCtx, msCache = app.cacheTxContext(ctx, txBytes) anteCtx = anteCtx.WithEventManager(sdk.NewEventManager()) newCtx, err := app.anteHandler(anteCtx, tx, mode == runTxModeSimulate) + if err != nil { + return gInfo, nil, nil, 0, err + } if !newCtx.IsZero() { // At this point, newCtx.MultiStore() is a store branch, or something else @@ -681,33 +689,30 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re ctx = newCtx.WithMultiStore(ms) } - events := ctx.EventManager().Events() - // GasMeter expected to be set in AnteHandler gasWanted = ctx.GasMeter().Limit() - - if err != nil { - return gInfo, nil, nil, 0, err - } - priority = ctx.Priority() - msCache.Write() + events := ctx.EventManager().Events() anteEvents = events.ToABCIEvents() + + msCache.Write() } - if mode == runTxModeCheck { + // insert or remove the transaction from the mempool + switch mode { + case runTxModeCheck: err = app.mempool.Insert(ctx, tx) - if err != nil { - return gInfo, nil, anteEvents, priority, err - } - } else if mode == runTxModeDeliver { + case runTxModeDeliver: err = app.mempool.Remove(tx) if err != nil && !errors.Is(err, mempool.ErrTxNotFound) { - return gInfo, nil, anteEvents, priority, - fmt.Errorf("failed to remove tx from mempool: %w", err) + err = fmt.Errorf("failed to remove tx from mempool: %w", err) } } + if err != nil { + return gInfo, nil, events, priority, err + } + // Create a new Context based off of the existing Context with a MultiStore branch // in case message processing fails. At this point, the MultiStore // is a branch of a branch. @@ -718,35 +723,85 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // Result if any single message fails or does not have a registered Handler. result, err = app.runMsgs(runMsgCtx, msgs, mode) - if err == nil { + // Case 1: the msg errors and the post handler is not set. + if err != nil && app.postHandler == nil { + return gInfo, nil, events, priority, err + } - // Run optional postHandlers. - // - // Note: If the postHandler fails, we also revert the runMsgs state. - if app.postHandler != nil { - // 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 - } + // Case 2: tx errors and the post handler is set. Run PostHandler and revert state from runMsgs + if err != nil && app.postHandler != nil { + // Run optional postHandlers with a context branched off the ante handler ctx + postCtx, postCache := app.cacheTxContext(ctx, txBytes) - result.Events = append(result.Events, newCtx.EventManager().ABCIEvents()...) + newCtx, err := app.postHandler(postCtx, tx, mode == runTxModeSimulate, err == nil) + if err != nil { + // return result in case the pointer has been modified by the post decorators + return gInfo, result, events, priority, err } if mode == runTxModeDeliver { // When block gas exceeds, it'll panic and won't commit the cached store. consumeBlockGas() - msCache.Write() + postCache.Write() } if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) { // append the events in the order of occurrence - result.Events = append(anteEvents, result.Events...) + postEvents = newCtx.EventManager().ABCIEvents() + events = make([]abci.Event, len(anteEvents)+len(postEvents)) + copy(events[:len(anteEvents)], anteEvents) + copy(events[len(anteEvents):], postEvents) + result.Events = append(result.Events, events...) + } else { + events = make([]abci.Event, len(postEvents)) + copy(events, postEvents) + result.Events = append(result.Events, postEvents...) + } + + return gInfo, result, events, priority, err + } + + // Case 3: tx successful and post handler is set. Run Post Handler with runMsgCtx so that the state from runMsgs is persisted + if app.postHandler != nil { + newCtx, err := app.postHandler(runMsgCtx, tx, mode == runTxModeSimulate, err == nil) + if err != nil { + return gInfo, nil, nil, priority, err } + + postEvents = newCtx.EventManager().Events().ToABCIEvents() + result.Events = append(result.Events, postEvents...) + + if len(anteEvents) > 0 { + events = make([]abci.Event, len(anteEvents)+len(postEvents)) + copy(events[:len(anteEvents)], anteEvents) + copy(events[len(anteEvents):], postEvents) + } else { + events = make([]abci.Event, len(postEvents)) + copy(events, postEvents) + } + } + + // Case 4: tx successful and post handler is not set. + + if mode == runTxModeDeliver { + // When block gas exceeds, it'll panic and won't commit the cached store. + consumeBlockGas() + + msCache.Write() + } + + if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) { + // append the events in the order of occurrence: + // 1. AnteHandler events + // 2. Transaction Result events + // 3. PostHandler events + result.Events = append(anteEvents, result.Events...) + + copy(events, result.Events) } - return gInfo, result, anteEvents, priority, err + return gInfo, result, events, priority, err } // runMsgs iterates through a list of messages and executes them with the provided diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index e757dd224885..3db0f30d9a5b 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -3,9 +3,14 @@ package baseapp_test import ( "fmt" "math/rand" + "os" "testing" "time" + "cosmossdk.io/depinject" + + "github.com/cosmos/cosmos-sdk/runtime" + dbm "github.com/cosmos/cosmos-db" "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" @@ -655,3 +660,167 @@ func TestLoadVersionPruning(t *testing.T) { require.Nil(t, err) testLoadVersionHelper(t, app, int64(7), lastCommitID) } + +func TestBaseAppPostHandler(t *testing.T) { + testCases := []struct { + name string + testFunc func() + }{ + { + "success case 1 - The msg errors and the PostHandler is not set", + func() { + // Setup baseapp. + var ( + appBuilder *runtime.AppBuilder + cdc codec.ProtoCodecMarshaler + ) + err := depinject.Inject(makeMinimalConfig(), &appBuilder, &cdc) + require.NoError(t, err) + + testCtx := testutil.DefaultContextWithDB(t, capKey1, sdk.NewTransientStoreKey("transient_test")) + + app := appBuilder.Build(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), testCtx.DB, nil) + app.SetCMS(testCtx.CMS) + baseapptestutil.RegisterInterfaces(cdc.InterfaceRegistry()) + + // patch in TxConfig instead of using an output from x/auth/tx + txConfig := authtx.NewTxConfig(cdc, authtx.DefaultSignModes) + // set the TxDecoder in the BaseApp for minimal tx simulations + app.SetTxDecoder(txConfig.TxDecoder()) + + app.InitChain(abci.RequestInitChain{}) + + deliverKey := []byte("deliver-key") + baseapptestutil.RegisterCounterServer(app.MsgServiceRouter(), CounterServerImpl{t, capKey1, deliverKey}) + + header := tmproto.Header{Height: int64(1)} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + + tx := newTxCounter(t, txConfig, 1, 0) + tx = setFailOnHandler(txConfig, tx, true) + txBytes, err := txConfig.TxEncoder()(tx) + require.NoError(t, err) + + r := app.CheckTx(abci.RequestCheckTx{Tx: txBytes}) + require.True(t, r.IsOK(), fmt.Sprintf("%v", r)) + + res := app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes}) + require.False(t, res.IsOK(), fmt.Sprintf("%v", res)) + + events := res.GetEvents() + require.Len(t, events, 0, "Should not contain any events as the post handler is not set") + + app.EndBlock(abci.RequestEndBlock{}) + app.Commit() + }, + }, + { + "success case 2 - The msg errors and the PostHandler is set", + func() { + postKey := []byte("post-key") + anteKey := []byte("ante-key") + // Setup baseapp. + var ( + appBuilder *runtime.AppBuilder + cdc codec.ProtoCodecMarshaler + ) + err := depinject.Inject(makeMinimalConfig(), &appBuilder, &cdc) + require.NoError(t, err) + + testCtx := testutil.DefaultContextWithDB(t, capKey1, sdk.NewTransientStoreKey("transient_test")) + + app := appBuilder.Build(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), testCtx.DB, nil) + app.SetCMS(testCtx.CMS) + baseapptestutil.RegisterInterfaces(cdc.InterfaceRegistry()) + + // patch in TxConfig instead of using an output from x/auth/tx + txConfig := authtx.NewTxConfig(cdc, authtx.DefaultSignModes) + // set the TxDecoder in the BaseApp for minimal tx simulations + app.SetTxDecoder(txConfig.TxDecoder()) + + app.InitChain(abci.RequestInitChain{}) + + deliverKey := []byte("deliver-key") + baseapptestutil.RegisterCounterServer(app.MsgServiceRouter(), CounterServerImpl{t, capKey1, deliverKey}) + + app.SetAnteHandler(anteHandlerTxTest(t, capKey1, anteKey)) + app.SetPostHandler(postHandlerTxTest(t, capKey1, postKey)) + header := tmproto.Header{Height: int64(1)} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + + tx := newTxCounter(t, txConfig, 0, 0) + tx = setFailOnHandler(txConfig, tx, true) + txBytes, err := txConfig.TxEncoder()(tx) + require.NoError(t, err) + + r := app.CheckTx(abci.RequestCheckTx{Tx: txBytes}) + require.True(t, r.IsOK(), fmt.Sprintf("%v", r)) + + res := app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes}) + require.False(t, res.IsOK(), fmt.Sprintf("%v", res)) + + events := res.GetEvents() + require.Len(t, events, 3, "Contains the AnteHandler and the PostHandler") + + app.EndBlock(abci.RequestEndBlock{}) + app.Commit() + }, + }, + { + "success case 3 - Run Post Handler with runMsgCtx so that the state from runMsgs is persisted", + func() { + postKey := []byte("post-key") + // Setup baseapp. + var ( + appBuilder *runtime.AppBuilder + cdc codec.ProtoCodecMarshaler + ) + err := depinject.Inject(makeMinimalConfig(), &appBuilder, &cdc) + require.NoError(t, err) + + testCtx := testutil.DefaultContextWithDB(t, capKey1, sdk.NewTransientStoreKey("transient_test")) + + app := appBuilder.Build(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), testCtx.DB, nil) + app.SetCMS(testCtx.CMS) + baseapptestutil.RegisterInterfaces(cdc.InterfaceRegistry()) + + // patch in TxConfig instead of using an output from x/auth/tx + txConfig := authtx.NewTxConfig(cdc, authtx.DefaultSignModes) + // set the TxDecoder in the BaseApp for minimal tx simulations + app.SetTxDecoder(txConfig.TxDecoder()) + + app.InitChain(abci.RequestInitChain{}) + + deliverKey := []byte("deliver-key") + baseapptestutil.RegisterCounterServer(app.MsgServiceRouter(), CounterServerImpl{t, capKey1, deliverKey}) + + app.SetPostHandler(postHandlerTxTest(t, capKey1, postKey)) + header := tmproto.Header{Height: int64(1)} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + + tx := newTxCounter(t, txConfig, 0, 0) + txBytes, err := txConfig.TxEncoder()(tx) + require.NoError(t, err) + + r := app.CheckTx(abci.RequestCheckTx{Tx: txBytes}) + require.True(t, r.IsOK(), fmt.Sprintf("%v", r)) + + res := app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes}) + require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) + + events := res.GetEvents() + require.Len(t, events, 3, "should contain ante handler, message type and counter events respectively") + + require.NoError(t, err) + app.EndBlock(abci.RequestEndBlock{}) + app.Commit() + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.testFunc() + }) + } +} diff --git a/baseapp/options.go b/baseapp/options.go index e78295917c38..849808efcc62 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -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") } diff --git a/baseapp/utils_test.go b/baseapp/utils_test.go index 4ce4feb0fad9..8112e1b3040d 100644 --- a/baseapp/utils_test.go +++ b/baseapp/utils_test.go @@ -207,6 +207,20 @@ func counterEvent(evType string, msgCount int64) sdk.Events { } } +func postHandlerTxTest(t *testing.T, capKey storetypes.StoreKey, storeKey []byte) sdk.PostHandler { + return func(ctx sdk.Context, tx sdk.Tx, simulate, success bool) (sdk.Context, error) { + counter, _ := parseTxMemo(t, tx) + + ctx.EventManager().EmitEvents( + counterEvent("post_handler", counter), + ) + + ctx = ctx.WithPriority(testTxPriority) + + return ctx, nil + } +} + func anteHandlerTxTest(t *testing.T, capKey storetypes.StoreKey, storeKey []byte) sdk.AnteHandler { return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (sdk.Context, error) { store := ctx.KVStore(capKey) 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/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 }