Skip to content

Commit

Permalink
Update QueuedRequestController to support Multichain API (#4718)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
adonesky1 authored Sep 27, 2024
1 parent b6a11a4 commit 54ddebf
Show file tree
Hide file tree
Showing 2 changed files with 422 additions and 101 deletions.
Loading

0 comments on commit 54ddebf

Please sign in to comment.