-
-
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: chain id to hexadecimal format #7999
Conversation
…r gas fee controller, replace breaking changes on network controller and a migration for it, NetworkChainId to chainId on controller-utils
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. |
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
…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
NEW PR smoke E2E pipeline: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f6fd0da9-20de-4fb0-9f89-de9d7717a9a4 |
@sleepytanya I can't reproduce this behaviour, could you share more details on how you reproduced this scenario? |
Hey @sleepytanya I was able to reproduce this issue and I was able to reproduce it on main as well: |
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, could you fix the lint issue?
QA build, Samsung s24 + (physical device) :
|
|
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
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8516b725-81b4-4ea9-b708-ee37000bed89 |
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.
Excellent PR description and videos! LGTM 🚀
- Checked local build and TabBar works as expected
- Searched for removed locales key
As an aside the TabBar could probably be moved out of the component-library folder it's not really a reusable component cc @brianacnguyen
Description
On this PR we updated the missing dependencies to the metamask controllers v53
(Small minor version of cockatiel update just because it was a missing dependency on solving some conflicts running
yarn add cockatiel
it added with a minor version update, the changelog seems pretty trivial but we can revert it, let me know your thoughts)It was needed to change the chain id across the project to use hexadecimal format, and on
Ramp
files was needed to have attention and change the selector to change it back to decimal and see all of the interaction across.Created a new patch for
SwapsController
to change the chainId to hexadecimal format. This PR addresses that changeCreated a a core branch to easily reading the assets controller current patch - v53-mobile-assets-controllers-patch
Implemented migrations for all the state objects that had chain id values and for other breaking changes like
networkDetails
object andrpcUrl
instead ofrpcTarget
Related issues
Fixes: #
Manual testing steps
I will mention flows instead of enumerating single steps and upload their recordings, just because it's a wide change around the app
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist