-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[Bug]: Long delay before balances update after a swap #31029
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
Comments
@micaelae @GustavoRSSilva @ghgoodreau can one of you take this on? |
@dylanbutler1 @chaoticgoodpanda I can investigate this one, but 3 mins sounds like the time that it takes for a transaction to get to finalized state (12 blocks * 12-15 seconds), if we are ready from the blockchain that is how long it will take. |
That can't be right. It's a bug, the balances should update immediately when the swap is complete, and those tokens should be tradeable immediately. Further, it doesn't make sense why the source token balance would update but not the destination token balance. Please investigate, thank you! |
https://github.com/user-attachments/assets/0d1a27d8-2a4c-4bde-aeb3-4aa8a1a881c0 |
Should be fixed with these: New tokens should be displayed with correct balances as soon as the swap is successful |
## **Description** This PR integrates [tokenBalances](MetaMask/core#5726) controller state update optimizations to extension. Also integrates changes to triggered detectTokens as soon as a tx is confirmed MetaMask/core#5859 TokenBalancesController used to make state updates even though none of the token balances has changed. When a user removes an account both TokensController and TokenBalancesController state would still have the tokens and balances related to that account; hence triggering api calls to still fetch the token balances and save to state even though the user has already removes that account. The migration in this PR checks if there are any tokenBalances or tokens in both tokensController state and TokenBalancesController state that belong to accounts that do not exist anymore and cleans them up. No functional changes are expected in this PR [](https://codespaces.new/MetaMask/metamask-extension/pull/32467?quickstart=1) ## **Related issues** Fixes: #31029 ## **Manual testing steps** Test migration: 1. Import new account (on main branch) 2. You should still be able to see balances correctly 3. Import new tokens and you should still be able to see balances correctly 4. Remove an account 5. Build on this branch 6. Download state logs and make sure the removed account no longer appears in tokensController or tokenBalancesController state Test: 1. Import new account 2. import new tokens for the account 3. You should be able to see token balances correctly Test: 1- Start a swap to a token that is not already imported in state ( you can use uniswap) 2- submit transaction 3- Notice that as soon as the tx if confirmed you are able to see your new token in the main token list with balance ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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/main/.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/main/.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. --------- Co-authored-by: MetaMask Bot <[email protected]>
## **Description** This PR integrates [tokenBalances](MetaMask/core#5726) controller state update optimizations to extension. Also integrates changes to triggered detectTokens as soon as a tx is confirmed MetaMask/core#5859 TokenBalancesController used to make state updates even though none of the token balances has changed. When a user removes an account both TokensController and TokenBalancesController state would still have the tokens and balances related to that account; hence triggering api calls to still fetch the token balances and save to state even though the user has already removes that account. The migration in this PR checks if there are any tokenBalances or tokens in both tokensController state and TokenBalancesController state that belong to accounts that do not exist anymore and cleans them up. No functional changes are expected in this PR [](https://codespaces.new/MetaMask/metamask-extension/pull/32467?quickstart=1) ## **Related issues** Fixes: #31029 ## **Manual testing steps** Test migration: 1. Import new account (on main branch) 2. You should still be able to see balances correctly 3. Import new tokens and you should still be able to see balances correctly 4. Remove an account 5. Build on this branch 6. Download state logs and make sure the removed account no longer appears in tokensController or tokenBalancesController state Test: 1. Import new account 2. import new tokens for the account 3. You should be able to see token balances correctly Test: 1- Start a swap to a token that is not already imported in state ( you can use uniswap) 2- submit transaction 3- Notice that as soon as the tx if confirmed you are able to see your new token in the main token list with balance ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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/main/.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/main/.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. --------- Co-authored-by: MetaMask Bot <[email protected]>
Describe the bug
This issue has been bugging me for a long time now. After a swap, the destination token balance update can take as long as 3 minutes to update. Please resolve.
Screen.Recording.2025-03-14.at.1.35.55.PM.mov
Expected behavior
No response
Screenshots/Recordings
No response
Steps to reproduce
I do not believe the problem is with Accounts API, as I saw the balance update practically right away in Portfolio, and this has been happening since before we migrated to Accounts API.
Error messages or log output
Detection stage
In production (default)
Version
12.12.0
Build type
None
Browser
Brave
Operating system
MacOS
Hardware wallet
No response
Additional context
No response
Severity
No response
The text was updated successfully, but these errors were encountered: