-
Notifications
You must be signed in to change notification settings - Fork 5k
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
fix: bump assets controller to 30.0.0 #24913
Conversation
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. |
I'm looking through remaining occurrences of |
for nonce-tracker/block-tracker bump
The currently replaced versions are verified compatible. Fixes nonce-tracker issue
6dd2c8d
to
c4ec3b2
Compare
@metamaskbot update-policies |
Policies updated |
…metamask-extension into bump-assets-controller-version
…metamask-extension into bump-assets-controller-version
NFT detection flow works as expected. Screen.Recording.2024-06-04.at.15.08.44.mov |
I confirm that i have seen same behavior happen when trying to send an ERC2O. Screen.Recording.2024-06-04.at.15.16.05.mov |
Builds ready [87dd240]
Page Load Metrics (51 ± 4 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [9fb556f]
Page Load Metrics (141 ± 173 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
"oidc.api.cx.metamask.io" | ||
"oidc.api.cx.metamask.io", | ||
"price.api.cx.metamask.io", | ||
"token.api.cx.metamask.io" |
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.
Given the inclusion of "token.api.cx.metamask.io"
should "token-api.metaswap.codefi.network",
be removed?
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.
I tried to remove it, but there was still a usage somewhere. I will tackle it separately from this upgrade.
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 like it's becuase this comes from @metamask/controller-utils
, and since this PR introduces the new version but is still using the older one(s) simultaneously, it will hit a different endpoint depending on which of the versions is used.
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.
> [!NOTE] > This PR is intended to be cherry-picked into RC 11.17.0 after being merged into develop ## **Description** Upgrade transaction-controller to get this fix: MetaMask/core#4343 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24947?quickstart=1) ## **Related issues** Fixes: A recent update to the transaction-controller has made the TransactionMeta object passed to the afterSign hook frozen. This change prevents adding new properties, leading to the error: “Cannot add property custodyId, object is not extensible.” This bug is breaking all transactions for MMI as the original txMeta cannot store required properties like custodyId. Blocked by #24861 Blocked by #24913 ## **Manual testing steps** I followed these steps to test the fix: 1. yarn && yarn start:mmi 2. create a new wallet 3. visit https://saturn-custody-ui.dev.metamask-institutional.io/ and add two custodial addresses 4. send 0ETH from one custodial address to the other You should see a popup with an Approve button. Before this fix, the transaction would appear in the activity tab with an error. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. --------- Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: Antonio Regadas <[email protected]>
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.
Approving the privacy-snapshot.json update on behalf of privacy reviewers
Description
Upgrades to version 30 of assets controllers, bringing a new
marketData
object to the token rates controller.Related issues
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist