diff --git a/app/ante/ante.go b/app/ante/ante.go index 74a43ce7..bb051744 100644 --- a/app/ante/ante.go +++ b/app/ante/ante.go @@ -19,32 +19,20 @@ import ( ibcante "github.com/cosmos/ibc-go/modules/core/ante" "github.com/tharsis/ethermint/crypto/ethsecp256k1" + evmtypes "github.com/tharsis/ethermint/x/evm/types" ) const ( secp256k1VerifyCost uint64 = 21000 ) -// AccountKeeper defines an expected keeper interface for the auth module's AccountKeeper -type AccountKeeper interface { - authante.AccountKeeper - NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI - GetSequence(sdk.Context, sdk.AccAddress) (uint64, error) -} - -// BankKeeper defines an expected keeper interface for the bank module's Keeper -type BankKeeper interface { - authtypes.BankKeeper - GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) sdk.Coin -} - // NewAnteHandler returns an ante handler responsible for attempting to route an // Ethereum or SDK transaction to an internal ante handler for performing // transaction-level processing (e.g. fee payment, signature verification) before // being passed onto it's respective handler. func NewAnteHandler( - ak AccountKeeper, - bankKeeper BankKeeper, + ak evmtypes.AccountKeeper, + bankKeeper evmtypes.BankKeeper, evmKeeper EVMKeeper, feeGrantKeeper authante.FeegrantKeeper, channelKeeper channelkeeper.Keeper, diff --git a/app/ante/eth.go b/app/ante/eth.go index bcdbb4f4..75b20235 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -9,6 +9,7 @@ import ( "github.com/palantir/stacktrace" ethermint "github.com/tharsis/ethermint/types" + evmkeeper "github.com/tharsis/ethermint/x/evm/keeper" evmtypes "github.com/tharsis/ethermint/x/evm/types" "github.com/ethereum/go-ethereum/common" @@ -89,13 +90,13 @@ func (esvd EthSigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, s // EthAccountVerificationDecorator validates an account balance checks type EthAccountVerificationDecorator struct { - ak AccountKeeper - bankKeeper BankKeeper + ak evmtypes.AccountKeeper + bankKeeper evmtypes.BankKeeper evmKeeper EVMKeeper } // NewEthAccountVerificationDecorator creates a new EthAccountVerificationDecorator -func NewEthAccountVerificationDecorator(ak AccountKeeper, bankKeeper BankKeeper, ek EVMKeeper) EthAccountVerificationDecorator { +func NewEthAccountVerificationDecorator(ak evmtypes.AccountKeeper, bankKeeper evmtypes.BankKeeper, ek EVMKeeper) EthAccountVerificationDecorator { return EthAccountVerificationDecorator{ ak: ak, bankKeeper: bankKeeper, @@ -154,18 +155,8 @@ func (avd EthAccountVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx avd.ak.SetAccount(ctx, acc) } - // validate sender has enough funds to pay for tx cost - balance := avd.bankKeeper.GetBalance(ctx, from, evmDenom) - cost := txData.Cost() - - if balance.Amount.BigInt().Cmp(cost) < 0 { - return ctx, stacktrace.Propagate( - sdkerrors.Wrapf( - sdkerrors.ErrInsufficientFunds, - "sender balance < tx cost (%s < %s%s)", balance, txData.Cost(), evmDenom, - ), - "sender should have had enough funds to pay for tx cost = fee + amount (%s = %s + %s)", cost, txData.Fee(), txData.GetValue(), - ) + if err := evmkeeper.CheckSenderBalance(ctx, avd.bankKeeper, from, txData, evmDenom); err != nil { + return ctx, stacktrace.Propagate(err, "failed to check sender balance") } } @@ -177,11 +168,11 @@ func (avd EthAccountVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx // EthNonceVerificationDecorator checks that the account nonce from the transaction matches // the sender account sequence. type EthNonceVerificationDecorator struct { - ak AccountKeeper + ak evmtypes.AccountKeeper } // NewEthNonceVerificationDecorator creates a new EthNonceVerificationDecorator -func NewEthNonceVerificationDecorator(ak AccountKeeper) EthNonceVerificationDecorator { +func NewEthNonceVerificationDecorator(ak evmtypes.AccountKeeper) EthNonceVerificationDecorator { return EthNonceVerificationDecorator{ ak: ak, } @@ -235,13 +226,13 @@ func (nvd EthNonceVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, // EthGasConsumeDecorator validates enough intrinsic gas for the transaction and // gas consumption. type EthGasConsumeDecorator struct { - ak AccountKeeper - bankKeeper BankKeeper + ak evmtypes.AccountKeeper + bankKeeper evmtypes.BankKeeper evmKeeper EVMKeeper } // NewEthGasConsumeDecorator creates a new EthGasConsumeDecorator -func NewEthGasConsumeDecorator(ak AccountKeeper, bankKeeper BankKeeper, ek EVMKeeper) EthGasConsumeDecorator { +func NewEthGasConsumeDecorator(ak evmtypes.AccountKeeper, bankKeeper evmtypes.BankKeeper, ek EVMKeeper) EthGasConsumeDecorator { return EthGasConsumeDecorator{ ak: ak, bankKeeper: bankKeeper, @@ -274,6 +265,7 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula blockHeight := big.NewInt(ctx.BlockHeight()) homestead := ethCfg.IsHomestead(blockHeight) istanbul := ethCfg.IsIstanbul(blockHeight) + evmDenom := params.EvmDenom var events sdk.Events @@ -291,45 +283,19 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula return ctx, stacktrace.Propagate(err, "failed to unpack tx data") } - isContractCreation := txData.GetTo() == nil + fees, err := evmkeeper.DeductTxCostsFromUserBalance( + ctx, + egcd.bankKeeper, + egcd.ak, + *msgEthTx, + txData, + evmDenom, + homestead, + istanbul, + ) - // fetch sender account from signature - signerAcc, err := authante.GetSignerAcc(ctx, egcd.ak, msgEthTx.GetFrom()) if err != nil { - return ctx, stacktrace.Propagate(err, "account not found for sender %s", msgEthTx.From) - } - - gasLimit := txData.GetGas() - - var accessList ethtypes.AccessList - if txData.GetAccessList() != nil { - accessList = txData.GetAccessList() - } - - intrinsicGas, err := core.IntrinsicGas(txData.GetData(), accessList, isContractCreation, homestead, istanbul) - if err != nil { - return ctx, stacktrace.Propagate( - sdkerrors.Wrap(err, "failed to compute intrinsic gas cost"), - "failed to retrieve intrinsic gas, contract creation = %t; homestead = %t, istanbul = %t", isContractCreation, homestead, istanbul) - } - - // intrinsic gas verification during CheckTx - if ctx.IsCheckTx() && gasLimit < intrinsicGas { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrOutOfGas, "gas limit too low: %d (gas limit) < %d (intrinsic gas)", gasLimit, intrinsicGas) - } - - // calculate the fees paid to validators based on gas limit and price - feeAmt := txData.Fee() // fee = gas limit * gas price - - evmDenom := egcd.evmKeeper.GetParams(ctx).EvmDenom - fees := sdk.Coins{sdk.NewCoin(evmDenom, sdk.NewIntFromBigInt(feeAmt))} - - // deduct the full gas cost from the user balance - if err := authante.DeductFees(egcd.bankKeeper, ctx, signerAcc, fees); err != nil { - return ctx, stacktrace.Propagate( - err, - "failed to deduct full gas cost %s from the user %s balance", fees, msgEthTx.From, - ) + return ctx, stacktrace.Propagate(err, "failed to deduct transaction costs from user balance") } events = append(events, sdk.NewEvent(sdk.EventTypeTx, sdk.NewAttribute(sdk.AttributeKeyFee, fees.String()))) @@ -477,11 +443,11 @@ func (ald AccessListDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b // EthIncrementSenderSequenceDecorator increments the sequence of the signers. type EthIncrementSenderSequenceDecorator struct { - ak AccountKeeper + ak evmtypes.AccountKeeper } // NewEthIncrementSenderSequenceDecorator creates a new EthIncrementSenderSequenceDecorator. -func NewEthIncrementSenderSequenceDecorator(ak AccountKeeper) EthIncrementSenderSequenceDecorator { +func NewEthIncrementSenderSequenceDecorator(ak evmtypes.AccountKeeper) EthIncrementSenderSequenceDecorator { return EthIncrementSenderSequenceDecorator{ ak: ak, } diff --git a/x/evm/keeper/utils.go b/x/evm/keeper/utils.go new file mode 100644 index 00000000..807458dc --- /dev/null +++ b/x/evm/keeper/utils.go @@ -0,0 +1,97 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + authante "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/palantir/stacktrace" + + evmtypes "github.com/tharsis/ethermint/x/evm/types" + + "github.com/ethereum/go-ethereum/core" + ethtypes "github.com/ethereum/go-ethereum/core/types" +) + +// AccountKeeper defines an expected keeper interface for the auth module's AccountKeeper +// DeductTxCostsFromUserBalance it calculates the tx costs and deducts the fees +func DeductTxCostsFromUserBalance( + ctx sdk.Context, + bankKeeper evmtypes.BankKeeper, + accountKeeper evmtypes.AccountKeeper, + msgEthTx evmtypes.MsgEthereumTx, + txData evmtypes.TxData, + denom string, + homestead bool, + istanbul bool, +) (sdk.Coins, error) { + isContractCreation := txData.GetTo() == nil + + // fetch sender account from signature + signerAcc, err := authante.GetSignerAcc(ctx, accountKeeper, msgEthTx.GetFrom()) + if err != nil { + return nil, stacktrace.Propagate(err, "account not found for sender %s", msgEthTx.From) + } + + gasLimit := txData.GetGas() + + var accessList ethtypes.AccessList + if txData.GetAccessList() != nil { + accessList = txData.GetAccessList() + } + + intrinsicGas, err := core.IntrinsicGas(txData.GetData(), accessList, isContractCreation, homestead, istanbul) + if err != nil { + return nil, stacktrace.Propagate(sdkerrors.Wrap( + err, + "failed to compute intrinsic gas cost"), "failed to retrieve intrinsic gas, contract creation = %t; homestead = %t, istanbul = %t", + isContractCreation, homestead, istanbul, + ) + } + + // intrinsic gas verification during CheckTx + if ctx.IsCheckTx() && gasLimit < intrinsicGas { + return nil, sdkerrors.Wrapf( + sdkerrors.ErrOutOfGas, + "gas limit too low: %d (gas limit) < %d (intrinsic gas)", gasLimit, intrinsicGas, + ) + } + + // calculate the fees paid to validators based on gas limit and price + feeAmt := txData.Fee() // fee = gas limit * gas price + + fees := sdk.Coins{sdk.NewCoin(denom, sdk.NewIntFromBigInt(feeAmt))} + + // deduct the full gas cost from the user balance + if err := authante.DeductFees(bankKeeper, ctx, signerAcc, fees); err != nil { + return nil, stacktrace.Propagate( + err, + "failed to deduct full gas cost %s from the user %s balance", + fees, msgEthTx.From, + ) + } + return fees, nil +} + +// CheckSenderBalance validates sender has enough funds to pay for tx cost +func CheckSenderBalance( + ctx sdk.Context, + bankKeeper evmtypes.BankKeeper, + sender sdk.AccAddress, + txData evmtypes.TxData, + denom string, +) error { + balance := bankKeeper.GetBalance(ctx, sender, denom) + cost := txData.Cost() + + if balance.Amount.BigInt().Cmp(cost) < 0 { + return stacktrace.Propagate( + sdkerrors.Wrapf( + sdkerrors.ErrInsufficientFunds, + "sender balance < tx cost (%s < %s%s)", balance, txData.Cost(), denom, + ), + "sender should have had enough funds to pay for tx cost = fee + amount (%s = %s + %s)", + cost, txData.Fee(), txData.GetValue(), + ) + } + return nil +} diff --git a/x/evm/keeper/utils_test.go b/x/evm/keeper/utils_test.go new file mode 100644 index 00000000..062aabb9 --- /dev/null +++ b/x/evm/keeper/utils_test.go @@ -0,0 +1,257 @@ +package keeper_test + +import ( + "math/big" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/ethereum/go-ethereum/common" + ethtypes "github.com/ethereum/go-ethereum/core/types" + evmkeeper "github.com/tharsis/ethermint/x/evm/keeper" + evmtypes "github.com/tharsis/ethermint/x/evm/types" +) + +func (suite *KeeperTestSuite) TestCheckSenderBalance() { + hundredInt := sdk.NewInt(100) + zeroInt := sdk.ZeroInt() + oneInt := sdk.NewInt(1) + fiveInt := sdk.NewInt(5) + fiftyInt := sdk.NewInt(50) + + testCases := []struct { + name string + to string + gasLimit uint64 + gasPrice *sdk.Int + cost *sdk.Int + from string + accessList *ethtypes.AccessList + expectPass bool + }{ + { + name: "Enough balance", + to: suite.address.String(), + gasLimit: 10, + gasPrice: &oneInt, + cost: &oneInt, + from: suite.address.String(), + accessList: ðtypes.AccessList{}, + expectPass: true, + }, + { + name: "Equal balance", + to: suite.address.String(), + gasLimit: 99, + gasPrice: &oneInt, + cost: &oneInt, + from: suite.address.String(), + accessList: ðtypes.AccessList{}, + expectPass: true, + }, + { + name: "Higher gas limit, not enough balance", + to: suite.address.String(), + gasLimit: 100, + gasPrice: &oneInt, + cost: &oneInt, + from: suite.address.String(), + accessList: ðtypes.AccessList{}, + expectPass: false, + }, + { + name: "Higher gas price, enough balance", + to: suite.address.String(), + gasLimit: 10, + gasPrice: &fiveInt, + cost: &oneInt, + from: suite.address.String(), + accessList: ðtypes.AccessList{}, + expectPass: true, + }, + { + name: "Higher gas price, not enough balance", + to: suite.address.String(), + gasLimit: 20, + gasPrice: &fiveInt, + cost: &oneInt, + from: suite.address.String(), + accessList: ðtypes.AccessList{}, + expectPass: false, + }, + { + name: "Higher cost, enough balance", + to: suite.address.String(), + gasLimit: 10, + gasPrice: &fiveInt, + cost: &fiftyInt, + from: suite.address.String(), + accessList: ðtypes.AccessList{}, + expectPass: true, + }, + { + name: "Higher cost, not enough balance", + to: suite.address.String(), + gasLimit: 10, + gasPrice: &fiveInt, + cost: &hundredInt, + from: suite.address.String(), + accessList: ðtypes.AccessList{}, + expectPass: false, + }, + } + + suite.app.EvmKeeper.AddBalance(suite.address, hundredInt.BigInt()) + balance := suite.app.EvmKeeper.GetBalance(suite.address) + suite.Require().Equal(balance, hundredInt.BigInt()) + + for i, tc := range testCases { + suite.Run(tc.name, func() { + to := common.HexToAddress(tc.from) + + var amount, gasPrice *big.Int + if tc.cost != nil { + amount = tc.cost.BigInt() + } + if tc.gasPrice != nil { + gasPrice = tc.gasPrice.BigInt() + } + + tx := evmtypes.NewTx(zeroInt.BigInt(), 1, &to, amount, tc.gasLimit, gasPrice, nil, tc.accessList) + tx.From = tc.from + + txData, _ := evmtypes.UnpackTxData(tx.Data) + + err := evmkeeper.CheckSenderBalance( + suite.app.EvmKeeper.Ctx(), + suite.app.BankKeeper, + suite.address[:], + txData, + evmtypes.DefaultEVMDenom, + ) + + if tc.expectPass { + suite.Require().NoError(err, "valid test %d failed", i) + } else { + suite.Require().Error(err, "invalid test %d passed", i) + } + + }) + } +} + +func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { + hundredInt := sdk.NewInt(100) + zeroInt := sdk.ZeroInt() + oneInt := sdk.NewInt(1) + fiveInt := sdk.NewInt(5) + fiftyInt := sdk.NewInt(50) + + testCases := []struct { + name string + gasLimit uint64 + gasPrice *sdk.Int + cost *sdk.Int + accessList *ethtypes.AccessList + expectPass bool + }{ + { + name: "Enough balance", + gasLimit: 10, + gasPrice: &oneInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPass: true, + }, + { + name: "Equal balance", + gasLimit: 100, + gasPrice: &oneInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPass: true, + }, + { + name: "Higher gas limit, not enough balance", + gasLimit: 105, + gasPrice: &oneInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPass: false, + }, + { + name: "Higher gas price, enough balance", + gasLimit: 20, + gasPrice: &fiveInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPass: true, + }, + { + name: "Higher gas price, not enough balance", + gasLimit: 20, + gasPrice: &fiftyInt, + cost: &oneInt, + accessList: ðtypes.AccessList{}, + expectPass: false, + }, + // This case is expected to be true because the fees can be deducted, but the tx + // execution is going to fail because there is no more balance to pay the cost + { + name: "Higher cost, enough balance", + gasLimit: 100, + gasPrice: &oneInt, + cost: &fiftyInt, + accessList: ðtypes.AccessList{}, + expectPass: true, + }, + } + + for i, tc := range testCases { + + suite.Run(tc.name, func() { + suite.SetupTest() + suite.app.EvmKeeper.AddBalance(suite.address, hundredInt.BigInt()) + balance := suite.app.EvmKeeper.GetBalance(suite.address) + suite.Require().Equal(balance, hundredInt.BigInt()) + + var amount, gasPrice *big.Int + if tc.cost != nil { + amount = tc.cost.BigInt() + } + if tc.gasPrice != nil { + gasPrice = tc.gasPrice.BigInt() + } + + tx := evmtypes.NewTx(zeroInt.BigInt(), 1, &suite.address, amount, tc.gasLimit, gasPrice, nil, tc.accessList) + tx.From = suite.address.String() + + txData, _ := evmtypes.UnpackTxData(tx.Data) + + fees, err := evmkeeper.DeductTxCostsFromUserBalance( + suite.app.EvmKeeper.Ctx(), + suite.app.BankKeeper, + suite.app.AccountKeeper, + *tx, + txData, + evmtypes.DefaultEVMDenom, + false, + false, + ) + + if tc.expectPass { + suite.Require().NoError(err, "valid test %d failed", i) + suite.Require().Equal( + fees, + sdk.NewCoins( + sdk.NewCoin(evmtypes.DefaultEVMDenom, tc.gasPrice.Mul(sdk.NewIntFromUint64(tc.gasLimit))), + ), + "valid test %d failed, fee value is wrong ", i, + ) + + } else { + suite.Require().Error(err, "invalid test %d passed", i) + suite.Require().Nil(fees, "invalid test %d passed. fees value must be nil", i) + } + + }) + } +} diff --git a/x/evm/types/interfaces.go b/x/evm/types/interfaces.go index 4b60f69a..642b7b9f 100644 --- a/x/evm/types/interfaces.go +++ b/x/evm/types/interfaces.go @@ -18,6 +18,7 @@ type AccountKeeper interface { GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI SetAccount(ctx sdk.Context, account authtypes.AccountI) RemoveAccount(ctx sdk.Context, account authtypes.AccountI) + GetParams(ctx sdk.Context) (params authtypes.Params) } // BankKeeper defines the expected interface needed to retrieve account balances.