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 #416

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
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
31 changes: 21 additions & 10 deletions app/ante/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (

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"

ethermint "github.com/evmos/ethermint/types"
"github.com/evmos/ethermint/x/evm/keeper"
Expand Down Expand Up @@ -105,23 +107,26 @@ func (avd EthAccountVerificationDecorator) AnteHandle(
// EthGasConsumeDecorator validates enough intrinsic gas for the transaction and
// gas consumption.
type EthGasConsumeDecorator struct {
evmKeeper EVMKeeper
maxGasWanted uint64
ethCfg *params.ChainConfig
evmDenom string
baseFee *big.Int
accountKeeper authante.AccountKeeper
bankKeeper authtypes.BankKeeper
maxGasWanted uint64
ethCfg *params.ChainConfig
evmDenom string
baseFee *big.Int
}

// NewEthGasConsumeDecorator creates a new EthGasConsumeDecorator
func NewEthGasConsumeDecorator(
evmKeeper EVMKeeper,
accountKeeper authante.AccountKeeper,
bankKeeper authtypes.BankKeeper,
maxGasWanted uint64,
ethCfg *params.ChainConfig,
evmDenom string,
baseFee *big.Int,
) EthGasConsumeDecorator {
return EthGasConsumeDecorator{
evmKeeper,
accountKeeper,
bankKeeper,
maxGasWanted,
ethCfg,
evmDenom,
Expand Down Expand Up @@ -195,9 +200,15 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
return ctx, errorsmod.Wrapf(err, "failed to verify the fees")
}

err = egcd.evmKeeper.DeductTxCostsFromUserBalance(ctx, fees, common.BytesToAddress(msgEthTx.From))
if err != nil {
return ctx, errorsmod.Wrapf(err, "failed to deduct transaction costs from user balance")
// fetch sender account
acc := egcd.accountKeeper.GetAccount(ctx, msgEthTx.From)
if acc == nil {
return ctx, errorsmod.Wrapf(errortypes.ErrUnknownAddress, "account %s does not exist", common.BytesToAddress(msgEthTx.From))
}

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

events = append(events,
Expand Down
2 changes: 1 addition & 1 deletion app/ante/eth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (suite *AnteTestSuite) TestEthGasConsumeDecorator() {
chainCfg := evmParams.GetChainConfig()
ethCfg := chainCfg.EthereumConfig(chainID)
baseFee := suite.app.EvmKeeper.GetBaseFee(suite.ctx, ethCfg)
dec := ante.NewEthGasConsumeDecorator(suite.app.EvmKeeper, config.DefaultMaxTxGasWanted, ethCfg, evmtypes.DefaultEVMDenom, baseFee)
dec := ante.NewEthGasConsumeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper, config.DefaultMaxTxGasWanted, ethCfg, evmtypes.DefaultEVMDenom, baseFee)

addr := tests.GenerateAddress()

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 @@ func NewDynamicFeeChecker(k DynamicFeeEVMKeeper) authante.TxFeeChecker {

// 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")
}

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

// Ensure that the provided fees meet a minimum threshold for the validator,
Expand Down
4 changes: 2 additions & 2 deletions app/ante/handler_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func newEthAnteHandler(ctx sdk.Context, options HandlerOptions, extra ...sdk.Ant
NewEthSigVerificationDecorator(chainID),
NewEthAccountVerificationDecorator(options.AccountKeeper, options.EvmKeeper, evmDenom),
NewCanTransferDecorator(options.EvmKeeper, baseFee, &evmParams, ethCfg),
NewEthGasConsumeDecorator(options.EvmKeeper, options.MaxTxGasWanted, ethCfg, evmDenom, baseFee),
NewEthGasConsumeDecorator(options.AccountKeeper, options.BankKeeper, options.MaxTxGasWanted, ethCfg, evmDenom, baseFee),
NewEthIncrementSenderSequenceDecorator(options.AccountKeeper), // innermost AnteDecorator.
NewGasWantedDecorator(options.FeeMarketKeeper, ethCfg),
NewEthEmitEventDecorator(options.EvmKeeper), // emit eth tx hash and index at the very last ante handler.
Expand All @@ -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
2 changes: 0 additions & 2 deletions app/ante/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
tx "github.com/cosmos/cosmos-sdk/types/tx"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/params"
"github.com/evmos/ethermint/x/evm/statedb"
evmtypes "github.com/evmos/ethermint/x/evm/types"
Expand All @@ -40,7 +39,6 @@ type EVMKeeper interface {
statedb.Keeper
DynamicFeeEVMKeeper

DeductTxCostsFromUserBalance(ctx sdk.Context, fees sdk.Coins, from common.Address) error
ResetTransientGasUsed(ctx sdk.Context)
GetTxIndexTransient(ctx sdk.Context) uint64
}
Expand Down
135 changes: 135 additions & 0 deletions app/ante/nativefee.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
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"
)

// 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")
}

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

var (
priority int64
err error
)

fee := feeTx.GetFee()
if !simulate {
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) 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")
}

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

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)
}
}

deductFeesFrom = feeGranter
}

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

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

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
}

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

err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees)
if err != nil {
return errors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error())
}

return nil
}
9 changes: 8 additions & 1 deletion x/evm/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/evmos/ethermint/app"
"github.com/evmos/ethermint/app/ante"
ethermint "github.com/evmos/ethermint/types"
"github.com/evmos/ethermint/x/evm"
"github.com/evmos/ethermint/x/evm/types"
Expand Down Expand Up @@ -500,7 +501,13 @@ func (suite *HandlerTestSuite) TestERC20TransferReverted() {
suite.Require().NoError(err)
fees, err := keeper.VerifyFee(txData, "aphoton", baseFee, true, true, true, suite.Ctx.IsCheckTx())
suite.Require().NoError(err)
err = k.DeductTxCostsFromUserBalance(suite.Ctx, fees, tx.GetSender())

// fetch sender account
signerAcc := suite.App.AccountKeeper.GetAccount(suite.Ctx, tx.GetSender().Bytes())
suite.Require().NotNil(signerAcc)

// deduct the full gas cost from the user balance
err = ante.DeductFees(suite.App.BankKeeper, suite.Ctx, signerAcc, fees)
suite.Require().NoError(err)

res, err := k.EthereumTx(sdk.WrapSDKContext(suite.Ctx), tx)
Expand Down
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,12 +9,12 @@ 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"

ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/evmos/ethermint/app/ante"
"github.com/evmos/ethermint/server/config"
"github.com/evmos/ethermint/testutil"
ethermint "github.com/evmos/ethermint/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 = ante.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 = ante.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
22 changes: 0 additions & 22 deletions x/evm/keeper/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
sdkmath "cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

"github.com/evmos/ethermint/x/evm/types"
Expand Down Expand Up @@ -55,27 +54,6 @@ func GetProposerAddress(ctx sdk.Context, proposerAddress sdk.ConsAddress) sdk.Co
return proposerAddress
}

// DeductTxCostsFromUserBalance deducts the fees from the user balance. Returns an
// error if the specified sender address does not exist or the account balance is not sufficient.
func (k *Keeper) DeductTxCostsFromUserBalance(
ctx sdk.Context,
fees sdk.Coins,
from common.Address,
) error {
// fetch sender account
signerAcc, err := authante.GetSignerAcc(ctx, k.accountKeeper, from.Bytes())
if err != nil {
return errorsmod.Wrapf(err, "account not found for sender %s", from)
}

// deduct the full gas cost from the user balance
if err := authante.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)
}

return nil
}

// VerifyFee is used to return the fee for the given transaction data in sdk.Coins. It checks that the
// gas limit is not reached, the gas limit is higher than the intrinsic gas and that the
// base fee is higher than the gas fee cap.
Expand Down
8 changes: 7 additions & 1 deletion x/evm/keeper/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
ethtypes "github.com/ethereum/go-ethereum/core/types"
ethparams "github.com/ethereum/go-ethereum/params"
"github.com/evmos/ethermint/app"
"github.com/evmos/ethermint/app/ante"
"github.com/evmos/ethermint/testutil"
"github.com/evmos/ethermint/x/evm/keeper"
evmtypes "github.com/evmos/ethermint/x/evm/types"
Expand Down Expand Up @@ -531,7 +532,12 @@ func (suite *UtilsTestSuite) TestVerifyFeeAndDeductTxCostsFromUserBalance() {
suite.Require().Nil(fees, "invalid test %d passed. fees value must be nil - '%s'", i, tc.name)
}

err = suite.App.EvmKeeper.DeductTxCostsFromUserBalance(suite.Ctx, fees, common.BytesToAddress(tx.From))
// fetch sender account
acc := suite.App.AccountKeeper.GetAccount(suite.Ctx, tx.From)
yihuang marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().NotNil(acc, "account not found for sender %s", tx.From)

// deduct the full gas cost from the user balance
err = ante.DeductFees(suite.App.BankKeeper, suite.Ctx, acc, fees)
if tc.expectPassDeduct {
suite.Require().NoError(err, "valid test %d failed - '%s'", i, tc.name)
} else {
Expand Down
Loading