Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix & Update EstimateGas API #462

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Fix & Update EstimateGas API #462

wants to merge 18 commits into from

Conversation

c98tristan
Copy link
Contributor

@c98tristan c98tristan commented Oct 10, 2024 •
edited
Loading

  1. Fix too many arguments, want at most 1 when call eth_estimateGas
  2. Fix default gasPrice depend on balanceTokenFee
  3. Add executionResult to emphasizes the difference between Consensus error and EVM internal error.
  4. Fix race condition by copy state from stateDB each execution.
  5. Optimize time execution estimate gas.
  6. Get balanceTokenFee from stateDB instead of default value
  7. Remove gasLimit and gasPrice when using contract call
  8. Remove logic set gasPrice when balanceTokenFee exist.
  9. Unit testing

Reference:

@c98tristan
fix(crypto/bn256/cloudflare): pull in upstream fix for Go 1.21 R18
603983b
@c98tristan
refactor: PR #18445 - CallArgs and estimateGas API
ff40ce2
@c98tristan
refactor: PR #20261 #20702 - Use from address only once, Set sender t…
7b74145 
…o 0x00
@c98tristan
refactor: comment balanceTokenFee related code & add unit test
1313614
@c98tristan
fix: PR #20783 unusual balance return, set gasPrice to zero
36ff14e
@c98tristan
refactor: PR #20830, #21083 add executionResult, return revert reason
6c30566
@c98tristan
feat: PR #21043 recap highest gas limit with account's balance
876f2d5
@c98tristan
feat: PR #21346 add more condition check prevent unnecessary failures…
09f740f 
… during gas estimation
@c98tristan
refactor: remove gas price and gas limit in validator call message
419e7fc
@c98tristan
feat: PR #21545, #24363: EstimateGas function add block number parame…
7f89afb 
…ter and use LatestBlockNumber by default
@c98tristan
refactor:
33c58ad 
PR #27505: Each binary search call retrieve new copy of the state
Remove old EstimateGas function
Fix unit test
@c98tristan
refactor:
e3d8644 
PR #27710: Optimize and clean EstimateGas function
@c98tristan
feat: add balanceTokenFee logic, remove comment code
7765eed
@c98tristan
fix: refundGas if balanceTokenFee == nil
1bc0cc5
beage666
beage666 reviewed Oct 17, 2024
View reviewed changes
Copy link

@beage666 beage666 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@c98tristan c98tristan marked this pull request as ready for review October 23, 2024 10:38
@imterryyy
Copy link
Collaborator

Does this change require protocol update ?

@beage666
Copy link

Yes

@imterryyy
Copy link
Collaborator

Fix: #483

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beage666 beage666 beage666 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@c98tristan @imterryyy @beage666