From ece3d0e9fe1da69966e32391931904607fca127f Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Tue, 31 May 2022 16:05:16 +0200 Subject: [PATCH] refactor: Mark tips as beta (#12089) ## Description Following our SDK call, we decided to mark the Tip decorator as beta for now, and not include it in the default posthandler chain. This PR also fixes events not all included in the response in SimulateTx. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- CHANGELOG.md | 5 ++++ baseapp/baseapp.go | 8 ++--- docs/core/tips.md | 53 +++++++++++++++++++++------------ simapp/app.go | 11 ++----- x/auth/client/testutil/suite.go | 3 ++ x/auth/posthandler/post.go | 18 ++--------- x/auth/posthandler/tips.go | 2 ++ 7 files changed, 53 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c9cf3f83191..0bb227e5b8f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,9 +41,14 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (cli) [#12028](https://github.com/cosmos/cosmos-sdk/pull/12028) Add the `tendermint key-migrate` to perform Tendermint v0.35 DB key migration. +### Improvements + +* [#12089](https://github.com/cosmos/cosmos-sdk/pull/12089) Mark the `TipDecorator` as beta, don't include it in simapp by default. + ### Bug Fixes * (migrations) [#12028](https://github.com/cosmos/cosmos-sdk/pull/12028) Fix v0.45->v0.46 in-place store migrations. +* (baseapp) [#12089](https://github.com/cosmos/cosmos-sdk/pull/12089) Include antehandler and runMsgs events in SimulateTx. ## [v0.46.0-rc1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.0-rc1) - 2022-05-23 diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 636fd60e61b4..9888011c648e 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -707,11 +707,11 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re consumeBlockGas() msCache.Write() + } - if len(anteEvents) > 0 { - // append the events in the order of occurrence - result.Events = append(anteEvents, result.Events...) - } + if len(anteEvents) > 0 && (mode == runTxModeDeliver || mode == runTxModeSimulate) { + // append the events in the order of occurrence + result.Events = append(anteEvents, result.Events...) } } diff --git a/docs/core/tips.md b/docs/core/tips.md index 2165f4bbcd38..45914a185c58 100644 --- a/docs/core/tips.md +++ b/docs/core/tips.md @@ -4,7 +4,7 @@ order: 14 # Transaction Tips -Transaction tips are a mechanism to pay for transaction fees using another denom than the native fee denom of the chain. {synopsis} +Transaction tips are a mechanism to pay for transaction fees using another denom than the native fee denom of the chain. They are still in beta, and are not included by default in the SDK. {synopsis} ## Context @@ -72,27 +72,42 @@ In both cases, using `SIGN_MODE_LEGACY_AMINO_JSON` is recommended only if hardwa ## Enabling Tips on your Chain -The transaction tips functionality is introduced in Cosmos SDK v0.46, so earlier versions do not have support for tips. If you're using v0.46 or later, then enabling tips on your chain is as simple as adding the `TipMiddleware` in your middleware stack: +The transaction tips functionality is introduced in Cosmos SDK v0.46, so earlier versions do not have support for tips. It is however not included by default in a v0.46 app. Enabling tips on your chain is done by adding the `TipDecorator` in your posthandler chain: ```go -// NewTxHandler defines a TxHandler middleware stack. -func NewTxHandler(options TxHandlerOptions) (tx.Handler, error) { - // --snip-- - - return ComposeMiddlewares( - // base tx handler that executes Msgs - NewRunMsgsTxHandler(options.MsgServiceRouter, options.LegacyRouter), - // --snip other middlewares-- - - // Add the TipMiddleware - NewTipMiddleware(options.BankKeeper), - ) +// HandlerOptions are the options required for constructing a default SDK PostHandler. +type HandlerOptions struct { + BankKeeper types.BankKeeper } -``` -Notice that `NewTipMiddleware` needs a reference to the BankKeeper, for transferring the tip to the fee payer. +// MyPostHandler returns a posthandler chain with the TipDecorator. +func MyPostHandler(options HandlerOptions) (sdk.AnteHandler, error) { + if options.BankKeeper == nil { + return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for posthandler") + } + + postDecorators := []sdk.AnteDecorator{ + posthandler.NewTipDecorator(options.bankKeeper), + } + + return sdk.ChainAnteDecorators(postDecorators...), nil +} + +func (app *SimApp) setPostHandler() { + postHandler, err := MyPostHandler( + HandlerOptions{ + BankKeeper: app.BankKeeper, + }, + ) + if err != nil { + panic(err) + } + + app.SetPostHandler(postHandler) +} +``` -If you are using the Cosmos SDK's default middleware stack `NewDefaultTxHandler()`, then the tip middleware is included by default. +Notice that `NewTipDecorator` needs a reference to the BankKeeper, for transferring the tip to the fee payer. ## CLI Usage @@ -170,7 +185,7 @@ For the fee payer, the SDK added a new method on the existing `TxBuilder` to imp txBuilder := clientCtx.TxConfig.NewTxBuilder() err := txBuilder.AddAuxSignerData(auxSignerData) if err != nil { - return err + return err } // A lot of fields will be populated in txBuilder, such as its Msgs, tip @@ -184,6 +199,6 @@ txBuilder.SetGasLimit(...) // Usual signing code err = authclient.SignTx(...) if err != nil { - return err + return err } ``` diff --git a/simapp/app.go b/simapp/app.go index cd6f19bb584a..46a641b30f8b 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -436,12 +436,7 @@ func NewSimApp( // meaning that both `runMsgs` and `postHandler` state will be committed if // both are successful, and both will be reverted if any of the two fails. // - // The SDK exposes a default postHandlers chain, which comprises of only - // one decorator: the Transaction Tips decorator. However, some chains do - // not need it by default, so feel free to comment the next line if you do - // not need tips. - // To read more about tips: - // https://docs.cosmos.network/main/core/tips.html + // The SDK exposes a default empty postHandlers chain. // // Please note that changing any of the anteHandler or postHandler chain is // likely to be a state-machine breaking change, which needs a coordinated @@ -480,9 +475,7 @@ func (app *SimApp) setAnteHandler(txConfig client.TxConfig, indexEventsStr []str func (app *SimApp) setPostHandler() { postHandler, err := posthandler.NewPostHandler( - posthandler.HandlerOptions{ - BankKeeper: app.BankKeeper, - }, + posthandler.HandlerOptions{}, ) if err != nil { panic(err) diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index a56885920220..86da20fb8be5 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -1562,6 +1562,9 @@ func (s *IntegrationTestSuite) TestAuxSigner() { } func (s *IntegrationTestSuite) TestAuxToFeeWithTips() { + // Skipping this test as it needs a simapp with the TipDecorator in post handler. + s.T().Skip() + require := s.Require() val := s.network.Validators[0] diff --git a/x/auth/posthandler/post.go b/x/auth/posthandler/post.go index 604b1203c4e4..8d3fb7776c63 100644 --- a/x/auth/posthandler/post.go +++ b/x/auth/posthandler/post.go @@ -2,26 +2,14 @@ package posthandler import ( sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/cosmos/cosmos-sdk/x/auth/types" ) // HandlerOptions are the options required for constructing a default SDK PostHandler. -type HandlerOptions struct { - BankKeeper types.BankKeeper -} +type HandlerOptions struct{} -// NewAnteHandler returns an AnteHandler that checks and increments sequence -// numbers, checks signatures & account numbers, and deducts fees from the first -// signer. +// NewPostHandler returns an empty posthandler chain. func NewPostHandler(options HandlerOptions) (sdk.AnteHandler, error) { - if options.BankKeeper == nil { - return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for posthandler") - } - - postDecorators := []sdk.AnteDecorator{ - NewTipDecorator(options.BankKeeper), - } + postDecorators := []sdk.AnteDecorator{} return sdk.ChainAnteDecorators(postDecorators...), nil } diff --git a/x/auth/posthandler/tips.go b/x/auth/posthandler/tips.go index a112a3e1c4a2..f956d53e5442 100644 --- a/x/auth/posthandler/tips.go +++ b/x/auth/posthandler/tips.go @@ -16,6 +16,8 @@ type tipDecorator struct { // NewTipDecorator returns a new decorator for handling transactions with // tips. +// +// IMPORTANT: This decorator is still in beta, please use it at your own risk. func NewTipDecorator(bankKeeper types.BankKeeper) sdk.AnteDecorator { return tipDecorator{ bankKeeper: bankKeeper,