Handling the error when eth_estimateGas fails #920
Merged
+190
−22
Conversation
This file contains hidden or 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
Closed
3 tasks
mcmire
reviewed
Sep 29, 2022
…imateGas error for the acual error
tommasini
force-pushed
the
fix/4646-bns-estimate-gas-rpc-error
branch
from
638715e to
9a48838
Compare
mcmire
reviewed
Oct 6, 2022
There was a problem hiding this comment.
Have a few more suggestions (just typos in test names) but this is looking good. Clearly there's more we need to do to improve these tests, and I think it would really help if we could simplify estimateGas because it does SO MUCH, but there's only so much you can do here, so I think what you have makes sense :)
|
@mcmire Thank you a lot for helping this PR have more quality! |
blackdevelopa
approved these changes
Oct 11, 2022
Merged
gantunesr
pushed a commit
that referenced
this pull request
Dec 8, 2022
When we did a transaction with binance smart chain and the dapp doesn't send a gas value, we were not being able to execute the eth_estimateGas, it was falling do a general RPC error. Handle the error like the extension does it. Added a value to the Transaction object to be filled with an error if the eth_estimateGas fails.
MajorLift
pushed a commit
that referenced
this pull request
Oct 11, 2023
When we did a transaction with binance smart chain and the dapp doesn't send a gas value, we were not being able to execute the eth_estimateGas, it was falling do a general RPC error. Handle the error like the extension does it. Added a value to the Transaction object to be filled with an error if the eth_estimateGas fails.
MajorLift
pushed a commit
that referenced
this pull request
Oct 11, 2023
When we did a transaction with binance smart chain and the dapp doesn't send a gas value, we were not being able to execute the eth_estimateGas, it was falling do a general RPC error. Handle the error like the extension does it. Added a value to the Transaction object to be filled with an error if the eth_estimateGas fails.
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.
Handling error when eth_estimateGas fails
Description
When we did a transaction with binance smart chain and the dapp doesn't send a gas value, we were not being able to execute the eth_estimateGas, it was falling do a general RPC error.
Proposed solution
Handle the error like the extension does it. Added a value to the Transaction object to be filled with an error if the eth_estimateGas fails.
FIXED:
JavaScript API does not work from within MetaMask's browser metamask-mobile#4621
Request for transaction that is going to fail, or with insufficient funds, results in silent failure with no error message metamask-mobile#4646
CHANGED:
If the eth_estimateGas fails, the transaction object will have a new variable with an error
Checklist
Issue
Resolves #???