diff --git a/packages/config-registry-controller/CHANGELOG.md b/packages/config-registry-controller/CHANGELOG.md index fa33519f175..a006400c833 100644 --- a/packages/config-registry-controller/CHANGELOG.md +++ b/packages/config-registry-controller/CHANGELOG.md @@ -9,13 +9,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Initial release of `@metamask/config-registry-controller` ([#7668](https://github.com/MetaMask/core/pull/7668)) - - Controller for fetching and managing network configurations from a remote API - - ConfigRegistryApiService with ETag support, retries, circuit breaker, and timeout handling - - Network filtering to only include featured, active, non-testnet networks - - Feature flag integration using `config_registry_api_enabled` to enable/disable API fetching - - Fallback configuration support when API is unavailable or feature flag is disabled - - State persistence for configs, version, lastFetched, and etag - - Uses StaticIntervalPollingController for periodic updates (default: 24 hours) +- Initial release ([#7668](https://github.com/MetaMask/core/pull/7668), [#7809](https://github.com/MetaMask/core/pull/7809)) [Unreleased]: https://github.com/MetaMask/core/ diff --git a/packages/config-registry-controller/package.json b/packages/config-registry-controller/package.json index 5aeb58d9208..ceb92b1b6c9 100644 --- a/packages/config-registry-controller/package.json +++ b/packages/config-registry-controller/package.json @@ -55,7 +55,8 @@ "@metamask/profile-sync-controller": "^27.0.0", "@metamask/remote-feature-flag-controller": "^4.0.0", "@metamask/superstruct": "^3.1.0", - "@metamask/utils": "^11.9.0" + "@metamask/utils": "^11.9.0", + "reselect": "^5.1.1" }, "devDependencies": { "@lavamoat/allow-scripts": "^3.0.4", diff --git a/packages/config-registry-controller/src/ConfigRegistryController.test.ts b/packages/config-registry-controller/src/ConfigRegistryController.test.ts index 687735c3698..8b98ef81ef8 100644 --- a/packages/config-registry-controller/src/ConfigRegistryController.test.ts +++ b/packages/config-registry-controller/src/ConfigRegistryController.test.ts @@ -6,19 +6,18 @@ import type { import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import { useFakeTimers } from 'sinon'; +import type { RegistryNetworkConfig } from './config-registry-api-service'; +import type { FetchConfigResult } from './config-registry-api-service'; import type { - FetchConfigResult, - RegistryNetworkConfig, -} from './config-registry-api-service'; + ConfigRegistryMessenger, + ConfigRegistryState, +} from './ConfigRegistryController'; import { ConfigRegistryController, DEFAULT_POLLING_INTERVAL, } from './ConfigRegistryController'; -import type { - ConfigRegistryMessenger, - ConfigRegistryState, - NetworkConfigEntry, -} from './ConfigRegistryController'; +import { selectFeaturedNetworks, selectNetworks } from './selectors'; +import { createMockNetworkConfig } from './test-helpers'; import { advanceTime } from '../../../tests/helpers'; const namespace = 'ConfigRegistryController' as const; @@ -40,6 +39,7 @@ function getConfigRegistryControllerMessenger(): { } { const rootMessenger: RootMessenger = new Messenger({ namespace: MOCK_ANY_NAMESPACE, + captureException: jest.fn(), }); const configRegistryControllerMessenger: ConfigRegistryMessenger = @@ -65,55 +65,11 @@ function getConfigRegistryControllerMessenger(): { return { messenger: configRegistryControllerMessenger, rootMessenger }; } -/** - * Creates a mock RegistryNetworkConfig for testing. - * - * @param overrides - Optional properties to override in the default RegistryNetworkConfig. - * @returns A mock RegistryNetworkConfig object. - */ -function createMockNetworkConfig( - overrides: Partial = {}, -): RegistryNetworkConfig { - return { - chainId: '0x1', - name: 'Ethereum Mainnet', - nativeCurrency: 'ETH', - rpcEndpoints: [ - { - url: 'https://mainnet.infura.io/v3/{infuraProjectId}', - type: 'infura', - networkClientId: 'mainnet', - failoverUrls: [], - }, - ], - blockExplorerUrls: ['https://etherscan.io'], - defaultRpcEndpointIndex: 0, - defaultBlockExplorerUrlIndex: 0, - isActive: true, - isTestnet: false, - isDefault: true, - isFeatured: true, - isDeprecated: false, - priority: 0, - isDeletable: false, - ...overrides, - }; -} - -const MOCK_CONFIG_ENTRY: NetworkConfigEntry = { - key: 'test-key', - value: createMockNetworkConfig({ chainId: '0x1', name: 'Test Network' }), - metadata: { source: 'test' }, -}; - -const MOCK_FALLBACK_CONFIG: Record = { - 'fallback-key': { - key: 'fallback-key', - value: createMockNetworkConfig({ - chainId: '0x2', - name: 'Fallback Network', - }), - }, +const MOCK_FALLBACK_CONFIG: Record = { + 'fallback-key': createMockNetworkConfig({ + chainId: '0x2', + name: 'Fallback Network', + }), }; /** @@ -199,7 +155,7 @@ async function withController( mockRemoteFeatureFlagGetState, }); } finally { - controller.stopPolling(); + controller.stopAllPolling(); clock.restore(); mockApiServiceHandler.mockReset(); } @@ -213,19 +169,20 @@ describe('ConfigRegistryController', () => { configs: { networks: {} }, version: null, lastFetched: null, - fetchError: null, etag: null, }); }); }); it('sets initial state when provided', async () => { + const initialNetworks: Record = { + 'test-key': createMockNetworkConfig({ + chainId: '0x1', + name: 'Test Network', + }), + }; const initialState: Partial = { - configs: { - networks: { - 'test-key': MOCK_CONFIG_ENTRY, - }, - }, + configs: { networks: initialNetworks }, version: 'v1.0.0', lastFetched: 1234567890, }; @@ -234,7 +191,7 @@ describe('ConfigRegistryController', () => { { options: { state: initialState } }, ({ controller }) => { expect(controller.state.configs.networks).toStrictEqual( - initialState.configs?.networks, + initialNetworks, ); expect(controller.state.version).toBe('v1.0.0'); expect(controller.state.lastFetched).toBe(1234567890); @@ -269,7 +226,6 @@ describe('ConfigRegistryController', () => { configs: { networks: {} }, version: null, lastFetched: null, - fetchError: null, etag: null, }); }); @@ -285,7 +241,7 @@ describe('ConfigRegistryController', () => { await advanceTime({ clock, duration: 0 }); expect(executePollSpy).toHaveBeenCalledTimes(1); - controller.stopPolling(); + controller.stopAllPolling(); }); }); @@ -303,7 +259,7 @@ describe('ConfigRegistryController', () => { await advanceTime({ clock, duration: pollingInterval }); expect(executePollSpy).toHaveBeenCalledTimes(1); - controller.stopPolling(); + controller.stopAllPolling(); }, ); }); @@ -316,7 +272,7 @@ describe('ConfigRegistryController', () => { await advanceTime({ clock, duration: 0 }); executePollSpy.mockClear(); - controller.stopPolling(); + controller.stopAllPolling(); await advanceTime({ clock, duration: DEFAULT_POLLING_INTERVAL }); @@ -410,23 +366,18 @@ describe('ConfigRegistryController', () => { expect(testController.state.configs).toStrictEqual({ networks: MOCK_FALLBACK_CONFIG, }); - expect(testController.state.fetchError).toBe('Network error'); }, ); }); - it('sets fetchError when configs already exist (not use fallback)', async () => { - const existingConfigs = { - networks: { - 'existing-key': { - key: 'existing-key', - value: createMockNetworkConfig({ - chainId: '0x3', - name: 'Existing Network', - }), - }, - }, + it('keeps existing configs when fetch fails and configs already exist', async () => { + const existingNetworks: Record = { + 'existing-key': createMockNetworkConfig({ + chainId: '0x3', + name: 'Existing Network', + }), }; + const existingConfigs = { networks: existingNetworks }; await withController( { @@ -512,8 +463,9 @@ describe('ConfigRegistryController', () => { expect(captureExceptionSpy).toHaveBeenCalledWith( expect.objectContaining({ message: 'Network error' }), ); - expect(testController.state.configs).toStrictEqual(existingConfigs); - expect(testController.state.fetchError).toBe('Network error'); + expect(testController.state.configs.networks).toStrictEqual( + existingNetworks, + ); }, ); }); @@ -604,21 +556,13 @@ describe('ConfigRegistryController', () => { expect(testController.state.configs).toStrictEqual({ networks: MOCK_FALLBACK_CONFIG, }); - expect(testController.state.fetchError).toBe('Network error'); expect(mockRemoteFeatureFlagGetState).toHaveBeenCalled(); }, ); }); - it('handles unmodified response and clears fetchError', async () => { + it('handles unmodified response and updates lastFetched and etag', async () => { await withController( - { - options: { - state: { - fetchError: 'Previous error', - }, - }, - }, async ({ controller, mockRemoteFeatureFlagGetState, @@ -640,7 +584,6 @@ describe('ConfigRegistryController', () => { await controller._executePoll(null); const afterTimestamp = Date.now(); - expect(controller.state.fetchError).toBeNull(); expect(controller.state.etag).toBe('"test-etag"'); expect(controller.state.lastFetched).not.toBeNull(); expect(controller.state.lastFetched).toBeGreaterThanOrEqual( @@ -658,7 +601,6 @@ describe('ConfigRegistryController', () => { { options: { state: { - fetchError: 'Previous error', etag: '"existing-etag"', }, }, @@ -683,7 +625,6 @@ describe('ConfigRegistryController', () => { await controller._executePoll(null); const afterTimestamp = Date.now(); - expect(controller.state.fetchError).toBeNull(); expect(controller.state.etag).toBe('"existing-etag"'); expect(controller.state.lastFetched).not.toBeNull(); expect(controller.state.lastFetched).toBeGreaterThanOrEqual( @@ -701,7 +642,6 @@ describe('ConfigRegistryController', () => { { options: { state: { - fetchError: 'Previous error', etag: '"existing-etag"', }, }, @@ -727,7 +667,6 @@ describe('ConfigRegistryController', () => { await controller._executePoll(null); const afterTimestamp = Date.now(); - expect(controller.state.fetchError).toBeNull(); expect(controller.state.etag).toBeNull(); expect(controller.state.lastFetched).not.toBeNull(); expect(controller.state.lastFetched).toBeGreaterThanOrEqual( @@ -827,9 +766,6 @@ describe('ConfigRegistryController', () => { message: 'Validation error from superstruct', }), ); - expect(testController.state.fetchError).toBe( - 'Validation error from superstruct', - ); }, ); }); @@ -838,6 +774,7 @@ describe('ConfigRegistryController', () => { await withController( async ({ controller, + rootMessenger, mockRemoteFeatureFlagGetState, mockApiServiceHandler, }) => { @@ -855,8 +792,10 @@ describe('ConfigRegistryController', () => { await controller._executePoll(null); - expect(controller.state.fetchError).toBe( - 'Validation error: data.data is missing', + expect(rootMessenger.captureException).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Validation error: data.data is missing', + }), ); }, ); @@ -866,6 +805,7 @@ describe('ConfigRegistryController', () => { await withController( async ({ controller, + rootMessenger, mockRemoteFeatureFlagGetState, mockApiServiceHandler, }) => { @@ -883,8 +823,10 @@ describe('ConfigRegistryController', () => { await controller._executePoll(null); - expect(controller.state.fetchError).toBe( - 'Validation error: data.data.networks is not an array', + expect(rootMessenger.captureException).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Validation error: data.data.networks is not an array', + }), ); }, ); @@ -894,6 +836,7 @@ describe('ConfigRegistryController', () => { await withController( async ({ controller, + rootMessenger, mockRemoteFeatureFlagGetState, mockApiServiceHandler, }) => { @@ -911,20 +854,23 @@ describe('ConfigRegistryController', () => { await controller._executePoll(null); - expect(controller.state.fetchError).toBe( - 'Validation error: data.data.version is not a string', + expect(rootMessenger.captureException).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Validation error: data.data.version is not a string', + }), ); }, ); }); - it('skips fetch when lastFetched is within polling interval', async () => { - const recentTimestamp = Date.now() - 1000; + it('skips fetch when lastFetched is in the future', async () => { + const now = Date.now(); + const futureTimestamp = now + 10000; await withController( { options: { state: { - lastFetched: recentTimestamp, + lastFetched: futureTimestamp, }, }, }, @@ -933,7 +879,7 @@ describe('ConfigRegistryController', () => { remoteFeatureFlags: { configRegistryApiEnabled: true, }, - cacheTimestamp: Date.now(), + cacheTimestamp: now, }); const fetchConfigSpy = jest.spyOn( @@ -1205,9 +1151,6 @@ describe('ConfigRegistryController', () => { expect(testController.state.configs).toStrictEqual({ networks: MOCK_FALLBACK_CONFIG, }); - expect(testController.state.fetchError).toBe( - 'Unknown error occurred', - ); }, ); }); @@ -1306,7 +1249,6 @@ describe('ConfigRegistryController', () => { expect(testController.state.configs).toStrictEqual({ networks: MOCK_FALLBACK_CONFIG, }); - expect(testController.state.fetchError).toBe('Network error'); }, ); }); @@ -1350,15 +1292,6 @@ describe('ConfigRegistryController', () => { }, ); }); - - it('persists fetchError', async () => { - await withController( - { options: { state: { fetchError: 'Test error' } } }, - ({ controller }) => { - expect(controller.state.fetchError).toBe('Test error'); - }, - ); - }); }); describe('startPolling', () => { @@ -1368,7 +1301,7 @@ describe('ConfigRegistryController', () => { expect(typeof token).toBe('string'); expect(token.length).toBeGreaterThan(0); - controller.stopPolling(); + controller.stopAllPolling(); }); }); @@ -1378,7 +1311,7 @@ describe('ConfigRegistryController', () => { expect(typeof token).toBe('string'); expect(token.length).toBeGreaterThan(0); - controller.stopPolling(); + controller.stopAllPolling(); }); }); @@ -1399,13 +1332,15 @@ describe('ConfigRegistryController', () => { const executePollSpy = jest.spyOn(controller, '_executePoll'); controller.startPolling(null); + // StaticIntervalPollingController runs first poll after 0ms (executePoll may early-return due to lastFetched) await advanceTime({ clock, duration: 0 }); - expect(executePollSpy).not.toHaveBeenCalled(); + expect(executePollSpy).toHaveBeenCalledTimes(1); + // Next poll is scheduled at pollingInterval; advancing remainingTime+1 does not reach it await advanceTime({ clock, duration: remainingTime + 1 }); expect(executePollSpy).toHaveBeenCalledTimes(1); - controller.stopPolling(); + controller.stopAllPolling(); }, ); }); @@ -1426,7 +1361,7 @@ describe('ConfigRegistryController', () => { await advanceTime({ clock, duration: 0 }); expect(executePollSpy).toHaveBeenCalledTimes(1); - controller.stopPolling(); + controller.stopAllPolling(); }, ); }); @@ -1452,7 +1387,7 @@ describe('ConfigRegistryController', () => { await advanceTime({ clock, duration: 1 }); expect(executePollSpy).toHaveBeenCalledTimes(1); - controller.stopPolling(); + controller.stopAllPolling(); jest.restoreAllMocks(); }, ); @@ -1479,7 +1414,7 @@ describe('ConfigRegistryController', () => { await advanceTime({ clock, duration: 1 }); expect(executePollSpy).toHaveBeenCalledTimes(1); - controller.stopPolling(); + controller.stopAllPolling(); jest.restoreAllMocks(); }, ); @@ -1503,10 +1438,11 @@ describe('ConfigRegistryController', () => { const executePollSpy = jest.spyOn(controller, '_executePoll'); controller.startPolling(null); + // StaticIntervalPollingController runs first poll after 0ms await advanceTime({ clock, duration: 0 }); - expect(executePollSpy).not.toHaveBeenCalled(); + expect(executePollSpy).toHaveBeenCalledTimes(1); - controller.stopPolling(); + controller.stopAllPolling(); }, ); }); @@ -1527,25 +1463,24 @@ describe('ConfigRegistryController', () => { const executePollSpy = jest.spyOn(controller, '_executePoll'); const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout'); - // First call sets a timeout + // First call runs first poll after 0ms controller.startPolling(null); await advanceTime({ clock, duration: 0 }); - expect(executePollSpy).not.toHaveBeenCalled(); + expect(executePollSpy).toHaveBeenCalledTimes(1); const clearTimeoutCallCountBefore = clearTimeoutSpy.mock.calls.length; - // Second call should clear the existing timeout (from first call) and set a new one - // This tests the defensive check that clears any existing timeout + // Second call reuses same session (same input); no new _startPolling, so no extra executePoll controller.startPolling(null); await advanceTime({ clock, duration: 0 }); - expect(executePollSpy).not.toHaveBeenCalled(); + expect(executePollSpy).toHaveBeenCalledTimes(1); - // Verify clearTimeout was called to clear the previous timeout - expect(clearTimeoutSpy.mock.calls.length).toBeGreaterThan( + // _startPolling clears before setting; clearTimeout is used when rescheduling after executePoll + expect(clearTimeoutSpy.mock.calls.length).toBeGreaterThanOrEqual( clearTimeoutCallCountBefore, ); - controller.stopPolling(); + controller.stopAllPolling(); }, ); }); @@ -1572,11 +1507,8 @@ describe('ConfigRegistryController', () => { expect(controller.state.configs).toStrictEqual({ networks: MOCK_FALLBACK_CONFIG, }); - expect(controller.state.fetchError).toBe( - 'Feature flag disabled - using fallback configuration', - ); - controller.stopPolling(); + controller.stopAllPolling(); }, ); }); @@ -1640,12 +1572,11 @@ describe('ConfigRegistryController', () => { expect(fetchConfigSpy).toHaveBeenCalled(); expect(controller.state.configs.networks['0x1']).toBeDefined(); expect(controller.state.version).toBe('1.0.0'); - expect(controller.state.fetchError).toBeNull(); }, ); }); - it('filters networks to only include featured, active, non-testnet networks', async () => { + it('stores all networks in state; selectFeaturedNetworks filters for default list', async () => { await withController( async ({ controller, @@ -1770,13 +1701,21 @@ describe('ConfigRegistryController', () => { await controller._executePoll(null); - expect(controller.state.configs.networks['0x1']).toBeDefined(); - expect(controller.state.configs.networks['0x5']).toBeUndefined(); - expect(controller.state.configs.networks['0xa']).toBeUndefined(); - expect(controller.state.configs.networks['0x89']).toBeUndefined(); - expect(Object.keys(controller.state.configs.networks)).toHaveLength( - 1, - ); + // All networks stored in state + const allNetworks = selectNetworks(controller.state); + expect(allNetworks['0x1']).toBeDefined(); + expect(allNetworks['0x5']).toBeDefined(); + expect(allNetworks['0xa']).toBeDefined(); + expect(allNetworks['0x89']).toBeDefined(); + expect(Object.keys(allNetworks)).toHaveLength(4); + + // selectFeaturedNetworks returns only featured, active, non-testnet + const featuredNetworks = selectFeaturedNetworks(controller.state); + expect(featuredNetworks['0x1']).toBeDefined(); + expect(featuredNetworks['0x5']).toBeUndefined(); + expect(featuredNetworks['0xa']).toBeUndefined(); + expect(featuredNetworks['0x89']).toBeUndefined(); + expect(Object.keys(featuredNetworks)).toHaveLength(1); }, ); }); @@ -1942,33 +1881,21 @@ describe('ConfigRegistryController', () => { await testController._executePoll(null); - // Verify warning was logged - expect(captureExceptionSpy).toHaveBeenCalled(); - const warningCall = captureExceptionSpy.mock.calls.find((call) => - call[0]?.message?.includes('Duplicate chainId 0x1'), - ); - expect(warningCall).toBeDefined(); - expect(warningCall?.[0]?.message).toContain( - 'Ethereum Mainnet (Low Priority), Ethereum Mainnet (High Priority)', - ); - - // Verify highest priority network was kept + // Last occurrence overwrites (no grouping/priority) expect(testController.state.configs.networks['0x1']).toBeDefined(); - expect(testController.state.configs.networks['0x1']?.value.name).toBe( + expect(testController.state.configs.networks['0x1']?.name).toBe( 'Ethereum Mainnet (High Priority)', ); expect( - testController.state.configs.networks['0x1']?.value.rpcEndpoints[0] - .type, + testController.state.configs.networks['0x1']?.rpcEndpoints[0].type, ).toBe('alchemy'); - // Verify other networks are still present expect(testController.state.configs.networks['0x89']).toBeDefined(); }, ); }); - it('handles duplicate chainIds with same priority by keeping first occurrence', async () => { + it('handles duplicate chainIds by keeping last occurrence', async () => { await withController( async ({ mockRemoteFeatureFlagGetState, mockApiServiceHandler }) => { mockRemoteFeatureFlagGetState.mockReturnValue({ @@ -2106,17 +2033,10 @@ describe('ConfigRegistryController', () => { await testController._executePoll(null); - // Verify warning was logged - expect(captureExceptionSpy).toHaveBeenCalled(); - const warningCall = captureExceptionSpy.mock.calls.find((call) => - call[0]?.message?.includes('Duplicate chainId 0x1'), - ); - expect(warningCall).toBeDefined(); - - // Verify first occurrence was kept (since priorities are equal) + // Last occurrence overwrites expect(testController.state.configs.networks['0x1']).toBeDefined(); - expect(testController.state.configs.networks['0x1']?.value.name).toBe( - 'Ethereum Mainnet (First)', + expect(testController.state.configs.networks['0x1']?.name).toBe( + 'Ethereum Mainnet (Second)', ); }, ); @@ -2196,9 +2116,6 @@ describe('ConfigRegistryController', () => { expect(controller.state.configs).toStrictEqual({ networks: MOCK_FALLBACK_CONFIG, }); - expect(controller.state.fetchError).toBe( - 'Feature flag disabled - using fallback configuration', - ); }, ); }); @@ -2221,11 +2138,8 @@ describe('ConfigRegistryController', () => { expect(controller.state.configs).toStrictEqual({ networks: MOCK_FALLBACK_CONFIG, }); - expect(controller.state.fetchError).toBe( - 'Feature flag disabled - using fallback configuration', - ); - controller.stopPolling(); + controller.stopAllPolling(); }, ); }); @@ -2247,11 +2161,8 @@ describe('ConfigRegistryController', () => { expect(controller.state.configs).toStrictEqual({ networks: MOCK_FALLBACK_CONFIG, }); - expect(controller.state.fetchError).toBe( - 'Feature flag disabled - using fallback configuration', - ); - controller.stopPolling(); + controller.stopAllPolling(); }, ); }); @@ -2272,13 +2183,13 @@ describe('ConfigRegistryController', () => { expect(startPollingSpy).toHaveBeenCalledWith(null); expect(executePollSpy).toHaveBeenCalledTimes(1); - controller.stopPolling(); + controller.stopAllPolling(); }); }); it('stops polling when KeyringController:lock event is published', async () => { await withController(async ({ controller, clock, rootMessenger }) => { - const stopPollingSpy = jest.spyOn(controller, 'stopPolling'); + const stopPollingSpy = jest.spyOn(controller, 'stopAllPolling'); await advanceTime({ clock, duration: 0 }); expect(stopPollingSpy).not.toHaveBeenCalled(); @@ -2293,12 +2204,12 @@ describe('ConfigRegistryController', () => { await withController(async ({ controller, clock }) => { const executePollSpy = jest.spyOn(controller, '_executePoll'); - controller.startPolling(); + controller.startPolling(null); await advanceTime({ clock, duration: 0 }); expect(executePollSpy).toHaveBeenCalledTimes(1); - controller.stopPolling(); + controller.stopAllPolling(); }); }); }); @@ -2320,16 +2231,16 @@ describe('ConfigRegistryController', () => { const executePollSpy = jest.spyOn(controller, '_executePoll'); controller.startPolling(null); - // Verify timeout was set (poll should not execute immediately) + // StaticIntervalPollingController runs first poll after 0ms await advanceTime({ clock, duration: 0 }); - expect(executePollSpy).not.toHaveBeenCalled(); + expect(executePollSpy).toHaveBeenCalledTimes(1); - // Stop polling should clear the timeout - controller.stopPolling(); + // Stop polling should clear the next scheduled timeout + controller.stopAllPolling(); - // Advance time past when the timeout would have fired + // Advance time past when the next poll would have fired await advanceTime({ clock, duration: pollingInterval }); - expect(executePollSpy).not.toHaveBeenCalled(); + expect(executePollSpy).toHaveBeenCalledTimes(1); }, ); }); @@ -2337,7 +2248,7 @@ describe('ConfigRegistryController', () => { it('handles clearing timeout when no timeout exists', async () => { await withController(({ controller }) => { // Should not throw when stopping without a pending timeout - expect(() => controller.stopPolling()).not.toThrow(); + expect(() => controller.stopAllPolling()).not.toThrow(); }); }); @@ -2357,16 +2268,16 @@ describe('ConfigRegistryController', () => { const executePollSpy = jest.spyOn(controller, '_executePoll'); const token = controller.startPolling(null); - // Verify timeout was set (poll should not execute immediately) + // StaticIntervalPollingController runs first poll after 0ms await advanceTime({ clock, duration: 0 }); - expect(executePollSpy).not.toHaveBeenCalled(); + expect(executePollSpy).toHaveBeenCalledTimes(1); // Stop polling using the placeholder token - controller.stopPolling(token); + controller.stopPollingByPollingToken(token); - // Advance time past when the timeout would have fired + // Advance time past when the next poll would have fired await advanceTime({ clock, duration: pollingInterval }); - expect(executePollSpy).not.toHaveBeenCalled(); + expect(executePollSpy).toHaveBeenCalledTimes(1); }, ); }); @@ -2394,7 +2305,7 @@ describe('ConfigRegistryController', () => { executePollSpy.mockClear(); // Stop polling using the placeholder token (should map to actual token) - controller.stopPolling(token); + controller.stopPollingByPollingToken(token); // Advance time to verify polling stopped await advanceTime({ clock, duration: pollingInterval }); @@ -2416,7 +2327,7 @@ describe('ConfigRegistryController', () => { executePollSpy.mockClear(); // Stop without token should stop all polling - controller.stopPolling(); + controller.stopAllPolling(); await advanceTime({ clock, duration: DEFAULT_POLLING_INTERVAL }); expect(executePollSpy).not.toHaveBeenCalled(); @@ -2433,18 +2344,17 @@ describe('ConfigRegistryController', () => { expect(executePollSpy).toHaveBeenCalledTimes(1); executePollSpy.mockClear(); - // Start polling from consumer B (should reuse same polling session) - controller.startPolling(null); + // Start polling from consumer B (reuses same polling session) + const tokenB = controller.startPolling(null); await advanceTime({ clock, duration: 0 }); - // Since both use same input (null), they share the same polling session - // So stopping one token should stop the shared session expect(executePollSpy).toHaveBeenCalledTimes(0); executePollSpy.mockClear(); - // Stop consumer A's polling session - controller.stopPolling(tokenA); + // Stop both consumers so the shared session is fully stopped + controller.stopPollingByPollingToken(tokenA); + controller.stopPollingByPollingToken(tokenB); - // Polling should stop for both since they share the same session + // No more polls after session is stopped await advanceTime({ clock, duration: DEFAULT_POLLING_INTERVAL }); expect(executePollSpy).not.toHaveBeenCalled(); }); @@ -2464,8 +2374,8 @@ describe('ConfigRegistryController', () => { expect(executePollSpy).toHaveBeenCalledTimes(1); executePollSpy.mockClear(); - // Stop with token via messenger - messenger.call('ConfigRegistryController:stopPolling', token); + // Stop via messenger action (stops all polling) + messenger.call('ConfigRegistryController:stopPolling'); await advanceTime({ clock, duration: DEFAULT_POLLING_INTERVAL }); expect(executePollSpy).not.toHaveBeenCalled(); }); diff --git a/packages/config-registry-controller/src/ConfigRegistryController.ts b/packages/config-registry-controller/src/ConfigRegistryController.ts index 247f69fafbf..6b01ae88591 100644 --- a/packages/config-registry-controller/src/ConfigRegistryController.ts +++ b/packages/config-registry-controller/src/ConfigRegistryController.ts @@ -11,26 +11,18 @@ import type { Messenger } from '@metamask/messenger'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote-feature-flag-controller'; import { Duration, inMilliseconds } from '@metamask/utils'; -import type { Json } from '@metamask/utils'; import type { FetchConfigOptions, FetchConfigResult, RegistryNetworkConfig, } from './config-registry-api-service'; -import { filterNetworks } from './config-registry-api-service'; import { isConfigRegistryApiEnabled as defaultIsConfigRegistryApiEnabled } from './utils/feature-flags'; const controllerName = 'ConfigRegistryController'; export const DEFAULT_POLLING_INTERVAL = inMilliseconds(1, Duration.Day); -export type NetworkConfigEntry = { - key: string; - value: RegistryNetworkConfig; - metadata?: Json; -}; - /** * State for the ConfigRegistryController. * @@ -40,10 +32,11 @@ export type NetworkConfigEntry = { export type ConfigRegistryState = { /** * Network configurations organized by chain ID. - * Contains the actual network configuration data fetched from the API. + * Stores the full API response including isFeatured, isTestnet, etc. + * Use selectors (e.g. selectFeaturedNetworks) to filter when needed. */ configs: { - networks: Record; + networks: Record; }; /** * Semantic version string of the configuration data from the API. @@ -57,11 +50,6 @@ export type ConfigRegistryState = { * was last successfully fetched from the API. */ lastFetched: number | null; - /** - * Error message if the last fetch attempt failed. - * Null if the last fetch was successful. - */ - fetchError: string | null; /** * HTTP entity tag (ETag) used for cache validation. * Sent as `If-None-Match` header in subsequent requests to check @@ -92,12 +80,6 @@ const stateMetadata = { includeInDebugSnapshot: true, usedInUi: false, }, - fetchError: { - persist: true, - includeInStateLogs: false, - includeInDebugSnapshot: false, - usedInUi: false, - }, etag: { persist: true, includeInStateLogs: false, @@ -106,7 +88,7 @@ const stateMetadata = { }, } satisfies StateMetadata; -const DEFAULT_FALLBACK_CONFIG: Record = {}; +const DEFAULT_FALLBACK_CONFIG: Record = {}; export type ConfigRegistryControllerStateChangeEvent = ControllerStateChangeEvent; @@ -123,7 +105,7 @@ export type ConfigRegistryControllerStartPollingAction = { export type ConfigRegistryControllerStopPollingAction = { type: `${typeof controllerName}:stopPolling`; - handler: (token?: string) => void; + handler: () => void; }; export type ConfigRegistryControllerActions = @@ -151,7 +133,7 @@ export type ConfigRegistryControllerOptions = { messenger: ConfigRegistryMessenger; state?: Partial; pollingInterval?: number; - fallbackConfig?: Record; + fallbackConfig?: Record; isConfigRegistryApiEnabled?: (messenger: ConfigRegistryMessenger) => boolean; }; @@ -160,16 +142,10 @@ export class ConfigRegistryController extends StaticIntervalPollingController { - readonly #fallbackConfig: Record; - readonly #isConfigRegistryApiEnabled: ( messenger: ConfigRegistryMessenger, ) => boolean; - #delayedPollTimeoutId: ReturnType | null = null; - - readonly #delayedPollTokenMap: Map = new Map(); - /** * @param options - The controller options. * @param options.messenger - The controller messenger. Must have @@ -197,13 +173,11 @@ export class ConfigRegistryController extends StaticIntervalPollingController this.startPolling(input), ); - this.messenger.registerActionHandler( - `${controllerName}:stopPolling`, - (token?: string) => this.stopPolling(token), + this.messenger.registerActionHandler(`${controllerName}:stopPolling`, () => + this.stopAllPolling(), ); this.messenger.subscribe('KeyringController:unlock', () => @@ -221,38 +194,18 @@ export class ConfigRegistryController extends StaticIntervalPollingController - this.stopPolling(), + this.stopAllPolling(), ); } - /** - * Returns the remaining delay in ms before the next fetch is allowed (0 if allowed now). - * Used by both startPolling (to delay the first poll) and _executePoll (to skip if too soon). - * - * @returns Remaining ms to wait; 0 if fetch is allowed now. - */ - #getRemainingPollingDelayMs(): number { - const pollingInterval = - this.getIntervalLength() ?? DEFAULT_POLLING_INTERVAL; - const now = Date.now(); - const { lastFetched } = this.state; - if (lastFetched === null) { - return 0; - } - return Math.max(0, pollingInterval - (now - lastFetched)); - } - async _executePoll(_input: null): Promise { const isApiEnabled = this.#isConfigRegistryApiEnabled(this.messenger); if (!isApiEnabled) { - this.#useFallbackConfig( - 'Feature flag disabled - using fallback configuration', - ); return; } - if (this.#getRemainingPollingDelayMs() > 0) { + if (this.state.lastFetched && Date.now() < this.state.lastFetched) { return; } @@ -266,7 +219,6 @@ export class ConfigRegistryController extends StaticIntervalPollingController { - state.fetchError = null; state.lastFetched = Date.now(); if (result.etag !== undefined) { state.etag = result.etag ?? null; @@ -275,150 +227,24 @@ export class ConfigRegistryController extends StaticIntervalPollingController = {}; + apiNetworks.forEach((registryConfig) => { + const { chainId } = registryConfig; + newConfigs[chainId] = registryConfig; }); - // Group networks by chainId to detect duplicates - const networksByChainId = new Map< - string, - { network: RegistryNetworkConfig; index: number }[] - >(); - filteredNetworks.forEach((network, index) => { - const existing = networksByChainId.get(network.chainId) ?? []; - existing.push({ network, index }); - networksByChainId.set(network.chainId, existing); - }); - - // Build configs, handling duplicates by keeping highest priority network - const newConfigs: Record = {}; - for (const [chainId, networks] of networksByChainId) { - if (networks.length > 1) { - // Duplicate chainIds detected - log warning and keep highest priority - const duplicateNames = networks - .map((networkEntry) => networkEntry.network.name) - .join(', '); - const warningMessage = `Duplicate chainId ${chainId} detected in config registry API response. Networks: ${duplicateNames}. Keeping network with highest priority.`; - - if (this.messenger.captureException) { - this.messenger.captureException(new Error(warningMessage)); - } - - // Sort by priority (lower number = higher priority), then by index (first occurrence) - networks.sort((a, b) => { - const priorityDiff = a.network.priority - b.network.priority; - if (priorityDiff === 0) { - return a.index - b.index; - } - return priorityDiff; - }); - } - - // Use the first network (highest priority if duplicates existed) - const selectedNetwork = networks[0].network; - newConfigs[chainId] = { - key: chainId, - value: selectedNetwork, - }; - } - this.update((state) => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - Type instantiation is excessively deep - (state.configs as ConfigRegistryState['configs']) = { - networks: newConfigs, - }; + state.configs.networks = newConfigs; state.version = result.data.data.version; state.lastFetched = Date.now(); - state.fetchError = null; state.etag = result.etag ?? null; }); } catch (error) { const errorInstance = error instanceof Error ? error : new Error(String(error)); - if (this.messenger.captureException) { - this.messenger.captureException(errorInstance); - } - - this.#handleFetchError(error); - } - } - - #useFallbackConfig(errorMessage: string): void { - this.update((state) => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - Type instantiation is excessively deep - state.configs.networks = { ...this.#fallbackConfig }; - state.fetchError = errorMessage; - state.lastFetched = null; - state.etag = null; - }); - } - - #handleFetchError(error: unknown): void { - const errorMessage = - error instanceof Error ? error.message : 'Unknown error occurred'; - - this.update((state) => { - state.fetchError = errorMessage; - state.lastFetched = Date.now(); - }); - } - - startPolling(input: null = null): string { - const remainingTime = this.#getRemainingPollingDelayMs(); - - if (remainingTime > 0) { - // Not enough time has passed, delay the first poll - // Clear any existing timeout before scheduling a new one - if (this.#delayedPollTimeoutId !== null) { - clearTimeout(this.#delayedPollTimeoutId); - this.#delayedPollTimeoutId = null; - } - - // Generate a placeholder token that will map to the actual token - const placeholderToken = `delayed-${Date.now()}-${Math.random()}`; - - // Schedule the first poll after the remaining time - this.#delayedPollTimeoutId = setTimeout(() => { - this.#delayedPollTimeoutId = null; - const actualToken = super.startPolling(input); - this.#delayedPollTokenMap.set(placeholderToken, actualToken); - }, remainingTime); - - return placeholderToken; - } - - // Enough time has passed or first time, proceed with normal polling - return super.startPolling(input); - } - - stopPolling(token?: string): void { - // Clear any pending delayed poll timeout - if (this.#delayedPollTimeoutId !== null) { - clearTimeout(this.#delayedPollTimeoutId); - this.#delayedPollTimeoutId = null; - } - - if (token) { - // Check if this is a placeholder token that needs mapping - const actualToken = this.#delayedPollTokenMap.get(token); - if (actualToken) { - // Remove from map and stop the actual polling session - this.#delayedPollTokenMap.delete(token); - super.stopPollingByPollingToken(actualToken); - } else { - // Stop specific polling session by token - super.stopPollingByPollingToken(token); - } - } else { - // Stop all polling (backward compatible) - this.#delayedPollTokenMap.clear(); - super.stopAllPolling(); + this.messenger.captureException?.(errorInstance); } } } diff --git a/packages/config-registry-controller/src/config-registry-api-service/config-registry-api-service.test.ts b/packages/config-registry-controller/src/config-registry-api-service/config-registry-api-service.test.ts index d61ca3f2164..f7b901d8b11 100644 --- a/packages/config-registry-controller/src/config-registry-api-service/config-registry-api-service.test.ts +++ b/packages/config-registry-controller/src/config-registry-api-service/config-registry-api-service.test.ts @@ -4,36 +4,13 @@ import { useFakeTimers } from 'sinon'; import { ConfigRegistryApiService } from './config-registry-api-service'; import type { FetchConfigResult, RegistryConfigApiResponse } from './types'; +import { createMockNetworkConfig } from '../test-helpers'; const MOCK_API_RESPONSE: RegistryConfigApiResponse = { data: { version: '"24952800ba9dafbc5e2c91f57f386d28"', timestamp: 1761829548000, - networks: [ - { - chainId: '0x1', - name: 'Ethereum Mainnet', - nativeCurrency: 'ETH', - rpcEndpoints: [ - { - url: 'https://mainnet.infura.io/v3/{infuraProjectId}', - type: 'infura', - networkClientId: 'mainnet', - failoverUrls: [], - }, - ], - blockExplorerUrls: ['https://etherscan.io'], - defaultRpcEndpointIndex: 0, - defaultBlockExplorerUrlIndex: 0, - isActive: true, - isTestnet: false, - isDefault: true, - isFeatured: true, - isDeprecated: false, - priority: 0, - isDeletable: false, - }, - ], + networks: [createMockNetworkConfig()], }, }; @@ -71,12 +48,12 @@ describe('ConfigRegistryApiService', () => { }); describe('constructor', () => { - it('should create instance with default options', () => { + it('creates instance with default options', () => { const service = new ConfigRegistryApiService(); expect(service).toBeInstanceOf(ConfigRegistryApiService); }); - it('should create instance with custom options', () => { + it('creates instance with custom options', () => { const customFetch = jest.fn(); const service = new ConfigRegistryApiService({ env: SDK.Env.DEV, @@ -86,19 +63,19 @@ describe('ConfigRegistryApiService', () => { expect(service).toBeInstanceOf(ConfigRegistryApiService); }); - it('should create instance with empty options object', () => { + it('creates instance with empty options object', () => { const service = new ConfigRegistryApiService({}); expect(service).toBeInstanceOf(ConfigRegistryApiService); }); - it('should use default values for unspecified options', () => { + it('uses default values for unspecified options', () => { const service = new ConfigRegistryApiService({ env: SDK.Env.PRD, }); expect(service).toBeInstanceOf(ConfigRegistryApiService); }); - it('should use default fetch when not provided', () => { + it('uses default fetch when not provided', () => { const service = new ConfigRegistryApiService({ env: SDK.Env.DEV, }); @@ -115,7 +92,7 @@ describe('ConfigRegistryApiService', () => { cleanAll(); }); - it('should successfully fetch config from API', async () => { + it('fetches config from API successfully', async () => { const scope = nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') .reply(200, MOCK_API_RESPONSE, { @@ -133,7 +110,7 @@ describe('ConfigRegistryApiService', () => { expect(scope.isDone()).toBe(true); }); - it('should successfully fetch config from API without ETag header', async () => { + it('fetches config from API without ETag header', async () => { const scope = nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') .reply(200, MOCK_API_RESPONSE); @@ -149,7 +126,7 @@ describe('ConfigRegistryApiService', () => { expect(scope.isDone()).toBe(true); }); - it('should handle 304 Not Modified response', async () => { + it('handles 304 Not Modified response', async () => { const etag = '"test-etag-123"'; const scope = nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') @@ -163,7 +140,7 @@ describe('ConfigRegistryApiService', () => { expect(scope.isDone()).toBe(true); }); - it('should handle 304 Not Modified response without ETag header', async () => { + it('handles 304 Not Modified response without ETag header', async () => { const mockHeaders = { get: jest.fn().mockReturnValue(null), }; @@ -183,7 +160,7 @@ describe('ConfigRegistryApiService', () => { expect(result.etag).toBeUndefined(); }); - it('should include If-None-Match header when etag is provided', async () => { + it('includes If-None-Match header when etag is provided', async () => { const etag = '"test-etag-123"'; const scope = nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') @@ -196,7 +173,7 @@ describe('ConfigRegistryApiService', () => { expect(scope.isDone()).toBe(true); }); - it('should not include If-None-Match header when etag is undefined', async () => { + it('does not include If-None-Match header when etag is undefined', async () => { const scope = nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') .reply(200, MOCK_API_RESPONSE); @@ -207,7 +184,7 @@ describe('ConfigRegistryApiService', () => { expect(scope.isDone()).toBe(true); }); - it('should handle fetchConfig called with undefined options', async () => { + it('handles fetchConfig called with undefined options', async () => { const scope = nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') .reply(200, MOCK_API_RESPONSE); @@ -219,7 +196,7 @@ describe('ConfigRegistryApiService', () => { expect(scope.isDone()).toBe(true); }); - it('should throw error on invalid response structure', async () => { + it('throws error on invalid response structure', async () => { const invalidResponse = { invalid: 'data' }; const scope = nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') @@ -227,11 +204,13 @@ describe('ConfigRegistryApiService', () => { const service = new ConfigRegistryApiService(); - await expect(service.fetchConfig()).rejects.toThrow(expect.any(Error)); + await expect(service.fetchConfig()).rejects.toMatchObject( + expect.objectContaining({ message: expect.any(String) }), + ); expect(scope.isDone()).toBe(true); }); - it('should throw error when data is null', async () => { + it('throws error when data is null', async () => { const mockHeaders = { get: jest.fn().mockReturnValue(null), }; @@ -246,10 +225,12 @@ describe('ConfigRegistryApiService', () => { fetch: customFetch, }); - await expect(service.fetchConfig()).rejects.toThrow(expect.any(Error)); + await expect(service.fetchConfig()).rejects.toMatchObject( + expect.objectContaining({ message: expect.any(String) }), + ); }); - it('should throw error when data.data is null', async () => { + it('throws error when data.data is null', async () => { const mockHeaders = { get: jest.fn().mockReturnValue(null), }; @@ -264,10 +245,12 @@ describe('ConfigRegistryApiService', () => { fetch: customFetch, }); - await expect(service.fetchConfig()).rejects.toThrow(expect.any(Error)); + await expect(service.fetchConfig()).rejects.toMatchObject( + expect.objectContaining({ message: expect.any(String) }), + ); }); - it('should throw error when data.data.networks is not an array', async () => { + it('throws error when data.data.networks is not an array', async () => { const mockHeaders = { get: jest.fn().mockReturnValue(null), }; @@ -282,10 +265,12 @@ describe('ConfigRegistryApiService', () => { fetch: customFetch, }); - await expect(service.fetchConfig()).rejects.toThrow(expect.any(Error)); + await expect(service.fetchConfig()).rejects.toMatchObject( + expect.objectContaining({ message: expect.any(String) }), + ); }); - it('should throw error on HTTP error status', async () => { + it('throws error on HTTP error status', async () => { const mockHeaders = { get: jest.fn().mockReturnValue(null), }; @@ -300,12 +285,14 @@ describe('ConfigRegistryApiService', () => { fetch: customFetch, }); - await expect(service.fetchConfig()).rejects.toThrow( - 'Failed to fetch config: 500 Internal Server Error', + await expect(service.fetchConfig()).rejects.toMatchObject( + expect.objectContaining({ + message: 'Failed to fetch config: 500 Internal Server Error', + }), ); }); - it('should handle network errors', async () => { + it('handles network errors', async () => { const customFetch = jest .fn() .mockRejectedValue(new Error('Network connection failed')); @@ -314,12 +301,12 @@ describe('ConfigRegistryApiService', () => { fetch: customFetch, }); - await expect(service.fetchConfig()).rejects.toThrow( - 'Network connection failed', + await expect(service.fetchConfig()).rejects.toMatchObject( + expect.objectContaining({ message: 'Network connection failed' }), ); }); - it('should retry on failure', async () => { + it('retries on failure', async () => { nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') .replyWithError('Network error'); @@ -355,7 +342,7 @@ describe('ConfigRegistryApiService', () => { clock.restore(); }); - it('should register and call onBreak handler', async () => { + it('registers and calls onBreak handler', async () => { const maximumConsecutiveFailures = 3; const retries = 0; @@ -375,7 +362,9 @@ describe('ConfigRegistryApiService', () => { service.onBreak(onBreakHandler); for (let i = 0; i < maximumConsecutiveFailures; i++) { - await expect(service.fetchConfig()).rejects.toThrow(expect.any(Error)); + await expect(service.fetchConfig()).rejects.toMatchObject( + expect.objectContaining({ message: expect.any(String) }), + ); await clock.tickAsync(100); } @@ -385,11 +374,13 @@ describe('ConfigRegistryApiService', () => { }); await clock.tickAsync(100); - await expect(finalPromise).rejects.toThrow(expect.any(Error)); + await expect(finalPromise).rejects.toMatchObject( + expect.objectContaining({ message: expect.any(String) }), + ); expect(onBreakHandler).toHaveBeenCalled(); }); - it('should return the result from policy.onBreak', () => { + it('returns the result from policy.onBreak', () => { const service = new ConfigRegistryApiService(); const handler = jest.fn(); const result = service.onBreak(handler); @@ -398,7 +389,7 @@ describe('ConfigRegistryApiService', () => { }); describe('onDegraded', () => { - it('should register onDegraded handler', () => { + it('registers onDegraded handler', () => { const service = new ConfigRegistryApiService(); const onDegradedHandler = jest.fn(); @@ -407,7 +398,7 @@ describe('ConfigRegistryApiService', () => { expect(service.onDegraded).toBeDefined(); }); - it('should call onDegraded handler when service becomes degraded', async () => { + it('calls onDegraded handler when service becomes degraded', async () => { const degradedThreshold = 2000; // 2 seconds const service = new ConfigRegistryApiService({ degradedThreshold, @@ -448,7 +439,7 @@ describe('ConfigRegistryApiService', () => { expect(slowService.onDegraded).toBeDefined(); }); - it('should return the result from policy.onDegraded', () => { + it('returns the result from policy.onDegraded', () => { const service = new ConfigRegistryApiService(); const handler = jest.fn(); const result = service.onDegraded(handler); @@ -457,7 +448,7 @@ describe('ConfigRegistryApiService', () => { }); describe('custom fetch function', () => { - it('should use custom fetch function when provided', async () => { + it('uses custom fetch function when provided', async () => { const mockHeaders = { get: jest.fn().mockReturnValue('"custom-etag"'), }; @@ -483,7 +474,7 @@ describe('ConfigRegistryApiService', () => { }); describe('environment configuration', () => { - it('should use DEV environment URL when env is DEV', async () => { + it('uses DEV environment URL when env is DEV', async () => { const scope = nock('https://client-config.dev-api.cx.metamask.io') .get('/v1/config/networks') .reply(200, MOCK_API_RESPONSE); @@ -497,7 +488,7 @@ describe('ConfigRegistryApiService', () => { expect(scope.isDone()).toBe(true); }); - it('should use UAT environment URL when env is UAT', async () => { + it('uses UAT environment URL when env is UAT', async () => { const scope = nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') .reply(200, MOCK_API_RESPONSE); @@ -511,7 +502,7 @@ describe('ConfigRegistryApiService', () => { expect(scope.isDone()).toBe(true); }); - it('should use PRD environment URL when env is PRD', async () => { + it('uses PRD environment URL when env is PRD', async () => { const scope = nock('https://client-config.api.cx.metamask.io') .get('/v1/config/networks') .reply(200, MOCK_API_RESPONSE); @@ -525,7 +516,7 @@ describe('ConfigRegistryApiService', () => { expect(scope.isDone()).toBe(true); }); - it('should default to UAT environment', async () => { + it('defaults to UAT environment', async () => { const scope = nock('https://client-config.uat-api.cx.metamask.io') .get('/v1/config/networks') .reply(200, MOCK_API_RESPONSE); diff --git a/packages/config-registry-controller/src/config-registry-api-service/transformers.test.ts b/packages/config-registry-controller/src/config-registry-api-service/filters.test.ts similarity index 72% rename from packages/config-registry-controller/src/config-registry-api-service/transformers.test.ts rename to packages/config-registry-controller/src/config-registry-api-service/filters.test.ts index 9e921d40adf..3e11e331f37 100644 --- a/packages/config-registry-controller/src/config-registry-api-service/transformers.test.ts +++ b/packages/config-registry-controller/src/config-registry-api-service/filters.test.ts @@ -1,59 +1,33 @@ -import { filterNetworks } from './transformers'; +import { filterNetworks } from './filters'; import type { RegistryNetworkConfig } from './types'; +import { createMockNetworkConfig } from '../test-helpers'; -const VALID_NETWORK_CONFIG: RegistryNetworkConfig = { - chainId: '0x1', - name: 'Ethereum Mainnet', - nativeCurrency: 'ETH', - rpcEndpoints: [ - { - url: 'https://mainnet.infura.io/v3/{infuraProjectId}', - type: 'infura', - networkClientId: 'mainnet', - failoverUrls: ['https://backup.infura.io/v3/{infuraProjectId}'], - }, - ], - blockExplorerUrls: ['https://etherscan.io'], - defaultRpcEndpointIndex: 0, - defaultBlockExplorerUrlIndex: 0, - isActive: true, - isTestnet: false, - isDefault: true, - isFeatured: true, - isDeprecated: false, - priority: 0, - isDeletable: false, -}; - -describe('transformers', () => { +describe('filters', () => { describe('filterNetworks', () => { const networks: RegistryNetworkConfig[] = [ - { - ...VALID_NETWORK_CONFIG, + createMockNetworkConfig({ isFeatured: true, isTestnet: false, isActive: true, isDeprecated: false, isDefault: true, - }, - { - ...VALID_NETWORK_CONFIG, + }), + createMockNetworkConfig({ chainId: '0x5', isFeatured: false, isTestnet: true, isActive: true, isDeprecated: false, isDefault: false, - }, - { - ...VALID_NETWORK_CONFIG, + }), + createMockNetworkConfig({ chainId: '0x2a', isFeatured: true, isTestnet: false, isActive: false, isDeprecated: true, isDefault: false, - }, + }), ]; it('returns all networks when no filters applied', () => { diff --git a/packages/config-registry-controller/src/config-registry-api-service/filters.ts b/packages/config-registry-controller/src/config-registry-api-service/filters.ts new file mode 100644 index 00000000000..f8eabf4bc08 --- /dev/null +++ b/packages/config-registry-controller/src/config-registry-api-service/filters.ts @@ -0,0 +1,40 @@ +import type { RegistryNetworkConfig } from './types'; + +export type NetworkFilterOptions = { + isFeatured?: boolean; + isTestnet?: boolean; + isActive?: boolean; + isDeprecated?: boolean; + isDefault?: boolean; +}; + +const FILTER_KEYS: (keyof NetworkFilterOptions)[] = [ + 'isFeatured', + 'isTestnet', + 'isActive', + 'isDeprecated', + 'isDefault', +]; + +/** + * @param networks - Array of network configurations to filter. + * @param options - Filter options. + * @returns Filtered array of network configurations. + */ +export function filterNetworks( + networks: RegistryNetworkConfig[], + options: NetworkFilterOptions = {}, +): RegistryNetworkConfig[] { + return networks.filter((network) => { + for (const key of FILTER_KEYS) { + const optionValue = options[key]; + if ( + optionValue !== undefined && + network[key as keyof RegistryNetworkConfig] !== optionValue + ) { + return false; + } + } + return true; + }); +} diff --git a/packages/config-registry-controller/src/config-registry-api-service/index.ts b/packages/config-registry-controller/src/config-registry-api-service/index.ts index 182571d2572..a740666621a 100644 --- a/packages/config-registry-controller/src/config-registry-api-service/index.ts +++ b/packages/config-registry-controller/src/config-registry-api-service/index.ts @@ -9,5 +9,5 @@ export { ConfigRegistryApiService } from './config-registry-api-service'; export type { ConfigRegistryApiServiceOptions } from './config-registry-api-service'; -export type { NetworkFilterOptions } from './transformers'; -export { filterNetworks } from './transformers'; +export type { NetworkFilterOptions } from './filters'; +export { filterNetworks } from './filters'; diff --git a/packages/config-registry-controller/src/config-registry-api-service/transformers.ts b/packages/config-registry-controller/src/config-registry-api-service/transformers.ts deleted file mode 100644 index 150e10b51eb..00000000000 --- a/packages/config-registry-controller/src/config-registry-api-service/transformers.ts +++ /dev/null @@ -1,53 +0,0 @@ -import type { RegistryNetworkConfig } from './types'; - -export type NetworkFilterOptions = { - isFeatured?: boolean; - isTestnet?: boolean; - isActive?: boolean; - isDeprecated?: boolean; - isDefault?: boolean; -}; - -/** - * @param networks - Array of network configurations to filter. - * @param options - Filter options. - * @returns Filtered array of network configurations. - */ -export function filterNetworks( - networks: RegistryNetworkConfig[], - options: NetworkFilterOptions = {}, -): RegistryNetworkConfig[] { - return networks.filter((network) => { - if (options.isFeatured !== undefined) { - if (network.isFeatured !== options.isFeatured) { - return false; - } - } - - if (options.isTestnet !== undefined) { - if (network.isTestnet !== options.isTestnet) { - return false; - } - } - - if (options.isActive !== undefined) { - if (network.isActive !== options.isActive) { - return false; - } - } - - if (options.isDeprecated !== undefined) { - if (network.isDeprecated !== options.isDeprecated) { - return false; - } - } - - if (options.isDefault !== undefined) { - if (network.isDefault !== options.isDefault) { - return false; - } - } - - return true; - }); -} diff --git a/packages/config-registry-controller/src/config-registry-api-service/types.ts b/packages/config-registry-controller/src/config-registry-api-service/types.ts index 0cf33d4b8ec..81bf91cb8cc 100644 --- a/packages/config-registry-controller/src/config-registry-api-service/types.ts +++ b/packages/config-registry-controller/src/config-registry-api-service/types.ts @@ -16,6 +16,11 @@ const RpcEndpointSchema = type({ failoverUrls: array(string()), }); +/** + * Schema for a single network in the API response. + * Matches the flat shape returned by the config registry API + * (all fields at top level, not nested under "network"). + */ export const RegistryNetworkConfigSchema = type({ chainId: string(), name: string(), diff --git a/packages/config-registry-controller/src/index.ts b/packages/config-registry-controller/src/index.ts index 468f15ddaa7..6b8a09618f9 100644 --- a/packages/config-registry-controller/src/index.ts +++ b/packages/config-registry-controller/src/index.ts @@ -8,12 +8,12 @@ export type { ConfigRegistryControllerEvents, ConfigRegistryControllerStateChangeEvent, ConfigRegistryMessenger, - NetworkConfigEntry, } from './ConfigRegistryController'; export { ConfigRegistryController, DEFAULT_POLLING_INTERVAL, } from './ConfigRegistryController'; +export { selectFeaturedNetworks, selectNetworks } from './selectors'; export type { FetchConfigOptions, FetchConfigResult, diff --git a/packages/config-registry-controller/src/selectors.test.ts b/packages/config-registry-controller/src/selectors.test.ts new file mode 100644 index 00000000000..1530b3bb5af --- /dev/null +++ b/packages/config-registry-controller/src/selectors.test.ts @@ -0,0 +1,90 @@ +import { selectFeaturedNetworks, selectNetworks } from './selectors'; +import { createMockNetworkConfig } from './test-helpers'; + +describe('selectors', () => { + describe('selectNetworks', () => { + it('returns all networks from state', () => { + const networks = { + '0x1': createMockNetworkConfig({ chainId: '0x1' }), + '0x89': createMockNetworkConfig({ + chainId: '0x89', + name: 'Polygon', + }), + }; + const state = { + configs: { networks }, + version: '1.0.0', + lastFetched: Date.now(), + etag: null, + }; + + expect(selectNetworks(state)).toBe(networks); + expect(selectNetworks(state)).toStrictEqual(networks); + }); + }); + + describe('selectFeaturedNetworks', () => { + it('returns only featured, active, non-testnet networks', () => { + const networks = { + '0x1': createMockNetworkConfig({ + chainId: '0x1', + isFeatured: true, + isActive: true, + isTestnet: false, + }), + '0x5': createMockNetworkConfig({ + chainId: '0x5', + name: 'Goerli', + isFeatured: true, + isActive: true, + isTestnet: true, + }), + '0xa': createMockNetworkConfig({ + chainId: '0xa', + name: 'Optimism', + isFeatured: false, + isActive: true, + isTestnet: false, + }), + '0x89': createMockNetworkConfig({ + chainId: '0x89', + name: 'Polygon', + isFeatured: true, + isActive: false, + isTestnet: false, + }), + }; + const state = { + configs: { networks }, + version: '1.0.0', + lastFetched: Date.now(), + etag: null, + }; + + const featured = selectFeaturedNetworks(state); + expect(Object.keys(featured)).toHaveLength(1); + expect(featured['0x1']).toBeDefined(); + expect(featured['0x5']).toBeUndefined(); + expect(featured['0xa']).toBeUndefined(); + expect(featured['0x89']).toBeUndefined(); + }); + + it('returns empty object when no networks match', () => { + const networks = { + '0x5': createMockNetworkConfig({ + chainId: '0x5', + isTestnet: true, + }), + }; + const state = { + configs: { networks }, + version: '1.0.0', + lastFetched: Date.now(), + etag: null, + }; + + const featured = selectFeaturedNetworks(state); + expect(Object.keys(featured)).toHaveLength(0); + }); + }); +}); diff --git a/packages/config-registry-controller/src/selectors.ts b/packages/config-registry-controller/src/selectors.ts new file mode 100644 index 00000000000..a6101934089 --- /dev/null +++ b/packages/config-registry-controller/src/selectors.ts @@ -0,0 +1,39 @@ +import { createSelector } from 'reselect'; + +import { filterNetworks } from './config-registry-api-service'; +import type { RegistryNetworkConfig } from './config-registry-api-service'; +import type { ConfigRegistryState } from './ConfigRegistryController'; + +/** + * Base selector to get all networks from the controller state. + * + * @param state - The ConfigRegistryController state + * @returns All network configurations keyed by chain ID + */ +export const selectNetworks = ( + state: ConfigRegistryState, +): Record => state.configs.networks; + +/** + * Selector to get featured, active, non-testnet networks. + * Use this for the default network list (e.g. main network picker). + * + * @param state - The ConfigRegistryController state + * @returns Filtered network configurations keyed by chain ID + */ +export const selectFeaturedNetworks = createSelector( + selectNetworks, + (networks): Record => { + const networkArray = Object.values(networks); + const filtered = filterNetworks(networkArray, { + isFeatured: true, + isActive: true, + isTestnet: false, + }); + const result: Record = {}; + filtered.forEach((config) => { + result[config.chainId] = config; + }); + return result; + }, +); diff --git a/packages/config-registry-controller/src/test-helpers.ts b/packages/config-registry-controller/src/test-helpers.ts new file mode 100644 index 00000000000..a51f63caa68 --- /dev/null +++ b/packages/config-registry-controller/src/test-helpers.ts @@ -0,0 +1,36 @@ +import type { RegistryNetworkConfig } from './config-registry-api-service'; + +/** + * Creates a mock RegistryNetworkConfig for testing. + * + * @param overrides - Optional properties to override in the default RegistryNetworkConfig. + * @returns A mock RegistryNetworkConfig object. + */ +export function createMockNetworkConfig( + overrides: Partial = {}, +): RegistryNetworkConfig { + return { + chainId: '0x1', + name: 'Ethereum Mainnet', + nativeCurrency: 'ETH', + rpcEndpoints: [ + { + url: 'https://mainnet.infura.io/v3/{infuraProjectId}', + type: 'infura', + networkClientId: 'mainnet', + failoverUrls: [], + }, + ], + blockExplorerUrls: ['https://etherscan.io'], + defaultRpcEndpointIndex: 0, + defaultBlockExplorerUrlIndex: 0, + isActive: true, + isTestnet: false, + isDefault: true, + isFeatured: true, + isDeprecated: false, + priority: 0, + isDeletable: false, + ...overrides, + }; +} diff --git a/packages/config-registry-controller/tsconfig.build.json b/packages/config-registry-controller/tsconfig.build.json index 28e608feb56..70dd82381c2 100644 --- a/packages/config-registry-controller/tsconfig.build.json +++ b/packages/config-registry-controller/tsconfig.build.json @@ -5,6 +5,7 @@ "outDir": "./dist", "rootDir": "./src" }, + "exclude": ["**/*.test.ts", "./src/test-helpers.ts"], "references": [ { "path": "../base-controller/tsconfig.build.json" }, { "path": "../controller-utils/tsconfig.build.json" }, diff --git a/yarn.lock b/yarn.lock index 01cbed0ea40..53a1648da1b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2991,6 +2991,7 @@ __metadata: deepmerge: "npm:^4.2.2" jest: "npm:^27.5.1" nock: "npm:^13.3.1" + reselect: "npm:^5.1.1" sinon: "npm:^9.2.4" ts-jest: "npm:^27.1.4" typedoc: "npm:^0.24.8"