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.

Sign up for GitHub

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

Jump to bottom

Core: Improve of native balance of the ERC-20 tokens #6101

Closed
tommasini opened this issue Apr 3, 2023 · 4 comments · Fixed by #8556
Closed

Core: Improve of native balance of the ERC-20 tokens #6101

tommasini opened this issue Apr 3, 2023 · 4 comments · Fixed by #8556
Assignees
@salimtb
Labels
release-7.17.0 Issue or pull request that will be included in release 7.17.0 team-assets

Comments

@tommasini
Copy link
Contributor

tommasini commented Apr 3, 2023 •
edited
Loading

Description

This issue aims to optimize the TokenBalancesController on the assets-controller.

When we change of network or account, the native balance of the ERC-20 tokens is not reset. It shows a wrong value for a couple seconds and then updates.

We should reset the state and show a skeleton text instead of a wrong value.

Technical Details

Acceptance Criteria

  • Be able to show consistently the right native balance to the user

Scenario: User detect tokens on mainnet

  • GIVEN a user have 15 tokens to detect
  • WHEN a user detect that tokens
  • THEN switch network to polygon
  • AND detect tokens
  • THEN switch network
  • THEN token native balance should always reset and appear with the right value

Scenario: User manually imports token

  • GIVEN a user go to import token screen
  • WHEN a user import an ERC-20 Token
  • THEN the native balance should not take longer to appear

References

  • References go here.
  • Issue numbers. Links.
  • Slack threads.
  • Etc.
@tommasini tommasini changed the title Native balance of ERC-20 tl,e a Improve of native balance of ERC-20 token Apr 3, 2023
@tommasini tommasini mentioned this issue Apr 3, 2023
Open
@sethkfman sethkfman changed the title Improve of native balance of ERC-20 token Core: Improve of native balance of ERC-20 token Apr 3, 2023
@gauthierpetetin
Copy link
Contributor

Hi @Gudahtt , do you think this falls in the scope of Shared Libraries team?

@Gudahtt
Copy link
Member

Gudahtt commented Apr 13, 2023

Conceivably, but it's not related to anything we're focusing on this quarter

@sethkfman sethkfman changed the title Core: Improve of native balance of ERC-20 token Core: Improve of native balance of the ERC-20 tokens Apr 17, 2023
@salimtb
Copy link
Contributor

salimtb commented Jan 31, 2024 •
edited
Loading

hey @tommasini ,
i wasn't able to reproduce this bug , can you pls provide me more informations/recorded video

Screen.Recording.2024-01-31.at.17.18.30.mov

@salimtb salimtb self-assigned this Jan 31, 2024
@tommasini
Copy link
Contributor Author

hey @salimtb, if you look at the ApeCoin, you can see the the native balance of it when switching account for some initial seconds is not correct: https://recordit.co/6uxFuxhAHn (Iphone 12, testflight v7.16.0)

@salimtb salimtb mentioned this issue Feb 8, 2024
Merged
13 tasks
chrisleewilcox pushed a commit that referenced this issue Feb 16, 2024
@salimtb
fix: improve native balance fetch logic on the UX (#8556)
542af2c 
## **Description**

Problem: When we change network or account, the native balance of the
ERC-20 tokens is not reset. It shows a wrong value for a couple seconds
and then updates.

We should reset the state and show a skeleton text instead of a wrong
value.

[PR](MetaMask/core#3586) already Merged on core 

## **Related issues**

Fixes: #6101 

## **Manual testing steps**

1. Go to any network where you hold native tokens
2. switch to another network when you have 0 balance
3. you can see that the wrong balance was displayed for a short time

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/MetaMask/metamask-mobile/assets/26223211/d96b474f-60fc-4c4a-a43f-fde18c4a4a26



### **After**



https://github.com/MetaMask/metamask-mobile/assets/26223211/462d8c0e-dcf0-4da7-ae2f-7fdba65b9076


## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
@metamaskbot metamaskbot added the release-7.17.0 Issue or pull request that will be included in release 7.17.0 label Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@salimtb salimtb

Labels
release-7.17.0 Issue or pull request that will be included in release 7.17.0 team-assets
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: improve native balance fetch logic on the UX MetaMask/metamask-mobile
6 participants
@Gudahtt @gauthierpetetin @salimtb @hesterbruikman @metamaskbot @tommasini