Skip to content

Commit

Permalink
Revert "chore: Remove useRequestQueue toggle to turn on/off Per Dap…
Browse files Browse the repository at this point in the history
…p Selected Network Feature (#4941)" (#5065)

This reverts commit 4814cf1.

## Explanation

We are not yet ready to release per-dapp selected network functionality
on the mobile client and with this change there is no clean way to
[update the version of
SelectedNetworkController](MetaMask/metamask-mobile#12434 (comment))
in the mobile client without having the Domains state starting to
populate and possibly become corrupt since its not being consumed
by/updated by the frontend in the expected way and may need to be
migrated away when its time to actually start using the controller.

Without this revert the @MetaMask/wallet-framework team is blocked from
completing their goal to get both clients up to the latest versions of
all controllers.

## 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/queued-request-controller`

- **ADDED**: **BREAKING:** `createQueuedRequestMiddleware` now expects a
`useRequestQueue` property in its param options


### `@metamask/selected-network-controller`

- **ADDED**: **BREAKING:** `SelectedNetworkController` constructor now
expects
both a `useRequestQueuePreference` and a `onPreferencesStateChange`
param

## 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 Dec 12, 2024
1 parent bf93458 commit 3f15e1e
Show file tree
Hide file tree
Showing 4 changed files with 746 additions and 394 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ describe('createQueuedRequestMiddleware', () => {
const mockEnqueueRequest = getMockEnqueueRequest();
const middleware = buildQueuedRequestMiddleware({
enqueueRequest: mockEnqueueRequest,
useRequestQueue: () => true,
});

const request = {
Expand All @@ -104,7 +105,7 @@ describe('createQueuedRequestMiddleware', () => {
const mockEnqueueRequest = getMockEnqueueRequest();
const middleware = buildQueuedRequestMiddleware({
enqueueRequest: mockEnqueueRequest,

useRequestQueue: () => true,
shouldEnqueueRequest: ({ method }) =>
method === 'method_with_confirmation',
});
Expand Down Expand Up @@ -144,6 +145,7 @@ describe('createQueuedRequestMiddleware', () => {
it('calls next after a request is queued and processed', async () => {
const middleware = buildQueuedRequestMiddleware({
enqueueRequest: getMockEnqueueRequest(),
useRequestQueue: () => true,
});
const request = {
...getRequestDefaults(),
Expand All @@ -165,7 +167,7 @@ describe('createQueuedRequestMiddleware', () => {
enqueueRequest: jest
.fn()
.mockRejectedValue(new Error('enqueuing error')),

useRequestQueue: () => true,
shouldEnqueueRequest: () => true,
});
const request = {
Expand All @@ -189,7 +191,7 @@ describe('createQueuedRequestMiddleware', () => {
enqueueRequest: jest
.fn()
.mockRejectedValue(new Error('enqueuing error')),

useRequestQueue: () => true,
shouldEnqueueRequest: () => true,
});
const request = {
Expand Down Expand Up @@ -269,6 +271,7 @@ function buildQueuedRequestMiddleware(
) {
const options = {
enqueueRequest: getMockEnqueueRequest(),
useRequestQueue: () => false,
shouldEnqueueRequest: () => false,
...overrideOptions,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,27 @@ function hasRequiredMetadata(
*
* @param options - Configuration options.
* @param options.enqueueRequest - A method for enqueueing a request.
* @param options.useRequestQueue - A function that determines if the request queue feature is enabled.
* @param options.shouldEnqueueRequest - A function that returns if a request should be handled by the QueuedRequestController.
* @returns The JSON-RPC middleware that manages queued requests.
*/
export const createQueuedRequestMiddleware = ({
enqueueRequest,
useRequestQueue,
shouldEnqueueRequest,
}: {
enqueueRequest: QueuedRequestController['enqueueRequest'];
useRequestQueue: () => boolean;
shouldEnqueueRequest: (
request: QueuedRequestMiddlewareJsonRpcRequest,
) => boolean;
}): JsonRpcMiddleware<JsonRpcParams, Json> => {
return createAsyncMiddleware(async (req: JsonRpcRequest, res, next) => {
hasRequiredMetadata(req);

// if this method is not a confirmation method bypass the queue completely
if (!shouldEnqueueRequest(req)) {
// if the request queue feature is turned off, or this method is not a confirmation method
// bypass the queue completely
if (!useRequestQueue() || !shouldEnqueueRequest(req)) {
return await next();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ export type SelectedNetworkControllerMessenger = RestrictedControllerMessenger<
export type SelectedNetworkControllerOptions = {
state?: SelectedNetworkControllerState;
messenger: SelectedNetworkControllerMessenger;
useRequestQueuePreference: boolean;
onPreferencesStateChange: (
listener: (preferencesState: { useRequestQueue: boolean }) => void,
) => void;
domainProxyMap: Map<Domain, NetworkProxy>;
};

Expand All @@ -120,17 +124,23 @@ export class SelectedNetworkController extends BaseController<
> {
#domainProxyMap: Map<Domain, NetworkProxy>;

#useRequestQueuePreference: boolean;

/**
* Construct a SelectedNetworkController controller.
*
* @param options - The controller options.
* @param options.messenger - The restricted controller messenger for the EncryptionPublicKey controller.
* @param options.state - The controllers initial state.
* @param options.useRequestQueuePreference - A boolean indicating whether to use the request queue preference.
* @param options.onPreferencesStateChange - A callback that is called when the preference state changes.
* @param options.domainProxyMap - A map for storing domain-specific proxies that are held in memory only during use.
*/
constructor({
messenger,
state = getDefaultState(),
useRequestQueuePreference,
onPreferencesStateChange,
domainProxyMap,
}: SelectedNetworkControllerOptions) {
super({
Expand All @@ -139,6 +149,7 @@ export class SelectedNetworkController extends BaseController<
messenger,
state,
});
this.#useRequestQueuePreference = useRequestQueuePreference;
this.#domainProxyMap = domainProxyMap;
this.#registerMessageHandlers();

Expand Down Expand Up @@ -236,6 +247,21 @@ export class SelectedNetworkController extends BaseController<
}
},
);

onPreferencesStateChange(({ useRequestQueue }) => {
if (this.#useRequestQueuePreference !== useRequestQueue) {
if (!useRequestQueue) {
// Loop through all domains and points each domain's proxy
// to the NetworkController's own proxy of the globally selected networkClient
Object.keys(this.state.domains).forEach((domain) => {
this.#unsetNetworkClientIdForDomain(domain);
});
} else {
this.#resetAllPermissionedDomains();
}
this.#useRequestQueuePreference = useRequestQueue;
}
});
}

#registerMessageHandlers(): void {
Expand Down Expand Up @@ -300,10 +326,31 @@ export class SelectedNetworkController extends BaseController<
);
}

// Loop through all domains and for those with permissions it points that domain's proxy
// to an unproxied instance of the globally selected network client.
// NOT the NetworkController's proxy of the globally selected networkClient
#resetAllPermissionedDomains() {
this.#domainProxyMap.forEach((_: NetworkProxy, domain: string) => {
const { selectedNetworkClientId } = this.messagingSystem.call(
'NetworkController:getState',
);
// can't use public setNetworkClientIdForDomain because it will throw an error
// rather than simply skip if the domain doesn't have permissions which can happen
// in this case since proxies are added for each site the user visits
if (this.#domainHasPermissions(domain)) {
this.#setNetworkClientIdForDomain(domain, selectedNetworkClientId);
}
});
}

setNetworkClientIdForDomain(
domain: Domain,
networkClientId: NetworkClientId,
) {
if (!this.#useRequestQueuePreference) {
return;
}

if (domain === METAMASK_DOMAIN) {
throw new Error(
`NetworkClientId for domain "${METAMASK_DOMAIN}" cannot be set on the SelectedNetworkController`,
Expand All @@ -326,6 +373,9 @@ export class SelectedNetworkController extends BaseController<
getNetworkClientIdForDomain(domain: Domain): NetworkClientId {
const { selectedNetworkClientId: metamaskSelectedNetworkClientId } =
this.messagingSystem.call('NetworkController:getState');
if (!this.#useRequestQueuePreference) {
return metamaskSelectedNetworkClientId;
}
return this.state.domains[domain] ?? metamaskSelectedNetworkClientId;
}

Expand Down Expand Up @@ -353,7 +403,10 @@ export class SelectedNetworkController extends BaseController<
let networkProxy = this.#domainProxyMap.get(domain);
if (networkProxy === undefined) {
let networkClient;
if (this.#domainHasPermissions(domain)) {
if (
this.#useRequestQueuePreference &&
this.#domainHasPermissions(domain)
) {
const networkClientId = this.getNetworkClientIdForDomain(domain);
networkClient = this.messagingSystem.call(
'NetworkController:getNetworkClientById',
Expand All @@ -363,11 +416,10 @@ export class SelectedNetworkController extends BaseController<
networkClient = this.messagingSystem.call(
'NetworkController:getSelectedNetworkClient',
);
if (networkClient === undefined) {
throw new Error('Selected network not initialized');
}
}
if (networkClient === undefined) {
throw new Error('Selected network not initialized');
}

networkProxy = {
provider: createEventEmitterProxy(networkClient.provider),
blockTracker: createEventEmitterProxy(networkClient.blockTracker, {
Expand Down
Loading

0 comments on commit 3f15e1e

Please sign in to comment.