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.

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: Override lower bound gas when gasLimit is defined #5396

Merged
merged 5 commits into from
Mar 12, 2025

Conversation

pedronfigueiredo
Copy link
Contributor

@pedronfigueiredo pedronfigueiredo commented Feb 26, 2025

Explanation

On the extension client, a strange behaviour occurrs when a dApp sends a request with a gasLimit lower than the minimum 21000, but the transaction still is confirmed with a gas of 21000.

This happens because updateGasProperties is called inside addTransaction. updateGasProperties calls updateGas that calls getGas in which if the gas property in the txParams is falsy, it defaults to FIXED_GAS which is 0x5208 (21000). To fix this issue, we can derive gas from gasLimit during normalization of transaction parameters when the first is falsy and the second is defined.

Note that transactions correctly failed when the user tweaked the gas prices using the appropriate modals. This is because gas is derived from gasLimit before new gas settings are submitted to the transaction controller in updateTransactionGasFees.

References

Changelog

@metamask/transaction-controller

  • FIXED: Derive gas from gasLimit when the first is undefined and the second is defined.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@pedronfigueiredo pedronfigueiredo self-assigned this Feb 26, 2025
@pedronfigueiredo pedronfigueiredo requested a review from a team as a code owner February 26, 2025 13:13
@pedronfigueiredo pedronfigueiredo force-pushed the pnf/fix-minored-gas-when-gas-limit-exists branch from c6e0a83 to 0c655ee Compare February 26, 2025 17:20
@pedronfigueiredo pedronfigueiredo force-pushed the pnf/fix-minored-gas-when-gas-limit-exists branch 3 times, most recently from 30bef28 to 5c40062 Compare February 28, 2025 12:07
@pedronfigueiredo pedronfigueiredo force-pushed the pnf/fix-minored-gas-when-gas-limit-exists branch from 5c40062 to e020ae2 Compare February 28, 2025 12:18
@pedronfigueiredo pedronfigueiredo enabled auto-merge (squash) March 12, 2025 09:33
@pedronfigueiredo pedronfigueiredo merged commit 9a930da into main Mar 12, 2025
136 checks passed
@pedronfigueiredo pedronfigueiredo deleted the pnf/fix-minored-gas-when-gas-limit-exists branch March 12, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants