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

[Bug]: Autodetect tokens display Mainnet tokens on another network (MXC zkEVM Mainnet) #7587

Closed
seaona opened this issue Oct 24, 2023 · 3 comments
Assignees
Labels
Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing stale Issues that have not had activity in the last 90 days team-assets type-bug Something isn't working

Comments

@seaona
Copy link
Contributor

seaona commented Oct 24, 2023

Describe the bug

Problem: I have some tokens on Mainnet, which are auto-detected by the wallet. Whenever I switch to MXC zkEVM Mainnet network, those same tokens are still auto-detected (now with 0 balance).

Expected behavior

We should not auto-detect mainnet tokens when I am on another network

Screenshots/Recordings

mobile-autodetect-tokens.mp4

Steps to reproduce

  1. Select Mainnet and have some tokens auto-detected
  2. Add MXC zkEVM Mainnet chain https://chainlist.org/?testnets=true&search=mxc+zk
  3. Switch to this network
  4. See how same tokens in step 1 are auto-detected in this new network

Error messages or log output

No response

Version

7.8.0 production (possibly old issue)

Build type

None

Device

Pixel 6

Operating system

Android

Additional context

No response

Severity

No response

@seaona seaona added type-bug Something isn't working Sev3-low A possible confusion or deception that is only hypothetical & has no known instances in the wild team-assets labels Oct 24, 2023
@vania221
Copy link

Опишіть помилку

Проблема: у мене є кілька токенів у мережі Mainnet, які автоматично визначаються гаманцем. Кожного разу, коли я перемикаюся на мережу MXC zkEVM Mainnet, ті самі токени все одно виявляються автоматично (тепер із балансом 0).

Очікувана поведінка

Ми не повинні автоматично виявляти маркери основної мережі, коли я перебуваю в іншій мережі

Скріншоти/записи

mobile-autodetect-tokens.mp4

Кроки для відтворення

  1. Виберіть Mainnet і автоматично виявляйте деякі маркери
  2. Додайте ланцюг основної мережі MXC zkEVM https://chainlist.org/?testnets=true&search=mxc+zk
  3. Переключитися на цю мережу
  4. Подивіться, як ті самі маркери на кроці 1 автоматично виявляються в цій новій мережі

Повідомлення про помилки або журнал

Немає відповіді

Версія

Версія 7.8.0 (можливо, стара версія)

Тип збірки

Жодного

пристрій

Pixel 6

Операційна система

Android

Додатковий контекст

Немає відповіді

Суворість

Немає відповіді

@gauthierpetetin gauthierpetetin added Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing and removed Sev3-low A possible confusion or deception that is only hypothetical & has no known instances in the wild labels Jan 19, 2024
bergeron added a commit to MetaMask/metamask-extension that referenced this issue Feb 13, 2024
## **Description**

Fixes a race condition during token detection after switching networks.

During the network switch, some controllers state are still on the old
chain id, and others are on the new chain id. This can cause different
issues depending which controllers win the race. The worst case is that
detected tokens are added to the wrong network (see related issues):


![image](https://github.com/MetaMask/metamask-extension/assets/3500406/c4dc4a21-9234-44a5-a915-7d2910a28a60)

In that ^ screenshot there are 2 mainnet tokens that we have a balance
of, but they incorrectly appear under linea.

There's a fix for each of the relevant controllers
(`DetectTokensController`, `TokensController`,
`AssetsContractController`) ensuring we use the chain ID being switched
*to*.

## **Related issues**

[Auto token detection list collision with other networks
#22512](#22512)

[Autodetect tokens display Mainnet tokens on another network
#7587](MetaMask/metamask-mobile#7587)


## **Manual testing steps**

It takes a few minutes of switching networks back and forth to reproduce
the bug. But basically keep doing that and we should not see tokens hop
from 1 network to the other.

## **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 Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained what problem this PR is solving and how it
is solved.
- [ ] I've linked related issues
- [ ] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [ ] 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.
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] 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.
sahar-fehri pushed a commit to MetaMask/metamask-extension that referenced this issue Feb 14, 2024
## **Description**

Fixes a race condition during token detection after switching networks.

During the network switch, some controllers state are still on the old
chain id, and others are on the new chain id. This can cause different
issues depending which controllers win the race. The worst case is that
detected tokens are added to the wrong network (see related issues):


![image](https://github.com/MetaMask/metamask-extension/assets/3500406/c4dc4a21-9234-44a5-a915-7d2910a28a60)

In that ^ screenshot there are 2 mainnet tokens that we have a balance
of, but they incorrectly appear under linea.

There's a fix for each of the relevant controllers
(`DetectTokensController`, `TokensController`,
`AssetsContractController`) ensuring we use the chain ID being switched
*to*.

## **Related issues**

[Auto token detection list collision with other networks
#22512](#22512)

[Autodetect tokens display Mainnet tokens on another network
#7587](MetaMask/metamask-mobile#7587)


## **Manual testing steps**

It takes a few minutes of switching networks back and forth to reproduce
the bug. But basically keep doing that and we should not see tokens hop
from 1 network to the other.

## **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 Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained what problem this PR is solving and how it
is solved.
- [ ] I've linked related issues
- [ ] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [ ] 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.
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] 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.
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Feb 19, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Apr 9, 2024
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 7 days. Thank you for your contributions.

@github-actions github-actions bot added the stale Issues that have not had activity in the last 90 days label Apr 18, 2024
Copy link
Contributor

This issue was closed because there has been no follow activity in 7 days. If you feel this was closed in error please provide evidence on the current production app in a new issue or comment in the existing issue to a maintainer. Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 25, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Apr 25, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing stale Issues that have not had activity in the last 90 days team-assets type-bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

4 participants