Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(x/feegrant): Rewrite feegrant ante handler as tx validator #19999

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion contrib/images/simd-env/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ COPY x/mint/go.mod x/mint/go.sum /work/x/mint/
COPY x/accounts/go.mod x/accounts/go.sum /work/x/accounts/
COPY runtime/v2/go.mod runtime/v2/go.sum /work/runtime/v2/
COPY server/v2/appmanager/go.mod server/v2/appmanager/go.sum /work/server/v2/appmanager/
COPY server/v2/core/go.mod server/v2/core/go.sum /work/server/v2/core/
COPY server/v2/stf/go.mod server/v2/stf/go.sum /work/server/v2/stf/

RUN go mod download
Expand Down
21 changes: 21 additions & 0 deletions core/gas/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
"context"
"errors"
"math"

cosmossdk_io_math "cosmossdk.io/math"

Check failure on line 9 in core/gas/service.go

View workflow job for this annotation

GitHub Actions / dependency-review

missing go.sum entry for module providing package cosmossdk.io/math (imported by cosmossdk.io/core/gas); to add:
)

// ErrOutOfGas must be used by GasMeter implementers to signal
Expand All @@ -20,6 +22,23 @@
// NoGasLimit signals that no gas limit must be applied.
const NoGasLimit Gas = math.MaxUint64

type DecCoins []DecCoin

type DecCoin struct {
Denom string
Amount cosmossdk_io_math.LegacyDec
}

// IsZero returns whether all coins are zero
func (coins DecCoins) IsZero() bool {
for _, coin := range coins {
if !coin.Amount.IsZero() {
return false
}
}
return true
}

// Service represents a gas service which can retrieve and set a gas meter in a context.
// gas.Service is a core API type that should be provided by the runtime module being used to
// build an app via depinject.
Expand All @@ -41,6 +60,8 @@
WithBlockGasMeter(ctx context.Context, meter Meter) context.Context

GetGasConfig(ctx context.Context) GasConfig

MinGasPrices(ctx context.Context) DecCoins
}

// Meter represents a gas meter for modules consumption
Expand Down
4 changes: 2 additions & 2 deletions core/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.21

require (
cosmossdk.io/log v1.3.1
cosmossdk.io/math v1.3.0
github.com/cosmos/gogoproto v1.4.12
github.com/stretchr/testify v1.9.0
google.golang.org/grpc v1.63.2
Expand All @@ -13,7 +14,7 @@ require (
require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/kr/pretty v0.3.1 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/pkg/errors v0.9.1 // indirect
Expand All @@ -25,6 +26,5 @@ require (
golang.org/x/sys v0.19.0 // indirect
golang.org/x/text v0.14.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
9 changes: 4 additions & 5 deletions core/go.sum
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
cosmossdk.io/log v1.3.1 h1:UZx8nWIkfbbNEWusZqzAx3ZGvu54TZacWib3EzUYmGI=
cosmossdk.io/log v1.3.1/go.mod h1:2/dIomt8mKdk6vl3OWJcPk2be3pGOS8OQaLUM/3/tCM=
cosmossdk.io/math v1.3.0 h1:RC+jryuKeytIiictDslBP9i1fhkVm6ZDmZEoNP316zE=
cosmossdk.io/math v1.3.0/go.mod h1:vnRTxewy+M7BtXBNFybkuhSH4WfedVAAnERHgVFhp3k=
github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
github.com/cosmos/gogoproto v1.4.12 h1:vB6Lbe/rtnYGjQuFxkPiPYiCybqFT8QvLipDZP8JpFE=
github.com/cosmos/gogoproto v1.4.12/go.mod h1:LnZob1bXRdUoqMMtwYlcR3wjiElmlC+FkjaZRv1/eLY=
Expand All @@ -9,11 +11,8 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
Expand All @@ -22,12 +21,10 @@ github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/
github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M=
github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA=
github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg=
Expand Down Expand Up @@ -59,3 +56,5 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gotest.tools/v3 v3.5.1 h1:EENdUnS3pdur5nybKYIh2Vfgc8IUNBjxDPSjtiJcOzU=
gotest.tools/v3 v3.5.1/go.mod h1:isy3WKz7GK6uNw/sbHzfKBLvlvXwUyV06n6brMxxopU=
sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY=
11 changes: 11 additions & 0 deletions runtime/gas.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ func (g GasService) GetGasConfig(ctx context.Context) gas.GasConfig {
return gas.GasConfig(sdk.UnwrapSDKContext(ctx).KVGasConfig())
}

func (g GasService) MinGasPrices(ctx context.Context) gas.DecCoins {
sdkCtx := sdk.UnwrapSDKContext(ctx)
minGasPrice := sdkCtx.MinGasPrices()
txDecCoins := make(gas.DecCoins, len(minGasPrice))
for i, s := range minGasPrice {
txDecCoins[i].Amount = s.Amount
txDecCoins[i].Denom = s.Denom
}
return txDecCoins
}

// ______________________________________________________________________________________________
// Gas Meter Wrappers
// ______________________________________________________________________________________________
Expand Down
4 changes: 4 additions & 0 deletions server/v2/stf/core_gas_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,7 @@ func (g gasService) WithGasMeter(ctx context.Context, meter gas.Meter) context.C
func (g gasService) WithBlockGasMeter(ctx context.Context, meter gas.Meter) context.Context {
panic("unimplemented")
}

func (g gasService) MinGasPrices(ctx context.Context) gas.DecCoins {
panic("unimplemented")
}
2 changes: 2 additions & 0 deletions x/auth/ante/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"cosmossdk.io/core/address"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/x/auth/types"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -18,6 +19,7 @@ type AccountKeeper interface {
GetModuleAddress(moduleName string) sdk.AccAddress
AddressCodec() address.Codec
NewAccountWithAddress(ctx context.Context, addr sdk.AccAddress) sdk.AccountI
Environment() appmodule.Environment
}

// FeegrantKeeper defines the expected feegrant keeper.
Expand Down
50 changes: 32 additions & 18 deletions x/auth/ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package ante

import (
"bytes"
"context"
"fmt"

"cosmossdk.io/core/event"
"cosmossdk.io/core/transaction"
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/x/auth/types"

Expand All @@ -13,7 +16,7 @@ import (

// 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, error)
type TxFeeChecker func(ctx context.Context, tx sdk.Tx, accountKeeper AccountKeeper) (sdk.Coins, error)

// DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx.
// If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error.
Expand All @@ -39,32 +42,43 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKee
}
}

func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) {
// ValidateTx implements an TxValidator decorator for the DeductFeeDecorator
func (dfd DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error {
txService := dfd.accountKeeper.Environment().TransactionService
feeTx, ok := tx.(sdk.FeeTx)
if !ok {
return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
}

if ctx.ExecMode() != sdk.ExecModeSimulate && ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 {
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas")
headerInfo := dfd.accountKeeper.Environment().HeaderService.GetHeaderInfo(ctx)
if txService.ExecMode(ctx) != transaction.ExecModeSimulate && headerInfo.Height > 0 && feeTx.GetGas() == 0 {
return errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas")
}

var err error
fee := feeTx.GetFee()
if ctx.ExecMode() != sdk.ExecModeSimulate {
fee, err = dfd.txFeeChecker(ctx, tx)
if txService.ExecMode(ctx) != transaction.ExecModeSimulate {
fee, err = dfd.txFeeChecker(ctx, tx, dfd.accountKeeper)
if err != nil {
return ctx, err
return err
}
}
if err := dfd.checkDeductFee(ctx, tx, fee); err != nil {
return err
}

return nil
}

func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) {
if err := dfd.ValidateTx(ctx, tx); err != nil {
return ctx, err
}

return next(ctx, tx, ctx.ExecMode() == sdk.ExecModeSimulate)
return next(ctx, tx, false)
}

func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error {
func (dfd DeductFeeDecorator) checkDeductFee(ctx context.Context, sdkTx sdk.Tx, fee sdk.Coins) error {
feeTx, ok := sdkTx.(sdk.FeeTx)
if !ok {
return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
Expand Down Expand Up @@ -103,20 +117,20 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee
}
}

events := sdk.Events{
sdk.NewEvent(
sdk.EventTypeTx,
sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()),
sdk.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(deductFeesFrom).String()),
),
eventManager := dfd.accountKeeper.Environment().EventService.EventManager(ctx)
if err := eventManager.EmitKV(
sdk.EventTypeTx,
event.NewAttribute(sdk.AttributeKeyFee, fee.String()),
event.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(deductFeesFrom).String()),
); err != nil {
return err
}
ctx.EventManager().EmitEvents(events)

return nil
}

// DeductFees deducts fees from the given account.
func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc []byte, fees sdk.Coins) error {
func DeductFees(bankKeeper types.BankKeeper, ctx context.Context, acc []byte, fees sdk.Coins) error {
if !fees.IsValid() {
return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees)
}
Expand Down
15 changes: 15 additions & 0 deletions x/auth/ante/testutil/expected_keepers_mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 8 additions & 3 deletions x/auth/ante/validator_tx_fee.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package ante

import (
"context"

"cosmossdk.io/core/transaction"
errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"

Expand All @@ -10,7 +13,7 @@ import (

// checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per
// unit of gas is fixed and set by each validator.
func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, error) {
func checkTxFeeWithValidatorMinGasPrices(ctx context.Context, tx sdk.Tx, accountKeeper AccountKeeper) (sdk.Coins, error) {
feeTx, ok := tx.(sdk.FeeTx)
if !ok {
return nil, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
Expand All @@ -19,11 +22,13 @@ func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins,
feeCoins := feeTx.GetFee()
gas := feeTx.GetGas()

gasService := accountKeeper.Environment().GasService
txService := accountKeeper.Environment().TransactionService
// 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.ExecMode() == sdk.ExecModeCheck {
minGasPrices := ctx.MinGasPrices()
if txService.ExecMode(ctx) == transaction.ExecModeCheck {
minGasPrices := gasService.MinGasPrices(ctx)
if !minGasPrices.IsZero() {
requiredFees := make(sdk.Coins, len(minGasPrices))

Expand Down
5 changes: 5 additions & 0 deletions x/auth/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,8 @@ func (ak AccountKeeper) GetParams(ctx context.Context) (params types.Params) {
}
return params
}

// Environment returns the module's environment.
func (ak AccountKeeper) Environment() appmodule.Environment {
return ak.environment
}
5 changes: 5 additions & 0 deletions x/feegrant/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@ import (
"context"

"cosmossdk.io/core/address"
"cosmossdk.io/x/auth/ante"
authtypes "cosmossdk.io/x/auth/types"

sdk "github.com/cosmos/cosmos-sdk/types"
)

// AccountKeeper defines the expected auth Account Keeper (noalias)
type AccountKeeper interface {
ante.AccountKeeper

AddressCodec() address.Codec

GetModuleAddress(moduleName string) sdk.AccAddress
Expand All @@ -22,6 +26,7 @@ type AccountKeeper interface {

// BankKeeper defines the expected supply Keeper (noalias)
type BankKeeper interface {
authtypes.BankKeeper
SpendableCoins(ctx context.Context, addr sdk.AccAddress) sdk.Coins
SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
}
20 changes: 19 additions & 1 deletion x/feegrant/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ import (
"cosmossdk.io/core/registry"
"cosmossdk.io/core/transaction"
"cosmossdk.io/errors"
"cosmossdk.io/x/auth/ante"
"cosmossdk.io/x/feegrant"
"cosmossdk.io/x/feegrant/client/cli"
"cosmossdk.io/x/feegrant/keeper"

sdkclient "github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
)

Expand Down Expand Up @@ -160,6 +162,22 @@ func (am AppModule) EndBlock(ctx context.Context) error {
// TxValidator implements appmodulev2.HasTxValidator.
// It replaces auth ante handlers for server/v2
func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error {
// TODO in follow-up
validators := []interface {
ValidateTx(ctx context.Context, tx sdk.Tx) error
}{
ante.NewDeductFeeDecorator(am.accountKeeper, am.bankKeeper, am.keeper, nil),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this forces an import of auth. can we just put it in auth and call it a day?

Copy link
Contributor Author

@likhita-809 likhita-809 Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could do that, but still this needs other changes as mentioned in comment (#19967 (review)) here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make these changes once after julien's #19949 is finished, as that pr handles most of the antehandler stuff for auth module.

}

sdkTx, ok := tx.(sdk.Tx)
if !ok {
return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx)
}

for _, validator := range validators {
if err := validator.ValidateTx(ctx, sdkTx); err != nil {
return err
}
}

return nil
}
Loading
Loading