-
-
Notifications
You must be signed in to change notification settings - Fork 181
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.
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
fix: change gas limit fallback #4739
base: main
Are you sure you want to change the base?
fix: change gas limit fallback #4739
Conversation
@@ -60,7 +60,7 @@ export async function estimateGas( | |||
const gasLimitBN = hexToBN(gasLimitHex); | |||
|
|||
request.data = data ? add0x(data) : data; | |||
request.gas = BNToHex(fractionBN(gasLimitBN, 19, 20)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we take the time to define both of these as standard constants?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -96,7 +96,7 @@ export function addGasBuffer( | |||
multiplier: number, | |||
) { | |||
const estimatedGasBN = hexToBN(estimatedGas); | |||
const maxGasBN = hexToBN(blockGasLimit).muln(0.9); | |||
const maxGasBN = hexToBN(blockGasLimit).muln(0.3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this also confirmed as a safe value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was not an intended change, removed it.
605eb03
to
1e6be12
Compare
…5-of-the-block-gas-limit-to-improve-how-we-handle-failed-gas-limit-estimations
Explanation
This PR reduce gas limit fallback from 95% to 35% of the block gas limit to improve how we handle failed gas limit estimations.
References
Fixes: MetaMask/metamask-extension#19692
Changelog
@metamask/transaction-controller
Checklist