feat: create merger and factory for the Caip25Permission #5283
+732
−24
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ffmcgee725
commented
Feb 5, 2025
jiexi
reviewed
Feb 5, 2025
jiexi
reviewed
Feb 5, 2025
jiexi
reviewed
Feb 5, 2025
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
jiexi
reviewed
Feb 11, 2025
jiexi
reviewed
Feb 11, 2025
ffmcgee725
commented
Feb 11, 2025
jiexi
reviewed
Feb 12, 2025
jiexi
reviewed
Feb 12, 2025
jiexi
reviewed
Feb 12, 2025
jiexi
reviewed
Feb 12, 2025
jiexi
reviewed
Feb 12, 2025
jiexi
reviewed
Feb 12, 2025
7 tasks
mcmire
reviewed
Feb 21, 2025
There was a problem hiding this comment.
Hi @ffmcgee725! I read some of the documentation for PermissionController as well as your extension PR and that helped me understand better what was going on here. It seems like we are doing the right thing there. I left some comments here. Most are minor. There are a couple on test readability and naming, but I will leave it up to you.
|
|
||
| describe('permission merger', () => { | ||
| describe('optionalScopes', () => { | ||
| it.each<{ |
There was a problem hiding this comment.
This list of tests looks similar to the ones written for mergeInternalScopes. Do we need to fully test the logic behind mergeInternalScopes again? I wonder whether it's worth it instead to focus on testing the logic behind diffScopesForCaip25CaveatValue.
There was a problem hiding this comment.
addressed in 757b88a
kept one complex test case for making sure permission merger functionality behaves as expected, and focused multiple testing of logic on diffScopesForCaip25CaveatValue function
| }); | ||
|
|
||
| describe('requiredScopes', () => { | ||
| it('incremental request an existing scope with new accounts, and 2 whole new scope with accounts - should return merged scope with previously existing chain and accounts, plus new requested chains with new accounts', () => { |
There was a problem hiding this comment.
Why are we testing requiredScopes differently from optionalScopes? Glancing at the code, it seems like we treat them the same. Shouldn't we have the same tests here as for optionalScopes? Or maybe we ought to include both requiredScopes and optionalScopes together in the same tests?
There was a problem hiding this comment.
Was previously requested by another engineer to explicitly test requiredScopes to make sure it behaves the same way. Although I do agree there is no point in separating these since they are treated the same way.
addressed in 757b88a
test case for permission merger interacts with both requiredScopes and optionalScopes.
Comment on lines
414
to
417
| (account) => | ||
| !diff[scopeToDiff][ | ||
| scopeString as InternalScopeString | ||
| ]?.accounts.includes(account), |
There was a problem hiding this comment.
Isn't this the same thing as:
Suggested change
| (account) => | |
| !diff[scopeToDiff][ | |
| scopeString as InternalScopeString | |
| ]?.accounts.includes(account), | |
| (account) => !originalScopeObject.accounts.includes(account), |
There was a problem hiding this comment.
forgot to change this after some refactoring, addressed in 757b88a
ty for noticing!
Comment on lines
399
to
402
| scopeToDiff: keyof Pick< | ||
| Caip25CaveatValue, | ||
| 'optionalScopes' | 'requiredScopes' | ||
| >, |
There was a problem hiding this comment.
Isn't this the same as:
Suggested change
| scopeToDiff: keyof Pick< | |
| Caip25CaveatValue, | |
| 'optionalScopes' | 'requiredScopes' | |
| >, | |
| scopeToDiff: 'optionalScopes' | 'requiredScopes' |
There was a problem hiding this comment.
my initial thought was making explicit for the reader to understand where exactly 'optionalScopes' | 'requiredScopes' come from. But from the context of the function itself, plus the documentation, I believe that can be deduced.
addressed in 757b88a
| @@ -347,3 +384,49 @@ function removeScope( | |||
| operation: CaveatMutatorOperation.RevokePermission, | |||
| }; | |||
| } | |||
|
|
|||
| /** | |||
| * Returns the differential between two provided CAIP-25 permission caveat value scopes. | |||
There was a problem hiding this comment.
Based on this description I would expect the data returned by this function to be smaller. For instance if I passed it optionalScopes as the scopeToDiff and two Caip25CaveatValues where there was one more account in optionalScopes than the other, then I would expect this function to give me back an array with just that account. However, it doesn't do that; it returns a full Caip25CaveatValue.
Maybe a better way to say this is that this function calculates a difference between one Caip25CaveatValue and another, only considering one a scope at a time? This would explain why we can call this function multiple times with different scope arguments, because we're essentially doing a mathematical operation: 10 - 3 - 1 is the same thing as 7 - 1.
Suggested change
| * Returns the differential between two provided CAIP-25 permission caveat value scopes. | |
| * Calculates the difference between two provided CAIP-25 permission caveat values, but only considering a single scope property at a time. |
There was a problem hiding this comment.
I agree. This is the meaning I was trying to convey, just bad choice of the english word for it on my part 😅
addressed in 757b88a
| * @param originalValue - The existing CAIP-25 permission caveat value. | ||
| * @param mergedValue - The result from merging existing and incoming CAIP-25 permission caveat values. | ||
| * @param scopeToDiff - The required or optional scopes from the [CAIP-25](https://github.com/ChainAgnostic/CAIPs/blob/main/CAIPs/caip-25.md) request. | ||
| * @returns The differential between original and merged CAIP-25 permission caveat values. |
There was a problem hiding this comment.
Similar as above, maybe we want "difference" and not "differential":
Suggested change
| * @returns The differential between original and merged CAIP-25 permission caveat values. | |
| * @returns The difference between original and merged CAIP-25 permission caveat values. |
mcmire
reviewed
Feb 24, 2025
| }); | ||
| }); | ||
|
|
||
| describe('diffScopesForCaip25CaveatValue', () => { |
There was a problem hiding this comment.
Hmm, it looks like we are still just testing this function with optionalScopes only, so the fact that it accepts requiredScopes is not being tested. Maybe we can copy these four describes and then change them so that we are setting requiredScopes?
| const mergedValue = mergeInternalScopes(leftValue, rightValue); | ||
|
|
||
| expect(mergedValue).toStrictEqual( | ||
| expect.objectContaining(expectedMergedValue), |
There was a problem hiding this comment.
It looks like we are using expect.objectContaining to check the contents of mergedValue but not the whole thing. Are there are parts of mergedValue that we don't care about in this test? Or can we use toStrictEqual?
There was a problem hiding this comment.
We can actually check the entirety of object, Changed in a4bba23
Gudahtt
reviewed
Feb 25, 2025
jiexi
approved these changes
Feb 25, 2025
github-merge-queue bot
pushed a commit
to MetaMask/metamask-extension
that referenced
this pull request
Mar 3, 2025
Addressing MetaMask/MetaMask-planning#4017 We want to create a merger for the Caip25Permission specification in order to enable wallet_addEthereumChain and wallet_switchEthereumChain to call to the PermissionController rather than the ApprovalController for incremental permission requests (i.e. for the permittedChains flow). Merger created in this [core PR](MetaMask/core#5283) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/30042?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** We can start everything off by making sure we use the dev build of extension via `yarn && yarn webpack` or `yarn && yarn start` ### `wallet_requestPermissions` empty `eth_accounts` request (default) 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': {}, } ] }); ``` 2. Should render the following UI <img width="353" alt="Screenshot 2025-02-26 at 18 15 26" src="https://github.com/user-attachments/assets/e6a01efa-a15a-4790-89e9-2710e387b02b" /> <img width="358" alt="Screenshot 2025-02-26 at 18 15 19" src="https://github.com/user-attachments/assets/03a777a0-da68-4f0f-b2d1-fc8c76ed6f7d" /> 3. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ### `wallet_requestPermissions` `eth_accounts` account permission request 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': { 'caveats': [ { 'type': 'restrictReturnedAccounts', 'value': ["0x0aC9D99d92AB266e33b27469f8ce84FE7C56b6d5"] // replace this account with your local account public address } ] }, } ] }); ``` 2. Should render the following UI <img width="358" alt="Screenshot 2025-02-26 at 18 19 32" src="https://github.com/user-attachments/assets/33384044-2205-4d06-bced-51fd4a3b0301" /> <img width="353" alt="Screenshot 2025-02-26 at 18 19 23" src="https://github.com/user-attachments/assets/27af0e19-cfe9-43f5-a039-06cf1b0cb939" /> 3. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ### `wallet_requestPermissions` `endowment:permitted-chains` chain permission request 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': { 'caveats': [ { 'type': 'restrictReturnedAccounts', 'value': [] } ] }, 'endowment:permitted-chains': { 'caveats': [ { 'type': 'restrictNetworkSwitching', 'value': ["0x1"] // replace with desired chain, here we default to mainnet } ] } } ] }); ``` 2. Should render the following UI <img width="352" alt="Screenshot 2025-02-26 at 18 22 15" src="https://github.com/user-attachments/assets/ad93a621-0282-4d59-b91e-4d6d66b1a64d" /> <img width="354" alt="Screenshot 2025-02-26 at 18 22 09" src="https://github.com/user-attachments/assets/5eb7a5bf-41f0-4c5a-9280-f1a926180e1c" /> 3. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ### `wallet_requestPermissions` `eth_accounts` and `endowment:permitted-chains` account + chain permission request 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': { 'caveats': [ { 'type': 'restrictReturnedAccounts', 'value': ["0x0aC9D99d92AB266e33b27469f8ce84FE7C56b6d5"] // replace this account with your local account public address } ] }, 'endowment:permitted-chains': { 'caveats': [ { 'type': 'restrictNetworkSwitching', 'value': ["0x1"] // replace with desired chain, here we default to mainnet } ] } } ] }); ``` 2. Should render the following UI <img width="356" alt="Screenshot 2025-02-26 at 18 23 34" src="https://github.com/user-attachments/assets/f63530ff-820a-4c43-a0e4-4b9bf7fa955a" /> <img width="350" alt="Screenshot 2025-02-26 at 18 23 29" src="https://github.com/user-attachments/assets/b2db17b6-0395-4c08-ad3f-2176cb4dcea8" /> 3. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ### `wallet_requestPermissions` malformed `eth_accounts` address 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': { 'caveats': [ { 'type': 'restrictReturnedAccounts', 'value': ["malformed.eth"] } ] }, 'endowment:permitted-chains': { 'caveats': [ { 'type': 'restrictNetworkSwitching', 'value': [] } ] } } ] }); ``` 2. Should see an exception being thrown in the console, with the message `Value must be a hexadecimal string, starting with "0x".` <img width="730" alt="Screenshot 2025-02-27 at 10 16 31" src="https://github.com/user-attachments/assets/0a13f3f9-3ec3-4093-a9fc-331faa4e29ee" /> ### `wallet_switchEthereumChain` Permissions for new chain 1. Send the following request through dev tools console via [wallet_switchEthereumChain](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_switchethereumchain/) ```js await window.ethereum.request({ jsonrpc: '2.0', method: 'wallet_switchEthereumChain', params: [{ chainId: '0xe705' }], // replace by a chain that's currently not permitted, I used Linea Sepolia }) ``` 2. Should render the following UI <img width="350" alt="Screenshot 2025-02-26 at 18 26 20" src="https://github.com/user-attachments/assets/c93a9487-ebab-43fd-9713-6ef4adbffc39" /> 4. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ## **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 Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.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/main/.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: ffmcgee <joao.carlos@consensys.net> Co-authored-by: ffmcgee <jc.992@hotmail.com> Co-authored-by: Alex Donesky <adonesky@gmail.com> Co-authored-by: ffmcgee <51971598+ffmcgee725@users.noreply.github.com>
dbrans
pushed a commit
to MetaMask/metamask-extension
that referenced
this pull request
Mar 3, 2025
](https://codespaces.new/MetaMask/metamask-extension/pull/30042?quickstart=1){kind=link}
Addressing MetaMask/MetaMask-planning#4017 We want to create a merger for the Caip25Permission specification in order to enable wallet_addEthereumChain and wallet_switchEthereumChain to call to the PermissionController rather than the ApprovalController for incremental permission requests (i.e. for the permittedChains flow). Merger created in this [core PR](MetaMask/core#5283) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/30042?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** We can start everything off by making sure we use the dev build of extension via `yarn && yarn webpack` or `yarn && yarn start` ### `wallet_requestPermissions` empty `eth_accounts` request (default) 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': {}, } ] }); ``` 2. Should render the following UI <img width="353" alt="Screenshot 2025-02-26 at 18 15 26" src="https://github.com/user-attachments/assets/e6a01efa-a15a-4790-89e9-2710e387b02b" /> <img width="358" alt="Screenshot 2025-02-26 at 18 15 19" src="https://github.com/user-attachments/assets/03a777a0-da68-4f0f-b2d1-fc8c76ed6f7d" /> 3. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ### `wallet_requestPermissions` `eth_accounts` account permission request 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': { 'caveats': [ { 'type': 'restrictReturnedAccounts', 'value': ["0x0aC9D99d92AB266e33b27469f8ce84FE7C56b6d5"] // replace this account with your local account public address } ] }, } ] }); ``` 2. Should render the following UI <img width="358" alt="Screenshot 2025-02-26 at 18 19 32" src="https://github.com/user-attachments/assets/33384044-2205-4d06-bced-51fd4a3b0301" /> <img width="353" alt="Screenshot 2025-02-26 at 18 19 23" src="https://github.com/user-attachments/assets/27af0e19-cfe9-43f5-a039-06cf1b0cb939" /> 3. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ### `wallet_requestPermissions` `endowment:permitted-chains` chain permission request 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': { 'caveats': [ { 'type': 'restrictReturnedAccounts', 'value': [] } ] }, 'endowment:permitted-chains': { 'caveats': [ { 'type': 'restrictNetworkSwitching', 'value': ["0x1"] // replace with desired chain, here we default to mainnet } ] } } ] }); ``` 2. Should render the following UI <img width="352" alt="Screenshot 2025-02-26 at 18 22 15" src="https://github.com/user-attachments/assets/ad93a621-0282-4d59-b91e-4d6d66b1a64d" /> <img width="354" alt="Screenshot 2025-02-26 at 18 22 09" src="https://github.com/user-attachments/assets/5eb7a5bf-41f0-4c5a-9280-f1a926180e1c" /> 3. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ### `wallet_requestPermissions` `eth_accounts` and `endowment:permitted-chains` account + chain permission request 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': { 'caveats': [ { 'type': 'restrictReturnedAccounts', 'value': ["0x0aC9D99d92AB266e33b27469f8ce84FE7C56b6d5"] // replace this account with your local account public address } ] }, 'endowment:permitted-chains': { 'caveats': [ { 'type': 'restrictNetworkSwitching', 'value': ["0x1"] // replace with desired chain, here we default to mainnet } ] } } ] }); ``` 2. Should render the following UI <img width="356" alt="Screenshot 2025-02-26 at 18 23 34" src="https://github.com/user-attachments/assets/f63530ff-820a-4c43-a0e4-4b9bf7fa955a" /> <img width="350" alt="Screenshot 2025-02-26 at 18 23 29" src="https://github.com/user-attachments/assets/b2db17b6-0395-4c08-ad3f-2176cb4dcea8" /> 3. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ### `wallet_requestPermissions` malformed `eth_accounts` address 1. Send the following request through dev tools console via [wallet_requestPermissions](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_requestPermissions/) ```js await window.ethereum.request({ method: 'wallet_requestPermissions', params: [ { 'eth_accounts': { 'caveats': [ { 'type': 'restrictReturnedAccounts', 'value': ["malformed.eth"] } ] }, 'endowment:permitted-chains': { 'caveats': [ { 'type': 'restrictNetworkSwitching', 'value': [] } ] } } ] }); ``` 2. Should see an exception being thrown in the console, with the message `Value must be a hexadecimal string, starting with "0x".` <img width="730" alt="Screenshot 2025-02-27 at 10 16 31" src="https://github.com/user-attachments/assets/0a13f3f9-3ec3-4093-a9fc-331faa4e29ee" /> ### `wallet_switchEthereumChain` Permissions for new chain 1. Send the following request through dev tools console via [wallet_switchEthereumChain](https://docs.metamask.io/wallet/reference/json-rpc-methods/wallet_switchethereumchain/) ```js await window.ethereum.request({ jsonrpc: '2.0', method: 'wallet_switchEthereumChain', params: [{ chainId: '0xe705' }], // replace by a chain that's currently not permitted, I used Linea Sepolia }) ``` 2. Should render the following UI <img width="350" alt="Screenshot 2025-02-26 at 18 26 20" src="https://github.com/user-attachments/assets/c93a9487-ebab-43fd-9713-6ef4adbffc39" /> 4. After approval, the dapp should also have `eth_accounts` and `endowment:permitted-chains` permission for the recently added chain via `wallet_getPermissions` ## **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 Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.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/main/.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: ffmcgee <joao.carlos@consensys.net> Co-authored-by: ffmcgee <jc.992@hotmail.com> Co-authored-by: Alex Donesky <adonesky@gmail.com> Co-authored-by: ffmcgee <51971598+ffmcgee725@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation
Addressing https://github.com/MetaMask/MetaMask-planning/issues/4017
We want to create a merger for the Caip25Permission specification in order to enable wallet_addEthereumChain and wallet_switchEthereumChain to call to the PermissionController rather than the ApprovalController for incremental permission requests (i.e. for the permittedChains flow).
References
Extension: MetaMask/metamask-extension#30042
Changelog
@metamask/multichain-apimergeScopestomergeNormalizedScopescaip25CaveatBuilder()mergeInternalScopeswhich merges twoInternalScopesObjectsChecklist