fix(evm): fixed the problem gas_used is 0 when using evm type tx to transfer token to module account

[luchenqun]

When we transfer token to a module account, such as the distribution module account evmos1jv65s3grqf6v6jl3dp4t6c9t9rk99cd8974jnh (ethereum-style account is 0x93354845030274cD4bf1686Abd60AB28EC52e1a7), two scenarios can occur:

The transaction receipt example below is obtained when using the cosmos type to send the transaction. The gas_used value is displayed as 70711, the gas_used value is correct.

{
    "tx":{
        "body":{
            "messages":[
                {
                    "@type":"/cosmos.bank.v1beta1.MsgSend",
                    "from_address":"evmos1qqqqhe5pnaq5qq39wqkn957aydnrm45sdn8583",
                    "to_address":"evmos1jv65s3grqf6v6jl3dp4t6c9t9rk99cd8974jnh",
                    "amount":[
                        {
                            "denom":"aevmos",
                            "amount":"1"
                        }
                    ]
                }
            ]
        }
    },
    "tx_response":{
        "code":4,
        "raw_log":"failed to execute message; message index: 0: evmos1jv65s3grqf6v6jl3dp4t6c9t9rk99cd8974jnh is not allowed to receive funds: unauthorized",
        "gas_wanted":"100000",
        "gas_used":"70711"
    }
}

When send the transaction using the evm type. The following is an example of a queried transaction receipt, in which gas_used is displayed as 0. Because when a evm transaction fails, all gas will be deducted, so the gas_used data here is incorrect (Note: Please make sure value is not 0, otherwise this error will not be triggered.).

{
    "tx":{
        "body":{
            "messages":[
                {
                    "@type":"/ethermint.evm.v1.MsgEthereumTx",
                    "data":{
                        "@type":"/ethermint.evm.v1.LegacyTx",
                        "nonce":"1",
                        "gas_price":"10000000000",
                        "gas":"21000",
                        "to":"0x93354845030274cD4bf1686Abd60AB28EC52e1a7",
                        "value":"1000000000000000000",
                        "data":null,
                        "v":"RnM=",
                        "r":"GAQdTFdMC3PnFion2mH0U0raEhBoW87F6ik7MRpzTQs=",
                        "s":"XVPUZUdBNgW44rIBTGAGGsBrh2EaWZa7+6Ujpog8CwE="
                    },
                    "size":0,
                    "hash":"0x4283c1f11d6d6fbe19dc36e17ffe6ead2bf40dd6823d62dfb20c11c6156ce9e9",
                    "from":""
                }
            ]
        }
    },
    "tx_response":{
        "code":4,
        "raw_log":"failed to execute message; message index: 0: failed to apply transaction: failed to apply ethereum core message: failed to commit stateDB: failed to set account: evmos1jv65s3grqf6v6jl3dp4t6c9t9rk99cd8974jnh is not allowed to receive funds: unauthorized",
        "gas_wanted":"21000",
        "gas_used":"0"
    }
}

At the same time, after the transaction is executed, we can query the transaction receipt through eth_getTransactionReceipt by transaction hash, but we cannot obtain the transaction receipt.

If we only solve this problem, we can prohibit transfers to the module account during the checking phase of the transaction. But considering the general situation, such as the smart contract below, the user can pass in the module account as a parameter, then we cannot detect it.。

pragma solidity ^0.8.0;

contract EthTransfer {
    function transferEther(address payable recipient) external payable {
        require(msg.value > 0, "No ether sent with the transaction.");
        recipient.transfer(msg.value);
    }
}

So we must have a general method to solve such problems.

The reason why gas_used=0 occurs is because when executing the transaction in the EVM module, an error is triggered during the stateDB.Commit() operation in the ApplyMessageWithConfig function. As a result, the ApplyTransaction function returns an error prematurely without executing k.ResetGasMeterAndConsumeGas(ctx, totalGasUsed), leading to incorrect gas_used value. In general, whenever an error occurs in the ApplyMessageWithConfig function, there can be a gas_used=0 issue. I only used the example of an error in stateDB.Commit() to illustrate this problem.

func (k *Keeper) ApplyMessageWithConfig(...) (*types.MsgEthereumTxResponse, error) {
	// ...
	// The dirty states in `StateDB` is either committed or discarded after return
	if commit {
		if err := stateDB.Commit(); err != nil {
			return nil, errorsmod.Wrap(err, "failed to commit stateDB")
		}
	}

	// ...
}

func (k *Keeper) ApplyTransaction(ctx sdk.Context, tx *ethtypes.Transaction) (*types.MsgEthereumTxResponse, error) {
	// ...

	// pass true to commit the StateDB
	res, err := k.ApplyMessageWithConfig(tmpCtx, msg, nil, true, cfg, txConfig)
	if err != nil {
		return nil, errorsmod.Wrap(err, "failed to apply ethereum core message")
	}
	
	// ...

	// reset the gas meter for current cosmos transaction
	k.ResetGasMeterAndConsumeGas(ctx, totalGasUsed)
	return res, nil
}

So, in order to solve this problem, when the ApplyMessageWithConfig function encounters an error, we should call the k.ResetGasMeterAndConsumeGas(ctx, txGasLimit) function before returning the error. Here is the modified code for the ApplyTransaction function:

// pass true to commit the StateDB
res, err := k.ApplyMessageWithConfig(tmpCtx, msg, nil, true, cfg, txConfig)
if err != nil {
	// when a transaction contains multiple messages, as long as one of the messages fails, all gas will be deducted.
	gasLimit := ctx.GasMeter().Limit()
	k.ResetGasMeterAndConsumeGas(ctx, gasLimit)
	return nil, errorsmod.Wrap(err, "failed to apply ethereum core message")
}

Attention: The parameter for ResetGasMeterAndConsumeGas must be the gas limit of the transaction, which is ctx.GasMeter().Limit(), instead of the gas limit of the current message, which is msg.Gas(). This is because if a transaction may contains multiple messages, if one of the messages fails, all messages will fail and all gas will be deducted.

However, there is currently no interface for including multiple messages within a single transaction. Below are some notes on how to construct multiple messages within a single transaction for reference:

First, sign a transaction using ethers.js. The code is as follows:

import { ethers } from 'ethers';

async function signTransaction() {
  const privateKey = '98fbd519ecc11f96bf2d00062e305c7caacf61440a4cf8e16440ac9c91d5f7e0'; // 0x73F725bb91632Ca76cf3aaA79dcFB6dece9A7c78
  const providerUrl = 'http://127.0.0.1:8545';
  const chainId = 9000;
  const provider = new ethers.providers.JsonRpcProvider(providerUrl);

  const wallet = new ethers.Wallet(privateKey, provider);
  const receivers = [
    '0x00000be6819f41400225702d32d3dd23663dd690',
    '0x1111102dd32160b064f2a512cdef74bfdb6a9f96',
    '0x93354845030274cD4bf1686Abd60AB28EC52e1a7' // distribution module address
  ];

  let signedTxs = [];
  let nonce = 0;
  for (const to of receivers) {
    const tx = {
      to,
      value: ethers.utils.parseEther('1'),
      gasLimit: 40000000,
      gasPrice: ethers.utils.parseUnits('50', 'gwei'),
      chainId,
      nonce
    };
    nonce++;
    const signedTx = await wallet.signTransaction(tx);
    signedTxs.push(signedTx);
  }
  console.log(signedTxs);
}

signTransaction().catch((error) => {
  console.error('Error:', error);
});

Then, the signed message is assembled into a cosmos type transaction. I write a function BuildTx in rpc/backend/call_tx.go to complete this function, and then call the function in GasPrice. The relevant code is as follows:

func BuildTx(b client.TxBuilder, evmDenom string) (signing.Tx, error) {
	perMsgFeeAmount, _ := sdkmath.NewIntFromString("2000000000000000000")
	perMsgGasLimit := uint64(40000000)
	
	// YOU SIGNED EVM TX
	txHexStrs := []string{
		"0xf87080850ba43b74008402625a009400000be6819f41400225702d32d3dd23663dd690880de0b6b3a764000080824673a0be45cb58290ff2c69989d302fd70170e7bcd0cc8ce353e9f4b073914a02db1c5a02c5e6aa1315d9f7417ec37710e2e582346caf66e20fc2983318ae9d7ad8b0932",
		"0xf87001850ba43b74008402625a00941111102dd32160b064f2a512cdef74bfdb6a9f96880de0b6b3a764000080824673a04b51e046994cfb933c4b70a97b27dcbcc5c497f0f1362994abafc3bfa09ef3a1a010b46ddd3e3717d742ee045a858e839490abcd2fac80757e2da70e99293b27ba",
		"0xf87002850ba43b74008402625a00942222207b1f7b8d37566d9a2778732451dbfbc5d0880de0b6b3a764000080824674a01b80d4c61f24127959529ee4798d5af5d3989b15070a1f12efe66876e295a5e4a04d22e02e207898934a49e679cc56a8489eeaa3c3de51e4bf4cf5d8eb303ff0c2",
	}
	var ethereumTxs []sdk.Msg
	for _, txHexStr := range txHexStrs {
		evmTx := hexutil.MustDecode(txHexStr)
		tx := &ethtypes.Transaction{}
		if err := tx.UnmarshalBinary(evmTx); err != nil {
			return nil, err
		}
		ethereumTx := &evmtypes.MsgEthereumTx{}
		if err := ethereumTx.FromEthereumTx(tx); err != nil {
			return nil, err
		}
		// A valid msg should have empty `From`
		ethereumTx.From = ""
		ethereumTxs = append(ethereumTxs, ethereumTx)
	}

	builder, ok := b.(authtx.ExtensionOptionsTxBuilder)
	if !ok {
		return nil, errors.New("unsupported builder")
	}

	option, err := codectypes.NewAnyWithValue(&evmtypes.ExtensionOptionsEthereumTx{})
	if err != nil {
		return nil, err
	}

	txCount := len(txHexStrs)
	fees := sdk.Coins{sdk.Coin{Amount: perMsgFeeAmount.MulRaw(int64(txCount)), Denom: evmDenom}}

	builder.SetExtensionOptions(option)

	err = builder.SetMsgs(ethereumTxs...)
	if err != nil {
		return nil, err
	}
	builder.SetFeeAmount(fees)
	builder.SetGasLimit(perMsgGasLimit * uint64(txCount))
	tx := builder.GetTx()
	return tx, nil
}

// GasPrice returns the current gas price based on Ethermint's gas price oracle.
func (b *Backend) GasPrice() (*hexutil.Big, error) {
	var (
		result *big.Int
		err    error
	)
	if head := b.CurrentHeader(); head.BaseFee != nil {
		result, err = b.SuggestGasTipCap(head.BaseFee)
		if err != nil {
			return nil, err
		}
		result = result.Add(result, head.BaseFee)
	} else {
		result = big.NewInt(b.RPCMinGasPrice())
	}

	// return at least GlobalMinGasPrice from FeeMarket module
	minGasPrice, err := b.GlobalMinGasPrice()
	if err != nil {
		return nil, err
	}
	minGasPriceInt := minGasPrice.TruncateInt().BigInt()
	if result.Cmp(minGasPriceInt) < 0 {
		result = minGasPriceInt
	}

	/////////////////////////// ADD ///////////////////////////
	{
		cosmosTx, err := BuildTx(b.clientCtx.TxConfig.NewTxBuilder(), utils.BaseDenom)
		if err != nil {
			b.logger.Error("failed to build cosmos tx", "error", err.Error())
		}
		txBytes, err := b.clientCtx.TxConfig.TxEncoder()(cosmosTx)
		b.logger.Error("cosmos tx with multiple messages", "tx bytes", hexutil.Encode(txBytes))
	}
	/////////////////////////// ADD ///////////////////////////

	return (*hexutil.Big)(result), nil
}

Finally, call Ethereum's rpc interface eth_gasPrice to trigger the assembly of the transaction. Then you can get a cosmos transaction containing multiple messages in the log, and send this transaction to the chain through http://127.0.0.1:26657/broadcast_tx_commit.

In order to facilitate testing, you can transfer 10evmos to the address 0x73F725bb91632Ca76cf3aaA79dcFB6dece9A7c78, and you can use the transaction I have assembled below for testing.

0x0aad070aa5020a1f2f65746865726d696e742e65766d2e76312e4d7367457468657265756d54781281020aba010a1a2f65746865726d696e742e65766d2e76312e4c65676163795478129b01120b35303030303030303030301880b48913222a3078393333353438343530333032373463443462663136383641626436304142323845433532653161372a13313030303030303030303030303030303030303a02467442209787e82add28eeced04ac1cd7b1fe1ddfd1b40eb44b12794f2e07c59e8a6d1734a2037e51635be25cc59d2acd7730c59cdd11b8d7583e2ac341b9df2ef3d822c7e7e1a423078396237623234396463353435313337613663383534633831306131383334316562306237373338313831623433653135636236376435653733343862613534370aa7020a1f2f65746865726d696e742e65766d2e76312e4d7367457468657265756d54781283020abc010a1a2f65746865726d696e742e65766d2e76312e4c65676163795478129d010801120b35303030303030303030301880b48913222a3078303030303042653638313966343134303032323537303244333264336464323336363344643639302a13313030303030303030303030303030303030303a02467342201e1122036c63185fab473e379b998bfd28e0e94585e77be5f935fcaf6e240d674a206dc49def72dd513e23d473bd6f86dcd49712766715af2da0a11fb923da686b241a423078646463376138393134333064346130396434646430303135393336353732363932643663613537313933323362646465663235316461656264623766373965330aa7020a1f2f65746865726d696e742e65766d2e76312e4d7367457468657265756d54781283020abc010a1a2f65746865726d696e742e65766d2e76312e4c65676163795478129d010802120b35303030303030303030301880b48913222a3078313131313130324464333231363042303634463241353132434445663734624664423661394639362a13313030303030303030303030303030303030303a0246744220cd92e971a98cea933c35545226ac55c9fca3cdbf26d98422efd27ef48dfce93b4a207f7e8fae91998f0a6fe6b55e375a4dd15d20c14dcee5b0e99af48ffd011405c91a42307834613233393037373931343636663666396239363730363534636530633339393637626664396438343461353830633533373439393637616432613463393163fa3f2e0a2c2f65746865726d696e742e65766d2e76312e457874656e73696f6e4f7074696f6e73457468657265756d5478122612240a1d0a066165766d6f7312133630303030303030303030303030303030303010809c9c39

As for the other question, if the receipt cannot be found when querying by transaction hash, it is because the returned data does not consider this special case. Add the corresponding handling logic to fix it. The relevant code is as follows:

// CommitError defines the error message when commit after executing EVM transaction, for example
// transfer native token to a distrubition module account 0x93354845030274cD4bf1686Abd60AB28EC52e1a7 using an evm type transaction
// note: the transfer amount cannot be set to 0, otherwise this problem will not be triggered
const CommitError = "failed to commit stateDB"

// TxCommitError returns true if the tx commit error.
func TxCommitError(res *abci.ResponseDeliverTx) bool {
	return strings.Contains(res.Log, CommitError)
}

// TxSuccessOrExceedsBlockGasLimit returnsrue if the transaction was successful
// or if it failed with an ExceedBlockGasLimit error
func TxSuccessOrExceedsBlockGasLimit(res *abci.ResponseDeliverTx) bool {
	return res.Code == 0 || TxExceedBlockGasLimit(res) || TxCommitError(res)
}

[codecov]

Codecov Report

Merging #1801 (c5e6ae2) into main (11265c8) will decrease coverage by 0.14%.
Report is 1 commits behind head on main.
The diff coverage is 84.09%.

@@            Coverage Diff             @@
##             main    #1801      +/-   ##
==========================================
- Coverage   70.07%   69.93%   -0.14%     
==========================================
  Files         324      327       +3     
  Lines       24017    24166     +149     
==========================================
+ Hits        16829    16901      +72     
- Misses       6327     6401      +74     
- Partials      861      864       +3     

Files	Coverage Δ
indexer/kv_indexer.go	64.96% <100.00%> (ø)
precompiles/outposts/osmosis/osmosis.go	0.00% <ø> (ø)
precompiles/outposts/osmosis/tx.go	0.00% <ø> (ø)
rpc/backend/blocks.go	84.97% <100.00%> (+0.04%)	⬆️
x/evm/keeper/state_transition.go	84.08% <100.00%> (+0.16%)	⬆️
rpc/backend/tx_info.go	63.92% <0.00%> (ø)
rpc/types/utils.go	0.00% <0.00%> (ø)
precompiles/outposts/osmosis/types.go	88.31% <88.31%> (ø)

[GAtom22]

Thanks for the contribution @luchenqun!!
Left a couple of comments

Please resolve conflicts

[GAtom22]

Also, would be great to add tests for this, e.g: we could add something like this in the test_patches.py

def test_send_funds_to_distr_mod_eth_tx(evmos):
    """
    This tests the transfer of funds to the distribution module account,
    via ethereum tx
    which should be forbidden, since this is a blocked address.
    """
    cli = evmos.cosmos_cli()
    w3 = evmos.w3

    sender = ADDRS["signer1"]
    mod_accs = cli.query_module_accounts()
    old_src_balance = cli.balance(eth_to_bech32(sender))

    for acc in mod_accs:
        if acc["name"] != "distribution":
            continue
        receiver = decode_bech32(acc["base_account"]["address"])

    assert receiver is not None

    txhash = w3.eth.send_transaction(
        {
            "from": sender,
            "to": receiver,
            "value": 1000,
        }
    )
    receipt = w3.eth.wait_for_transaction_receipt(txhash)
    assert receipt.status == 0  # failed status expected

    wait_for_new_blocks(cli, 2)
    # only fees should be deducted from sender balance
    fees = receipt["gasUsed"] * receipt["effectiveGasPrice"]
    assert fees > 0

    new_src_balance = cli.balance(eth_to_bech32(sender))
    assert old_src_balance - fees == new_src_balance

[CLAassistant]

All committers have signed the CLA.

[luchenqun]

Also, would be great to add tests for this, e.g: we could add something like this in the test_patches.py

def test_send_funds_to_distr_mod_eth_tx(evmos):
    """
    This tests the transfer of funds to the distribution module account,
    via ethereum tx
    which should be forbidden, since this is a blocked address.
    """
    cli = evmos.cosmos_cli()
    w3 = evmos.w3

    sender = ADDRS["signer1"]
    mod_accs = cli.query_module_accounts()
    old_src_balance = cli.balance(eth_to_bech32(sender))

    for acc in mod_accs:
        if acc["name"] != "distribution":
            continue
        receiver = decode_bech32(acc["base_account"]["address"])

    assert receiver is not None

    txhash = w3.eth.send_transaction(
        {
            "from": sender,
            "to": receiver,
            "value": 1000,
        }
    )
    receipt = w3.eth.wait_for_transaction_receipt(txhash)
    assert receipt.status == 0  # failed status expected

    wait_for_new_blocks(cli, 2)
    # only fees should be deducted from sender balance
    fees = receipt["gasUsed"] * receipt["effectiveGasPrice"]
    assert fees > 0

    new_src_balance = cli.balance(eth_to_bech32(sender))
    assert old_src_balance - fees == new_src_balance

Sorry, I'm not very proficient in Python (still learning). Please help me complete the relevant tests.
In addition, I merged the contents of the main branch of evmos into my main branch, which seems to have a lot of changes. Do I need to reopen a branch and submit a new Pull Request?

[GAtom22]

Sorry, I'm not very proficient in Python (still learning). Please help me complete the relevant tests. In addition, I merged the contents of the main branch of evmos into my main branch, which seems to have a lot of changes. Do I need to reopen a branch and submit a new Pull Request?

No worries about the test, I can add it later

[facs95]

Please fix lint. Besides that things look good to me. Thanks for the contribution!

We will not merge this PR yet because we have a release coming soon so we want to keep main clean but we will merge it afterwards once we have the according tests.

[luchenqun]

Please fix lint. Besides that things look good to me. Thanks for the contribution!

We will not merge this PR yet because we have a release coming soon so we want to keep main clean but we will merge it afterwards once we have the according tests.

I won’t take the blame for this. It was @GAtom22 who helped me update the comments and forgot to execute make format, and I have already updated it. Hahaha.