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.

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

Fix issue with selectedNetworkMiddlware where all domains are added to state regardless of whether they have permissions #3908

Merged
merged 11 commits into from
Feb 22, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
BlockTrackerProxy,
NetworkClientId,
NetworkControllerGetNetworkClientByIdAction,
NetworkControllerGetStateAction,
NetworkControllerStateChangeEvent,
ProviderProxy,
} from '@metamask/network-controller';
Expand All @@ -24,7 +25,7 @@ const getDefaultState = () => ({

type Domain = string;

const METAMASK_DOMAIN = 'metamask' as const;
export const METAMASK_DOMAIN = 'metamask' as const;

export const SelectedNetworkControllerActionTypes = {
getState: `${controllerName}:getState` as const,
Expand Down Expand Up @@ -60,20 +61,28 @@ export type SelectedNetworkControllerGetSelectedNetworkStateAction = {

export type SelectedNetworkControllerGetNetworkClientIdForDomainAction = {
type: typeof SelectedNetworkControllerActionTypes.getNetworkClientIdForDomain;
handler: (domain: string) => NetworkClientId;
handler: SelectedNetworkController['getNetworkClientIdForDomain'];
};

export type SelectedNetworkControllerSetNetworkClientIdForDomainAction = {
type: typeof SelectedNetworkControllerActionTypes.setNetworkClientIdForDomain;
handler: (domain: string, NetworkClientId: NetworkClientId) => void;
handler: SelectedNetworkController['setNetworkClientIdForDomain'];
};

type PermissionControllerHasPermissions = {
type: `PermissionController:hasPermissions`;
mcmire marked this conversation as resolved.
Show resolved Hide resolved
handler: (domain: string) => boolean;
};

export type SelectedNetworkControllerActions =
| SelectedNetworkControllerGetSelectedNetworkStateAction
| SelectedNetworkControllerGetNetworkClientIdForDomainAction
| SelectedNetworkControllerSetNetworkClientIdForDomainAction;

export type AllowedActions = NetworkControllerGetNetworkClientByIdAction;
export type AllowedActions =
| NetworkControllerGetNetworkClientByIdAction
| NetworkControllerGetStateAction
| PermissionControllerHasPermissions;

export type SelectedNetworkControllerEvents =
SelectedNetworkControllerStateChangeEvent;
Expand Down Expand Up @@ -133,17 +142,12 @@ export class SelectedNetworkController extends BaseController<
SelectedNetworkControllerActionTypes.getNetworkClientIdForDomain,
this.getNetworkClientIdForDomain.bind(this),
);

this.messagingSystem.registerActionHandler(
SelectedNetworkControllerActionTypes.setNetworkClientIdForDomain,
this.setNetworkClientIdForDomain.bind(this),
);
}

setNetworkClientIdForMetamask(networkClientId: NetworkClientId) {
this.setNetworkClientIdForDomain(METAMASK_DOMAIN, networkClientId);
}

#setNetworkClientIdForDomain(
domain: Domain,
networkClientId: NetworkClientId,
Expand All @@ -167,36 +171,42 @@ export class SelectedNetworkController extends BaseController<

this.update((state) => {
state.domains[domain] = networkClientId;
if (!state.perDomainNetwork) {
state.domains[METAMASK_DOMAIN] = networkClientId;
}
});
}

#domainHasPermissions(domain: Domain): boolean {
return this.messagingSystem.call(
'PermissionController:hasPermissions',
domain,
);
}

setNetworkClientIdForDomain(
domain: Domain,
networkClientId: NetworkClientId,
) {
if (!this.state.perDomainNetwork) {
Object.entries(this.state.domains).forEach(
([entryDomain, networkClientIdForDomain]) => {
if (
networkClientIdForDomain !== networkClientId &&
entryDomain !== domain
) {
this.#setNetworkClientIdForDomain(entryDomain, networkClientId);
}
},
if (domain === METAMASK_DOMAIN) {
throw new Error(
`NetworkClientId for domain "${METAMASK_DOMAIN}" cannot be set on the SelectedNetworkController`,
);
}

if (!this.#domainHasPermissions(domain)) {
throw new Error(
'NetworkClientId for domain cannot be called with a domain that has not yet been granted permissions',
);
}

this.#setNetworkClientIdForDomain(domain, networkClientId);
}

getNetworkClientIdForDomain(domain: Domain): NetworkClientId {
if (this.state.perDomainNetwork) {
return this.state.domains[domain] ?? this.state.domains[METAMASK_DOMAIN];
const { selectedNetworkClientId: metamaskSelectedNetworkClientId } =
this.messagingSystem.call('NetworkController:getState');
if (!this.state.perDomainNetwork) {
return metamaskSelectedNetworkClientId;
mcmire marked this conversation as resolved.
Show resolved Hide resolved
}
return this.state.domains[METAMASK_DOMAIN];
return this.state.domains[domain] ?? metamaskSelectedNetworkClientId;
}

/**
Expand All @@ -206,11 +216,22 @@ export class SelectedNetworkController extends BaseController<
* @returns The proxy and block tracker proxies.
*/
getProviderAndBlockTracker(domain: Domain): NetworkProxy {
if (!this.state.perDomainNetwork) {
throw new Error(
'Provider and BlockTracker should be fetched from NetworkController when perDomainNetwork is false',
);
}
const networkClientId = this.state.domains[domain];
if (!networkClientId) {
throw new Error(
'NetworkClientId has not been set for the requested domain',
);
}
let networkProxy = this.#proxies.get(domain);
if (networkProxy === undefined) {
const networkClient = this.messagingSystem.call(
'NetworkController:getNetworkClientById',
this.getNetworkClientIdForDomain(domain),
networkClientId,
);
networkProxy = {
provider: createEventEmitterProxy(networkClient.provider),
Expand All @@ -229,12 +250,5 @@ export class SelectedNetworkController extends BaseController<
state.perDomainNetwork = enabled;
return state;
});
Object.keys(this.state.domains).forEach((domain) => {
// when perDomainNetwork is false, getNetworkClientIdForDomain always returns the networkClientId for the domain 'metamask'
this.setNetworkClientIdForDomain(
domain,
this.getNetworkClientIdForDomain(domain),
);
});
Comment on lines -232 to -238
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to update the domain state when the perDomainNetwork flag is toggled on or off, we can just reference this flag in the relevant methods to determine which networkClient to access in either case.

}
}
Original file line number Diff line number Diff line change
@@ -1,64 +1,29 @@
import type { ControllerMessenger } from '@metamask/base-controller';
import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine';
import type {
NetworkClientId,
NetworkControllerGetStateAction,
NetworkControllerStateChangeEvent,
} from '@metamask/network-controller';
import type { NetworkClientId } from '@metamask/network-controller';
import type { Json, JsonRpcParams, JsonRpcRequest } from '@metamask/utils';

import type {
SelectedNetworkControllerGetNetworkClientIdForDomainAction,
SelectedNetworkControllerSetNetworkClientIdForDomainAction,
} from './SelectedNetworkController';
import type { SelectedNetworkControllerMessenger } from './SelectedNetworkController';
import { SelectedNetworkControllerActionTypes } from './SelectedNetworkController';

export type MiddlewareAllowedActions = NetworkControllerGetStateAction;
export type MiddlewareAllowedEvents = NetworkControllerStateChangeEvent;

export type SelectedNetworkMiddlewareMessenger = ControllerMessenger<
| SelectedNetworkControllerGetNetworkClientIdForDomainAction
| SelectedNetworkControllerSetNetworkClientIdForDomainAction
| MiddlewareAllowedActions,
MiddlewareAllowedEvents
>;

export type SelectedNetworkMiddlewareJsonRpcRequest = JsonRpcRequest & {
networkClientId?: NetworkClientId;
origin?: string;
};

export const createSelectedNetworkMiddleware = (
messenger: SelectedNetworkMiddlewareMessenger,
messenger: SelectedNetworkControllerMessenger,
): JsonRpcMiddleware<JsonRpcParams, Json> => {
const getNetworkClientIdForDomain = (origin: string) =>
messenger.call(
SelectedNetworkControllerActionTypes.getNetworkClientIdForDomain,
origin,
);

const setNetworkClientIdForDomain = (
origin: string,
networkClientId: NetworkClientId,
) =>
messenger.call(
SelectedNetworkControllerActionTypes.setNetworkClientIdForDomain,
origin,
networkClientId,
);

const getDefaultNetworkClientId = () =>
messenger.call('NetworkController:getState').selectedNetworkClientId;

adonesky1 marked this conversation as resolved.
Show resolved Hide resolved
return (req: SelectedNetworkMiddlewareJsonRpcRequest, _, next) => {
if (!req.origin) {
throw new Error("Request object is lacking an 'origin'");
}

if (getNetworkClientIdForDomain(req.origin) === undefined) {
setNetworkClientIdForDomain(req.origin, getDefaultNetworkClientId());
}

req.networkClientId = getNetworkClientIdForDomain(req.origin);
return next();
};
Expand Down
1 change: 1 addition & 0 deletions packages/selected-network-controller/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export {
SelectedNetworkControllerActionTypes,
SelectedNetworkControllerEventTypes,
SelectedNetworkController,
METAMASK_DOMAIN,
} from './SelectedNetworkController';
export type { SelectedNetworkMiddlewareJsonRpcRequest } from './SelectedNetworkMiddleware';
export { createSelectedNetworkMiddleware } from './SelectedNetworkMiddleware';
Loading
Loading