search

cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.31k stars 3.64k forks source link

Automatic gas calculation works incorrect #4864

Closed tochka closed 5 years ago

tochka commented 5 years ago

Summary of Bug

The function of Automatic gas calculation works incorrectly. When a client sends --gas auto from the CLI the client gets the incorrect amount which is rejected after sending the real Tx. It's related to the reward of a node (fee). The gas is used to move from a signer account to the fee pool (get and write account data to a store). When the CLI sends a simulation Tx the fee amount is empty so the program skips fee movement (~13000 of gas) for more details see https://github.com/cosmos/cosmos-sdk/blob/v0.36.0-rc3/x/auth/ante.go#L120

Version

github.com/cosmos/cosmos-sdk v0.36.0-rc3

Steps to Reproduce

Build cli and create tx with --gas auto.

For Admin Use

alexanderbez commented 5 years ago

Correct and this is documented (albeit poorly) (e.g. https://cosmos.network/docs/cosmos-hub/gaiacli.html#gaia-cli). Gas estimation is not accurate and hence we have the gas-adjustment flag. This is because during simulation:

  1. We don't factor in the reads/writes after message execution.
  2. State mutations could occur between simulation and actual tx execution.

What are you looking to propose here?

tochka commented 5 years ago

I assume to add:

alexanderbez commented 5 years ago

@tochka not sure I follow. The ante handler has nothing to do (directly) with variations in gas costs between simulation and actual execution. Also, I don't see how you would come up with min_gas_price -- this value is not part of consensus.

tochka commented 5 years ago

@alexanderbez you can add minimal fees when you send Tx to simulation it'll help enter inside this condition. GasMeter used for read and write data to the storage (there is the table of operation gas price.

alexanderbez commented 5 years ago

@tochka you can already provide fees when simulating.

tochka commented 5 years ago

@alexanderbez you cannot do that when you set --gas auto so txBld.gas is 0 so fees is 0.

It happens when you execute the following command

cli tx send {from_address} {to_address} --gas auto --gas-prices 1atom
alexanderbez commented 5 years ago

@tochka yes, you can.

$ gaiacli tx send cosmos1dfp05pasnts7a4lupn889vptjtrxzkk5f7027f cosmos1dfp05pasnts7a4lupn889vptjtrxzkk5f7027f 10stake --chain-id=chain-ZTQObJ -b block -y --home ./build/node0/gaiacli --gas=auto --dry-run
BUILDING TX FOR SIMULATION
FEES: { 0}
gas estimate: 26812
$ gaiacli tx send cosmos1dfp05pasnts7a4lupn889vptjtrxzkk5f7027f cosmos1dfp05pasnts7a4lupn889vptjtrxzkk5f7027f 10stake --chain-id=chain-ZTQObJ -b block -y --home ./build/node0/gaiacli --gas=auto --dry-run --fees=2stake
BUILDING TX FOR SIMULATION
FEES: {2stake 0}
gas estimate: 43513

Notice how I pass --gas=auto and --fees.

tochka commented 5 years ago

@alexanderbez so when I want to send Tx I need do two operations:

Do you think it's an easy way for customers? Please look at your estimated gas with and without fees they differ by two orders of magnitude.

alexanderbez commented 5 years ago

@tochka

so when I want to send Tx I need do two operations: ...

No, you misunderstand. Just a single command/request. My post demonstrates that you can supply fees when simulating gas. The --dry-run flag in my post just prohibits the tx from being broadcast. It is just showing the gas difference between using fees and no fees which you stated was not possible, but it is.

Please look at your estimated gas with and without fees they differ by two orders of magnitude.

Yes, exactly the point. Showing that gas simulation does take fees into account.

I recommend reading up on this issue: https://github.com/cosmos/cosmos-sdk/issues/4938 for further details on what we plan on improving.