-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: Update assets controllers v14 #9182
Conversation
…o delete networkId property from the network controller state and added a network id property to our own internal property, created the reducer and the action for that property and added the dispatch to the engine event subscriber, update the patch of network controller, updated test cases and initial states with new data structure with the networkId property
…only look for network id when the network did change
…ion we do not need to ensure was being called with the right argument, we have separate unit tests for that function
…etworkProvider that will contain our internal property regarding a network provider, updated on the migrations files, backgroundBridge and related components, the test files were updated accordingly as well. On Engine file was added a subscribe of the networkWillChange event to make sure we have the networkId property reseted if the intention of switching starts
… want to investigate any issue, this is the better approach to not polute Sentry and save it for the investigations
… to fetch network id
…ageProvider with the property networkId, updated files with the new property inpageProvider, deleting old actions and reducers files about networkProvider, addressed review on unit tests, changed initial root state and fixture builder initial state with the inpageProvider, and updated respective migrations 033 and 037 with reviews and new property, updated also engine file imports with the action of the slice of inpageprovider
… fresh install the networkId would not be updated because the event network did change did not trigger
Bitrise✅✅✅ Commit hash: 8f5dfa8 Note
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes in app/components/UI/Ramp/hooks/useHandleSuccessfulOrder.ts
LGTM
Bitrise❌❌❌ Commit hash: 8b20aee Note
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Tested detecting and importing tokens. switching accounts / chains. And my recent patch for linea pricing.
As the PR description mentions, NFT detection requires #9149
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Bitrise❌❌❌ Commit hash: f9259b3 Note
|
Bitrise❌❌❌ Commit hash: 53e5ccf Note
|
Quality Gate passedIssues Measures |
Description
This PR addresses the update to the Asset Controller v14
Created Core branch to address patch changes: patch/mobile-assets-controllers-v-14-0-0
Fixed issue of token being automatically imported when requested by a dapp.
Changed breaking changes on addToken and watchAsset function signature.
Important to note that this version of assets controller is pointing to version 5 of controller utils, therefore the auto detect tokens will only be available when this PR is merged
Related issues
Fixes: https://github.com/MetaMask/mobile-planning/issues/1620
Manual testing steps
Screenshots/Recordings
Auto detect tokens, import token via dapp:
Screen.Recording.2024-04-11.at.10.13.51.mov
Add custom token/ hide token:
Screen.Recording.2024-04-11.at.10.14.44.mov
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist