diff --git a/CHANGELOG.md b/CHANGELOG.md index 8703e0aaaa3d..806f8232fa47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking +* (baseapp) [#14593](https://github.com/cosmos/cosmos-sdk/pull/14593) Refactor `runTx` to handle `PostHandler` cases. * (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. diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index cff907546447..a1dba3cf0744 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -613,12 +613,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() @@ -707,23 +712,25 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re } priority = ctx.Priority() - msCache.Write() 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. @@ -733,38 +740,92 @@ 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 err != nil { + // Case 1: the msg errors and the post handler is not set. + if app.postHandler == nil { + return gInfo, result, anteEvents, priority, err + } + + // Case 2: tx errors and the post handler is set. Run PostHandler and revert state from runMsgs if app.postHandler != nil { + // Run optional postHandlers with a context branched off the ante handler ctx + postCtx, postCache := app.cacheTxContext(ctx, txBytes) + // 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()) + postCtx = runMsgCtx.WithEventManager(sdk.NewEventManager()) newCtx, err := app.postHandler(postCtx, tx, mode == runTxModeSimulate, err == nil) if err != nil { - return gInfo, nil, anteEvents, priority, err + // return result in case the pointer has been modified by the post decorators + return gInfo, result, events, priority, err } - result.Events = append(result.Events, newCtx.EventManager().ABCIEvents()...) - } + if mode == runTxModeDeliver { + // When block gas exceeds, it'll panic and won't commit the cached store. + consumeBlockGas() + + postCache.Write() + } - if mode == runTxModeDeliver { - // When block gas exceeds, it'll panic and won't commit the cached store. - consumeBlockGas() + if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) { + // append the events in the order of occurrence + 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...) + } - msCache.Write() + return gInfo, result, events, priority, err } + } - if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) { - // append the events in the order of occurrence - result.Events = append(anteEvents, result.Events...) + // 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) } } - return gInfo, result, anteEvents, priority, err + // 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, 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 4874a371c6e2..c3549b47b097 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -3,9 +3,11 @@ package baseapp_test import ( "fmt" "math/rand" + "os" "testing" "time" + "cosmossdk.io/depinject" errorsmod "cosmossdk.io/errors" "cosmossdk.io/log" "cosmossdk.io/store/metrics" @@ -15,8 +17,10 @@ import ( snapshottypes "cosmossdk.io/store/snapshots/types" storetypes "cosmossdk.io/store/types" abci "github.com/cometbft/cometbft/abci/types" + cmtlog "github.com/cometbft/cometbft/libs/log" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" dbm "github.com/cosmos/cosmos-db" + "github.com/cosmos/cosmos-sdk/runtime" "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/baseapp" @@ -655,3 +659,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, storetypes.NewTransientStoreKey("transient_test")) + + app := appBuilder.Build(cmtlog.NewTMLogger(cmtlog.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 := cmtproto.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, storetypes.NewTransientStoreKey("transient_test")) + + app := appBuilder.Build(cmtlog.NewTMLogger(cmtlog.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(testTxPostHandler(t, capKey1, postKey)) + header := cmtproto.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, 2, "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, storetypes.NewTransientStoreKey("transient_test")) + + app := appBuilder.Build(cmtlog.NewTMLogger(cmtlog.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(testTxPostHandler(t, capKey1, postKey)) + header := cmtproto.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(_ *testing.T) { + tc.testFunc() + }) + } +} diff --git a/baseapp/utils_test.go b/baseapp/utils_test.go index 53eb3cfaab31..6266dab88ea6 100644 --- a/baseapp/utils_test.go +++ b/baseapp/utils_test.go @@ -232,6 +232,20 @@ func anteHandlerTxTest(t *testing.T, capKey storetypes.StoreKey, storeKey []byte } } +func testTxPostHandler(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 incrementingCounter(t *testing.T, store storetypes.KVStore, counterKey []byte, counter int64) (*sdk.Result, error) { storedCounter := getIntFromStore(t, store, counterKey) require.Equal(t, storedCounter, counter)