imp(fees): only allow user to pass in aevmos native token as transaction fees

[luchenqun]

Description

Because evmos = cosmos + evm. Cosmos can include multiple native tokens as transaction fees, but for evmos, which needs to be compatible with evm, the transaction fees can only be in the form of a single native token, which is aevmos. However, currently there are no restrictions on transaction fees in the evmos code.

Note that for testing convenience, the baseFee and minGasPrice in the feemarket module have been set to 0.

Using the command evmosd tx bank send node0 evmos1qqqqhe5pnaq5qq39wqkn957aydnrm45sdn8583 1024aevmos --fees=1000000000aevmos,666stake,888unknowntoken --home ./nodes/node0/evmosd --broadcast-mode sync -y to send a transaction will be successful. The transaction receipt information is as follows:

{
    "body":{
        "messages":[
            {
                "@type":"/cosmos.bank.v1beta1.MsgSend",
                "from_address":"evmos1hajh6rhhkjqkwet6wqld3lgx8ur4y3khjuxkxh",
                "to_address":"evmos1qqqqhe5pnaq5qq39wqkn957aydnrm45sdn8583",
                "amount":[
                    {
                        "denom":"aevmos",
                        "amount":"1024"
                    }
                ]
            }
        ]
    },
    "auth_info":{
        "fee":{
            "amount":[
                {
                    "denom":"aevmos",
                    "amount":"1000000000"
                },
                {
                    "denom":"stake",
                    "amount":"666"
                },
                {
                    "denom":"unknowntoken",
                    "amount":"888"
                }
            ],
            "gas_limit":"200000",
            "payer":"",
            "granter":""
        },
        "tip":null
    },
    "signatures":[
        "xBBWbe5440tRwoRtJubOR4JrgRse7FzG2B05vg0oH+8KD4Y45w7TiYRTrCFD+CCuxJgIfI7nugdHAQbSxO6ZyAA="
    ]
}

However, there are two unreasonable issues:

Issue 1: The sender doesn't actually have stake and unknowntoken, but the transaction still succeeds.

Issue 2: We can allocate stake and unknowntoken native tokens to the address corresponding to node0 in the genesis block's bank module(the mainnet can replace the native token with an IBC type token), and the transaction will still succeed. However, the system does not deduct the 666stake and 888unknowntoken transaction fees provided by the sender, only deducting the 1000000000aevmos in the fees.

Since evmos can only use aevmos native token as fees, we should add restrictions in checkTx. However, considering the special scenario where baseFee and minGasPrice in the feemarket module are set to 0, it is reasonable to have fees as nil, fees array as empty, and fees array as 1 with denom being aevmos, while others are considered illegal fees.

Of course, after fixing this issue, many test cases run command make test would fail because cosmos-sdk defaults to using stake native token as fees. So I am raising this issue first to see your thoughts on this problem.

[fedekunze]

Thanks @luchenqun left a comment

[0xstepit]

Thanks for the contribution @luchenqun !

[CLAassistant]

All committers have signed the CLA.

[luchenqun]

@fedekunze @0xstepit
Please be careful when merging this PR, if merge will cause some test cases to fail when executing make test, because cosmos-sdk defaults to using stake native token as fees

[fedekunze]

if merge will cause some test cases to fail when executing make test, because cosmos-sdk defaults to using stake native token as fees

Tests will need to be updated in that case so that they pass

[luchenqun]

if merge will cause some test cases to fail when executing make test, because cosmos-sdk defaults to using stake native token as fees

Tests will need to be updated in that case so that they pass

Due to the peculiarities of evmos fees, it seems difficult to fix these tests.
Take the test "pass - MaxInt256 allocation" in the file precompiles/ics20/approve_test.go as an example。
When executing s.coordinator.Setup(path) https://github.com/evmos/evmos/blob/main/precompiles/ics20/approve_test.go#L159 , the ibc-go project will be called https://github.com/cosmos/ibc-go/blob/v7.3.1/testing/simapp/test_helpers.go#L219 ,By default, denom stake is used as the gas fee. It is inconvenient for us to modify it.

[codecov]

Codecov Report

Merging #1998 (1d8b114) into main (6e2812d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1998   +/-   ##
=======================================
  Coverage   70.44%   70.45%           
=======================================
  Files         340      340           
  Lines       25492    25498    +6     
=======================================
+ Hits        17959    17965    +6     
  Misses       6609     6609           
  Partials      924      924           

Files	Coverage Δ
app/ante/cosmos/min_price.go	90.74% <100.00%> (+1.15%)	⬆️

[luchenqun]

if merge will cause some test cases to fail when executing make test, because cosmos-sdk defaults to using stake native token as fees

Tests will need to be updated in that case so that they pass

Due to the peculiarities of evmos fees, it seems difficult to fix these tests. Take the test "pass - MaxInt256 allocation" in the file precompiles/ics20/approve_test.go as an example。 When executing s.coordinator.Setup(path) https://github.com/evmos/evmos/blob/main/precompiles/ics20/approve_test.go#L159 , the ibc-go project will be called https://github.com/cosmos/ibc-go/blob/v7.3.1/testing/simapp/test_helpers.go#L219 ,By default, denom stake is used as the gas fee. It is inconvenient for us to modify it.

I made a compromise modification.

I have imposed the following limitations on transaction fees:

• The length of the transaction fee must be 0 or 1.
• When the length of the transaction fee is 1, the denom is allowed to be aevmos or stake.

Now all unit tests can pass. The only unresolved case is that when the chain allows users to submit transactions with a gasPrice of 0, users can still use stake native tokens as transaction fees.

However, although not perfect, the current issues mentioned above on the evmos mainnet have been resolved. Because the gasPrice on the evmos mainnet is not 0, if stake native tokens are used as transaction fees, they will definitely not pass the sufficien fee checks.