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

Don't hardcode 21000 for gas limit for ETH transfers #19835

Closed
bbondy opened this issue Nov 30, 2021 · 3 comments · Fixed by brave/brave-core#11414
Closed

Don't hardcode 21000 for gas limit for ETH transfers #19835

bbondy opened this issue Nov 30, 2021 · 3 comments · Fixed by brave/brave-core#11414
Assignees
@onyb
Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Android Fixes related to Android browser functionality OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include
Projects
Wallet
Milestone
1.32.x - Release #4

Comments

@bbondy
Copy link
Member

bbondy commented Nov 30, 2021 •
edited
Loading

Some contracts require more gas so we should estimate gas in all cases and use the default values only as a fallback.
See for example this case: https://etherscan.io/tx/0x79144e4af4324d992438bcdf867d61e299147b1fa6d93c59b56c4e1253165d53

The correct gas that we should’ve suggested is 34914.
@bbondy bbondy added OS/Android Fixes related to Android browser functionality OS/Desktop labels Nov 30, 2021
@bbondy bbondy added this to Untriaged in Wallet via automation Nov 30, 2021
@bbondy bbondy added the priority/P2 A bad problem. We might uplift this to the next planned release. label Nov 30, 2021
@onyb onyb self-assigned this Nov 30, 2021
@onyb onyb moved this from Untriaged to Backlog in Wallet Dec 1, 2021
@onyb onyb added feature/web3/wallet Integrating Ethereum+ wallet support QA/Yes release-notes/include labels Dec 1, 2021
@onyb onyb moved this from Backlog to In progress in Wallet Dec 2, 2021
@onyb onyb changed the title Don't hardcode 21000 for gas limit for sending erc20-tokens Don't hardcode 21000 for gas limit for ETH transfers Dec 3, 2021
@onyb onyb mentioned this issue Dec 3, 2021
Merged
24 tasks
@onyb onyb moved this from In progress to In Review in Wallet Dec 3, 2021
Wallet automation moved this from In Review to Closed Dec 3, 2021
This was referenced Dec 3, 2021
Merged
Merged
@onyb onyb mentioned this issue Dec 3, 2021
Merged
7 tasks
@onyb onyb added this to the 1.35.x - Nightly milestone Dec 3, 2021
@srirambv
Copy link
Contributor

srirambv commented Dec 6, 2021

Brave 1.32.114 Chromium: 96.0.4664.55 (Official Build) (64-bit)
Revision 38cededc5d09b785d12203f1d3209aa6eb293e79-refs/branch-heads/4664@{#1090}
OS ☑️ Linux ☑️ Windows 11 Version Dev
(Build 22504.1010) 		☑️ macOS Version 12.0.1
(Build 21A559)
  • Verified steps from brave/brave-core#11414
  • Verified gas fee isn't hardcoded to 21000 for various kind of transactions
Linux.mp4
Windows.mov
macOS.mov

@kjozwiak kjozwiak mentioned this issue Dec 7, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@onyb onyb

Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Android Fixes related to Android browser functionality OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include
Projects
No open projects
Wallet
Closed
Milestone
1.32.x - Release #4
Development

Successfully merging a pull request may close this issue.

Fix gas estimation for ETHSend transactions brave/brave-core
5 participants
@bbondy @kjozwiak @onyb @srirambv and others