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

chore(deps): bump assets controller to v34.0.0 #25540

Merged
merged 12 commits into from
Jun 28, 2024

Conversation

montelaidev
Copy link
Contributor

@montelaidev montelaidev commented Jun 27, 2024

Description

This PR bumps @metamask/assets-controllers to ^34.0.0.

Related issues

Fixes https://github.com/MetaMask/accounts-planning/issues/481

Manual testing steps

This PR affects all assets related tokens

  1. Test added and removing of and tokens
  2. Transfer tokens
  3. Turn on off token detection and see the detected tokens
  4. Check if the token rate values on mainnet

Screenshots/Recordings

Before

After

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.

Copy link

socket-security bot commented Jun 27, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/accounts-controller@17.1.1 Transitive: environment, filesystem, network +41 7.8 MB
npm/@metamask/assets-controllers@34.0.0 Transitive: environment +22 4.21 MB metamaskbot

🚮 Removed packages: npm/@metamask/accounts-controller@17.0.0, npm/@metamask/assets-controllers@33.0.0

View full report↗︎

@montelaidev
Copy link
Contributor Author

@metamaskbot update-policies

Comment on lines 225 to 229
const supportsInterfaceStub = jest.fn().mockResolvedValue(false);
jest
.spyOn(metamaskController.tokensController, '_createEthersContract')
.mockResolvedValue({ supportsInterface: supportsInterfaceStub });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer part of tokens controller

Comment on lines 238 to 241
const supportsInterfaceStub = jest.fn().mockResolvedValue(false);
jest
.spyOn(metamaskController.tokensController, '_createEthersContract')
.mockResolvedValue({ supportsInterface: supportsInterfaceStub });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer part of tokens controller

@metamaskbot
Copy link
Collaborator

Policy update failed. You can review the logs or retry the policy update here

@montelaidev
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated

@montelaidev montelaidev changed the title chore(deps): bump assets and transaction controller chore(deps): bump assets controller to v34.0.0 Jun 27, 2024
@montelaidev montelaidev marked this pull request as ready for review June 27, 2024 14:44
@montelaidev montelaidev requested review from a team as code owners June 27, 2024 14:44
@montelaidev montelaidev added the DO-NOT-MERGE Pull requests that should not be merged label Jun 27, 2024
Copy link

codecov bot commented Jun 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.70%. Comparing base (b37e39d) to head (0485474).
Report is 7 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #25540      +/-   ##
===========================================
+ Coverage    69.68%   69.70%   +0.02%     
===========================================
  Files         1358     1358              
  Lines        48079    48064      -15     
  Branches     13262    13260       -2     
===========================================
- Hits         33501    33499       -2     
+ Misses       14578    14565      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

Builds ready [463b447]
Page Load Metrics (139 ± 190 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6212378136
domContentLoaded9201121
load411861139395190
domInteractive9201121
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -1.78 KiB (-0.05%)
  • ui: 0 Bytes (0.00%)
  • common: 1.39 KiB (0.02%)

@metamaskbot
Copy link
Collaborator

Builds ready [53617fe]
Page Load Metrics (156 ± 163 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint653481155727
domContentLoaded96827199
load421620156339163
domInteractive96827199
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -1.97 KiB (-0.06%)
  • ui: 0 Bytes (0.00%)
  • common: 1.39 KiB (0.02%)

@ccharly
Copy link
Contributor

ccharly commented Jun 28, 2024

I did run a very small tests where I add tokens, and it was working fine:

Screen.Recording.2024-06-28.at.12.06.04.mov

@metamaskbot
Copy link
Collaborator

Builds ready [0485474]
Page Load Metrics (275 ± 289 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint733061174823
domContentLoaded1079322110
load482398275601289
domInteractive1079312110
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -1.97 KiB (-0.06%)
  • ui: 0 Bytes (0.00%)
  • common: 1.39 KiB (0.02%)

@plasmacorral
Copy link
Contributor

plasmacorral commented Jun 28, 2024

Tested mac sonoma 14.5, Chrome 126.0.6478.127 and Firefox: 127.0.2
Used HD, Imported Snap, QR, Ledger accounts

  • Migration- imported tokens persisted through update (tested on Chrome only)
  • Import autodetected tokens
    • Hide autodetected imported token
    • Re-import removed token
    • Imported Orangefox assets on all networks from add list
  • Turn off Token Autodetection in settings>Security & Privacy
  • Manual token import
    • By contract address
    • By search
  • Autodetect NFTs
    • Turn on autodetect and:
      • see NFTs
      • imported NFTs persist
      • Keep display NFT media on
      • Turn off Display NFT media, keeping autodetect off
      • Turn on Display NFT media, keeping autodetect off
    • Turn off autodetect
      • Imported NFTs continue to display even after lock/unlock
  • Manual NFT import 721 & 1155
  • Send NFT 721 & 1155
  • Switch selected account
  • Add networks
  • Switch networks
  • Test dapp
    • ERC 20/721/1155
      • Deploy
      • Approve
      • Send
      • Reject
      • Import to wallet
    • Send
      • Legacy
      • 1559
      • 1559 w/o gas

Observed some delay on Polygon asset prices in Firefox, but on retest was not able to reproduce and prices appeared immediately after import.

LGTM!

@plasmacorral plasmacorral added QA Passed and removed DO-NOT-MERGE Pull requests that should not be merged labels Jun 28, 2024
@plasmacorral plasmacorral merged commit f353f0c into develop Jun 28, 2024
78 of 79 checks passed
@plasmacorral plasmacorral deleted the bump-assets-and-transaction-controllers branch June 28, 2024 22:42
@github-actions github-actions bot locked and limited conversation to collaborators Jun 28, 2024
@metamaskbot metamaskbot added the release-12.2.0 Issue or pull request that will be included in release 12.2.0 label Jun 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
QA Passed release-12.2.0 Issue or pull request that will be included in release 12.2.0 team-accounts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants