[Bug]: When the 'Max' amount option is selected, changing the source token doesn't reset the amount #25594
Assignees
Labels
regression-RC-12.0.0
send-swap
Sev2-normal
Normal severity; minor loss of service or inconvenience.
team-wallet-ux
type-bug
Comments
gauthierpetetin
added
team-wallet-ux
Sev2-normal
7 tasks
ejwessel
added a commit
that referenced
this issue
Jul 10, 2024
## **Description** When performing a send or swap+send there are two outstanding issues: 1. swap+send, nativeToken -> ERC20 and "insufficient funds for gas" error message appears. This behavior is incorrect estimation and should only occur when there are insufficient funds for simple send. In other words, the current logic is treating nativeToken -> ERC20 swaps as if they were nativeToken -> nativeToken swaps, which is causing this error. 1. when swapping between src assets the amount mode max is maintained. Amounts should reset between differently chosen srcAssets. Fixes are: * Hiding the max button when the source token is native, but the destination token is an ERC20 * Resetting the srcAmount whenever the srcAsset changes <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 3. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/25574?quickstart=1) ## **Related issues** Fixes: * #25285 * #25594 ## **Manual testing steps** For each every combination of srcToken to destToken either being nativeToken or an ERC20 Verify that the insufficient funds message does not show unless it is nativeToken -> nativeToken and there is insufficient funds for gas when maxing srcToken (current behavior) | srcToken | destToken | | --- | --- | | nativeToken | nativeToken | | nativeToken | ERC20 | | ERC20 | nativeToken | | ERC20 | ERC20 | 1. click on 'Send' 3. perform a 'Send' or 'Swap + Send' based on the above table 4. verify that the error message appears only in the situation where there is insufficient native token to pay for gas for a 'Send' ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="1187" alt="Screenshot 2024-06-20 at 00 01 00" src="https://github.com/MetaMask/metamask-extension/assets/104780023/1d5dd5f7-3ad2-4f8b-a9a5-503927070458"> ### **After** <img width="200" alt="Screenshot 2024-06-28 at 15 15 43" src="https://github.com/MetaMask/metamask-extension/assets/3590816/d5645dba-dc91-4da7-b909-bec2440131ca"> <img width="200" alt="Screenshot 2024-06-28 at 15 15 46" src="https://github.com/MetaMask/metamask-extension/assets/3590816/d66f0906-581b-48cd-b1b2-08f78a961670"> <img width="200" alt="Screenshot 2024-06-28 at 15 15 53" src="https://github.com/MetaMask/metamask-extension/assets/3590816/0bb74da7-41d6-4004-acbe-cb263fbef411"> <img width="200" alt="Screenshot 2024-06-28 at 15 15 56" src="https://github.com/MetaMask/metamask-extension/assets/3590816/e3e20655-bb05-4ad6-b90f-bda96d42c677"> <img width="200" alt="Screenshot 2024-06-28 at 15 16 16" src="https://github.com/MetaMask/metamask-extension/assets/3590816/75e766fe-a1a0-4709-ad6f-1683762c64f2"> <img width="200" alt="Screenshot 2024-06-28 at 15 16 24" src="https://github.com/MetaMask/metamask-extension/assets/3590816/fdfab83f-4d69-45da-8ca7-81ae56c9538e"> <img width="200" alt="Screenshot 2024-06-28 at 15 16 32" src="https://github.com/MetaMask/metamask-extension/assets/3590816/39b4315d-cd86-4f63-9edb-8ed838761e72"> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
|
Present in the latest v12.00 build #25098 (comment) Screen.Recording.2024-07-25.at.23.12.46.mov |
ejwessel
added a commit
that referenced
this issue
Jul 27, 2024
## **Description** When performing a send or swap+send there are two outstanding issues: 1. swap+send, nativeToken -> ERC20 and "insufficient funds for gas" error message appears. This behavior is incorrect estimation and should only occur when there are insufficient funds for simple send. In other words, the current logic is treating nativeToken -> ERC20 swaps as if they were nativeToken -> nativeToken swaps, which is causing this error. 1. when swapping between src assets the amount mode max is maintained. Amounts should reset between differently chosen srcAssets. Fixes are: * Hiding the max button when the source token is native, but the destination token is an ERC20 * Resetting the srcAmount whenever the srcAsset changes <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 3. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/25574?quickstart=1) ## **Related issues** Fixes: * #25285 * #25594 ## **Manual testing steps** For each every combination of srcToken to destToken either being nativeToken or an ERC20 Verify that the insufficient funds message does not show unless it is nativeToken -> nativeToken and there is insufficient funds for gas when maxing srcToken (current behavior) | srcToken | destToken | | --- | --- | | nativeToken | nativeToken | | nativeToken | ERC20 | | ERC20 | nativeToken | | ERC20 | ERC20 | 1. click on 'Send' 3. perform a 'Send' or 'Swap + Send' based on the above table 4. verify that the error message appears only in the situation where there is insufficient native token to pay for gas for a 'Send' ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="1187" alt="Screenshot 2024-06-20 at 00 01 00" src="https://github.com/MetaMask/metamask-extension/assets/104780023/1d5dd5f7-3ad2-4f8b-a9a5-503927070458"> ### **After** <img width="200" alt="Screenshot 2024-06-28 at 15 15 43" src="https://github.com/MetaMask/metamask-extension/assets/3590816/d5645dba-dc91-4da7-b909-bec2440131ca"> <img width="200" alt="Screenshot 2024-06-28 at 15 15 46" src="https://github.com/MetaMask/metamask-extension/assets/3590816/d66f0906-581b-48cd-b1b2-08f78a961670"> <img width="200" alt="Screenshot 2024-06-28 at 15 15 53" src="https://github.com/MetaMask/metamask-extension/assets/3590816/0bb74da7-41d6-4004-acbe-cb263fbef411"> <img width="200" alt="Screenshot 2024-06-28 at 15 15 56" src="https://github.com/MetaMask/metamask-extension/assets/3590816/e3e20655-bb05-4ad6-b90f-bda96d42c677"> <img width="200" alt="Screenshot 2024-06-28 at 15 16 16" src="https://github.com/MetaMask/metamask-extension/assets/3590816/75e766fe-a1a0-4709-ad6f-1683762c64f2"> <img width="200" alt="Screenshot 2024-06-28 at 15 16 24" src="https://github.com/MetaMask/metamask-extension/assets/3590816/fdfab83f-4d69-45da-8ca7-81ae56c9538e"> <img width="200" alt="Screenshot 2024-06-28 at 15 16 32" src="https://github.com/MetaMask/metamask-extension/assets/3590816/39b4315d-cd86-4f63-9edb-8ed838761e72"> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
|
Fixed in prod v12.1.0: Screen.Recording.2024-08-09.at.19.43.05.mov |
](https://codespaces.new/MetaMask/metamask-extension/pull/25574?quickstart=1){kind=link}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
When a specific amount is selected, it resets upon changing the source tokens. However, when the 'Max' option is chosen, the amount remains unchanged even after switching the source tokens.
Expected behavior
Screenshots/Recordings
maxAmount.mov
Steps to reproduce
Error messages or log output
No response
Version
12.0.0
Build type
None
Browser
Chrome
Operating system
MacOS
Hardware wallet
No response
Additional context
No response
Severity
No response
The text was updated successfully, but these errors were encountered: