Fix gas estimate for tokens #7753
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Gudahtt
reviewed
Jan 7, 2020
ui/app/pages/send/send-content/send-amount-row/send-amount-row.component.js
Outdated
Show resolved
Hide resolved
ui/app/pages/send/send-content/send-amount-row/send-amount-row.component.js
Outdated
Show resolved
Hide resolved
ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js
Outdated
Show resolved
Hide resolved
danjm
force-pushed
the
fix-gas-limit-estimation
branch
2 times, most recently
from
eb99c5c to
7be74b1
Compare
…n blur after change
Gudahtt
force-pushed
the
fix-gas-limit-estimation
branch
3 times, most recently
from
643239f to
f6f3d65
Compare
Gudahtt
force-pushed
the
fix-gas-limit-estimation
branch
from
f6f3d65 to
0237426
Compare
Builds ready [0237426]
|
Gudahtt
approved these changes
Jan 9, 2020
Gudahtt
added a commit
that referenced
this pull request
Jan 10, 2020
* Make gas estimate update on debounced token amount change, not just on blur after change * Updated tests * Ensure `updateGas` is bound early Co-authored-by: Mark Stacey <markjstacey@gmail.com>
yqrashawn
pushed a commit
to yqrashawn/conflux-portal
that referenced
this pull request
Jan 14, 2020
* Make gas estimate update on debounced token amount change, not just on blur after change * Updated tests * Ensure `updateGas` is bound early Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Gudahtt
added a commit
that referenced
this pull request
Jan 15, 2020
As of #7753, the `onBlur` prop is no longer used for the `token-input` and `currency-input` components, and the associated wrapper components and the shared underlying component. It as been removed from all of them.
yqrashawn
pushed a commit
to Conflux-Chain/conflux-portal
that referenced
this pull request
Jan 15, 2020
* Make gas estimate update on debounced token amount change, not just on blur after change * Updated tests * Ensure `updateGas` is bound early Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Gudahtt
added a commit
that referenced
this pull request
Jan 15, 2020
As of #7753, the `onBlur` prop is no longer used for the `token-input` and `currency-input` components, and the associated wrapper components and the shared underlying component. It as been removed from all of them.
|
I think this is still a problem with v7.7.2. I don't want to link my name here with my address/txs, but I just had some transactions that tried to transfer some tokens revert. I manually increased the gas limit and they succeeded. |
|
I've had this same issue just occur. 2 DAI transfers failing with gas limits of 43k and 50k which finally went through once the gas limit was raised to 70k (although ironically only 37k of gas was used) |
|
Thanks for the feedback @wysenynja and @TomAFrench ; would either of you be willing to create a separate issue to track the problem you've encountered? Any more details about browser version, MetaMask version, and circumstances of the event would be greatly appreciated as well. I suspect it's a distinct problem, unrelated to the changes in this PR, that just happens to have a similar symptom. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #7709
This PR fixes an outstanding issue with the send token screen. When advanced gas inputs were showing and token amounts where change, the newly estimated gas value would not be the submitted gas value if there was no blur event before clicking "Submit".
This PR ensures that the gas value is updated on a debounced change of the amount field so that the correct estimate is submitted.