Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

gas estimation is not accurate #268

Closed
yihuang opened this issue Jul 13, 2021 · 2 comments · Fixed by #272
Closed

gas estimation is not accurate #268

yihuang opened this issue Jul 13, 2021 · 2 comments · Fixed by #272
Assignees

Comments

@yihuang
Copy link
Contributor

yihuang commented Jul 13, 2021

System info: ethermint main branch

Steps to reproduce:

  1. set tx gas to the value returned by estimateGas api
  2. execute tx, run out of gas

Expected behavior: should just work

Actual behavior: run out of gas

Additional info:

Might have to do a binary search like go-ethereum.

Because the gasUsed returned by EthCall is not the peak gas needed during execution, due to different kinds of gas refund.

@leejw51crypto
Copy link
Contributor

this is the value for estimate gas
Ganache: 0xa2ae = 41646
Tharsis: 0x52d4 = 21204

i'm wondering why the difference comes from?

@yihuang
Copy link
Contributor Author

yihuang commented Jul 13, 2021

I guess the tharsis is the fully refunded one, while the ganache is the real peak gas limit needed to run the tx.

yihuang referenced this issue in yihuang/ethermint Jul 13, 2021
Closes #268

- Also refactor ApplyMessage to be more reuseable

move binary search to rpc api side to have a clean context each try

remove EstimateGas grpc api
yihuang referenced this issue in yihuang/ethermint Jul 15, 2021
Closes #268

- Also refactor ApplyMessage to be more reuseable

move binary search to rpc api side to have a clean context each try

remove EstimateGas grpc api
yihuang referenced this issue in crypto-org-chain/ethermint Jul 16, 2021
Closes #268

- Also refactor ApplyMessage to be more reuseable

move binary search to rpc api side to have a clean context each try

remove EstimateGas grpc api

extract BinSearch function and add unit test
fedekunze added a commit that referenced this issue Jul 19, 2021
* do binary search to estimate gas

Closes #268

- Also refactor ApplyMessage to be more reuseable

move binary search to rpc api side to have a clean context each try

remove EstimateGas grpc api

* extract BinSearch function and add unit test

* do estimateGas in grpc query

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants