-
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
Jl/caip25 permission migration/update mutators #28709
Jl/caip25 permission migration/update mutators #28709
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. |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@metamask/[email protected] |
👍 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. |
…oved (#4978) ## Explanation Updates the CAIP-25 mutators so that if the only eip155 account or scope is removed then the entire permission is revoked. ## References Related: MetaMask/metamask-extension#28709 ## Changelog <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> ### `@metamask/multichain` - **CHANGED**: `Caip25CaveatMutators.authorizedScopes.removeAccount` now revokes the CAIP-25 endowment if the only account is removed. - **CHANGED**: `Caip25CaveatMutators.authorizedScopes.removeScope` now revokes the CAIP-25 endowment if the only non-wallet scope is removed. - **CHANGED**: `setEthAccounts` no longer adds `wallet:eip155` to the optionalScopes if it does not already exist. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --------- Co-authored-by: Alex Donesky <[email protected]>
…migration/update-mutators
@SocketSecurity ignore npm/@metamask/[email protected] metamaskbot is us |
just tested manually. seems to work as expected |
Builds ready [2d78f33]
Page Load Metrics (1685 ± 67 ms)
Bundle size diffs [🚀 Bundle size reduced!]
|
@@ -297,7 +297,7 @@ export const NetworkListMenu = ({ onClose }: { onClose: () => void }) => { | |||
} | |||
|
|||
if (permittedAccountAddresses.length > 0) { | |||
addPermittedChain(selectedTabOrigin, network.chainId); | |||
dispatch(addPermittedChain(selectedTabOrigin, network.chainId)); |
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.
Fixing this upstream in develop here #28741
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.
nevermind, this is the result of a bad merge against develop. Develop actually properly dispatches this
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.
resolved in the CAIP-25 permission branch upstream of this
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 other than figuring out what's going on with the permission auto-granting piece
…migration/update-mutators
Description
Replaces existing caveat mutators. Handles ensuring
wallet:eip155
is only upserted for permissions granted to snaps.Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist