diff --git a/CHANGELOG.md b/CHANGELOG.md index b150a82a217f..4eb2b85a1a14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* (types) [#11985](https://github.com/cosmos/cosmos-sdk/pull/11985) Add a `Priority` field on `sdk.Context`, which represents the CheckTx priority field. It is only used during CheckTx. * (gRPC) [#11889](https://github.com/cosmos/cosmos-sdk/pull/11889) Support custom read and write gRPC options in `app.toml`. See `max-recv-msg-size` and `max-send-msg-size` respectively. * (cli) [\#11738](https://github.com/cosmos/cosmos-sdk/pull/11738) Add `tx auth multi-sign` as alias of `tx auth multisign` for consistency with `multi-send`. * (cli) [\#11738](https://github.com/cosmos/cosmos-sdk/pull/11738) Add `tx bank multi-send` command for bulk send of coins to multiple accounts. @@ -90,6 +91,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* (x/auth/ante) [#11985](https://github.com/cosmos/cosmos-sdk/pull/11985) The `MempoolFeeDecorator` has been removed. Instead, the `DeductFeeDecorator` takes a new argument of type `TxFeeChecker`, to define custom fee models. If `nil` is passed to this `TxFeeChecker` argument, then it will default to `checkTxFeeWithValidatorMinGasPrices`, which is the exact same behavior as the old `MempoolFeeDecorator` (i.e. checking fees against validator's own min gas price). +* (x/auth/ante) [#11985](https://github.com/cosmos/cosmos-sdk/pull/11985) The `ExtensionOptionsDecorator` takes an argument of type `ExtensionOptionChecker`. For backwards-compatibility, you can pass `nil`, which defaults to the old behavior of rejecting all tx extensions. * (crypto/keyring) [#11932](https://github.com/cosmos/cosmos-sdk/pull/11932) Remove `Unsafe*` interfaces from keyring package. Please use interface casting if you wish to access those unsafe functions. * (types) [#11881](https://github.com/cosmos/cosmos-sdk/issues/11881) Rename `AccAddressFromHex` to `AccAddressFromHexUnsafe`. * (types) [#11788](https://github.com/cosmos/cosmos-sdk/pull/11788) The `Int` and `Uint` types have been moved to their own dedicated module, `math`. Aliases are kept in the SDK's root `types` package, however, it is encouraged to utilize the new `math` module. As a result, the `Int#ToDec` API has been removed. @@ -277,6 +280,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking +* (baseapp) [\#11985](https://github.com/cosmos/cosmos-sdk/pull/11985) Add a `postHandler` to baseapp. This `postHandler` is like antehandler, but is run _after_ the `runMsgs` execution. It is in the same store branch that `runMsgs`, meaning that both `runMsgs` and `postHandler` * (x/gov) [#11998](https://github.com/cosmos/cosmos-sdk/pull/11998) Tweak the `x/gov` `ModuleAccountInvariant` invariant to ensure deposits are `<=` total module account balance instead of strictly equal. * (x/upgrade) [\#11800](https://github.com/cosmos/cosmos-sdk/pull/11800) Fix `GetLastCompleteUpgrade` to properly return the latest upgrade. * [\#10564](https://github.com/cosmos/cosmos-sdk/pull/10564) Fix bug when updating allowance inside AllowedMsgAllowance diff --git a/baseapp/abci.go b/baseapp/abci.go index 9b0b971343ac..1c7a36927e8b 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -251,7 +251,7 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx { panic(fmt.Sprintf("unknown RequestCheckTx type: %s", req.Type)) } - gInfo, result, anteEvents, err := app.runTx(mode, req.Tx) + gInfo, result, anteEvents, priority, err := app.runTx(mode, req.Tx) if err != nil { return sdkerrors.ResponseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, app.trace) } @@ -262,6 +262,7 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) abci.ResponseCheckTx { Log: result.Log, Data: result.Data, Events: sdk.MarkEventsToIndex(result.Events, app.indexEvents), + Priority: priority, } } @@ -281,7 +282,7 @@ func (app *BaseApp) DeliverTx(req abci.RequestDeliverTx) abci.ResponseDeliverTx telemetry.SetGauge(float32(gInfo.GasWanted), "tx", "gas", "wanted") }() - gInfo, result, anteEvents, err := app.runTx(runTxModeDeliver, req.Tx) + gInfo, result, anteEvents, _, err := app.runTx(runTxModeDeliver, req.Tx) if err != nil { resultStr = "failed" return sdkerrors.ResponseDeliverTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, app.trace) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 31122c29799b..636fd60e61b4 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -58,6 +58,7 @@ type BaseApp struct { // nolint: maligned txDecoder sdk.TxDecoder // unmarshal []byte into sdk.Tx anteHandler sdk.AnteHandler // ante handler for fee and auth + postHandler sdk.AnteHandler // 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 endBlocker sdk.EndBlocker // logic to run after all txs, and to determine valset changes @@ -584,7 +585,7 @@ 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, err error) { +func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []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. @@ -595,7 +596,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re // only run the tx if there is block gas remaining if mode == runTxModeDeliver && ctx.BlockGasMeter().IsOutOfGas() { - return gInfo, nil, nil, sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "no block gas left to run tx") + return gInfo, nil, nil, 0, sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "no block gas left to run tx") } defer func() { @@ -630,12 +631,12 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re tx, err := app.txDecoder(txBytes) if err != nil { - return sdk.GasInfo{}, nil, nil, err + return sdk.GasInfo{}, nil, nil, 0, err } msgs := tx.GetMsgs() if err := validateBasicTxMsgs(msgs); err != nil { - return sdk.GasInfo{}, nil, nil, err + return sdk.GasInfo{}, nil, nil, 0, err } if app.anteHandler != nil { @@ -671,9 +672,10 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re gasWanted = ctx.GasMeter().Limit() if err != nil { - return gInfo, nil, nil, err + return gInfo, nil, nil, 0, err } + priority = ctx.Priority() msCache.Write() anteEvents = events.ToABCIEvents() } @@ -687,19 +689,33 @@ 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 && mode == runTxModeDeliver { - // When block gas exceeds, it'll panic and won't commit the cached store. - consumeBlockGas() + if err == nil { + // Run optional postHandlers. + // + // Note: If the postHandler fails, we also revert the runMsgs state. + if app.postHandler != nil { + newCtx, err := app.postHandler(runMsgCtx, tx, mode == runTxModeSimulate) + if err != nil { + return gInfo, nil, nil, priority, err + } - msCache.Write() + result.Events = append(result.Events, newCtx.EventManager().ABCIEvents()...) + } - if len(anteEvents) > 0 { - // append the events in the order of occurrence - result.Events = append(anteEvents, result.Events...) + if mode == runTxModeDeliver { + // When block gas exceeds, it'll panic and won't commit the cached store. + consumeBlockGas() + + msCache.Write() + + if len(anteEvents) > 0 { + // append the events in the order of occurrence + result.Events = append(anteEvents, result.Events...) + } } } - return gInfo, result, anteEvents, err + return gInfo, result, anteEvents, 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 fcf8b1a788c8..8d67a7a5f001 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -36,6 +36,10 @@ import ( var ( capKey1 = sdk.NewKVStoreKey("key1") capKey2 = sdk.NewKVStoreKey("key2") + + // testTxPriority is the CheckTx priority that we set in the test + // antehandler. + testTxPriority = int64(42) ) type paramStore struct { @@ -826,6 +830,8 @@ func anteHandlerTxTest(t *testing.T, capKey storetypes.StoreKey, storeKey []byte counterEvent("ante_handler", txTest.Counter), ) + ctx = ctx.WithPriority(testTxPriority) + return ctx, nil } } @@ -933,6 +939,7 @@ func TestCheckTx(t *testing.T) { txBytes, err := codec.Marshal(tx) require.NoError(t, err) r := app.CheckTx(abci.RequestCheckTx{Tx: txBytes}) + require.Equal(t, testTxPriority, r.Priority) require.Empty(t, r.GetEvents()) require.True(t, r.IsOK(), fmt.Sprintf("%v", r)) } diff --git a/baseapp/options.go b/baseapp/options.go index 139e04c8c536..721fb612ffd8 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -153,6 +153,14 @@ func (app *BaseApp) SetAnteHandler(ah sdk.AnteHandler) { app.anteHandler = ah } +func (app *BaseApp) SetPostHandler(ph sdk.AnteHandler) { + if app.sealed { + panic("SetPostHandler() on sealed BaseApp") + } + + app.postHandler = ph +} + func (app *BaseApp) SetAddrPeerFilter(pf sdk.PeerFilter) { if app.sealed { panic("SetAddrPeerFilter() on sealed BaseApp") diff --git a/baseapp/test_helpers.go b/baseapp/test_helpers.go index eda2815da410..6489595bc8c1 100644 --- a/baseapp/test_helpers.go +++ b/baseapp/test_helpers.go @@ -16,13 +16,13 @@ func (app *BaseApp) SimCheck(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo, * if err != nil { return sdk.GasInfo{}, nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err) } - gasInfo, result, _, err := app.runTx(runTxModeCheck, bz) + gasInfo, result, _, _, err := app.runTx(runTxModeCheck, bz) return gasInfo, result, err } // Simulate executes a tx in simulate mode to get result and gas info. func (app *BaseApp) Simulate(txBytes []byte) (sdk.GasInfo, *sdk.Result, error) { - gasInfo, result, _, err := app.runTx(runTxModeSimulate, txBytes) + gasInfo, result, _, _, err := app.runTx(runTxModeSimulate, txBytes) return gasInfo, result, err } @@ -32,7 +32,7 @@ func (app *BaseApp) SimDeliver(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo, if err != nil { return sdk.GasInfo{}, nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err) } - gasInfo, result, _, err := app.runTx(runTxModeDeliver, bz) + gasInfo, result, _, _, err := app.runTx(runTxModeDeliver, bz) return gasInfo, result, err } diff --git a/simapp/app.go b/simapp/app.go index b660009d753a..171fb557c58a 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -34,6 +34,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth" "github.com/cosmos/cosmos-sdk/x/auth/ante" authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" + "github.com/cosmos/cosmos-sdk/x/auth/posthandler" authsims "github.com/cosmos/cosmos-sdk/x/auth/simulation" authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" @@ -443,6 +444,27 @@ func NewSimApp( app.SetBeginBlocker(app.BeginBlocker) app.SetEndBlocker(app.EndBlocker) app.setAnteHandler(encodingConfig.TxConfig, cast.ToStringSlice(appOpts.Get(server.FlagIndexEvents))) + // In v0.46, the SDK introduces _postHandlers_. PostHandlers are like + // antehandlers, but are run _after_ the `runMsgs` execution. They are also + // defined as a chain, and have the same signature as antehandlers. + // + // In baseapp, postHandlers are run in the same store branch as `runMsgs`, + // 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 the following line is commented out. You can + // uncomment it to include the tips decorator, or define your own + // postHandler chain. To read more about tips: + // https://docs.cosmos.network/main/core/tips.html + // + // Please note that changing any of the anteHandler or postHandler chain is + // likely to be a state-machine breaking change, which needs a coordinated + // upgrade. + // + // Uncomment to enable postHandlers: + // app.setPostHandler() if loadLatest { if err := app.LoadLatestVersion(); err != nil { @@ -475,6 +497,19 @@ func (app *SimApp) setAnteHandler(txConfig client.TxConfig, indexEventsStr []str app.SetAnteHandler(anteHandler) } +func (app *SimApp) setPostHandler() { + postHandler, err := posthandler.NewPostHandler( + posthandler.HandlerOptions{ + BankKeeper: app.BankKeeper, + }, + ) + if err != nil { + panic(err) + } + + app.SetPostHandler(postHandler) +} + // Name returns the name of the App func (app *SimApp) Name() string { return app.BaseApp.Name() } diff --git a/types/context.go b/types/context.go index 30bec2154f5d..ec5f2c3774dd 100644 --- a/types/context.go +++ b/types/context.go @@ -38,6 +38,7 @@ type Context struct { minGasPrice DecCoins consParams *tmproto.ConsensusParams eventManager *EventManager + priority int64 // The tx priority, only relevant in CheckTx } // Proposed rename, not done to avoid API breakage @@ -58,6 +59,7 @@ func (c Context) IsCheckTx() bool { return c.checkTx } func (c Context) IsReCheckTx() bool { return c.recheckTx } func (c Context) MinGasPrices() DecCoins { return c.minGasPrice } func (c Context) EventManager() *EventManager { return c.eventManager } +func (c Context) Priority() int64 { return c.priority } // clone the header before returning func (c Context) BlockHeader() tmproto.Header { @@ -226,6 +228,12 @@ func (c Context) WithEventManager(em *EventManager) Context { return c } +// WithEventManager returns a Context with an updated tx priority +func (c Context) WithPriority(p int64) Context { + c.priority = p + return c +} + // TODO: remove??? func (c Context) IsZero() bool { return c.ms == nil diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index 3b4aa6a56f11..a562e67ee39c 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -10,11 +10,13 @@ import ( // HandlerOptions are the options required for constructing a default SDK AnteHandler. type HandlerOptions struct { - AccountKeeper AccountKeeper - BankKeeper types.BankKeeper - FeegrantKeeper FeegrantKeeper - SignModeHandler authsigning.SignModeHandler - SigGasConsumer func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error + AccountKeeper AccountKeeper + BankKeeper types.BankKeeper + ExtensionOptionChecker ExtensionOptionChecker + FeegrantKeeper FeegrantKeeper + SignModeHandler authsigning.SignModeHandler + SigGasConsumer func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error + TxFeeChecker TxFeeChecker } // NewAnteHandler returns an AnteHandler that checks and increments sequence @@ -33,23 +35,17 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) { return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder") } - var sigGasConsumer = options.SigGasConsumer - if sigGasConsumer == nil { - sigGasConsumer = DefaultSigVerificationGasConsumer - } - anteDecorators := []sdk.AnteDecorator{ NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first - NewRejectExtensionOptionsDecorator(), - NewMempoolFeeDecorator(), + NewExtensionOptionsDecorator(options.ExtensionOptionChecker), NewValidateBasicDecorator(), NewTxTimeoutHeightDecorator(), NewValidateMemoDecorator(options.AccountKeeper), NewConsumeGasForTxSizeDecorator(options.AccountKeeper), - NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper), + NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker), NewSetPubKeyDecorator(options.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators NewValidateSigCountDecorator(options.AccountKeeper), - NewSigGasConsumeDecorator(options.AccountKeeper, sigGasConsumer), + NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer), NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler), NewIncrementSequenceDecorator(options.AccountKeeper), } diff --git a/x/auth/ante/ext.go b/x/auth/ante/ext.go index 362b8d32a971..6a072526aa0f 100644 --- a/x/auth/ante/ext.go +++ b/x/auth/ante/ext.go @@ -2,7 +2,7 @@ package ante import ( codectypes "github.com/cosmos/cosmos-sdk/codec/types" - "github.com/cosmos/cosmos-sdk/types" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) @@ -11,26 +11,55 @@ type HasExtensionOptionsTx interface { GetNonCriticalExtensionOptions() []*codectypes.Any } +// ExtensionOptionChecker is a function that returns true if the extension option is accepted. +type ExtensionOptionChecker func(*codectypes.Any) bool + +// rejectExtensionOption is the default extension check that reject all tx +// extensions. +func rejectExtensionOption(*codectypes.Any) bool { + return false +} + // RejectExtensionOptionsDecorator is an AnteDecorator that rejects all extension // options which can optionally be included in protobuf transactions. Users that // need extension options should create a custom AnteHandler chain that handles // needed extension options properly and rejects unknown ones. -type RejectExtensionOptionsDecorator struct{} +type RejectExtensionOptionsDecorator struct { + checker ExtensionOptionChecker +} + +// NewExtensionOptionsDecorator creates a new antehandler that rejects all extension +// options which can optionally be included in protobuf transactions that don't pass the checker. +// Users that need extension options should pass a custom checker that returns true for the +// needed extension options. +func NewExtensionOptionsDecorator(checker ExtensionOptionChecker) sdk.AnteDecorator { + if checker == nil { + checker = rejectExtensionOption + } -// NewRejectExtensionOptionsDecorator creates a new RejectExtensionOptionsDecorator -func NewRejectExtensionOptionsDecorator() RejectExtensionOptionsDecorator { - return RejectExtensionOptionsDecorator{} + return RejectExtensionOptionsDecorator{checker: checker} } -var _ types.AnteDecorator = RejectExtensionOptionsDecorator{} +var _ sdk.AnteDecorator = RejectExtensionOptionsDecorator{} // AnteHandle implements the AnteDecorator.AnteHandle method -func (r RejectExtensionOptionsDecorator) AnteHandle(ctx types.Context, tx types.Tx, simulate bool, next types.AnteHandler) (newCtx types.Context, err error) { +func (r RejectExtensionOptionsDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + err = checkExtOpts(tx, r.checker) + if err != nil { + return ctx, err + } + + return next(ctx, tx, simulate) +} + +func checkExtOpts(tx sdk.Tx, checker ExtensionOptionChecker) error { if hasExtOptsTx, ok := tx.(HasExtensionOptionsTx); ok { - if len(hasExtOptsTx.GetExtensionOptions()) != 0 { - return ctx, sdkerrors.ErrUnknownExtensionOptions + for _, opt := range hasExtOptsTx.GetExtensionOptions() { + if !checker(opt) { + return sdkerrors.ErrUnknownExtensionOptions + } } } - return next(ctx, tx, simulate) + return nil } diff --git a/x/auth/ante/ext_test.go b/x/auth/ante/ext_test.go index 89ce6a7d649f..3bd0f8f60275 100644 --- a/x/auth/ante/ext_test.go +++ b/x/auth/ante/ext_test.go @@ -1,7 +1,7 @@ package ante_test import ( - "github.com/cosmos/cosmos-sdk/codec/types" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/ante" @@ -10,27 +10,45 @@ import ( func (suite *AnteTestSuite) TestRejectExtensionOptionsDecorator() { suite.SetupTest(true) // setup - suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() - reod := ante.NewRejectExtensionOptionsDecorator() - antehandler := sdk.ChainAnteDecorators(reod) + testCases := []struct { + msg string + allow bool + }{ + {"allow extension", true}, + {"reject extension", false}, + } + for _, tc := range testCases { + suite.Run(tc.msg, func() { + txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() - // no extension options should not trigger an error - theTx := suite.txBuilder.GetTx() - _, err := antehandler(suite.ctx, theTx, false) - suite.Require().NoError(err) + reod := ante.NewExtensionOptionsDecorator(func(_ *codectypes.Any) bool { + return tc.allow + }) + antehandler := sdk.ChainAnteDecorators(reod) - extOptsTxBldr, ok := suite.txBuilder.(tx.ExtensionOptionsTxBuilder) - if !ok { - // if we can't set extension options, this decorator doesn't apply and we're done - return - } + // no extension options should not trigger an error + theTx := txBuilder.GetTx() + _, err := antehandler(suite.ctx, theTx, false) + suite.Require().NoError(err) - // setting any extension option should cause an error - any, err := types.NewAnyWithValue(testdata.NewTestMsg()) - suite.Require().NoError(err) - extOptsTxBldr.SetExtensionOptions(any) - theTx = suite.txBuilder.GetTx() - _, err = antehandler(suite.ctx, theTx, false) - suite.Require().EqualError(err, "unknown extension options") + extOptsTxBldr, ok := txBuilder.(tx.ExtensionOptionsTxBuilder) + if !ok { + // if we can't set extension options, this decorator doesn't apply and we're done + return + } + + // set an extension option and check + any, err := codectypes.NewAnyWithValue(testdata.NewTestMsg()) + suite.Require().NoError(err) + extOptsTxBldr.SetExtensionOptions(any) + theTx = txBuilder.GetTx() + _, err = antehandler(suite.ctx, theTx, false) + if tc.allow { + suite.Require().NoError(err) + } else { + suite.Require().EqualError(err, "unknown extension options") + } + }) + } } diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 19e8258cfa73..7a38dbc399d6 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -8,121 +8,96 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/types" ) -// MempoolFeeDecorator will check if the transaction's fee is at least as large -// as the local validator's minimum gasFee (defined in validator config). -// If fee is too low, decorator returns error and tx is rejected from mempool. -// Note this only applies when ctx.CheckTx = true -// If fee is high enough or not CheckTx, then call next AnteHandler -// CONTRACT: Tx must implement FeeTx to use MempoolFeeDecorator -type MempoolFeeDecorator struct{} - -func NewMempoolFeeDecorator() MempoolFeeDecorator { - return MempoolFeeDecorator{} -} - -func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - feeTx, ok := tx.(sdk.FeeTx) - if !ok { - return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") - } - - feeCoins := feeTx.GetFee() - gas := feeTx.GetGas() - - // Ensure that the provided fees meet a minimum threshold for the validator, - // if this is a CheckTx. This is only for local mempool purposes, and thus - // is only ran on check tx. - if ctx.IsCheckTx() && !simulate { - minGasPrices := ctx.MinGasPrices() - if !minGasPrices.IsZero() { - requiredFees := make(sdk.Coins, len(minGasPrices)) - - // Determine the required fees by multiplying each required minimum gas - // price by the gas limit, where fee = ceil(minGasPrice * gasLimit). - glDec := sdk.NewDec(int64(gas)) - for i, gp := range minGasPrices { - fee := gp.Amount.Mul(glDec) - requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) - } - - if !feeCoins.IsAnyGTE(requiredFees) { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) - } - } - } - - return next(ctx, tx, simulate) -} +// TxFeeChecker check if the provided fee is enough and returns the effective fee and tx priority, +// the effective fee should be deducted later, and the priority should be returned in abci response. +type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) // DeductFeeDecorator deducts fees from the first signer of the tx // If the first signer does not have the funds to pay for the fees, return with InsufficientFunds error // Call next AnteHandler if fees successfully deducted // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator type DeductFeeDecorator struct { - ak AccountKeeper + accountKeeper AccountKeeper bankKeeper types.BankKeeper feegrantKeeper FeegrantKeeper + txFeeChecker TxFeeChecker } -func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper) DeductFeeDecorator { +func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) DeductFeeDecorator { + if tfc == nil { + tfc = checkTxFeeWithValidatorMinGasPrices + } + return DeductFeeDecorator{ - ak: ak, + accountKeeper: ak, bankKeeper: bk, feegrantKeeper: fk, + txFeeChecker: tfc, + } +} + +func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + fee, priority, err := dfd.txFeeChecker(ctx, tx) + if err != nil { + return ctx, err } + if err := dfd.checkDeductFee(ctx, tx, fee); err != nil { + return ctx, err + } + + newCtx := ctx.WithPriority(priority) + + return next(newCtx, tx, simulate) } -func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - feeTx, ok := tx.(sdk.FeeTx) +func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error { + feeTx, ok := sdkTx.(sdk.FeeTx) if !ok { - return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + return sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } - if addr := dfd.ak.GetModuleAddress(types.FeeCollectorName); addr == nil { - return ctx, fmt.Errorf("Fee collector module account (%s) has not been set", types.FeeCollectorName) + if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { + return fmt.Errorf("Fee collector module account (%s) has not been set", types.FeeCollectorName) } - fee := feeTx.GetFee() feePayer := feeTx.FeePayer() feeGranter := feeTx.FeeGranter() - deductFeesFrom := feePayer // if feegranter set deduct fee from feegranter account. // this works with only when feegrant enabled. if feeGranter != nil { if dfd.feegrantKeeper == nil { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "fee grants are not enabled") + return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled") } else if !feeGranter.Equals(feePayer) { - err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, tx.GetMsgs()) - + err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, sdkTx.GetMsgs()) if err != nil { - return ctx, sdkerrors.Wrapf(err, "%s not allowed to pay fees from %s", feeGranter, feePayer) + return sdkerrors.Wrapf(err, "%s does not not allow to pay fees for %s", feeGranter, feePayer) } } deductFeesFrom = feeGranter } - deductFeesFromAcc := dfd.ak.GetAccount(ctx, deductFeesFrom) + deductFeesFromAcc := dfd.accountKeeper.GetAccount(ctx, deductFeesFrom) if deductFeesFromAcc == nil { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "fee payer address: %s does not exist", deductFeesFrom) + return sdkerrors.ErrUnknownAddress.Wrapf("fee payer address: %s does not exist", deductFeesFrom) } // deduct the fees - if !feeTx.GetFee().IsZero() { - err = DeductFees(dfd.bankKeeper, ctx, deductFeesFromAcc, feeTx.GetFee()) + if !fee.IsZero() { + err := DeductFees(dfd.bankKeeper, ctx, deductFeesFromAcc, fee) if err != nil { - return ctx, err + return err } } events := sdk.Events{sdk.NewEvent(sdk.EventTypeTx, - sdk.NewAttribute(sdk.AttributeKeyFee, feeTx.GetFee().String()), + sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), )} ctx.EventManager().EmitEvents(events) - return next(ctx, tx, simulate) + return nil } // DeductFees deducts fees from the given account. diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go index 06ccb4d3948f..21a0a2d7ff8e 100644 --- a/x/auth/ante/fee_test.go +++ b/x/auth/ante/fee_test.go @@ -12,11 +12,13 @@ func (suite *AnteTestSuite) TestEnsureMempoolFees() { suite.SetupTest(true) // setup suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() - mfd := ante.NewMempoolFeeDecorator() + mfd := ante.NewDeductFeeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper, suite.app.FeeGrantKeeper, nil) antehandler := sdk.ChainAnteDecorators(mfd) // keys and addresses priv1, _, addr1 := testdata.KeyTestPubAddr() + coins := sdk.NewCoins(sdk.NewCoin("atom", sdk.NewInt(300))) + testutil.FundAccount(suite.app.BankKeeper, suite.ctx, addr1, coins) // msg and signatures msg := testdata.NewTestMsg(addr1) @@ -56,8 +58,11 @@ func (suite *AnteTestSuite) TestEnsureMempoolFees() { lowGasPrice := []sdk.DecCoin{atomPrice} suite.ctx = suite.ctx.WithMinGasPrices(lowGasPrice) - _, err = antehandler(suite.ctx, tx, false) + newCtx, err := antehandler(suite.ctx, tx, false) suite.Require().Nil(err, "Decorator should not have errored on fee higher than local gasPrice") + // Priority is the smallest amount in any denom. Since we have only 1 fee + // of 150atom, the priority here is 150. + suite.Require().Equal(feeAmount.AmountOf("atom").Int64(), newCtx.Priority()) } func (suite *AnteTestSuite) TestDeductFees() { @@ -86,7 +91,7 @@ func (suite *AnteTestSuite) TestDeductFees() { err = testutil.FundAccount(suite.app.BankKeeper, suite.ctx, addr1, coins) suite.Require().NoError(err) - dfd := ante.NewDeductFeeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper, nil) + dfd := ante.NewDeductFeeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper, nil, nil) antehandler := sdk.ChainAnteDecorators(dfd) _, err = antehandler(suite.ctx, tx, false) diff --git a/x/auth/ante/feegrant_test.go b/x/auth/ante/feegrant_test.go index c88cf781eb3a..c8689ca3c7a1 100644 --- a/x/auth/ante/feegrant_test.go +++ b/x/auth/ante/feegrant_test.go @@ -32,7 +32,7 @@ func (suite *AnteTestSuite) TestDeductFeesNoDelegation() { protoTxCfg := tx.NewTxConfig(codec.NewProtoCodec(app.InterfaceRegistry()), tx.DefaultSignModes) // this just tests our handler - dfd := ante.NewDeductFeeDecorator(app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper) + dfd := ante.NewDeductFeeDecorator(app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, nil) feeAnteHandler := sdk.ChainAnteDecorators(dfd) // this tests the whole stack diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index fa29f0268e1f..b32f46728ff1 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -136,6 +136,10 @@ type SigGasConsumeDecorator struct { } func NewSigGasConsumeDecorator(ak AccountKeeper, sigGasConsumer SignatureVerificationGasConsumer) SigGasConsumeDecorator { + if sigGasConsumer == nil { + sigGasConsumer = DefaultSigVerificationGasConsumer + } + return SigGasConsumeDecorator{ ak: ak, sigGasConsumer: sigGasConsumer, diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go new file mode 100644 index 000000000000..b1725d62ddc0 --- /dev/null +++ b/x/auth/ante/validator_tx_fee.go @@ -0,0 +1,62 @@ +package ante + +import ( + "math" + + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +// checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per +// unit of gas is fixed and set by each validator, can the tx priority is computed from the gas price. +func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) { + feeTx, ok := tx.(sdk.FeeTx) + if !ok { + return nil, 0, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + } + + feeCoins := feeTx.GetFee() + gas := feeTx.GetGas() + + // Ensure that the provided fees meet a minimum threshold for the validator, + // if this is a CheckTx. This is only for local mempool purposes, and thus + // is only ran on check tx. + if ctx.IsCheckTx() { + minGasPrices := ctx.MinGasPrices() + if !minGasPrices.IsZero() { + requiredFees := make(sdk.Coins, len(minGasPrices)) + + // Determine the required fees by multiplying each required minimum gas + // price by the gas limit, where fee = ceil(minGasPrice * gasLimit). + glDec := sdk.NewDec(int64(gas)) + for i, gp := range minGasPrices { + fee := gp.Amount.Mul(glDec) + requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) + } + + if !feeCoins.IsAnyGTE(requiredFees) { + return nil, 0, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + } + } + } + + priority := getTxPriority(feeCoins) + return feeCoins, priority, nil +} + +// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the fee +// provided in a transaction. +func getTxPriority(fee sdk.Coins) int64 { + var priority int64 + for _, c := range fee { + p := int64(math.MaxInt64) + if c.Amount.IsInt64() { + p = c.Amount.Int64() + } + if priority == 0 || p < priority { + priority = p + } + } + + return priority +} diff --git a/x/auth/client/testutil/suite.go b/x/auth/client/testutil/suite.go index 891facc0b666..a8d86bfd462b 100644 --- a/x/auth/client/testutil/suite.go +++ b/x/auth/client/testutil/suite.go @@ -1561,8 +1561,14 @@ func (s *IntegrationTestSuite) TestAuxSigner() { } } -func (s *IntegrationTestSuite) TestAuxToFee() { +func (s *IntegrationTestSuite) TestAuxToFeeWithTips() { + // Currently, simapp doesn't have Tips decorator enabled by default in its + // posthandlers, so this test will fail. + // + // TODO Find a way to test Tips integratin test with a custom simapp with + // tips posthandler. s.T().Skip() + require := s.Require() val := s.network.Validators[0] @@ -1578,13 +1584,13 @@ func (s *IntegrationTestSuite) TestAuxToFee() { fee := sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(1000)) tip := sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(1000)) - s.Require().NoError(s.network.WaitForNextBlock()) + require.NoError(s.network.WaitForNextBlock()) _, err = s.createBankMsg(val, tipper, sdk.NewCoins(tipperInitialBal)) require.NoError(err) - s.Require().NoError(s.network.WaitForNextBlock()) + require.NoError(s.network.WaitForNextBlock()) bal := s.getBalances(val.ClientCtx, tipper, tip.Denom) - s.Require().True(bal.Equal(tipperInitialBal.Amount)) + require.True(bal.Equal(tipperInitialBal.Amount)) testCases := []struct { name string @@ -1735,7 +1741,7 @@ func (s *IntegrationTestSuite) TestAuxToFee() { feePayerArgs: []string{ fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagSignMode, flags.SignModeDirect), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), fmt.Sprintf("--%s=%s", flags.FlagFrom, feePayer), fmt.Sprintf("--%s=%s", flags.FlagFees, fee.String()), }, @@ -1792,21 +1798,21 @@ func (s *IntegrationTestSuite) TestAuxToFee() { require.NoError(err) var txRes sdk.TxResponse - s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(res.Bytes(), &txRes)) + require.NoError(val.ClientCtx.Codec.UnmarshalJSON(res.Bytes(), &txRes)) require.Contains(txRes.RawLog, tc.errMsg) } else { require.NoError(err) var txRes sdk.TxResponse - s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(res.Bytes(), &txRes)) + require.NoError(val.ClientCtx.Codec.UnmarshalJSON(res.Bytes(), &txRes)) - s.Require().Equal(uint32(0), txRes.Code) - s.Require().NotNil(int64(0), txRes.Height) + require.Equal(uint32(0), txRes.Code) + require.NotNil(int64(0), txRes.Height) bal = s.getBalances(val.ClientCtx, tipper, tc.tip.Denom) tipperInitialBal = tipperInitialBal.Sub(tc.tip) - s.Require().True(bal.Equal(tipperInitialBal.Amount)) + require.True(bal.Equal(tipperInitialBal.Amount)) } } }) diff --git a/x/auth/posthandler/post.go b/x/auth/posthandler/post.go new file mode 100644 index 000000000000..604b1203c4e4 --- /dev/null +++ b/x/auth/posthandler/post.go @@ -0,0 +1,27 @@ +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 +} + +// NewAnteHandler returns an AnteHandler that checks and increments sequence +// numbers, checks signatures & account numbers, and deducts fees from the first +// signer. +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), + } + + return sdk.ChainAnteDecorators(postDecorators...), nil +} diff --git a/x/auth/posthandler/tips.go b/x/auth/posthandler/tips.go new file mode 100644 index 000000000000..a112a3e1c4a2 --- /dev/null +++ b/x/auth/posthandler/tips.go @@ -0,0 +1,49 @@ +package posthandler + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +// ValidateBasicDecorator will call tx.ValidateBasic and return any non-nil error. +// If ValidateBasic passes, decorator calls next AnteHandler in chain. Note, +// ValidateBasicDecorator decorator will not get executed on ReCheckTx since it +// is not dependent on application state. +type tipDecorator struct { + bankKeeper types.BankKeeper +} + +// NewTipDecorator returns a new decorator for handling transactions with +// tips. +func NewTipDecorator(bankKeeper types.BankKeeper) sdk.AnteDecorator { + return tipDecorator{ + bankKeeper: bankKeeper, + } +} + +func (d tipDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + err := d.transferTip(ctx, tx) + if err != nil { + return ctx, err + } + + return next(ctx, tx, simulate) +} + +// transferTip transfers the tip from the tipper to the fee payer. +func (d tipDecorator) transferTip(ctx sdk.Context, sdkTx sdk.Tx) error { + tipTx, ok := sdkTx.(tx.TipTx) + + // No-op if the tx doesn't have tips. + if !ok || tipTx.GetTip() == nil { + return nil + } + + tipper, err := sdk.AccAddressFromBech32(tipTx.GetTip().Tipper) + if err != nil { + return err + } + + return d.bankKeeper.SendCoins(ctx, tipper, tipTx.FeePayer(), tipTx.GetTip().Amount) +}