From 50973b4e404a7619691a8a6b42122d51c6c4853f Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Thu, 12 Jan 2023 15:19:28 +0200 Subject: [PATCH 01/12] refactor: runTx now includes 3 cases for PostHandler and unit tests --- baseapp/baseapp.go | 100 ++++++++++++++++++------ baseapp/baseapp_test.go | 167 ++++++++++++++++++++++++++++++++++++++++ baseapp/utils_test.go | 14 ++++ 3 files changed, 257 insertions(+), 24 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 22ed0abd881a..7522f1e90504 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,18 +689,12 @@ 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 { @@ -718,35 +720,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) } } - 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 9e93a81422ef..cc033c4b9b91 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -1,8 +1,11 @@ package baseapp_test import ( + "cosmossdk.io/depinject" "fmt" + "github.com/cosmos/cosmos-sdk/runtime" "math/rand" + "os" "testing" "time" @@ -655,3 +658,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(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, storetypes.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, storetypes.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/utils_test.go b/baseapp/utils_test.go index bdd45d35894a..10fc97925e23 100644 --- a/baseapp/utils_test.go +++ b/baseapp/utils_test.go @@ -230,6 +230,20 @@ func anteHandlerTxTest(t *testing.T, capKey storetypes.StoreKey, storeKey []byte } } +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 incrementingCounter(t *testing.T, store storetypes.KVStore, counterKey []byte, counter int64) (*sdk.Result, error) { storedCounter := getIntFromStore(t, store, counterKey) require.Equal(t, storedCounter, counter) From 40561cafec0b3ae944c1061d6d1693b8919ef344 Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Thu, 12 Jan 2023 15:28:42 +0200 Subject: [PATCH 02/12] Added Changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf3820959c19..06df427773d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,6 +128,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. From 67345ae1380dffa77136f5e27b390e32e241f477 Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Thu, 12 Jan 2023 20:26:18 +0200 Subject: [PATCH 03/12] Update baseapp/utils_test.go Co-authored-by: Aleksandr Bezobchuk --- baseapp/utils_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/utils_test.go b/baseapp/utils_test.go index 10fc97925e23..48284110fd2a 100644 --- a/baseapp/utils_test.go +++ b/baseapp/utils_test.go @@ -230,7 +230,7 @@ func anteHandlerTxTest(t *testing.T, capKey storetypes.StoreKey, storeKey []byte } } -func postHandlerTxTest(t *testing.T, capKey storetypes.StoreKey, storeKey []byte) sdk.PostHandler { +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) From 5ebbfa915de05230905d33de31ec997159c1d30d Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Fri, 13 Jan 2023 15:44:03 +0200 Subject: [PATCH 04/12] fix: linting --- baseapp/baseapp_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index cc033c4b9b91..3d725252cf21 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -1,14 +1,16 @@ package baseapp_test import ( - "cosmossdk.io/depinject" "fmt" - "github.com/cosmos/cosmos-sdk/runtime" "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" @@ -742,7 +744,7 @@ func TestBaseAppPostHandler(t *testing.T) { baseapptestutil.RegisterCounterServer(app.MsgServiceRouter(), CounterServerImpl{t, capKey1, deliverKey}) app.SetAnteHandler(anteHandlerTxTest(t, capKey1, anteKey)) - app.SetPostHandler(postHandlerTxTest(t, capKey1, postKey)) + app.SetPostHandler(testTxPostHandler(t, capKey1, postKey)) header := tmproto.Header{Height: int64(1)} app.BeginBlock(abci.RequestBeginBlock{Header: header}) @@ -792,7 +794,7 @@ func TestBaseAppPostHandler(t *testing.T) { deliverKey := []byte("deliver-key") baseapptestutil.RegisterCounterServer(app.MsgServiceRouter(), CounterServerImpl{t, capKey1, deliverKey}) - app.SetPostHandler(postHandlerTxTest(t, capKey1, postKey)) + app.SetPostHandler(testTxPostHandler(t, capKey1, postKey)) header := tmproto.Header{Height: int64(1)} app.BeginBlock(abci.RequestBeginBlock{Header: header}) From 2f345d409586c3935b7097febd86306a6a7da2f0 Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Fri, 13 Jan 2023 15:51:07 +0200 Subject: [PATCH 05/12] fix: added missing gasWanted --- baseapp/baseapp.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 7522f1e90504..9604a220c796 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -675,9 +675,6 @@ 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 @@ -690,6 +687,12 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re } // GasMeter expected to be set in AnteHandler + gasWanted = ctx.GasMeter().Limit() + + if err != nil { + return gInfo, nil, nil, 0, err + } + priority = ctx.Priority() events := ctx.EventManager().Events() anteEvents = events.ToABCIEvents() From 3d1a227c0a7a3ebb39099bb5fa3e31f3ab3c2544 Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Fri, 13 Jan 2023 16:08:40 +0200 Subject: [PATCH 06/12] Update baseapp/baseapp.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- baseapp/baseapp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 9604a220c796..313f03684345 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -725,7 +725,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // Case 1: the msg errors and the post handler is not set. if err != nil && app.postHandler == nil { - return gInfo, nil, events, priority, err + return gInfo, nil, anteEvents, priority, err } // Case 2: tx errors and the post handler is set. Run PostHandler and revert state from runMsgs From a117cf4ed9368480b7a4164ec83f1c5b50d02d09 Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Fri, 13 Jan 2023 16:16:49 +0200 Subject: [PATCH 07/12] fix: applied changes from code review --- baseapp/baseapp.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 313f03684345..c573209d4dcd 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -686,6 +686,8 @@ 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() @@ -694,9 +696,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re } priority = ctx.Priority() - events := ctx.EventManager().Events() anteEvents = events.ToABCIEvents() - msCache.Write() } From c7eeaa48e5b32552c8a8ec2d6102c46b166beb97 Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Mon, 6 Feb 2023 16:43:16 +0100 Subject: [PATCH 08/12] fix: fixed conflicts from merge --- baseapp/baseapp.go | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 8fe0a0f8bf54..0a2e8f9a86c0 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -701,19 +701,21 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re 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. @@ -734,19 +736,6 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // Run optional postHandlers with a context branched off the ante handler ctx postCtx, postCache := app.cacheTxContext(ctx, txBytes) - // 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, err == nil) - if err != nil { - return gInfo, nil, anteEvents, priority, err - } - 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 From ad96192fbd89e65cc0a14cdfa64a12491efc836d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Sun, 19 Feb 2023 09:02:31 +0100 Subject: [PATCH 09/12] Update baseapp/baseapp.go --- baseapp/baseapp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 0a2e8f9a86c0..cd5d38ce3617 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -728,7 +728,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // Case 1: the msg errors and the post handler is not set. if err != nil && app.postHandler == nil { - return gInfo, nil, anteEvents, priority, err + return gInfo, result, anteEvents, priority, err } // Case 2: tx errors and the post handler is set. Run PostHandler and revert state from runMsgs From 60a0705321df218c39871bfd4585838e165e0bfe Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Tue, 21 Feb 2023 10:33:19 +0200 Subject: [PATCH 10/12] fix: combine err case into single if statement --- baseapp/baseapp.go | 72 ++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index f5eca7d3eab7..f66b56922382 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -740,48 +740,50 @@ 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) - // Case 1: the msg errors and the post handler is not set. - if err != nil && app.postHandler == nil { - return gInfo, result, anteEvents, priority, err - } + 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 err != nil && app.postHandler != nil { - // Run optional postHandlers with a context branched off the ante handler ctx - postCtx, postCache := app.cacheTxContext(ctx, txBytes) + // 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()) + // 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, err == nil) - if err != nil { - // return result in case the pointer has been modified by the post decorators - return gInfo, result, events, priority, err - } + 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() + if mode == runTxModeDeliver { + // When block gas exceeds, it'll panic and won't commit the cached store. + consumeBlockGas() - postCache.Write() - } + postCache.Write() + } - 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...) - } + 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...) + } - return gInfo, result, events, priority, err + 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 From 89021c9d25d138b7528801f165106d381ea4dfe3 Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Wed, 22 Feb 2023 18:12:08 +0200 Subject: [PATCH 11/12] fix test --- baseapp/baseapp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 3f74437ad2be..c3549b47b097 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -759,7 +759,7 @@ func TestBaseAppPostHandler(t *testing.T) { require.False(t, res.IsOK(), fmt.Sprintf("%v", res)) events := res.GetEvents() - require.Len(t, events, 3, "Contains the AnteHandler and the PostHandler") + require.Len(t, events, 2, "Contains the AnteHandler and the PostHandler") app.EndBlock(abci.RequestEndBlock{}) app.Commit() From c29e7818810e7a93c85e7f4cb1922ffee7783045 Mon Sep 17 00:00:00 2001 From: Vladislav Varadinov Date: Wed, 1 Mar 2023 10:01:03 +0200 Subject: [PATCH 12/12] Update baseapp/baseapp.go Co-authored-by: Aleksandr Bezobchuk --- baseapp/baseapp.go | 1 - 1 file changed, 1 deletion(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 0d923e463b93..a1dba3cf0744 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -740,7 +740,6 @@ 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 { // Case 1: the msg errors and the post handler is not set. if app.postHandler == nil {