From a99fbfdc2b4c73d69038f37c522e9dbe3ec77f56 Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Mon, 4 May 2020 18:41:17 -0400 Subject: [PATCH] fix AnteHandler gas consumption (#275) * fix antehandler gas consumption * fix gas * typo --- app/ante/ante_test.go | 2 +- app/ante/eth.go | 65 ++++++++++++++++++++++++------------------- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index db5f9ff1..c01bf1d5 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -55,7 +55,7 @@ func (suite *AnteTestSuite) TestValidEthTx() { to := ethcmn.BytesToAddress(addr2.Bytes()) amt := big.NewInt(32) gas := big.NewInt(20) - ethMsg := evmtypes.NewMsgEthereumTx(0, &to, amt, 34910, gas, []byte("test")) + ethMsg := evmtypes.NewMsgEthereumTx(0, &to, amt, 22000, gas, []byte("test")) tx, err := newTestEthTx(suite.ctx, ethMsg, priv1) suite.Require().NoError(err) diff --git a/app/ante/eth.go b/app/ante/eth.go index 29e68568..4f1297ab 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -94,16 +94,17 @@ func (emfd EthMempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula minGasPrices := ctx.MinGasPrices() - allGTE := true - for _, v := range minGasPrices { - if !fee.IsGTE(v) { - allGTE = false - } + // check that fee provided is greater than the minimum + // NOTE: we only check if photons are present in min gas prices. It is up to the + // sender if they want to send additional fees in other denominations. + var hasEnoughFees bool + if fee.Amount.GTE(minGasPrices.AmountOf(emint.DenomDefault)) { + hasEnoughFees = true } - // it is assumed that the minimum fees will only include the single valid denom - if !ctx.MinGasPrices().IsZero() && !allGTE { - // reject the transaction that does not meet the minimum fee + // reject transaction if minimum gas price is positive and the transaction does not + // meet the minimum fee + if !ctx.MinGasPrices().IsZero() && !hasEnoughFees { return ctx, sdkerrors.Wrap( sdkerrors.ErrInsufficientFee, fmt.Sprintf("insufficient fee, got: %q required: %q", fee, ctx.MinGasPrices()), @@ -135,12 +136,14 @@ func (esvd EthSigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, s } // validate sender/signature - // NOTE: signer is retrieved from the transaction on the next AnteDecorator _, err = msgEthTx.VerifySig(chainID) if err != nil { - return ctx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "signature verification failed") + return ctx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, fmt.Sprintf("signature verification failed: %s", err.Error())) } + // NOTE: when signature verification succeeds, a non-empty signer address can be + // retrieved from the transaction on the next AnteDecorators. + return next(ctx, msgEthTx, simulate) } @@ -169,10 +172,10 @@ func (avd AccountVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, s return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type: %T", tx) } - // sender address should be in the tx cache + // sender address should be in the tx cache from the previous AnteHandle call address := msgEthTx.From() - if address == nil { - panic("sender address is nil") + if address.Empty() { + panic("sender address cannot be empty") } acc := avd.ak.GetAccount(ctx, address) @@ -188,19 +191,20 @@ func (avd AccountVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, s ) } - // validate sender has enough funds + // validate sender has enough funds to pay for gas cost balance := avd.bk.GetBalance(ctx, acc.GetAddress(), emint.DenomDefault) if balance.Amount.BigInt().Cmp(msgEthTx.Cost()) < 0 { return ctx, sdkerrors.Wrapf( sdkerrors.ErrInsufficientFunds, - "%s < %s%s", balance.String(), msgEthTx.Cost().String(), emint.DenomDefault, + "sender balance < tx gas cost (%s < %s%s)", balance.String(), msgEthTx.Cost().String(), emint.DenomDefault, ) } return next(ctx, tx, simulate) } -// NonceVerificationDecorator that the nonce matches +// NonceVerificationDecorator that the account nonce from the transaction matches +// the sender account sequence. type NonceVerificationDecorator struct { ak auth.AccountKeeper } @@ -220,10 +224,10 @@ func (nvd NonceVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type: %T", tx) } - // sender address should be in the tx cache + // sender address should be in the tx cache from the previous AnteHandle call address := msgEthTx.From() - if address == nil { - panic("sender address is nil") + if address.Empty() { + panic("sender address cannot be empty") } acc := nvd.ak.GetAccount(ctx, address) @@ -272,8 +276,8 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // sender address should be in the tx cache address := msgEthTx.From() - if address == nil { - panic("sender address is nil") + if address.Empty() { + panic("sender address cannot be empty") } // Fetch sender account from signature @@ -337,28 +341,33 @@ func NewIncrementSenderSequenceDecorator(ak auth.AccountKeeper) IncrementSenderS // AnteHandle handles incrementing the sequence of the sender. func (issd IncrementSenderSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { - // no need to increment sequence on RecheckTx + // get and set account must be called with an infinite gas meter in order to prevent + // additional gas from being deducted. + gasMeter := ctx.GasMeter() + ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + + // no need to increment sequence on RecheckTx mode if ctx.IsReCheckTx() && !simulate { + ctx = ctx.WithGasMeter(gasMeter) return next(ctx, tx, simulate) } - // get and set account must be called with an infinite gas meter in order to prevent - // additional gas from being deducted. - oldCtx := ctx.WithBlockGasMeter(sdk.NewInfiniteGasMeter()) - msgEthTx, ok := tx.(evmtypes.MsgEthereumTx) if !ok { + ctx = ctx.WithGasMeter(gasMeter) return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type: %T", tx) } // increment sequence of all signers for _, addr := range msgEthTx.GetSigners() { - acc := issd.ak.GetAccount(oldCtx, addr) + acc := issd.ak.GetAccount(ctx, addr) if err := acc.SetSequence(acc.GetSequence() + 1); err != nil { panic(err) } - issd.ak.SetAccount(oldCtx, acc) + issd.ak.SetAccount(ctx, acc) } + // set the original gas meter + ctx = ctx.WithGasMeter(gasMeter) return next(ctx, tx, simulate) }