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: token autodetection multi chain #28553

Closed
wants to merge 1 commit into from

Conversation

salimtb
Copy link
Contributor

@salimtb salimtb commented Nov 19, 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

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 force-pushed the salim/token-autodetection-multi-chain branch from 615178a to e1f4656 Compare November 19, 2024 23:05
@salimtb salimtb changed the title Salim/token autodetection multi chain feat: token autodetection multi chain Nov 19, 2024
@salimtb salimtb force-pushed the salim/token-autodetection-multi-chain branch 3 times, most recently from b5a9b82 to 26be2ad Compare November 19, 2024 23:20
@metamaskbot
Copy link
Collaborator

Builds ready [817e528]
Page Load Metrics (1865 ± 63 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16542139187412861
domContentLoaded16432056183611756
load16522148186513263
domInteractive2811641199
backgroundConnect888282311
firstReactRender462961248038
getState461981114220
initialActions01000
loadScripts12181591138310148
setupStore682182613
uiStartup198026942280228110
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.45 KiB (0.02%)
  • common: 354 Bytes (0.00%)

@salimtb salimtb marked this pull request as ready for review November 20, 2024 10:52
@salimtb salimtb requested review from a team as code owners November 20, 2024 10:52
@sahar-fehri
Copy link
Contributor

Left couple questions 🙏 Did QA on this looks good!

@salimtb salimtb force-pushed the salim/token-autodetection-multi-chain branch from 83f706b to 23ec4ee Compare November 20, 2024 11:49
@salimtb
Copy link
Contributor Author

salimtb commented Nov 20, 2024

@metamaskbot update-policies

@sahar-fehri
Copy link
Contributor

One thing i noticed, is that when i am on a network and i import a token on another network; you would see the import notification but you wont see the token you just imported because its on a different network;
Is it worth updating the import msg to indicate the network where the token was imported?

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@metamaskbot metamaskbot requested a review from a team as a code owner November 20, 2024 12:03
@salimtb
Copy link
Contributor Author

salimtb commented Nov 20, 2024

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@salimtb salimtb requested a review from a team as a code owner November 20, 2024 13:46
@metamaskbot
Copy link
Collaborator

Builds ready [5d51658]
Page Load Metrics (2080 ± 101 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint41827832002421202
domContentLoaded175027762055211101
load175927802080211101
domInteractive28251555024
backgroundConnect674302411
firstReactRender563031479043
getState421911144421
initialActions01000
loadScripts12972024154215675
setupStore68312168
uiStartup218430442541253121
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 2.64 KiB (0.03%)
  • common: 511 Bytes (0.01%)

@salimtb salimtb removed request for a team November 20, 2024 17:18
@metamaskbot
Copy link
Collaborator

Builds ready [8194330]
Page Load Metrics (2041 ± 78 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint34624911957402193
domContentLoaded17882479200315574
load17982493204116278
domInteractive30252584823
backgroundConnect10109353216
firstReactRender6415091199
getState4611589189
initialActions00000
loadScripts13011917150913967
setupStore611811
uiStartup20812822235118790
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.75 KiB (0.02%)
  • common: 506 Bytes (0.01%)

gambinish
gambinish previously approved these changes Nov 20, 2024
.storybook/test-data.js Outdated Show resolved Hide resolved
vinnyhoward
vinnyhoward previously approved these changes Nov 20, 2024
bergeron
bergeron previously approved these changes Nov 20, 2024
@bergeron
Copy link
Contributor

Tested with and without PORTFOLIO_VIEW=1, detections and imports worked as expected

@bergeron
Copy link
Contributor

here's a unit test fix: 954ca55

@salimtb
Copy link
Contributor Author

salimtb commented Nov 20, 2024

954ca55

yes just pushed

vinnyhoward
vinnyhoward previously approved these changes Nov 20, 2024
bergeron
bergeron previously approved these changes Nov 20, 2024
@salimtb salimtb dismissed stale reviews from bergeron and vinnyhoward via ba2c169 November 20, 2024 21:33
vinnyhoward
vinnyhoward previously approved these changes Nov 20, 2024
bergeron
bergeron previously approved these changes Nov 20, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [ba2c169]
Page Load Metrics (2085 ± 133 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint31129281995471226
domContentLoaded182328102048256123
load184029392085278133
domInteractive27194513517
backgroundConnect8128332813
firstReactRender542921036531
getState52189943316
initialActions00000
loadScripts131322201537225108
setupStore612821
uiStartup207534762431392188
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.61 KiB (0.02%)
  • common: 513 Bytes (0.01%)

@gambinish gambinish added the DO-NOT-MERGE Pull requests that should not be merged label Nov 20, 2024
@gambinish

This comment was marked as resolved.

@salimtb salimtb dismissed stale reviews from bergeron and vinnyhoward via 180037e November 21, 2024 08:23
@salimtb salimtb removed the DO-NOT-MERGE Pull requests that should not be merged label Nov 21, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [a48805c]
Page Load Metrics (2545 ± 100 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint37728742100847407
domContentLoaded20792911251220598
load214029222545208100
domInteractive4110163199
backgroundConnect8153403215
firstReactRender1013021694421
getState585262311
initialActions01000
loadScripts15252252189517986
setupStore85718147
uiStartup239736342890328158
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 251 Bytes (0.00%)
  • ui: 1.73 KiB (0.02%)
  • common: 513 Bytes (0.01%)

@salimtb salimtb force-pushed the salim/token-autodetection-multi-chain branch from ef4f2aa to 8325697 Compare November 21, 2024 12:53
@salimtb salimtb force-pushed the salim/token-autodetection-multi-chain branch from 8325697 to 0cafd75 Compare November 21, 2024 12:55
@salimtb salimtb requested a review from sahar-fehri November 21, 2024 13:07
@metamaskbot
Copy link
Collaborator

Builds ready [0cafd75]
Page Load Metrics (1873 ± 149 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint35431041721538258
domContentLoaded161530841845299144
load163031641873311149
domInteractive24200704622
backgroundConnect878282010
firstReactRender652041142713
getState46116199
initialActions00000
loadScripts116022511371220106
setupStore612711
uiStartup184534002085319153
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.98 KiB (0.03%)
  • common: 709 Bytes (0.01%)

@salimtb salimtb closed this Nov 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants