Skip to content

Commit

Permalink
Refactor: Expose SelectedNetworkController proxies map in the const…
Browse files Browse the repository at this point in the history
…ructor params (#4104)

## Explanation

In a [preceding PR](#4063), the
SelectedNetworkController will start to keep proxy instances for all
domains seen without a way to remove them when they become stale. This
PR exposes the private proxies cache map as a constructor param which
will enable callers to implement their own cache invalidation strategies
without needing to concern the SelectedNetworkController of the
implementation details themselves. This is needed because Mobile and
Extension will need to follow different cache invalidation strategies.

This PR should be merged before
#4063

## References

Fixes: #4062
See: #4063

## Changelog

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

- **BREAKING**: `SelectedNetworkController` now expects the
`domainProxyMap` param which is a `Map` of `Domain` to `NetworkProxy`
- **ADDED**: exports the `Domain` type

## 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

---------

Co-authored-by: Alex Donesky <adonesky@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
  • Loading branch information
3 people authored Mar 26, 2024
1 parent 8f4c698 commit 319bcfc
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const stateMetadata = {

const getDefaultState = () => ({ domains: {} });

type Domain = string;
export type Domain = string;

export const METAMASK_DOMAIN = 'metamask' as const;

Expand Down Expand Up @@ -96,6 +96,7 @@ export type SelectedNetworkControllerOptions = {
state?: SelectedNetworkControllerState;
messenger: SelectedNetworkControllerMessenger;
getUseRequestQueue: GetUseRequestQueue;
domainProxyMap: Map<Domain, NetworkProxy>;
};

export type NetworkProxy = {
Expand All @@ -111,7 +112,7 @@ export class SelectedNetworkController extends BaseController<
SelectedNetworkControllerState,
SelectedNetworkControllerMessenger
> {
#proxies = new Map<Domain, NetworkProxy>();
#domainProxyMap: Map<Domain, NetworkProxy>;

#getUseRequestQueue: GetUseRequestQueue;

Expand All @@ -122,11 +123,13 @@ export class SelectedNetworkController extends BaseController<
* @param options.messenger - The restricted controller messenger for the EncryptionPublicKey controller.
* @param options.state - The controllers initial state.
* @param options.getUseRequestQueue - feature flag for perDappNetwork & request queueing features
* @param options.domainProxyMap - A map for storing domain-specific proxies that are held in memory only during use.
*/
constructor({
messenger,
state = getDefaultState(),
getUseRequestQueue,
domainProxyMap,
}: SelectedNetworkControllerOptions) {
super({
name: controllerName,
Expand All @@ -135,6 +138,7 @@ export class SelectedNetworkController extends BaseController<
state,
});
this.#getUseRequestQueue = getUseRequestQueue;
this.#domainProxyMap = domainProxyMap;
this.#registerMessageHandlers();

// this is fetching all the dapp permissions from the PermissionsController and looking for any domains that are not in domains state in this controller. Then we take any missing domains and add them to state here, setting it with the globally selected networkClientId (fetched from the NetworkController)
Expand Down Expand Up @@ -218,9 +222,9 @@ export class SelectedNetworkController extends BaseController<
'NetworkController:getNetworkClientById',
networkClientId,
);
const networkProxy = this.#proxies.get(domain);
const networkProxy = this.#domainProxyMap.get(domain);
if (networkProxy === undefined) {
this.#proxies.set(domain, {
this.#domainProxyMap.set(domain, {
provider: createEventEmitterProxy(networkClient.provider),
blockTracker: createEventEmitterProxy(networkClient.blockTracker, {
eventFilter: 'skipInternal',
Expand Down Expand Up @@ -289,7 +293,7 @@ export class SelectedNetworkController extends BaseController<
'NetworkClientId has not been set for the requested domain',
);
}
let networkProxy = this.#proxies.get(domain);
let networkProxy = this.#domainProxyMap.get(domain);
if (networkProxy === undefined) {
const networkClient = this.messagingSystem.call(
'NetworkController:getNetworkClientById',
Expand All @@ -301,7 +305,7 @@ export class SelectedNetworkController extends BaseController<
eventFilter: 'skipInternal',
}),
};
this.#proxies.set(domain, networkProxy);
this.#domainProxyMap.set(domain, networkProxy);
}

return networkProxy;
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 @@ -9,6 +9,7 @@ export type {
SelectedNetworkControllerMessenger,
SelectedNetworkControllerOptions,
NetworkProxy,
Domain,
} from './SelectedNetworkController';
export {
SelectedNetworkControllerActionTypes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import type {
SelectedNetworkControllerEvents,
SelectedNetworkControllerMessenger,
SelectedNetworkControllerState,
Domain,
NetworkProxy,
} from '../src/SelectedNetworkController';
import {
SelectedNetworkController,
Expand Down Expand Up @@ -92,11 +94,13 @@ const setup = ({
getSubjectNames = [],
state,
getUseRequestQueue = () => false,
domainProxyMap = new Map<Domain, NetworkProxy>(),
}: {
hasPermissions?: boolean;
state?: SelectedNetworkControllerState;
getSubjectNames?: string[];
getUseRequestQueue?: GetUseRequestQueue;
domainProxyMap?: Map<Domain, NetworkProxy>;
} = {}) => {
const mockProviderProxy = {
setTarget: jest.fn(),
Expand Down Expand Up @@ -143,6 +147,7 @@ const setup = ({
messenger: selectedNetworkControllerMessenger,
state,
getUseRequestQueue,
domainProxyMap,
});
return {
controller,
Expand Down

0 comments on commit 319bcfc

Please sign in to comment.