-
-
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 v^9.0.0 #8378
Conversation
…r gas fee controller, replace breaking changes on network controller and a migration for it, NetworkChainId to chainId on controller-utils
…tches for controller utils and assets-controllers, changed on new missing plances rpcTarget for rpcUrl, upgraded controller utils to v4
…onverting the chainId on that state for hexadecimal
… toggles on security and privacy settings
Fix chain ID middleware.
…st on networks constants for hexadecimal format
…Switch and some on Engine file
…h_chainId, also removed a type on test file of rpcMethodMiddleware
…ts-controllers-v9
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.
LGTM Tomas!
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/bf360295-9ce3-4815-845b-a72bf268a1ac |
|
Approving as code owner to unblock merge but have not tested this in any way. Please get another review from an engineer with context of these changes |
Description
The issue still needs validation.
On this PR was rebased all the packages on assets controllers present on this core branch:
v53-mobile-assets-controllers-patch
with the exception of:Still have a warning when it renders the app from a cold start
undefined is not an object (evaluating 'inputBn.toString')
This error is on therefresh
function on the AccountTrackerController and still under investigation. Good to mention that the behavior of the app still seems the same, since it recovers from the warning.Core branch with specific updates: v55-mobile-assets-controllers-patch
Patches needed
(Keep in mind this commit have more changes Because I'm updating all the changes of the assets-controller since version 8 in this branch, for being easier to update the patches on mobile)
I believe this is fixed on future versions where the networkId is implemented together with a mutex logic
Still need recordings of everything around assets: Tokens, NFTs, privacy settings working
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist