Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore: Remove useRequestQueue toggle to turn on/off Per Dapp Selected Network Feature #4941

Merged
merged 4 commits into from
Nov 22, 2024
Merged

chore: Remove useRequestQueue toggle to turn on/off Per Dapp Selected Network Feature #4941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fc955bd
remove queuedRequestPreference
adonesky1 Nov 15, 2024
1c32559
Merge branch 'main' into ad/remove-request-queue-toggleability
adonesky1 Nov 20, 2024
8ecd45d
Merge branch 'main' into ad/remove-request-queue-toggleability
adonesky1 Nov 21, 2024
d483359
Merge branch 'main' into ad/remove-request-queue-toggleability
adonesky1 Nov 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions packages/queued-request-controller/src/QueuedRequestMiddleware.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ describe('createQueuedRequestMiddleware', () => {
const mockEnqueueRequest = getMockEnqueueRequest();
const middleware = buildQueuedRequestMiddleware({
enqueueRequest: mockEnqueueRequest,
useRequestQueue: () => true,
});

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

shouldEnqueueRequest: ({ method }) =>
method === 'method_with_confirmation',
});
Expand Down Expand Up @@ -145,7 +144,6 @@ 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 @@ -167,7 +165,7 @@ describe('createQueuedRequestMiddleware', () => {
enqueueRequest: jest
.fn()
.mockRejectedValue(new Error('enqueuing error')),
useRequestQueue: () => true,

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

shouldEnqueueRequest: () => true,
});
const request = {
Expand Down Expand Up @@ -271,7 +269,6 @@ function buildQueuedRequestMiddleware(
) {
const options = {
enqueueRequest: getMockEnqueueRequest(),
useRequestQueue: () => false,
shouldEnqueueRequest: () => false,
...overrideOptions,
};
Expand Down
8 changes: 2 additions & 6 deletions packages/queued-request-controller/src/QueuedRequestMiddleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,27 +38,23 @@ 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 the request queue feature is turned off, or this method is not a confirmation method
// bypass the queue completely
if (!useRequestQueue() || !shouldEnqueueRequest(req)) {
// if this method is not a confirmation method bypass the queue completely
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// if this method is not a confirmation method bypass the queue completely
// if this method is not a confirmation method then bypass the queue completely

nit. not important

if (!shouldEnqueueRequest(req)) {
return await next();
}

Expand Down
62 changes: 5 additions & 57 deletions packages/selected-network-controller/src/SelectedNetworkController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,6 @@ 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 @@ -124,23 +120,17 @@ 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 @@ -149,7 +139,6 @@ export class SelectedNetworkController extends BaseController<
messenger,
state,
});
this.#useRequestQueuePreference = useRequestQueuePreference;
this.#domainProxyMap = domainProxyMap;
this.#registerMessageHandlers();

Expand Down Expand Up @@ -247,21 +236,6 @@ 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 @@ -326,31 +300,10 @@ 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 @@ -373,9 +326,6 @@ 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 @@ -403,10 +353,7 @@ export class SelectedNetworkController extends BaseController<
let networkProxy = this.#domainProxyMap.get(domain);
if (networkProxy === undefined) {
let networkClient;
if (
this.#useRequestQueuePreference &&
this.#domainHasPermissions(domain)
) {
if (this.#domainHasPermissions(domain)) {
const networkClientId = this.getNetworkClientIdForDomain(domain);
networkClient = this.messagingSystem.call(
'NetworkController:getNetworkClientById',
Expand All @@ -416,10 +363,11 @@ 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
Loading