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

Problem: native fee handler don't respect DefaultPriorityReduction #422

Merged
merged 3 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion app/ante/eip712.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func NewLegacyCosmosAnteHandlerEip712(ctx sdk.Context, options HandlerOptions, e
NewMinGasPriceDecorator(options.FeeMarketKeeper, evmDenom),
authante.NewValidateMemoDecorator(options.AccountKeeper),
authante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
authante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker),
NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker),
// SetPubKeyDecorator must be called before all signature verification decorators
authante.NewSetPubKeyDecorator(options.AccountKeeper),
authante.NewValidateSigCountDecorator(options.AccountKeeper),
Expand Down
11 changes: 8 additions & 3 deletions app/ante/fee_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,14 @@

// checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per
// unit of gas is fixed and set by each validator, and the tx priority is computed from the gas price.
func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.FeeTx) (sdk.Coins, int64, error) {
feeCoins := tx.GetFee()
gas := tx.GetGas()
func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) {
feeTx, ok := tx.(sdk.FeeTx)
if !ok {
return nil, 0, errorsmod.Wrap(errortypes.ErrTxDecode, "Tx must be a FeeTx")
}

Check warning on line 118 in app/ante/fee_checker.go

View check run for this annotation

Codecov / codecov/patch

app/ante/fee_checker.go#L117-L118

Added lines #L117 - L118 were not covered by tests

feeCoins := feeTx.GetFee()
gas := feeTx.GetGas()
minGasPrices := ctx.MinGasPrices()

// Ensure that the provided fees meet a minimum threshold for the validator,
Expand Down
2 changes: 1 addition & 1 deletion app/ante/handler_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func newCosmosAnteHandler(ctx sdk.Context, options HandlerOptions, extra ...sdk.
NewMinGasPriceDecorator(options.FeeMarketKeeper, evmDenom),
ante.NewValidateMemoDecorator(options.AccountKeeper),
ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
ante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker),
NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker),
// SetPubKeyDecorator must be called before all signature verification decorators
ante.NewSetPubKeyDecorator(options.AccountKeeper),
ante.NewValidateSigCountDecorator(options.AccountKeeper),
Expand Down
122 changes: 122 additions & 0 deletions app/ante/nativefee.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package ante

import (
"fmt"

"cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/auth/ante"
"github.com/cosmos/cosmos-sdk/x/auth/types"
evmkeeper "github.com/evmos/ethermint/x/evm/keeper"
)

// 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.
// Call next AnteHandler if fees successfully deducted.
// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator
type DeductFeeDecorator struct {
accountKeeper ante.AccountKeeper
bankKeeper types.BankKeeper
feegrantKeeper ante.FeegrantKeeper
txFeeChecker ante.TxFeeChecker
}

func NewDeductFeeDecorator(ak ante.AccountKeeper, bk types.BankKeeper, fk ante.FeegrantKeeper, tfc ante.TxFeeChecker) DeductFeeDecorator {
if tfc == nil {
tfc = checkTxFeeWithValidatorMinGasPrices
}

return DeductFeeDecorator{
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) {
feeTx, ok := tx.(sdk.FeeTx)
if !ok {
return ctx, errors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
}

Check warning on line 43 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L42-L43

Added lines #L42 - L43 were not covered by tests

if !simulate && ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 {
return ctx, errors.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas")
}

Check warning on line 47 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L46-L47

Added lines #L46 - L47 were not covered by tests

var (
priority int64
err error
)

fee := feeTx.GetFee()
if !simulate {
fee, priority, err = dfd.txFeeChecker(ctx, tx)
if err != nil {
return ctx, err
}

Check warning on line 59 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L58-L59

Added lines #L58 - L59 were not covered by tests
}
if err := dfd.checkDeductFee(ctx, tx, fee); err != nil {
return ctx, err
}

Check warning on line 63 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L62-L63

Added lines #L62 - L63 were not covered by tests

newCtx := ctx.WithPriority(priority)

return next(newCtx, tx, simulate)
}

func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error {
feeTx, ok := sdkTx.(sdk.FeeTx)
if !ok {
return errors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
}

Check warning on line 74 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L73-L74

Added lines #L73 - L74 were not covered by tests

if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil {
return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName)
}

Check warning on line 78 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L77-L78

Added lines #L77 - L78 were not covered by tests

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 sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled")
} else if !feeGranter.Equals(feePayer) {
err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, sdkTx.GetMsgs())
if err != nil {
return errors.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer)
}

Check warning on line 93 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L87-L93

Added lines #L87 - L93 were not covered by tests
}

deductFeesFrom = feeGranter

Check warning on line 96 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L96

Added line #L96 was not covered by tests
}

deductFeesFromAcc := dfd.accountKeeper.GetAccount(ctx, deductFeesFrom)
if deductFeesFromAcc == nil {
return sdkerrors.ErrUnknownAddress.Wrapf("fee payer address: %s does not exist", deductFeesFrom)
}

Check warning on line 102 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L101-L102

Added lines #L101 - L102 were not covered by tests

// deduct the fees
if !fee.IsZero() {
err := evmkeeper.DeductFees(dfd.bankKeeper, ctx, deductFeesFromAcc, fee)
if err != nil {
return err
}

Check warning on line 109 in app/ante/nativefee.go

View check run for this annotation

Codecov / codecov/patch

app/ante/nativefee.go#L108-L109

Added lines #L108 - L109 were not covered by tests
}

events := sdk.Events{
sdk.NewEvent(
sdk.EventTypeTx,
sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()),
sdk.NewAttribute(sdk.AttributeKeyFeePayer, deductFeesFrom.String()),
),
}
ctx.EventManager().EmitEvents(events)

return nil
}
6 changes: 3 additions & 3 deletions x/evm/keeper/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/stretchr/testify/require"

sdk "github.com/cosmos/cosmos-sdk/types"
authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/crypto"
Expand All @@ -18,6 +17,7 @@ import (
"github.com/evmos/ethermint/server/config"
"github.com/evmos/ethermint/testutil"
ethermint "github.com/evmos/ethermint/types"
evmkeeper "github.com/evmos/ethermint/x/evm/keeper"
"github.com/evmos/ethermint/x/evm/types"
)

Expand Down Expand Up @@ -115,7 +115,7 @@ func doBenchmark(b *testing.B, txBuilder TxBuilder) {
require.NoError(b, err)

fees := sdk.Coins{sdk.NewCoin(suite.EvmDenom(), sdkmath.NewIntFromBigInt(txData.Fee()))}
err = authante.DeductFees(suite.App.BankKeeper, suite.Ctx, suite.App.AccountKeeper.GetAccount(ctx, msg.GetFrom()), fees)
err = evmkeeper.DeductFees(suite.App.BankKeeper, suite.Ctx, suite.App.AccountKeeper.GetAccount(ctx, msg.GetFrom()), fees)
require.NoError(b, err)

rsp, err := suite.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(ctx), msg)
Expand Down Expand Up @@ -182,7 +182,7 @@ func BenchmarkMessageCall(b *testing.B) {
require.NoError(b, err)

fees := sdk.Coins{sdk.NewCoin(suite.EvmDenom(), sdkmath.NewIntFromBigInt(txData.Fee()))}
err = authante.DeductFees(suite.App.BankKeeper, suite.Ctx, suite.App.AccountKeeper.GetAccount(ctx, msg.GetFrom()), fees)
err = evmkeeper.DeductFees(suite.App.BankKeeper, suite.Ctx, suite.App.AccountKeeper.GetAccount(ctx, msg.GetFrom()), fees)
require.NoError(b, err)

rsp, err := suite.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(ctx), msg)
Expand Down
17 changes: 16 additions & 1 deletion x/evm/keeper/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
sdk "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

"github.com/evmos/ethermint/x/evm/types"
Expand Down Expand Up @@ -69,7 +70,7 @@
}

// deduct the full gas cost from the user balance
if err := authante.DeductFees(k.bankKeeper, ctx, signerAcc, fees); err != nil {
if err := DeductFees(k.bankKeeper, ctx, signerAcc, fees); err != nil {
return errorsmod.Wrapf(err, "failed to deduct full gas cost %s from the user %s balance", fees, from)
}

Expand Down Expand Up @@ -150,3 +151,17 @@
}
return nil
}

// DeductFees deducts fees from the given account.
func DeductFees(bankKeeper authtypes.BankKeeper, ctx sdk.Context, acc authtypes.AccountI, fees sdk.Coins) error {
if !fees.IsValid() {
return errorsmod.Wrapf(errortypes.ErrInsufficientFee, "invalid fee amount: %s", fees)
}

Check warning on line 159 in x/evm/keeper/utils.go

View check run for this annotation

Codecov / codecov/patch

x/evm/keeper/utils.go#L158-L159

Added lines #L158 - L159 were not covered by tests

err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), authtypes.FeeCollectorName, fees)
if err != nil {
return errorsmod.Wrapf(errortypes.ErrInsufficientFunds, err.Error())
}

return nil
}
Loading