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

feat: multichain token detection #28380

Merged
merged 4 commits into from
Nov 21, 2024

Conversation

salimtb
Copy link
Contributor

@salimtb salimtb commented Nov 8, 2024

Description

TokenDetectionController is responsible for detecting and keeping an updated list of all tokens across supported chains. This dataset is stored in the detectedTokens state variable within Metamask’s state. After completing this task, token detection will be enhanced by implementing periodic polling across all networks linked to the wallet, resulting in a more comprehensive dataset available to users.

For each network added to the wallet, the polling loop will receive the network as a parameter and execute token autodetection for it. Once results are available, they will be stored in detectedTokensAllChains, organized by chainId. This approach enables us to retrieve a comprehensive list of detected tokens across all networks.

Open in GitHub Codespaces

Related issues

Fixes: #3431

Manual testing steps

  1. install dependencies using yarn
  2. start the project using PORTFOLIO_VIEW=1 yarn start
  3. add other networks where you have tokens
  4. the autodetection should be multichain

Screenshots/Recordings

Before

After

Screen.Recording.2024-11-08.at.14.27.23.mov

Pre-merge author checklist

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.

Copy link
Contributor

github-actions bot commented Nov 8, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@salimtb salimtb changed the title Salim/multichain token detection final feat: multichain token detection Nov 8, 2024
@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch from 4583bad to 1b8fe67 Compare November 8, 2024 13:34
@salimtb salimtb marked this pull request as ready for review November 8, 2024 13:53
@salimtb salimtb requested review from a team as code owners November 8, 2024 13:53
@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch from 1b8fe67 to fde2ca2 Compare November 8, 2024 14:01
@metamaskbot
Copy link
Collaborator

Builds ready [fde2ca2]
Page Load Metrics (2038 ± 79 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint44023261955384184
domContentLoaded16852313200116981
load17392328203816579
domInteractive24109512110
backgroundConnect797352813
firstReactRender603041275326
getState457242110
initialActions01000
loadScripts12021711146913766
setupStore68412178
uiStartup189629382314248119
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 126 Bytes (0.00%)
  • ui: 2.28 KiB (0.03%)
  • common: 620 Bytes (0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [e899713]
Page Load Metrics (1965 ± 125 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint171128091969255122
domContentLoaded165227371932241116
load166128111965259125
domInteractive21106472211
backgroundConnect7127343617
firstReactRender512631194320
getState45516168
initialActions01000
loadScripts113922081411232111
setupStore558202010
uiStartup184930172208273131
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 126 Bytes (0.00%)
  • ui: 2.28 KiB (0.03%)
  • common: 620 Bytes (0.01%)

@salimtb salimtb closed this Nov 13, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 13, 2024
@salimtb salimtb reopened this Nov 14, 2024
@gambinish
Copy link
Contributor

gambinish commented Nov 19, 2024

I'm noticing an unexpected behavior. When I am on a network, and I try to import my account's tokens, in the detected-token popover, I only see balances and tokenFiatBalance of the detected tokens on the chain that I am on.

Is this expected? I'm not sure if this was a limitation to our previous core dependency. I think we now have the token balances being polled cross-chain, so I wonder if it would be worth it to integrate these cross chain balances on this PR as well.

Screen.Recording.2024-11-18.at.6.37.31.PM.mov

@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch from e899713 to 638dadd Compare November 19, 2024 10:13
@salimtb salimtb changed the base branch from develop to feat/mmassets-432_network-filter-extension--integration-balances November 19, 2024 10:14
@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch from 638dadd to 87722d9 Compare November 20, 2024 14:39
@salimtb salimtb requested a review from a team as a code owner November 20, 2024 14:39
@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch from 87722d9 to 5a4fad5 Compare November 20, 2024 15:32
@salimtb salimtb changed the title feat: multichain token detection feat: multichain token detection ( experimental ) Nov 20, 2024
@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch from 5a4fad5 to dce2001 Compare November 20, 2024 21:51
@salimtb salimtb requested a review from a team as a code owner November 20, 2024 21:51
@gambinish
Copy link
Contributor

I'm noticing two bugs, that we've discussed on Slack. Posting them here for tracking:

  1. On a newly imported wallet, I cannot import from my previously existing networks (this could be an existing behavior). After adding a new network, the tokens on my previously added chains get detected:
Screen.Recording.2024-11-20.at.2.39.11.PM.mov
  1. There are certain scenarios where switching networks and toggling between network filter options pollutes the detected tokens, resulting in improperly imported tokens:
import-bug-1.mov

@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch 2 times, most recently from 9b5a122 to 6012a46 Compare November 21, 2024 12:56
@salimtb salimtb changed the base branch from feat/mmassets-432_network-filter-extension--integration-balances to feat/portfolio-view November 21, 2024 12:57
@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch from 6012a46 to c290045 Compare November 21, 2024 12:59
@metamaskbot
Copy link
Collaborator

Builds ready [c290045]
Page Load Metrics (1905 ± 109 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint45225051819380183
domContentLoaded156124281877225108
load157024501905227109
domInteractive17178553718
backgroundConnect970302010
firstReactRender89160107168
getState55715178
initialActions00000
loadScripts11101827140219091
setupStore611821
uiStartup175627322106247118
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 10.5 KiB (0.14%)
  • common: 2.48 KiB (0.03%)

@salimtb salimtb removed the request for review from a team November 21, 2024 14:32
@salimtb salimtb removed the request for review from a team November 21, 2024 14:32
gambinish
gambinish previously approved these changes Nov 21, 2024
Copy link
Contributor

@gambinish gambinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulled down, tested, LGTM. The bugs I reported earlier now seem to be resolved ✅

Once #28593 gets through the merge queue, I think we can just rebase to develop and get these changes in 🚀

Base automatically changed from feat/portfolio-view to develop November 21, 2024 18:14
@gambinish gambinish dismissed their stale review November 21, 2024 18:14

The base branch was changed.

@salimtb salimtb changed the title feat: multichain token detection ( experimental ) feat: multichain token detection Nov 21, 2024
gambinish
gambinish previously approved these changes Nov 21, 2024
@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch from 2795d33 to d25ab6c Compare November 21, 2024 18:26
@gambinish gambinish self-requested a review November 21, 2024 18:27
@salimtb salimtb force-pushed the salim/multichain-token-detection-final branch from d25ab6c to 29742fd Compare November 21, 2024 18:29
vinnyhoward
vinnyhoward previously approved these changes Nov 21, 2024
NidhiKJha

This comment was marked as resolved.

@sahar-fehri
Copy link
Contributor

Hey @salimtb Im seeing this behavior where after hiding a token and going back to main page i dnt see the same token list?

Screen.Recording.2024-11-21.at.21.14.38.mov

@salimtb
Copy link
Contributor Author

salimtb commented Nov 21, 2024

Hey @salimtb Im seeing this behavior where after hiding a token and going back to main page i dnt see the same token list?

Screen.Recording.2024-11-21.at.21.14.38.mov

i couldn't reproduce on my side , i'll try to do it separately and follow up with a PR if needed

@metamaskbot
Copy link
Collaborator

Builds ready [373a8c5]
Page Load Metrics (1757 ± 52 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15841963175810952
domContentLoaded15741941173310249
load15841956175710752
domInteractive22154413115
backgroundConnect105226157
firstReactRender563731146330
getState47013168
initialActions01000
loadScripts1131147912749244
setupStore55215178
uiStartup17792398197615675
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 2.29 KiB (0.03%)
  • common: 645 Bytes (0.01%)

@salimtb salimtb added this pull request to the merge queue Nov 21, 2024
Merged via the queue into develop with commit 842a63f Nov 21, 2024
75 checks passed
@salimtb salimtb deleted the salim/multichain-token-detection-final branch November 21, 2024 21:08
@metamaskbot metamaskbot added the release-12.9.0 Issue or pull request that will be included in release 12.9.0 label Nov 21, 2024
@salimtb salimtb added the portfolio-view Used for PRs and issues related to Q4 2024 portfolio view label Nov 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
portfolio-view Used for PRs and issues related to Q4 2024 portfolio view release-12.9.0 Issue or pull request that will be included in release 12.9.0 team-assets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants