Ad/multichain/update queued request controller #27408
Draft
+13,317
−2,548
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
<!-- 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** This PR adds a new command `yarn test:api-specs-multichain` that uses externally_connectable as a transport and runs a custom `MultichainAuthorizationConfirmation` rule. It also writes an html report to `html-report-multichain`. [](https://codespaces.new/MetaMask/metamask-extension/pull/26643?quickstart=1) ## **Related issues** Fixes: MetaMask/MetaMask-planning#2834 ## **Manual testing steps** 1. `BARAD_DUR=1 yarn build:test` 2. `yarn test:api-specs-multichain` ## **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/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.
<!-- 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** Split methods into `eip155`, `wallet`, and `wallet:eip155` namespaces [](https://codespaces.new/MetaMask/metamask-extension/pull/26732?quickstart=1) ## **Related issues** See: MetaMask/MetaMask-planning#3036 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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.
<!-- 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** As discussed with Alex, we don't actually need to prepopulate the CAIP-25 scopes granted from the EIP-1193 API with methods/notifications [](https://codespaces.new/MetaMask/metamask-extension/pull/26877?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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.
<!-- 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** Error cleanup [](https://codespaces.new/MetaMask/metamask-extension/pull/26825?quickstart=1) ## **Related issues** See: MetaMask/MetaMask-planning#3048 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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.
<!-- 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** Whelp, my bad. [](https://codespaces.new/MetaMask/metamask-extension/pull/26926?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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.
<!-- 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** * Rename `provider_authorize` -> `wallet_createSession` * Rename `provider_requests` -> `wallet_invokeMethod` * Format `eth_subscription` responses with `wallet_notify` [](https://codespaces.new/MetaMask/metamask-extension/pull/26928?quickstart=1) ## **Related issues** See: MetaMask/MetaMask-planning#3037 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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.
<!-- 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** As discussed in the CASA meeting, we're using the 5301 error wrong. We wouldn't want to fire it in our implementation either way so just removing it now [](https://codespaces.new/MetaMask/metamask-extension/pull/26915?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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.
<!-- 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** * remove wallet_watchAsset from wallet:eip155 * (which makes it a `eip155:x` scoped method) [](https://codespaces.new/MetaMask/metamask-extension/pull/26954?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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.
<!-- 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** An effort to cleanup some lazy permission types and differentiate the potentially unflattened/malformed ScopeObjects from a CAIP-25 request and the internal flattened ScopeObjects that get persisted into the PermissionController state. I don't love the current Internal/External prefixing. Perhaps making the external type `unknown` and keeping the unprefixed as the internal type would be better. Thoughts? [](https://codespaces.new/MetaMask/metamask-extension/pull/26690?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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: Alex Donesky <adonesky@gmail.com>
<!-- 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/26828?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. `CHAIN_PERMISSIONS=1 BARAD_DUR=1 yarn build:test` 2. `yarn test:api-specs-multichain` ## **Screenshots/Recordings** <img width="1609" alt="image" src="https://github.com/user-attachments/assets/7db111ad-e194-4799-83a3-783d5e943e65"> ## **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/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 <metamaskbot@users.noreply.github.com>
# Conflicts: # lavamoat/browserify/beta/policy.json # lavamoat/browserify/flask/policy.json # lavamoat/browserify/main/policy.json # lavamoat/browserify/mmi/policy.json
<!-- 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** * Fix `wallet:eip155` not being considered a valid scopeString * Fix EIP-1193 permission adapter not checking `wallet` and `wallet:eip155` scopes for methods [](https://codespaces.new/MetaMask/metamask-extension/pull/27223?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **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/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.
|
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/queued-request-controller@2.0.0 |
4 tasks
|
|
Legacy Flow and Multichain Flow (different dapp, same dapp, different chain, same chain combinations) all seem functional! |
adonesky1
added a commit
to MetaMask/core
that referenced
this pull request
Sep 27, 2024
## Explanation The `QueuedRequestController` previously batched and processed requests based solely on their `origin`. This approach doesn't account for scenarios where a dapp interacts with multiple networks simultaneously from the same origin as will be the case on the multichain API. This PR updates the `QueuedRequestController` to batch and process requests based on both `origin` and `networkClientId`. This ensures that: - Requests from the same origin but targeting different networks are processed in separate batches. - Network switches occur appropriately between batches when the `networkClientId` changes. - Each request is processed on the correct network. Key changes: - **Batching Logic**: Modified the batching mechanism to consider both `origin` and `networkClientId`, for the legacy API this shouldn't change anything, but it allows us to cleanly batch for the multichain API until we remove queueing altogther. - **Request Validation**: Added validation to throw an error if a request doesn't include a `networkClientId`. - **Dependency Removal**: Removed reliance on `SelectedNetworkController` for determining `networkClientId`. Now `NetworkClientId` is expected on every request. Which should always be the case if the middleware ordering is correct on both APIs. - **Test Enhancements**: Added and updated tests to cover new scenarios involving multiple `origin`s and `networkClientId`s. Draft PR/Branch off caip-multichain feature branch with preview build: MetaMask/metamask-extension#27408 ## Videos demonstrating functionality w/ multichain API: ### 2 requests same network different dapps: https://github.com/user-attachments/assets/b7087322-093b-4229-86f9-d643540a5f5e ### 2 requests different network same dapp https://github.com/user-attachments/assets/80b9e820-30fd-4bd7-a82e-588243fa3a44 ### 2 requests same network same dapp https://github.com/user-attachments/assets/91e2e35d-2ab6-4863-a9e4-0b955815354e ### More complex flows: https://drive.google.com/file/d/1PvCmmCQbxXqglsFLUlyT_7P_znukLUxI/view?usp=drive_link https://drive.google.com/file/d/18R8zEPz_zsfhsw-DOkRFwkYRI1URynis/view?usp=sharing ### Queueing working same as before on legacy API [Branch/PR with preview builds on develop used to test ](MetaMask/metamask-extension#27430) https://github.com/user-attachments/assets/485d0a28-7075-4c65-be4b-b9022b6ef28f ## References - N/A ## Changelog ### `@metamask/queued-request-controller` - **CHANGED**: Batch processing now considers both `origin` and `networkClientId`, ensuring requests targeting different networks are processed separately. - **REMOVED**: Dependency on `SelectedNetworkController`; the controller no longer uses `SelectedNetworkController:getNetworkClientIdForDomain`. - **CHANGED**: Incoming requests to `enqueueRequest` now **must** include a `networkClientId`; an error is thrown if it's missing. This was previously a [required part of the type](https://github.com/MetaMask/core/blob/66c94ae4f0a54764ca890c927ffdbe3c2d6cd846/packages/queued-request-controller/src/types.ts#L5) but since consumers like the extension do not have extensive typescript coverage this isn't definitively enforced. ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
](https://codespaces.new/MetaMask/metamask-extension/pull/26643?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26732?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26877?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26825?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26926?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26928?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26915?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26954?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26690?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/26828?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/27223?quickstart=1){kind=link}
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.
Description
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist