diff --git a/CHANGELOG.md b/CHANGELOG.md index c28aba27f7..8f860b7c78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (evm, ante) [tharsis#620](https://github.com/tharsis/ethermint/pull/620) Add fee market field to EVM `Keeper` and `AnteHandler`. * (all) [tharsis#231](https://github.com/tharsis/ethermint/pull/231) Bump go-ethereum version to [`v1.10.9`](https://github.com/ethereum/go-ethereum/releases/tag/v1.10.9) * (ante) [tharsis#703](https://github.com/tharsis/ethermint/pull/703) Fix some fields in transaction are not authenticated by signature. +* (evm) [tharsis#751](https://github.com/tharsis/ethermint/pull/751) don't revert gas refund logic when transaction reverted ### Features diff --git a/x/evm/handler_test.go b/x/evm/handler_test.go index 5982077037..a7d6264bb9 100644 --- a/x/evm/handler_test.go +++ b/x/evm/handler_test.go @@ -525,3 +525,85 @@ func (suite *EvmTestSuite) TestErrorWhenDeployContract() { // TODO: snapshot checking } + +func (suite *EvmTestSuite) deployERC20Contract() common.Address { + k := suite.app.EvmKeeper + nonce := k.GetNonce(suite.from) + ctorArgs, err := types.ERC20Contract.ABI.Pack("", suite.from, big.NewInt(0)) + suite.Require().NoError(err) + msg := ethtypes.NewMessage( + suite.from, + nil, + nonce, + big.NewInt(0), + 2000000, + big.NewInt(1), + nil, + nil, + append(types.ERC20Contract.Bin, ctorArgs...), + nil, + true, + ) + rsp, err := k.ApplyMessage(msg, nil, true) + suite.Require().NoError(err) + suite.Require().False(rsp.Failed()) + return crypto.CreateAddress(suite.from, nonce) +} + +// TestGasRefundWhenReverted check that when transaction reverted, gas refund should still work. +func (suite *EvmTestSuite) TestGasRefundWhenReverted() { + suite.SetupTest() + k := suite.app.EvmKeeper + + // the bug only reproduce when there are hooks + k.SetHooks(&DummyHook{}) + + // add some fund to pay gas fee + k.AddBalance(suite.from, big.NewInt(10000000000)) + + contract := suite.deployERC20Contract() + + // the call will fail because no balance + data, err := types.ERC20Contract.ABI.Pack("transfer", common.BigToAddress(big.NewInt(1)), big.NewInt(10)) + suite.Require().NoError(err) + + tx := types.NewTx( + suite.chainID, + k.GetNonce(suite.from), + &contract, + big.NewInt(0), + 41000, + big.NewInt(1), + nil, + nil, + data, + nil, + ) + tx.From = suite.from.String() + err = tx.Sign(suite.ethSigner, suite.signer) + suite.Require().NoError(err) + + before := k.GetBalance(suite.from) + + txData, err := types.UnpackTxData(tx.Data) + suite.Require().NoError(err) + _, err = k.DeductTxCostsFromUserBalance(suite.ctx, *tx, txData, "aphoton", true, true, true) + suite.Require().NoError(err) + + res, err := k.EthereumTx(sdk.WrapSDKContext(suite.ctx), tx) + suite.Require().NoError(err) + suite.Require().True(res.Failed()) + + after := k.GetBalance(suite.from) + + suite.Require().Equal(uint64(21861), res.GasUsed) + // check gas refund works + suite.Require().Equal(big.NewInt(21861), new(big.Int).Sub(before, after)) +} + +// DummyHook implements EvmHooks interface +type DummyHook struct{} + +func (dh *DummyHook) PostTxProcessing(ctx sdk.Context, txHash common.Hash, logs []*ethtypes.Log) error { + return nil +} diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index cd05a1d9d4..1b3b0d20c1 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -218,12 +218,6 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT return nil, stacktrace.Propagate(err, "failed to apply ethereum core message") } - // refund gas prior to handling the vm error in order to match the Ethereum gas consumption instead of the default SDK one. - err = k.RefundGas(msg, msg.Gas()-res.GasUsed, cfg.Params.EvmDenom) - if err != nil { - return nil, stacktrace.Propagate(err, "failed to refund gas leftover gas to sender %s", msg.From()) - } - res.Hash = txHash.Hex() logs := k.GetTxLogsTransient(txHash) @@ -241,6 +235,14 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT } } + // change to original context + k.WithContext(ctx) + + // refund gas according to Ethereum gas accounting rules. + if err := k.RefundGas(msg, msg.Gas()-res.GasUsed, cfg.Params.EvmDenom); err != nil { + return nil, stacktrace.Propagate(err, "failed to refund gas leftover gas to sender %s", msg.From()) + } + if len(logs) > 0 { res.Logs = types.NewLogsFromEth(logs) // Update transient block bloom filter