Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 300
Fix issue with selectedNetworkMiddlware where all domains are added to state regardless of whether they have permissions#3908
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
73d907d84c980d65dcb47fd30413c784438c1bd25fa71ecdd2188f9908605805c269ec68105c0File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4,6 +4,7 @@ import type { | ||
| BlockTrackerProxy, | ||
| NetworkClientId, | ||
| NetworkControllerGetNetworkClientByIdAction, | ||
| NetworkControllerGetStateAction, | ||
| NetworkControllerStateChangeEvent, | ||
| ProviderProxy, | ||
| } from '@metamask/network-controller'; | ||
| @@ -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, | ||
| @@ -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`; | ||
| handler: (domain: string) => boolean; | ||
| }; | ||
| export type SelectedNetworkControllerActions = | ||
| | SelectedNetworkControllerGetSelectedNetworkStateAction | ||
| | SelectedNetworkControllerGetNetworkClientIdForDomainAction | ||
| | SelectedNetworkControllerSetNetworkClientIdForDomainAction; | ||
| export type AllowedActions = NetworkControllerGetNetworkClientByIdAction; | ||
| export type AllowedActions = | ||
| | NetworkControllerGetNetworkClientByIdAction | ||
| | NetworkControllerGetStateAction | ||
| | PermissionControllerHasPermissions; | ||
| export type SelectedNetworkControllerEvents = | ||
| SelectedNetworkControllerStateChangeEvent; | ||
| @@ -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, | ||
| @@ -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.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| return this.state.domains[METAMASK_DOMAIN]; | ||
| return this.state.domains[domain] ?? metamaskSelectedNetworkClientId; | ||
| } | ||
| /** | ||
| @@ -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), | ||
| @@ -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
ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to update the domain state when the | ||
| } | ||
| } | ||
| 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.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| 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(); | ||
| }; | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.