diff --git a/packages/gas-fee-controller/src/GasFeeController.test.ts b/packages/gas-fee-controller/src/GasFeeController.test.ts index 9f9e8d74c89..562589f8064 100644 --- a/packages/gas-fee-controller/src/GasFeeController.test.ts +++ b/packages/gas-fee-controller/src/GasFeeController.test.ts @@ -63,7 +63,16 @@ type AllEvents = AllGasFeeControllerEvents | AllNetworkControllerEvents; type RootMessenger = Messenger; const getRootMessenger = (): RootMessenger => { - return new Messenger({ namespace: MOCK_ANY_NAMESPACE }); + const rootMessenger = new Messenger< + MockAnyNamespace, + MessengerActions, + MessengerEvents + >({ namespace: MOCK_ANY_NAMESPACE }); + rootMessenger.registerActionHandler( + 'ErrorReportingService:captureException', + jest.fn(), + ); + return rootMessenger; }; const setupNetworkController = async ({ @@ -86,6 +95,10 @@ const setupNetworkController = async ({ namespace: 'NetworkController', parent: rootMessenger, }); + rootMessenger.delegate({ + messenger: networkControllerMessenger, + actions: ['ErrorReportingService:captureException'], + }); const infuraProjectId = '123'; diff --git a/packages/network-controller/CHANGELOG.md b/packages/network-controller/CHANGELOG.md index 2a91681de86..879bc61d255 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -13,6 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Providers accessible either via network clients or global proxies no longer emit events (or inherit from EventEmitter, for that matter). - Bump `@metamask/controller-utils` from `^11.14.1` to `^11.15.0` ([#7003](https://github.com/MetaMask/core/pull/7003)) +### Fixed + +- Ensure `networksMetadata` never references old network client IDs ([#7047](https://github.com/MetaMask/core/pull/7047)) + - When removing a network configuration, ensure that metadata for all RPC endpoints in the network configuration are also removed from `networksMetadata` + - When initializing the controller, remove metadata for RPC endpoints in `networksMetadata` that are not present in a network configuration + ## [25.0.0] ### Changed diff --git a/packages/network-controller/src/NetworkController.ts b/packages/network-controller/src/NetworkController.ts index 2449fe7c53d..e8204301abc 100644 --- a/packages/network-controller/src/NetworkController.ts +++ b/packages/network-controller/src/NetworkController.ts @@ -1082,12 +1082,17 @@ function correctInitialState( const networkConfigurationsSortedByChainId = getNetworkConfigurations( state, ).sort((a, b) => a.chainId.localeCompare(b.chainId)); - const networkClientIds = getAvailableNetworkClientIds( + const availableNetworkClientIds = getAvailableNetworkClientIds( networkConfigurationsSortedByChainId, ); + const invalidNetworkClientIdsWithMetadata = Object.keys( + state.networksMetadata, + ).filter( + (networkClientId) => !availableNetworkClientIds.includes(networkClientId), + ); return produce(state, (newState) => { - if (!networkClientIds.includes(state.selectedNetworkClientId)) { + if (!availableNetworkClientIds.includes(state.selectedNetworkClientId)) { const firstNetworkConfiguration = networkConfigurationsSortedByChainId[0]; const newSelectedNetworkClientId = firstNetworkConfiguration.rpcEndpoints[ @@ -1101,6 +1106,18 @@ function correctInitialState( ); newState.selectedNetworkClientId = newSelectedNetworkClientId; } + + if (invalidNetworkClientIdsWithMetadata.length > 0) { + for (const invalidNetworkClientId of invalidNetworkClientIdsWithMetadata) { + delete newState.networksMetadata[invalidNetworkClientId]; + } + messenger.call( + 'ErrorReportingService:captureException', + new Error( + '`networksMetadata` had invalid network client IDs, which have been removed', + ), + ); + } }); } @@ -2401,6 +2418,10 @@ export class NetworkController extends BaseController< mode: 'remove', existingNetworkConfiguration, }); + + for (const rpcEndpoint of existingNetworkConfiguration.rpcEndpoints) { + delete state.networksMetadata[rpcEndpoint.networkClientId]; + } }); this.messenger.publish( diff --git a/packages/network-controller/tests/NetworkController.test.ts b/packages/network-controller/tests/NetworkController.test.ts index ab7ea3aa877..77be127ecbd 100644 --- a/packages/network-controller/tests/NetworkController.test.ts +++ b/packages/network-controller/tests/NetworkController.test.ts @@ -350,90 +350,111 @@ describe('NetworkController', () => { ); }); - describe('if selectedNetworkClientId does not match the networkClientId of an RPC endpoint in networkConfigurationsByChainId', () => { - it('corrects selectedNetworkClientId to the default RPC endpoint of the first chain', () => { - const messenger = buildRootMessenger(); - messenger.registerActionHandler( - 'ErrorReportingService:captureException', - jest.fn(), - ); - const controllerMessenger = buildNetworkControllerMessenger(messenger); - const controller = new NetworkController({ - messenger: controllerMessenger, - state: { - selectedNetworkClientId: 'nonexistent', - networkConfigurationsByChainId: { - '0x1': buildCustomNetworkConfiguration({ - chainId: '0x1', - defaultRpcEndpointIndex: 1, - rpcEndpoints: [ - buildCustomRpcEndpoint({ - networkClientId: 'AAAA-AAAA-AAAA-AAAA', - }), - buildCustomRpcEndpoint({ - networkClientId: 'BBBB-BBBB-BBBB-BBBB', - }), - ], - }), - '0x2': buildCustomNetworkConfiguration({ chainId: '0x2' }), - '0x3': buildCustomNetworkConfiguration({ chainId: '0x3' }), - }, - }, - infuraProjectId: 'infura-project-id', - getRpcServiceOptions: () => ({ - fetch, - btoa, - }), - }); + it('corrects an invalid selectedNetworkClientId to the default RPC endpoint of the first chain, logging this fact', () => { + const captureExceptionMock = jest.fn(); + const messenger = buildRootMessenger({ + actionHandlers: { + 'ErrorReportingService:captureException': captureExceptionMock, + }, + }); + const controllerMessenger = buildNetworkControllerMessenger(messenger); - expect(controller.state.selectedNetworkClientId).toBe( - 'BBBB-BBBB-BBBB-BBBB', - ); + const controller = new NetworkController({ + messenger: controllerMessenger, + state: { + selectedNetworkClientId: 'nonexistent', + networkConfigurationsByChainId: { + '0x1': buildCustomNetworkConfiguration({ + chainId: '0x1', + defaultRpcEndpointIndex: 1, + rpcEndpoints: [ + buildCustomRpcEndpoint({ + networkClientId: 'AAAA-AAAA-AAAA-AAAA', + }), + buildCustomRpcEndpoint({ + networkClientId: 'BBBB-BBBB-BBBB-BBBB', + }), + ], + }), + '0x2': buildCustomNetworkConfiguration({ chainId: '0x2' }), + '0x3': buildCustomNetworkConfiguration({ chainId: '0x3' }), + }, + }, + infuraProjectId: 'infura-project-id', + getRpcServiceOptions: () => ({ + fetch, + btoa, + }), }); - it('logs a Sentry error', () => { - const messenger = buildRootMessenger(); - const captureExceptionMock = jest.fn(); - messenger.registerActionHandler( - 'ErrorReportingService:captureException', - captureExceptionMock, - ); - const controllerMessenger = buildNetworkControllerMessenger(messenger); + expect(controller.state.selectedNetworkClientId).toBe( + 'BBBB-BBBB-BBBB-BBBB', + ); + expect(captureExceptionMock).toHaveBeenCalledWith( + new Error( + "`selectedNetworkClientId` 'nonexistent' does not refer to an RPC endpoint within a network configuration; correcting to 'BBBB-BBBB-BBBB-BBBB'", + ), + ); + }); - new NetworkController({ - messenger: controllerMessenger, - state: { - selectedNetworkClientId: 'nonexistent', - networkConfigurationsByChainId: { - '0x1': buildCustomNetworkConfiguration({ - chainId: '0x1', - defaultRpcEndpointIndex: 1, - rpcEndpoints: [ - buildCustomRpcEndpoint({ - networkClientId: 'AAAA-AAAA-AAAA-AAAA', - }), - buildCustomRpcEndpoint({ - networkClientId: 'BBBB-BBBB-BBBB-BBBB', - }), - ], - }), - '0x2': buildCustomNetworkConfiguration({ chainId: '0x2' }), - '0x3': buildCustomNetworkConfiguration({ chainId: '0x3' }), + it('removes invalid network client IDs from networksMetadata, logging this fact', () => { + const captureExceptionMock = jest.fn(); + const messenger = buildRootMessenger({ + actionHandlers: { + 'ErrorReportingService:captureException': captureExceptionMock, + }, + }); + const controllerMessenger = buildNetworkControllerMessenger(messenger); + + const controller = new NetworkController({ + messenger: controllerMessenger, + state: { + selectedNetworkClientId: InfuraNetworkType.sepolia, + networkConfigurationsByChainId: { + [ChainId.sepolia]: buildInfuraNetworkConfiguration( + InfuraNetworkType.sepolia, + ), + }, + networksMetadata: { + [InfuraNetworkType.sepolia]: { + status: NetworkStatus.Available, + EIPS: {}, + }, + 'AAAA-AAAA-AAAA-AAAA': { + status: NetworkStatus.Available, + EIPS: {}, + }, + 'BBBB-BBBB-BBBB-BBBB': { + status: NetworkStatus.Available, + EIPS: {}, + }, + 'CCCC-CCCC-CCCC-CCCC': { + status: NetworkStatus.Available, + EIPS: {}, }, }, - infuraProjectId: 'infura-project-id', - getRpcServiceOptions: () => ({ - fetch, - btoa, - }), - }); - - expect(captureExceptionMock).toHaveBeenCalledWith( - new Error( - "`selectedNetworkClientId` 'nonexistent' does not refer to an RPC endpoint within a network configuration; correcting to 'BBBB-BBBB-BBBB-BBBB'", - ), - ); + }, + infuraProjectId: 'infura-project-id', + getRpcServiceOptions: () => ({ + fetch, + btoa, + }), }); + + expect(controller.state.networksMetadata).not.toHaveProperty( + 'AAAA-AAAA-AAAA-AAAA', + ); + expect(controller.state.networksMetadata).not.toHaveProperty( + 'BBBB-BBBB-BBBB-BBBB', + ); + expect(controller.state.networksMetadata).not.toHaveProperty( + 'CCCC-CCCC-CCCC-CCCC', + ); + expect(captureExceptionMock).toHaveBeenCalledWith( + new Error( + '`networksMetadata` had invalid network client IDs, which have been removed', + ), + ); }); const invalidInfuraProjectIds = [undefined, null, {}, 1]; @@ -831,7 +852,7 @@ describe('NetworkController', () => { }, }, networksMetadata: { - mainnet: { + [TESTNET.networkType]: { EIPS: { 1559: true }, status: NetworkStatus.Unknown, }, @@ -865,7 +886,7 @@ describe('NetworkController', () => { }, }, "networksMetadata": Object { - "mainnet": Object { + "sepolia": Object { "EIPS": Object { "1559": true, }, @@ -13130,6 +13151,41 @@ describe('NetworkController', () => { ); }); + it('removes the existing metadata for the network from state', async () => { + await withController( + { + state: { + selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA', + networkConfigurationsByChainId: { + [infuraChainId]: + buildInfuraNetworkConfiguration(infuraNetworkType), + '0x1337': buildCustomNetworkConfiguration({ + chainId: '0x1337', + rpcEndpoints: [ + buildCustomRpcEndpoint({ + networkClientId: 'AAAA-AAAA-AAAA-AAAA', + }), + ], + }), + }, + networksMetadata: { + [infuraNetworkType]: { + status: NetworkStatus.Available, + EIPS: {}, + }, + }, + }, + }, + ({ controller }) => { + controller.removeNetwork(infuraChainId); + + expect(controller.state.networksMetadata).not.toHaveProperty( + infuraNetworkType, + ); + }, + ); + }); + it('destroys and unregisters the network clients for each of the RPC endpoints defined in the network configuration (even the Infura endpoint)', async () => { const defaultRpcEndpoint = buildInfuraRpcEndpoint(infuraNetworkType); @@ -13190,7 +13246,7 @@ describe('NetworkController', () => { } describe('given the ID of a non-Infura-supported chain', () => { - it('removes the existing network configuration', async () => { + it('removes the existing network configuration from state', async () => { await withController( { state: { @@ -13217,6 +13273,61 @@ describe('NetworkController', () => { ); }); + it('removes the existing metadata for all RPC endpoints in the network from state', async () => { + await withController( + { + state: { + selectedNetworkClientId: TESTNET.networkType, + networkConfigurationsByChainId: { + '0x1337': buildCustomNetworkConfiguration({ + rpcEndpoints: [ + buildCustomRpcEndpoint({ + networkClientId: 'AAAA-AAAA-AAAA-AAAA', + }), + buildCustomRpcEndpoint({ + networkClientId: 'BBBB-BBBB-BBBB-BBBB', + }), + buildCustomRpcEndpoint({ + networkClientId: 'CCCC-CCCC-CCCC-CCCC', + }), + ], + }), + [TESTNET.chainId]: buildInfuraNetworkConfiguration( + TESTNET.networkType, + ), + }, + networksMetadata: { + 'AAAA-AAAA-AAAA-AAAA': { + status: NetworkStatus.Available, + EIPS: {}, + }, + 'BBBB-BBBB-BBBB-BBBB': { + status: NetworkStatus.Available, + EIPS: {}, + }, + 'CCCC-CCCC-CCCC-CCCC': { + status: NetworkStatus.Available, + EIPS: {}, + }, + }, + }, + }, + ({ controller }) => { + controller.removeNetwork('0x1337'); + + expect(controller.state.networksMetadata).not.toHaveProperty( + 'AAAA-AAAA-AAAA-AAAA', + ); + expect(controller.state.networksMetadata).not.toHaveProperty( + 'BBBB-BBBB-BBBB-BBBB', + ); + expect(controller.state.networksMetadata).not.toHaveProperty( + 'CCCC-CCCC-CCCC-CCCC', + ); + }, + ); + }); + it('destroys the network clients for each of the RPC endpoints defined in the network configuration', async () => { await withController( { @@ -15571,7 +15682,7 @@ function lookupNetworkTests({ state: { ...initialState, networksMetadata: { - mainnet: { + [expectedNetworkClientId]: { EIPS: { 1559: false }, status: NetworkStatus.Unknown, }, @@ -15614,7 +15725,7 @@ function lookupNetworkTests({ state: { ...initialState, networksMetadata: { - mainnet: { + [expectedNetworkClientId]: { EIPS: { 1559: true }, status: NetworkStatus.Unknown, }, diff --git a/packages/network-controller/tests/helpers.ts b/packages/network-controller/tests/helpers.ts index bff0d7f5010..a709fb799c6 100644 --- a/packages/network-controller/tests/helpers.ts +++ b/packages/network-controller/tests/helpers.ts @@ -5,6 +5,7 @@ import { NetworksTicker, toHex, } from '@metamask/controller-utils'; +import type { ErrorReportingServiceCaptureExceptionAction } from '@metamask/error-reporting-service'; import { Messenger, type MockAnyNamespace, @@ -82,10 +83,29 @@ export const TESTNET = { * Build a root messenger that includes all events used by the network * controller. * + * @param options - Options. + * @param options.actionHandlers - Handlers for actions that are pre-registered + * on the messenger. * @returns The messenger. */ -export function buildRootMessenger(): RootMessenger { - return new Messenger({ namespace: MOCK_ANY_NAMESPACE }); +export function buildRootMessenger({ + actionHandlers = {}, +}: { + actionHandlers?: { + 'ErrorReportingService:captureException'?: ErrorReportingServiceCaptureExceptionAction['handler']; + }; +} = {}): RootMessenger { + const rootMessenger = new Messenger< + MockAnyNamespace, + MessengerActions, + MessengerEvents + >({ namespace: MOCK_ANY_NAMESPACE }); + rootMessenger.registerActionHandler( + 'ErrorReportingService:captureException', + actionHandlers['ErrorReportingService:captureException'] ?? + ((error) => console.error(error)), + ); + return rootMessenger; } /**