From ad4ec39d95890352faa0c54ef6ce527380a3503f Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Fri, 15 Nov 2024 15:13:45 +0000 Subject: [PATCH 1/3] feat: use original networkClientId if provided when adding a network this is to support network syncing, where we add a new network to a synced device --- .../src/NetworkController.ts | 8 ++++-- .../tests/NetworkController.test.ts | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/network-controller/src/NetworkController.ts b/packages/network-controller/src/NetworkController.ts index 6b42d86d27..8da6233896 100644 --- a/packages/network-controller/src/NetworkController.ts +++ b/packages/network-controller/src/NetworkController.ts @@ -212,8 +212,11 @@ export type NetworkConfiguration = { * Custom RPC endpoints do not need a `networkClientId` property because it is * assumed that they have not already been added and therefore network clients * do not exist for them yet (and hence IDs need to be generated). + * + * However Custom RPC endpoints, that are synchronized between devices, + * can contain a `networkClientId` set on both devices. */ -export type AddNetworkCustomRpcEndpointFields = Omit< +export type AddNetworkCustomRpcEndpointFields = Partialize< CustomRpcEndpoint, 'networkClientId' >; @@ -1557,7 +1560,8 @@ export class NetworkController extends BaseController< defaultOrCustomRpcEndpointFields.type === RpcEndpointType.Custom ? { ...defaultOrCustomRpcEndpointFields, - networkClientId: uuidV4(), + networkClientId: + defaultOrCustomRpcEndpointFields.networkClientId ?? uuidV4(), } : defaultOrCustomRpcEndpointFields; return { diff --git a/packages/network-controller/tests/NetworkController.test.ts b/packages/network-controller/tests/NetworkController.test.ts index 24665f9c88..93307587f4 100644 --- a/packages/network-controller/tests/NetworkController.test.ts +++ b/packages/network-controller/tests/NetworkController.test.ts @@ -3844,6 +3844,32 @@ describe('NetworkController', () => { }); }); }); + + it('adds the network with an existing networkClientId', async () => { + // Arrange + const customNetworkClientId = 'my-custom-client-config'; + const networkConfigToAdd = buildCustomNetworkConfiguration({ + chainId: '0x1337', + rpcEndpoints: [ + buildCustomRpcEndpoint({ + networkClientId: customNetworkClientId, + url: 'https://test.endpoint/1', + }), + ], + }); + + // Act + const result = await withController(({ controller }) => { + const newNetworkConfig = controller.addNetwork(networkConfigToAdd); + return newNetworkConfig; + }); + + // Assert - ensure that we use existing networkClientId & do not call uuidV4() + expect(result.rpcEndpoints[0].networkClientId).toBe( + customNetworkClientId, + ); + expect(uuidV4Mock).not.toHaveBeenCalled(); + }); }); }); From 6e6596179cf438d6aec8637692a7d7c2fde865f6 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Tue, 19 Nov 2024 10:22:26 +0000 Subject: [PATCH 2/3] feat: add dangerouslySetNetworkConfiguration method to override networkConfiguration this is used for network syncing to override local state with the remove state. --- .../src/NetworkController.ts | 138 +++++++++++- packages/network-controller/src/index.ts | 1 + .../tests/NetworkController.test.ts | 199 ++++++++++++++++++ 3 files changed, 337 insertions(+), 1 deletion(-) diff --git a/packages/network-controller/src/NetworkController.ts b/packages/network-controller/src/NetworkController.ts index 8da6233896..27339ad37d 100644 --- a/packages/network-controller/src/NetworkController.ts +++ b/packages/network-controller/src/NetworkController.ts @@ -476,6 +476,11 @@ export type NetworkControllerGetNetworkConfigurationByNetworkClientId = { handler: NetworkController['getNetworkConfigurationByNetworkClientId']; }; +export type NetworkControllerDangerouslySetNetworkConfigurationAction = { + type: 'NetworkController:dangerouslySetNetworkConfiguration'; + handler: NetworkController['dangerouslySetNetworkConfiguration']; +}; + export type NetworkControllerActions = | NetworkControllerGetStateAction | NetworkControllerGetEthQueryAction @@ -486,7 +491,8 @@ export type NetworkControllerActions = | NetworkControllerSetActiveNetworkAction | NetworkControllerSetProviderTypeAction | NetworkControllerGetNetworkConfigurationByChainId - | NetworkControllerGetNetworkConfigurationByNetworkClientId; + | NetworkControllerGetNetworkConfigurationByNetworkClientId + | NetworkControllerDangerouslySetNetworkConfigurationAction; export type NetworkControllerMessenger = RestrictedControllerMessenger< typeof controllerName, @@ -957,6 +963,13 @@ export class NetworkController extends BaseController< `${this.name}:getSelectedNetworkClient`, this.getSelectedNetworkClient.bind(this), ); + + this.messagingSystem.registerActionHandler( + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `${this.name}:dangerouslySetNetworkConfiguration`, + this.dangerouslySetNetworkConfiguration.bind(this), + ); } /** @@ -1945,6 +1958,129 @@ export class NetworkController extends BaseController< ); } + /** + * This is used to override an existing network configuration. + * This is only meant for internal use only and not to be exposed via the UI. + * It is used as part of "Network Syncing", to sync networks, RPCs and block explorers cross devices. + * + * This will subsequently update the network client registry; state.networksMetadata, and state.selectedNetworkClientId + * @param networkConfiguration - the network configuration to override + */ + async dangerouslySetNetworkConfiguration( + networkConfiguration: NetworkConfiguration, + ) { + const prevNetworkConfig: NetworkConfiguration | undefined = + networkConfiguration.chainId in this.state.networkConfigurationsByChainId + ? this.state.networkConfigurationsByChainId[ + networkConfiguration.chainId + ] + : undefined; + + if (!prevNetworkConfig) { + // We only want to perform overrides, not add new network configurations + return; + } + + // Update Registry (remove old and add new) + const updateRegistry = () => { + // Unregister old networks we want to override + const autoManagedNetworkClientRegistry = + this.#ensureAutoManagedNetworkClientRegistryPopulated(); + const networkClientRemoveOperations = prevNetworkConfig.rpcEndpoints.map( + (rpcEndpoint) => { + return { + type: 'remove' as const, + rpcEndpoint, + }; + }, + ); + this.#unregisterNetworkClientsAsNeeded({ + networkClientOperations: networkClientRemoveOperations, + autoManagedNetworkClientRegistry, + }); + + // Register new networks we want to override + const networkClientAddOperations = networkConfiguration.rpcEndpoints.map( + (rpcEndpoint) => { + return { + type: 'add' as const, + rpcEndpoint, + }; + }, + ); + this.#registerNetworkClientsAsNeeded({ + networkFields: networkConfiguration, + networkClientOperations: networkClientAddOperations, + autoManagedNetworkClientRegistry, + }); + }; + + // Replace the networkConfiguration with our new networkConfiguration + // This is a full replace (no merging) + const replaceNetworkConfiguration = () => { + // Update State + this.update((state) => { + state.networkConfigurationsByChainId[networkConfiguration.chainId] = + networkConfiguration; + }); + + // Update Cache + this.#networkConfigurationsByNetworkClientId = + buildNetworkConfigurationsByNetworkClientId( + this.state.networkConfigurationsByChainId, + ); + }; + + // Updates the NetworksMetadata State + const updateNetworksMetadata = async () => { + // Remove old metadata state + this.update((state) => { + prevNetworkConfig.rpcEndpoints.forEach((r) => { + if (state.networksMetadata?.[r.networkClientId]) { + delete state.networksMetadata[r.networkClientId]; + } + }); + }); + + // Add new metadata state + for (const r of networkConfiguration.rpcEndpoints) { + await this.lookupNetwork(r.networkClientId); + } + }; + + // Update selectedNetworkId State + // Will try to keep the same OR will select a new RPC from new network OR any network (edge case) + const updateSelectedNetworkId = async () => { + const selectedClientId = this.state.selectedNetworkClientId; + const wasClientIdReplaced = prevNetworkConfig.rpcEndpoints.some( + (r) => r.networkClientId === selectedClientId, + ); + const doesExistInNewNetwork = networkConfiguration.rpcEndpoints.some( + (r) => r.networkClientId === selectedClientId, + ); + + const shouldUpdateSelectedNetworkId = + wasClientIdReplaced && !doesExistInNewNetwork; + if (shouldUpdateSelectedNetworkId) { + // Update the clientId to "something" that exists + const newRPCClientId = networkConfiguration.rpcEndpoints.find( + (r) => r.networkClientId in this.state.networksMetadata, + )?.networkClientId; + const anyRPCClientId = Object.keys(this.state.networksMetadata)[0]; + /* istanbul ignore next: anyRPCClientId and selectedClientId are fallbacks and should be impossible to reach */ + const newlySelectedNetwork = + newRPCClientId ?? anyRPCClientId ?? selectedClientId; + await this.#refreshNetwork(newlySelectedNetwork); + } + }; + + // Execute Set Network Config + updateRegistry(); + replaceNetworkConfiguration(); + await updateNetworksMetadata(); + await updateSelectedNetworkId(); + } + /** * Assuming that the network has been previously switched, switches to this * new network. diff --git a/packages/network-controller/src/index.ts b/packages/network-controller/src/index.ts index e05622fc65..07b4dfe65c 100644 --- a/packages/network-controller/src/index.ts +++ b/packages/network-controller/src/index.ts @@ -26,6 +26,7 @@ export type { NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerSetProviderTypeAction, NetworkControllerSetActiveNetworkAction, + NetworkControllerDangerouslySetNetworkConfigurationAction, NetworkControllerGetNetworkConfigurationByNetworkClientId, NetworkControllerActions, NetworkControllerMessenger, diff --git a/packages/network-controller/tests/NetworkController.test.ts b/packages/network-controller/tests/NetworkController.test.ts index 93307587f4..5212d231ef 100644 --- a/packages/network-controller/tests/NetworkController.test.ts +++ b/packages/network-controller/tests/NetworkController.test.ts @@ -11436,6 +11436,205 @@ describe('NetworkController', () => { }); }); + describe('dangerouslySetNetworkConfiguration', () => { + const TEST_CHAIN_ID = '0x1337'; + const ORIGINAL_NETWORK_CLIENT_ID = '1111'; + + const arrangeTestUtils = (props: { newNetworkClientIds: string[] }) => { + mockCreateNetworkClient().mockReturnValue(buildFakeClient()); + + const originalNetwork = buildCustomNetworkConfiguration({ + chainId: TEST_CHAIN_ID, + rpcEndpoints: [ + buildCustomRpcEndpoint({ + networkClientId: ORIGINAL_NETWORK_CLIENT_ID, + }), + ], + }); + + const overrideNetwork = buildCustomNetworkConfiguration({ + chainId: TEST_CHAIN_ID, + rpcEndpoints: props.newNetworkClientIds.map((id) => + buildCustomRpcEndpoint({ networkClientId: id }), + ), + }); + + const controllerState = + buildNetworkControllerStateWithDefaultSelectedNetworkClientId({ + networkConfigurationsByChainId: { + [originalNetwork.chainId]: originalNetwork, + }, + networksMetadata: { + [ORIGINAL_NETWORK_CLIENT_ID]: { + EIPS: { + '1559': true, + }, + status: NetworkStatus.Available, + }, + }, + }); + + return { originalNetwork, overrideNetwork, controllerState }; + }; + + const actTest = async ( + props: Pick< + ReturnType, + 'controllerState' | 'overrideNetwork' + >, + ) => { + const result = await withController( + { state: props.controllerState }, + async ({ controller }) => { + await controller.dangerouslySetNetworkConfiguration( + props.overrideNetwork, + ); + return { + state: controller.state, + controller, + }; + }, + ); + + return result; + }; + + const arrangeActTest = async (props: { newNetworkClientIds: string[] }) => { + const arrange = arrangeTestUtils(props); + + // Act + const result = await actTest({ + controllerState: arrange.controllerState, + overrideNetwork: arrange.overrideNetwork, + }); + return { ...arrange, result }; + }; + + const assertStateHasBeenUpdated = (props: { + state: NetworkState; + newNetworkConfiguration: NetworkConfiguration; + networkClientIdsRemoved: string[]; + }) => { + const { state, newNetworkConfiguration, networkClientIdsRemoved } = props; + + // Assert - new network config has been set + expect(state.networkConfigurationsByChainId[TEST_CHAIN_ID]).toStrictEqual( + newNetworkConfiguration, + ); + + // Assert - networks metadata removed some endpoints (from original network) + networkClientIdsRemoved.forEach((id) => { + expect(state.networksMetadata[id]).toBeUndefined(); + }); + + // Assert - networks metadata has been set + newNetworkConfiguration.rpcEndpoints.forEach((r) => { + expect(state.networksMetadata[r.networkClientId]).toBeDefined(); + }); + }; + + const assertNetworkRegistryHasBeenUpdated = (props: { + controller: NetworkController; + newNetworkConfiguration: NetworkConfiguration; + networkClientIdsRemoved: string[]; + }) => { + const { controller, newNetworkConfiguration, networkClientIdsRemoved } = + props; + const registry = controller.getNetworkClientRegistry(); + + // Assert - networks that were removed (from original network that was overwritten) + networkClientIdsRemoved.forEach((id) => { + expect(registry[id]).toBeUndefined(); + }); + + // Assert - new network config RPCs has been updated in the network registry + newNetworkConfiguration.rpcEndpoints.forEach((r) => { + expect(registry[r.networkClientId]).toBeDefined(); + }); + }; + + const assertNetworkConfigurationsByIdCacheHasBeenUpdated = (props: { + controller: NetworkController; + newNetworkConfiguration: NetworkConfiguration; + }) => { + const { controller, newNetworkConfiguration } = props; + expect( + controller.getNetworkConfigurationByChainId(TEST_CHAIN_ID), + ).toStrictEqual(newNetworkConfiguration); + }; + + it('overrides a set network configuration', async () => { + const { result, overrideNetwork } = await arrangeActTest({ + newNetworkClientIds: ['2222', '3333'], + }); + + assertStateHasBeenUpdated({ + state: result.state, + newNetworkConfiguration: overrideNetwork, + networkClientIdsRemoved: [ORIGINAL_NETWORK_CLIENT_ID], + }); + assertNetworkRegistryHasBeenUpdated({ + controller: result.controller, + newNetworkConfiguration: overrideNetwork, + networkClientIdsRemoved: [ORIGINAL_NETWORK_CLIENT_ID], + }); + assertNetworkConfigurationsByIdCacheHasBeenUpdated({ + controller: result.controller, + newNetworkConfiguration: overrideNetwork, + }); + + // Selected network has changed (since original was removed) + // We will select next available RPC for the given chain + expect(result.state.selectedNetworkClientId).toBe('2222'); + }); + + it('overrides network config, but keeps same selected network', async () => { + const { result, overrideNetwork } = await arrangeActTest({ + newNetworkClientIds: [ORIGINAL_NETWORK_CLIENT_ID, '2222'], + }); + + assertStateHasBeenUpdated({ + state: result.state, + newNetworkConfiguration: overrideNetwork, + // no networks were removed, as the new network config contains the original network client id + networkClientIdsRemoved: [], + }); + assertNetworkRegistryHasBeenUpdated({ + controller: result.controller, + newNetworkConfiguration: overrideNetwork, + // no networks were removed, as the new network config contains the original network client id + networkClientIdsRemoved: [], + }); + assertNetworkConfigurationsByIdCacheHasBeenUpdated({ + controller: result.controller, + newNetworkConfiguration: overrideNetwork, + }); + + // selected RPC has not changed, as it was not removed + expect(result.state.selectedNetworkClientId).toBe( + ORIGINAL_NETWORK_CLIENT_ID, + ); + }); + + it('does nothing if there is no network to override', async () => { + const { controllerState, overrideNetwork } = arrangeTestUtils({ + newNetworkClientIds: ['2222'], + }); + + // shim/mock the controller state to not contain a network we are overriding + controllerState.networkConfigurationsByChainId['0xDiffChain'] = + controllerState.networkConfigurationsByChainId[TEST_CHAIN_ID]; + delete controllerState.networkConfigurationsByChainId[TEST_CHAIN_ID]; + controllerState.networkConfigurationsByChainId['0xDiffChain'].chainId = + '0xDiffChain'; + + const result = await actTest({ controllerState, overrideNetwork }); + + // No changes should have occurred + expect(result.state).toStrictEqual(controllerState); + }); + }); + describe('rollbackToPreviousProvider', () => { describe('when called not following any network switches', () => { for (const infuraNetworkType of Object.values(InfuraNetworkType)) { From 6b693352f2996050a4f2515fa412bcbdc2c28e74 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Wed, 20 Nov 2024 14:42:05 +0000 Subject: [PATCH 3/3] refactor: make dangerouslySetNetworkConfiguration a private method This is now only accessible through the messaging systems --- packages/network-controller/src/NetworkController.ts | 6 +++--- packages/network-controller/tests/NetworkController.test.ts | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/network-controller/src/NetworkController.ts b/packages/network-controller/src/NetworkController.ts index 27339ad37d..15828164ba 100644 --- a/packages/network-controller/src/NetworkController.ts +++ b/packages/network-controller/src/NetworkController.ts @@ -478,7 +478,7 @@ export type NetworkControllerGetNetworkConfigurationByNetworkClientId = { export type NetworkControllerDangerouslySetNetworkConfigurationAction = { type: 'NetworkController:dangerouslySetNetworkConfiguration'; - handler: NetworkController['dangerouslySetNetworkConfiguration']; + handler: (networkConfiguration: NetworkConfiguration) => Promise; }; export type NetworkControllerActions = @@ -968,7 +968,7 @@ export class NetworkController extends BaseController< // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/restrict-template-expressions `${this.name}:dangerouslySetNetworkConfiguration`, - this.dangerouslySetNetworkConfiguration.bind(this), + this.#dangerouslySetNetworkConfiguration.bind(this), ); } @@ -1966,7 +1966,7 @@ export class NetworkController extends BaseController< * This will subsequently update the network client registry; state.networksMetadata, and state.selectedNetworkClientId * @param networkConfiguration - the network configuration to override */ - async dangerouslySetNetworkConfiguration( + async #dangerouslySetNetworkConfiguration( networkConfiguration: NetworkConfiguration, ) { const prevNetworkConfig: NetworkConfiguration | undefined = diff --git a/packages/network-controller/tests/NetworkController.test.ts b/packages/network-controller/tests/NetworkController.test.ts index 5212d231ef..665e9ea2e9 100644 --- a/packages/network-controller/tests/NetworkController.test.ts +++ b/packages/network-controller/tests/NetworkController.test.ts @@ -11485,8 +11485,9 @@ describe('NetworkController', () => { ) => { const result = await withController( { state: props.controllerState }, - async ({ controller }) => { - await controller.dangerouslySetNetworkConfiguration( + async ({ controller, messenger }) => { + await messenger.call( + 'NetworkController:dangerouslySetNetworkConfiguration', props.overrideNetwork, ); return {