diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts index 70d65648a65..feb2c8d1d38 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.test.ts @@ -1,19 +1,13 @@ -import { ControllerMessenger } from '@metamask/base-controller'; import type { InternalAccount } from '@metamask/keyring-api'; import type nock from 'nock'; import encryption, { createSHA256Hash } from '../../shared/encryption'; import { USER_STORAGE_FEATURE_NAMES } from '../../shared/storage-schema'; -import type { - AuthenticationControllerGetBearerToken, - AuthenticationControllerGetSessionProfile, - AuthenticationControllerIsSignedIn, - AuthenticationControllerPerformSignIn, -} from '../authentication/AuthenticationController'; import { MOCK_INTERNAL_ACCOUNTS, MOCK_USER_STORAGE_ACCOUNTS, } from './__fixtures__/mockAccounts'; +import { mockUserStorageMessenger as _mockUserStorageMessenger } from './__fixtures__/mockMessenger'; import { mockEndpointBatchUpsertUserStorage, mockEndpointGetUserStorage, @@ -26,23 +20,16 @@ import { import { MOCK_STORAGE_DATA, MOCK_STORAGE_KEY, - MOCK_STORAGE_KEY_SIGNATURE, } from './__fixtures__/mockStorage'; +import { waitFor } from './__fixtures__/test-utils'; import * as AccountsUserStorageModule from './accounts/user-storage'; +import * as NetworkSyncIntegrationModule from './network-syncing/controller-integration'; import type { GetUserStorageAllFeatureEntriesResponse, GetUserStorageResponse, + UserStorageBaseOptions, } from './services'; -import type { - AllowedActions, - AllowedEvents, - NotificationServicesControllerDisableNotificationServices, - NotificationServicesControllerSelectIsNotificationServicesEnabled, -} from './UserStorageController'; -import UserStorageController from './UserStorageController'; - -const typedMockFn = unknown>() => - jest.fn, Parameters>(); +import UserStorageController, { defaultState } from './UserStorageController'; describe('user-storage/user-storage-controller - constructor() tests', () => { const arrangeMocks = () => { @@ -60,6 +47,41 @@ describe('user-storage/user-storage-controller - constructor() tests', () => { expect(controller.state.isProfileSyncingEnabled).toBe(true); }); + + it('should call startNetworkSyncing', async () => { + // Arrange Mock Syncing + const mockStartNetworkSyncing = jest.spyOn( + NetworkSyncIntegrationModule, + 'startNetworkSyncing', + ); + let storageConfig: UserStorageBaseOptions | null = null; + let isSyncingBlocked: boolean | null = null; + mockStartNetworkSyncing.mockImplementation( + ({ getStorageConfig, isMutationSyncBlocked }) => { + // eslint-disable-next-line no-void + void getStorageConfig().then((s) => (storageConfig = s)); + + isSyncingBlocked = isMutationSyncBlocked(); + }, + ); + + const { messengerMocks } = arrangeMocks(); + new UserStorageController({ + messenger: messengerMocks.messenger, + getMetaMetricsState: () => true, + env: { + isNetworkSyncingEnabled: true, + }, + state: { + ...defaultState, + hasNetworkSyncingSyncedAtLeastOnce: true, + }, + }); + + // Assert Syncing Properties + await waitFor(() => expect(storageConfig).toBeDefined()); + expect(isSyncingBlocked).toBe(false); + }); }); describe('user-storage/user-storage-controller - performGetStorage() tests', () => { @@ -1451,13 +1473,13 @@ describe('user-storage/user-storage-controller - syncInternalAccountsWithUserSto expect( messengerMocks.mockAccountsUpdateAccountMetadata, - ).toHaveBeenCalledWith([ + ).toHaveBeenCalledWith( MOCK_USER_STORAGE_ACCOUNTS.ONE_CUSTOM_NAME_WITHOUT_LAST_UPDATED[0].i, { name: MOCK_USER_STORAGE_ACCOUNTS .ONE_CUSTOM_NAME_WITHOUT_LAST_UPDATED[0].n, }, - ]); + ); }); it('does not update internal account name if both user storage and internal accounts have custom names without last updated', async () => { @@ -1626,13 +1648,13 @@ describe('user-storage/user-storage-controller - syncInternalAccountsWithUserSto expect( messengerMocks.mockAccountsUpdateAccountMetadata, - ).toHaveBeenCalledWith([ + ).toHaveBeenCalledWith( MOCK_USER_STORAGE_ACCOUNTS.ONE_CUSTOM_NAME_WITH_LAST_UPDATED[0].i, { name: MOCK_USER_STORAGE_ACCOUNTS.ONE_CUSTOM_NAME_WITH_LAST_UPDATED[0] .n, }, - ]); + ); }); it('updates the internal account name and last updated if the internal account name is a custom name without last updated', async () => { @@ -1669,7 +1691,7 @@ describe('user-storage/user-storage-controller - syncInternalAccountsWithUserSto expect( messengerMocks.mockAccountsUpdateAccountMetadata, - ).toHaveBeenCalledWith([ + ).toHaveBeenCalledWith( MOCK_USER_STORAGE_ACCOUNTS.ONE_CUSTOM_NAME_WITH_LAST_UPDATED[0].i, { name: MOCK_USER_STORAGE_ACCOUNTS.ONE_CUSTOM_NAME_WITH_LAST_UPDATED[0] @@ -1677,7 +1699,7 @@ describe('user-storage/user-storage-controller - syncInternalAccountsWithUserSto nameLastUpdatedAt: MOCK_USER_STORAGE_ACCOUNTS.ONE_CUSTOM_NAME_WITH_LAST_UPDATED[0].nlu, }, - ]); + ); }); it('updates the internal account name and last updated if the user storage account is more recent', async () => { @@ -1715,7 +1737,7 @@ describe('user-storage/user-storage-controller - syncInternalAccountsWithUserSto expect( messengerMocks.mockAccountsUpdateAccountMetadata, - ).toHaveBeenCalledWith([ + ).toHaveBeenCalledWith( MOCK_USER_STORAGE_ACCOUNTS.ONE_CUSTOM_NAME_WITH_LAST_UPDATED[0].i, { name: MOCK_USER_STORAGE_ACCOUNTS.ONE_CUSTOM_NAME_WITH_LAST_UPDATED[0] @@ -1723,7 +1745,7 @@ describe('user-storage/user-storage-controller - syncInternalAccountsWithUserSto nameLastUpdatedAt: MOCK_USER_STORAGE_ACCOUNTS.ONE_CUSTOM_NAME_WITH_LAST_UPDATED[0].nlu, }, - ]); + ); }); it('does not update the internal account if the user storage account is less recent', async () => { @@ -1978,6 +2000,86 @@ describe('user-storage/user-storage-controller - saveInternalAccountToUserStorag }); }); +describe('user-storage/user-storage-controller - syncNetworks() tests', () => { + const arrangeMocks = () => { + const messengerMocks = mockUserStorageMessenger(); + const mockPerformMainNetworkSync = jest.spyOn( + NetworkSyncIntegrationModule, + 'performMainNetworkSync', + ); + return { + messenger: messengerMocks.messenger, + mockPerformMainNetworkSync, + mockGetSessionProfile: messengerMocks.mockAuthGetSessionProfile, + }; + }; + + const nonImportantControllerProps = () => ({ + getMetaMetricsState: () => true, + }); + + it('should not be invoked if the feature is not enabled', async () => { + const { messenger, mockGetSessionProfile, mockPerformMainNetworkSync } = + arrangeMocks(); + const controller = new UserStorageController({ + ...nonImportantControllerProps(), + messenger, + env: { + isNetworkSyncingEnabled: false, + }, + }); + + await controller.syncNetworks(); + + expect(mockGetSessionProfile).not.toHaveBeenCalled(); + expect(mockPerformMainNetworkSync).not.toHaveBeenCalled(); + }); + + // NOTE the actual testing of the implementation is done in `controller-integration.ts` file. + // See relevant unit tests to see how this feature works and is tested + it('should invoke syncing if feature is enabled', async () => { + const { messenger, mockGetSessionProfile, mockPerformMainNetworkSync } = + arrangeMocks(); + const controller = new UserStorageController({ + ...nonImportantControllerProps(), + messenger, + env: { + isNetworkSyncingEnabled: true, + }, + config: { + networkSyncing: { + onNetworkAdded: jest.fn(), + onNetworkRemoved: jest.fn(), + onNetworkUpdated: jest.fn(), + }, + }, + }); + + // For test-coverage, we will simulate calling the analytic callback events + // This has been correctly tested in `controller-integration.test.ts` + mockPerformMainNetworkSync.mockImplementation( + async ({ + onNetworkAdded, + onNetworkRemoved, + onNetworkUpdated, + getStorageConfig, + }) => { + const config = await getStorageConfig(); + expect(config).toBeDefined(); + onNetworkAdded?.('0x1'); + onNetworkRemoved?.('0x1'); + onNetworkUpdated?.('0x1'); + }, + ); + + await controller.syncNetworks(); + + expect(mockGetSessionProfile).toHaveBeenCalled(); + expect(mockPerformMainNetworkSync).toHaveBeenCalled(); + expect(controller.state.hasNetworkSyncingSyncedAtLeastOnce).toBe(true); + }); +}); + /** * Jest Mock Utility - create a mock user storage messenger * @@ -1991,191 +2093,22 @@ function mockUserStorageMessenger(options?: { accountsList?: InternalAccount[]; }; }) { - const baseMessenger = new ControllerMessenger< - AllowedActions, - AllowedEvents - >(); - - const messenger = baseMessenger.getRestricted({ - name: 'UserStorageController', - allowedActions: [ - 'KeyringController:getState', - 'SnapController:handleRequest', - 'AuthenticationController:getBearerToken', - 'AuthenticationController:getSessionProfile', - 'AuthenticationController:isSignedIn', - 'AuthenticationController:performSignIn', - 'AuthenticationController:performSignOut', - 'NotificationServicesController:disableNotificationServices', - 'NotificationServicesController:selectIsNotificationServicesEnabled', - 'AccountsController:listAccounts', - 'AccountsController:updateAccountMetadata', - 'KeyringController:addNewAccount', - ], - allowedEvents: [ - 'KeyringController:lock', - 'KeyringController:unlock', - 'AccountsController:accountAdded', - 'AccountsController:accountRenamed', - ], - }); - - const mockSnapGetPublicKey = jest.fn().mockResolvedValue('MOCK_PUBLIC_KEY'); - const mockSnapSignMessage = jest - .fn() - .mockResolvedValue(MOCK_STORAGE_KEY_SIGNATURE); - - const mockAuthGetBearerToken = - typedMockFn< - AuthenticationControllerGetBearerToken['handler'] - >().mockResolvedValue('MOCK_BEARER_TOKEN'); - - const mockAuthGetSessionProfile = typedMockFn< - AuthenticationControllerGetSessionProfile['handler'] - >().mockResolvedValue({ - identifierId: '', - profileId: 'MOCK_PROFILE_ID', - }); + const messengerMocks = _mockUserStorageMessenger(); - const mockAuthPerformSignIn = - typedMockFn< - AuthenticationControllerPerformSignIn['handler'] - >().mockResolvedValue('New Access Token'); - - const mockAuthIsSignedIn = - typedMockFn< - AuthenticationControllerIsSignedIn['handler'] - >().mockReturnValue(true); - - const mockAuthPerformSignOut = - typedMockFn< - AuthenticationControllerIsSignedIn['handler'] - >().mockReturnValue(true); - - const mockNotificationServicesIsEnabled = - typedMockFn< - NotificationServicesControllerSelectIsNotificationServicesEnabled['handler'] - >().mockReturnValue(true); - - const mockNotificationServicesDisableNotifications = - typedMockFn< - NotificationServicesControllerDisableNotificationServices['handler'] - >().mockResolvedValue(); - - const mockKeyringAddNewAccount = jest.fn(() => { - baseMessenger.publish( + messengerMocks.mockKeyringAddNewAccount.mockImplementation(async () => { + messengerMocks.baseMessenger.publish( 'AccountsController:accountAdded', MOCK_INTERNAL_ACCOUNTS.ONE[0] as InternalAccount, ); return MOCK_INTERNAL_ACCOUNTS.ONE[0].address; }); - const mockAccountsListAccounts = jest - .fn() - .mockResolvedValue( - options?.accounts?.accountsList ?? MOCK_INTERNAL_ACCOUNTS.ALL, - ); - - const mockAccountsUpdateAccountMetadata = jest.fn().mockResolvedValue(true); - - jest.spyOn(messenger, 'call').mockImplementation((...args) => { - // Creates the correct typed call params for mocks - type CallParams = { - [K in AllowedActions['type']]: [ - K, - ...Parameters['handler']>, - ]; - }[AllowedActions['type']]; - - const [actionType, params] = args as unknown as CallParams; - - if (actionType === 'SnapController:handleRequest') { - if (params.request.method === 'getPublicKey') { - return mockSnapGetPublicKey(); - } - - if (params.request.method === 'signMessage') { - return mockSnapSignMessage(); - } - - throw new Error( - `MOCK_FAIL - unsupported SnapController:handleRequest call: ${ - params.request.method as string - }`, - ); - } - - if (actionType === 'AuthenticationController:getBearerToken') { - return mockAuthGetBearerToken(); - } - - if (actionType === 'AuthenticationController:getSessionProfile') { - return mockAuthGetSessionProfile(); - } - - if (actionType === 'AuthenticationController:performSignIn') { - return mockAuthPerformSignIn(); - } - - if (actionType === 'AuthenticationController:isSignedIn') { - return mockAuthIsSignedIn(); - } - - if ( - actionType === - 'NotificationServicesController:selectIsNotificationServicesEnabled' - ) { - return mockNotificationServicesIsEnabled(); - } - - if ( - actionType === - 'NotificationServicesController:disableNotificationServices' - ) { - return mockNotificationServicesDisableNotifications(); - } - - if (actionType === 'AuthenticationController:performSignOut') { - return mockAuthPerformSignOut(); - } - - if (actionType === 'KeyringController:getState') { - return { isUnlocked: true }; - } - - if (actionType === 'KeyringController:addNewAccount') { - return mockKeyringAddNewAccount(); - } - - if (actionType === 'AccountsController:listAccounts') { - return mockAccountsListAccounts(); - } - - if (actionType === 'AccountsController:updateAccountMetadata') { - return mockAccountsUpdateAccountMetadata(args.slice(1)); - } - - throw new Error( - `MOCK_FAIL - unsupported messenger call: ${actionType as string}`, - ); - }); + messengerMocks.mockAccountsListAccounts.mockReturnValue( + (options?.accounts?.accountsList ?? + MOCK_INTERNAL_ACCOUNTS.ALL) as InternalAccount[], + ); - return { - baseMessenger, - messenger, - mockSnapGetPublicKey, - mockSnapSignMessage, - mockAuthGetBearerToken, - mockAuthGetSessionProfile, - mockAuthPerformSignIn, - mockAuthIsSignedIn, - mockNotificationServicesIsEnabled, - mockNotificationServicesDisableNotifications, - mockAuthPerformSignOut, - mockKeyringAddNewAccount, - mockAccountsUpdateAccountMetadata, - mockAccountsListAccounts, - }; + return messengerMocks; } /** diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts index ee71b0dc2af..c698b101bc4 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts @@ -19,7 +19,13 @@ import { type KeyringControllerAddNewAccountAction, KeyringTypes, } from '@metamask/keyring-controller'; -import type { NetworkConfiguration } from '@metamask/network-controller'; +import type { + NetworkControllerAddNetworkAction, + NetworkControllerGetStateAction, + NetworkControllerNetworkRemovedEvent, + NetworkControllerRemoveNetworkAction, + NetworkControllerUpdateNetworkAction, +} from '@metamask/network-controller'; import type { HandleSnapRequest } from '@metamask/snaps-controllers'; import { createSHA256Hash } from '../../shared/encryption'; @@ -43,7 +49,10 @@ import { isNameDefaultAccountName, mapInternalAccountToUserStorageAccount, } from './accounts/user-storage'; -import { startNetworkSyncing } from './network-syncing/controller-integration'; +import { + performMainNetworkSync, + startNetworkSyncing, +} from './network-syncing/controller-integration'; import { batchDeleteUserStorage, batchUpsertUserStorage, @@ -54,27 +63,6 @@ import { upsertUserStorage, } from './services'; -// TODO: add external NetworkController event -// Need to listen for when a network gets added -type NetworkControllerNetworkAddedEvent = { - type: 'NetworkController:networkAdded'; - payload: [networkConfiguration: NetworkConfiguration]; -}; - -// TODO: add external NetworkController event -// Need to listen for when a network is updated, or the default rpc/block explorer changes -type NetworkControllerNetworkChangedEvent = { - type: 'NetworkController:networkChanged'; - payload: [networkConfiguration: NetworkConfiguration]; -}; - -// TODO: add external NetworkController event -// Need to listen for when a network gets deleted -type NetworkControllerNetworkDeletedEvent = { - type: 'NetworkController:networkDeleted'; - payload: [networkConfiguration: NetworkConfiguration]; -}; - // TODO: fix external dependencies export declare type NotificationServicesControllerDisableNotificationServices = { @@ -108,6 +96,10 @@ export type UserStorageControllerState = { * Condition used by UI to determine if account syncing is ready to be dispatched. */ isAccountSyncingReadyToBeDispatched: boolean; + /** + * Condition used to ensure that we do not perform any network sync mutations until we have synced at least once + */ + hasNetworkSyncingSyncedAtLeastOnce?: boolean; }; export const defaultState: UserStorageControllerState = { @@ -128,12 +120,16 @@ const metadata: StateMetadata = { }, hasAccountSyncingSyncedAtLeastOnce: { persist: true, - anonymous: true, + anonymous: false, }, isAccountSyncingReadyToBeDispatched: { persist: false, anonymous: false, }, + hasNetworkSyncingSyncedAtLeastOnce: { + persist: true, + anonymous: false, + }, }; type ControllerConfig = { @@ -160,6 +156,31 @@ type ControllerConfig = { situationMessage: string, ) => void; }; + + networkSyncing?: { + maxNumberOfNetworksToAdd?: number; + /** + * Callback that fires when network sync adds a network + * This is used for analytics. + * @param profileId - ID for a given User (shared cross devices once authenticated) + * @param chainId - Chain ID for the network added (in hex) + */ + onNetworkAdded?: (profileId: string, chainId: string) => void; + /** + * Callback that fires when network sync updates a network + * This is used for analytics. + * @param profileId - ID for a given User (shared cross devices once authenticated) + * @param chainId - Chain ID for the network added (in hex) + */ + onNetworkUpdated?: (profileId: string, chainId: string) => void; + /** + * Callback that fires when network sync deletes a network + * This is used for analytics. + * @param profileId - ID for a given User (shared cross devices once authenticated) + * @param chainId - Chain ID for the network added (in hex) + */ + onNetworkRemoved?: (profileId: string, chainId: string) => void; + }; }; // Messenger Actions @@ -216,10 +237,15 @@ export type AllowedActions = // Metamask Notifications | NotificationServicesControllerDisableNotificationServices | NotificationServicesControllerSelectIsNotificationServicesEnabled - // Account syncing + // Account Syncing | AccountsControllerListAccountsAction | AccountsControllerUpdateAccountMetadataAction - | KeyringControllerAddNewAccountAction; + | KeyringControllerAddNewAccountAction + // Network Syncing + | NetworkControllerGetStateAction + | NetworkControllerAddNetworkAction + | NetworkControllerRemoveNetworkAction + | NetworkControllerUpdateNetworkAction; // Messenger events export type UserStorageControllerStateChangeEvent = ControllerStateChangeEvent< @@ -249,9 +275,7 @@ export type AllowedEvents = | AccountsControllerAccountAddedEvent | AccountsControllerAccountRenamedEvent // Network Syncing Events - | NetworkControllerNetworkAddedEvent - | NetworkControllerNetworkChangedEvent - | NetworkControllerNetworkDeletedEvent; + | NetworkControllerNetworkRemovedEvent; // Messenger export type UserStorageControllerMessenger = RestrictedControllerMessenger< @@ -275,6 +299,13 @@ export default class UserStorageController extends BaseController< UserStorageControllerState, UserStorageControllerMessenger > { + // This is replaced with the actual value in the constructor + // We will remove this once the feature will be released + #env = { + isAccountSyncingEnabled: false, + isNetworkSyncingEnabled: false, + }; + #auth = { getBearerToken: async () => { return await this.messagingSystem.call( @@ -303,8 +334,6 @@ export default class UserStorageController extends BaseController< }; #accounts = { - // This is replaced with the actual value in the constructor - isAccountSyncingEnabled: false, isAccountSyncingInProgress: false, maxNumberOfAccountsToAdd: 0, canSync: () => { @@ -315,7 +344,7 @@ export default class UserStorageController extends BaseController< return false; } - if (!this.#accounts.isAccountSyncingEnabled) { + if (!this.#env.isAccountSyncingEnabled) { return false; } @@ -482,12 +511,10 @@ export default class UserStorageController extends BaseController< state: { ...defaultState, ...state }, }); + this.#env.isAccountSyncingEnabled = Boolean(env?.isAccountSyncingEnabled); + this.#env.isNetworkSyncingEnabled = Boolean(env?.isNetworkSyncingEnabled); this.#config = config; - this.#accounts.isAccountSyncingEnabled = Boolean( - env?.isAccountSyncingEnabled, - ); - this.#accounts.maxNumberOfAccountsToAdd = config?.accountSyncing?.maxNumberOfAccountsToAdd ?? 100; @@ -498,18 +525,12 @@ export default class UserStorageController extends BaseController< this.#accounts.setupAccountSyncingSubscriptions(); // Network Syncing - if (env?.isNetworkSyncingEnabled) { + if (this.#env.isNetworkSyncingEnabled) { startNetworkSyncing({ messenger, - getStorageConfig: async () => { - const { storageKey, bearerToken } = - await this.#getStorageKeyAndBearerToken(); - return { - storageKey, - bearerToken, - nativeScryptCrypto: this.#nativeScryptCrypto, - }; - }, + getStorageConfig: () => this.#getStorageOptions(), + isMutationSyncBlocked: () => + !this.state.hasNetworkSyncingSyncedAtLeastOnce, }); } } @@ -560,6 +581,20 @@ export default class UserStorageController extends BaseController< ); } + async #getStorageOptions() { + if (!this.state.isProfileSyncingEnabled) { + return null; + } + + const { storageKey, bearerToken } = + await this.#getStorageKeyAndBearerToken(); + return { + storageKey, + bearerToken, + nativeScryptCrypto: this.#nativeScryptCrypto, + }; + } + public async enableProfileSyncing(): Promise { try { this.#setIsProfileSyncingUpdateLoading(true); @@ -1128,4 +1163,28 @@ export default class UserStorageController extends BaseController< ); } } + + async syncNetworks() { + if (!this.#env.isNetworkSyncingEnabled) { + return; + } + + const profileId = await this.#auth.getProfileId(); + + await performMainNetworkSync({ + messenger: this.messagingSystem, + getStorageConfig: () => this.#getStorageOptions(), + maxNetworksToAdd: this.#config?.networkSyncing?.maxNumberOfNetworksToAdd, + onNetworkAdded: (cId) => + this.#config?.networkSyncing?.onNetworkAdded?.(profileId, cId), + onNetworkUpdated: (cId) => + this.#config?.networkSyncing?.onNetworkUpdated?.(profileId, cId), + onNetworkRemoved: (cId) => + this.#config?.networkSyncing?.onNetworkRemoved?.(profileId, cId), + }); + + this.update((s) => { + s.hasNetworkSyncingSyncedAtLeastOnce = true; + }); + } } diff --git a/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts new file mode 100644 index 00000000000..6281b4d0d79 --- /dev/null +++ b/packages/profile-sync-controller/src/controllers/user-storage/__fixtures__/mockMessenger.ts @@ -0,0 +1,287 @@ +import type { NotNamespacedBy } from '@metamask/base-controller'; +import { ControllerMessenger } from '@metamask/base-controller'; + +import { MOCK_STORAGE_KEY_SIGNATURE } from '.'; +import type { + AllowedActions, + AllowedEvents, + UserStorageControllerMessenger, +} from '..'; + +type GetHandler = Extract< + AllowedActions, + { type: ActionType } +>['handler']; + +type CallParams = { + [K in AllowedActions['type']]: [ + K, + ...Parameters['handler']>, + ]; +}[AllowedActions['type']]; + +const typedMockFn = < + ActionType extends AllowedActions['type'], + Handler = GetHandler, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Func extends (...args: any) => any = Handler extends (...args: any[]) => any + ? Handler + : never, +>( + _type: ActionType, +) => jest.fn, Parameters>(); + +type ExternalEvents = NotNamespacedBy< + 'UserStorageController', + AllowedEvents['type'] +>; + +/** + * creates a custom user storage messenger, in case tests need different permissions + * @param props - overrides + * @param props.overrideEvents - override events + * @returns base messenger, and messenger. You can pass this into the mocks below to mock messenger calls + */ +export function createCustomUserStorageMessenger(props?: { + overrideEvents?: ExternalEvents[]; +}) { + const baseMessenger = new ControllerMessenger< + AllowedActions, + AllowedEvents + >(); + const messenger = baseMessenger.getRestricted({ + name: 'UserStorageController', + allowedActions: [ + 'KeyringController:getState', + 'KeyringController:addNewAccount', + 'SnapController:handleRequest', + 'AuthenticationController:getBearerToken', + 'AuthenticationController:getSessionProfile', + 'AuthenticationController:isSignedIn', + 'AuthenticationController:performSignOut', + 'AuthenticationController:performSignIn', + 'NotificationServicesController:disableNotificationServices', + 'NotificationServicesController:selectIsNotificationServicesEnabled', + 'AccountsController:listAccounts', + 'AccountsController:updateAccountMetadata', + 'NetworkController:getState', + 'NetworkController:addNetwork', + 'NetworkController:updateNetwork', + 'NetworkController:removeNetwork', + ], + allowedEvents: props?.overrideEvents ?? [ + 'KeyringController:lock', + 'KeyringController:unlock', + 'AccountsController:accountAdded', + 'AccountsController:accountRenamed', + 'NetworkController:networkRemoved', + ], + }); + + return { + baseMessenger, + messenger, + }; +} + +type OverrideMessengers = { + baseMessenger: ControllerMessenger; + messenger: UserStorageControllerMessenger; +}; + +/** + * Jest Mock Utility to generate a mock User Storage Messenger + * @param overrideMessengers - override messengers if need to modify the underlying permissions + * @returns series of mocks to actions that can be called + */ +export function mockUserStorageMessenger( + overrideMessengers?: OverrideMessengers, +) { + const { baseMessenger, messenger } = + overrideMessengers ?? createCustomUserStorageMessenger(); + + const mockSnapGetPublicKey = jest.fn().mockResolvedValue('MOCK_PUBLIC_KEY'); + const mockSnapSignMessage = jest + .fn() + .mockResolvedValue(MOCK_STORAGE_KEY_SIGNATURE); + + const mockAuthGetBearerToken = typedMockFn( + 'AuthenticationController:getBearerToken', + ).mockResolvedValue('MOCK_BEARER_TOKEN'); + + const mockAuthGetSessionProfile = typedMockFn( + 'AuthenticationController:getSessionProfile', + ).mockResolvedValue({ + identifierId: '', + profileId: 'MOCK_PROFILE_ID', + }); + + const mockAuthPerformSignIn = typedMockFn( + 'AuthenticationController:performSignIn', + ).mockResolvedValue('New Access Token'); + + const mockAuthIsSignedIn = typedMockFn( + 'AuthenticationController:isSignedIn', + ).mockReturnValue(true); + + const mockAuthPerformSignOut = typedMockFn( + 'AuthenticationController:performSignOut', + ); + + const mockNotificationServicesIsEnabled = typedMockFn( + 'NotificationServicesController:selectIsNotificationServicesEnabled', + ).mockReturnValue(true); + + const mockNotificationServicesDisableNotifications = typedMockFn( + 'NotificationServicesController:disableNotificationServices', + ).mockResolvedValue(); + + const mockKeyringAddNewAccount = typedMockFn( + 'KeyringController:addNewAccount', + ); + + // Untyped mock as there is a TS(2742) issue. + // This will return `InternalAccount[]` + const mockAccountsListAccounts = jest.fn(); + + const mockAccountsUpdateAccountMetadata = typedMockFn( + 'AccountsController:updateAccountMetadata', + ).mockResolvedValue(true as never); + + const mockNetworkControllerGetState = typedMockFn( + 'NetworkController:getState', + ).mockReturnValue({ + selectedNetworkClientId: '', + networksMetadata: {}, + networkConfigurationsByChainId: {}, + }); + + const mockNetworkControllerAddNetwork = typedMockFn( + 'NetworkController:addNetwork', + ); + + const mockNetworkControllerRemoveNetwork = typedMockFn( + 'NetworkController:removeNetwork', + ); + + const mockNetworkControllerUpdateNetwork = typedMockFn( + 'NetworkController:updateNetwork', + ); + + jest.spyOn(messenger, 'call').mockImplementation((...args) => { + const typedArgs = args as unknown as CallParams; + const [actionType] = typedArgs; + + if (actionType === 'SnapController:handleRequest') { + const [, params] = typedArgs; + if (params.request.method === 'getPublicKey') { + return mockSnapGetPublicKey(); + } + + if (params.request.method === 'signMessage') { + return mockSnapSignMessage(); + } + + throw new Error( + `MOCK_FAIL - unsupported SnapController:handleRequest call: ${ + params.request.method as string + }`, + ); + } + + if (actionType === 'AuthenticationController:getBearerToken') { + return mockAuthGetBearerToken(); + } + + if (actionType === 'AuthenticationController:getSessionProfile') { + return mockAuthGetSessionProfile(); + } + + if (actionType === 'AuthenticationController:performSignIn') { + return mockAuthPerformSignIn(); + } + + if (actionType === 'AuthenticationController:isSignedIn') { + return mockAuthIsSignedIn(); + } + + if ( + actionType === + 'NotificationServicesController:selectIsNotificationServicesEnabled' + ) { + return mockNotificationServicesIsEnabled(); + } + + if ( + actionType === + 'NotificationServicesController:disableNotificationServices' + ) { + return mockNotificationServicesDisableNotifications(); + } + + if (actionType === 'AuthenticationController:performSignOut') { + return mockAuthPerformSignOut(); + } + + if (actionType === 'KeyringController:getState') { + return { isUnlocked: true }; + } + + if (actionType === 'KeyringController:addNewAccount') { + return mockKeyringAddNewAccount(); + } + + if (actionType === 'AccountsController:listAccounts') { + return mockAccountsListAccounts(); + } + + if (typedArgs[0] === 'AccountsController:updateAccountMetadata') { + const [, ...params] = typedArgs; + return mockAccountsUpdateAccountMetadata(...params); + } + + if (actionType === 'NetworkController:getState') { + return mockNetworkControllerGetState(); + } + + if (actionType === 'NetworkController:addNetwork') { + const [, ...params] = typedArgs; + return mockNetworkControllerAddNetwork(...params); + } + + if (actionType === 'NetworkController:removeNetwork') { + const [, ...params] = typedArgs; + return mockNetworkControllerRemoveNetwork(...params); + } + + if (actionType === 'NetworkController:updateNetwork') { + const [, ...params] = typedArgs; + return mockNetworkControllerUpdateNetwork(...params); + } + + throw new Error( + `MOCK_FAIL - unsupported messenger call: ${actionType as string}`, + ); + }); + + return { + baseMessenger, + messenger, + mockSnapGetPublicKey, + mockSnapSignMessage, + mockAuthGetBearerToken, + mockAuthGetSessionProfile, + mockAuthPerformSignIn, + mockAuthIsSignedIn, + mockNotificationServicesIsEnabled, + mockNotificationServicesDisableNotifications, + mockAuthPerformSignOut, + mockKeyringAddNewAccount, + mockAccountsUpdateAccountMetadata, + mockAccountsListAccounts, + mockNetworkControllerGetState, + mockNetworkControllerAddNetwork, + mockNetworkControllerRemoveNetwork, + mockNetworkControllerUpdateNetwork, + }; +} diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/__fixtures__/mockNetwork.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/__fixtures__/mockNetwork.ts index 373c7d117da..e4b1caf9906 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/__fixtures__/mockNetwork.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/__fixtures__/mockNetwork.ts @@ -1,14 +1,16 @@ -import type { - NetworkConfiguration, - RemoteNetworkConfiguration, -} from '../types'; +import type { NetworkConfiguration } from '@metamask/network-controller'; +import { RpcEndpointType } from '@metamask/network-controller'; + +import type { RemoteNetworkConfiguration } from '../types'; + +export type RPCEndpoint = NetworkConfiguration['rpcEndpoints'][number]; export const createMockNetworkConfiguration = ( override?: Partial, ): NetworkConfiguration => { return { chainId: '0x1337', - blockExplorerUrls: [], + blockExplorerUrls: ['https://etherscan.io'], defaultRpcEndpointIndex: 0, name: 'Mock Network', nativeCurrency: 'MOCK TOKEN', @@ -27,3 +29,22 @@ export const createMockRemoteNetworkConfiguration = ( ...override, }; }; + +export const createMockCustomRpcEndpoint = ( + override: Partial>, +): RPCEndpoint => { + return { + type: RpcEndpointType.Custom, + networkClientId: '1111-1111-1111', + url: `https://FAKE_RPC/`, + ...override, + } as RPCEndpoint; +}; + +export const createMockInfuraRpcEndpoint = (): RPCEndpoint => { + return { + type: RpcEndpointType.Infura, + networkClientId: 'mainnet', + url: `https://mainnet.infura.io/v3/{infuraProjectId}`, + }; +}; diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/add-network-utils.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/add-network-utils.test.ts new file mode 100644 index 00000000000..ae394585f53 --- /dev/null +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/add-network-utils.test.ts @@ -0,0 +1,68 @@ +import { createMockNetworkConfiguration } from './__fixtures__/mockNetwork'; +import { + calculateAvailableSpaceToAdd, + getBoundedNetworksToAdd, +} from './add-network-utils'; + +describe('calculateAvailableSpaceToAdd()', () => { + it('returns available space to add', () => { + expect(calculateAvailableSpaceToAdd(5, 10)).toBe(5); + expect(calculateAvailableSpaceToAdd(9, 10)).toBe(1); + }); + it('returns 0 if there is no available space to add', () => { + expect(calculateAvailableSpaceToAdd(5, 5)).toBe(0); + expect(calculateAvailableSpaceToAdd(10, 5)).toBe(0); + }); +}); + +describe('getBoundedNetworksToAdd()', () => { + it('returns networks to add if within bounds', () => { + const originalNetworks = arrangeTestNetworks(['0x1', '0x2']); + const networksToAdd = arrangeTestNetworks(['0x3', '0x4']); + const result = getBoundedNetworksToAdd(originalNetworks, networksToAdd); + expect(result).toHaveLength(2); // we can all networks + }); + + it('returns a max size of networks to add if larger than max bounds', () => { + const originalNetworks = arrangeTestNetworks(['0x1', '0x2']); + const networksToAdd = arrangeTestNetworks(['0x3', '0x4']); + const result = getBoundedNetworksToAdd(originalNetworks, networksToAdd, 3); // max size set to 3 + expect(result).toHaveLength(1); // we can only add 1 network + }); + + it('returns an empty array if there is not available space to add networks', () => { + const originalNetworks = arrangeTestNetworks(['0x1', '0x2']); + const networksToAdd = arrangeTestNetworks(['0x3', '0x4']); + + const result2 = getBoundedNetworksToAdd(originalNetworks, networksToAdd, 2); // max size is set to 2 + expect(result2).toHaveLength(0); // we've used up all the available space, so no networks can be added + + const result3 = getBoundedNetworksToAdd(originalNetworks, networksToAdd, 1); // max size is set to 1 + expect(result3).toHaveLength(0); // we've used up all the available space, so no networks can be added + }); + + it('returns a list of networks ordered by chainId to add', () => { + const originalNetworks = arrangeTestNetworks(['0x1', '0x2']); + const networksToAdd = arrangeTestNetworks(['0x3', '0x4', '0x33']); + + const result = getBoundedNetworksToAdd(originalNetworks, networksToAdd, 4); // Max size is set to 4 + expect(result).toHaveLength(2); // We can only add 2 of the 3 networks to add + + // we are only adding 0x3 and 0x33 since the list was ordered + // 0x4 was dropped as we ran out of available space + expect(result.map((n) => n.chainId)).toStrictEqual(['0x3', '0x33']); + }); + + /** + * Test Utility - creates an array of network configurations + * @param chains - list of chains to create + * @returns array of mock network configurations + */ + function arrangeTestNetworks(chains: `0x${string}`[]) { + return chains.map((chainId) => { + const n = createMockNetworkConfiguration(); + n.chainId = chainId; + return n; + }); + } +}); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/add-network-utils.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/add-network-utils.ts new file mode 100644 index 00000000000..7cb7e12d0f3 --- /dev/null +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/add-network-utils.ts @@ -0,0 +1,47 @@ +import type { NetworkConfiguration } from '@metamask/network-controller'; + +export const MAX_NETWORKS_SIZE = 50; + +/** + * Calculates the available space to add new networks + * exported for testability. + * @param originalListSize - size of original list + * @param maxSize - max size + * @returns a positive number on the available space + */ +export const calculateAvailableSpaceToAdd = ( + originalListSize: number, + maxSize: number, +) => { + return Math.max(0, maxSize - originalListSize); +}; + +/** + * Returns a bounded number of networks to add (set by a max bound) + * The items will be ordered to give determinism on items to append (not random) + * + * @param originalNetworks - The original list of network configurations. + * @param networksToAdd - The list of network configurations to add. + * @param maxSize - The maximum allowed size of the list. Defaults to MAX_NETWORKS_SIZE. + * @returns The networks to add, sorted by chainId. + */ +export const getBoundedNetworksToAdd = ( + originalNetworks: NetworkConfiguration[], + networksToAdd: NetworkConfiguration[], + maxSize = MAX_NETWORKS_SIZE, +) => { + const availableSpace = calculateAvailableSpaceToAdd( + originalNetworks.length, + maxSize, + ); + const numberOfNetworksToAppend = Math.min( + availableSpace, + networksToAdd.length, + ); + + // Order and slice the networks to append + // Ordering so we have some determinism on the order of items + return networksToAdd + .sort((a, b) => a.chainId.localeCompare(b.chainId)) + .slice(0, numberOfNetworksToAppend); +}; diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.test.ts index 162d865dcab..35cab221557 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.test.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.test.ts @@ -1,14 +1,24 @@ -import type { NotNamespacedBy } from '@metamask/base-controller'; -import { ControllerMessenger } from '@metamask/base-controller'; import log from 'loglevel'; -import type { AllowedActions, AllowedEvents } from '..'; import { MOCK_STORAGE_KEY } from '../__fixtures__'; +import { + createCustomUserStorageMessenger, + mockUserStorageMessenger, +} from '../__fixtures__/mockMessenger'; import { waitFor } from '../__fixtures__/test-utils'; import type { UserStorageBaseOptions } from '../services'; -import { createMockNetworkConfiguration } from './__fixtures__/mockNetwork'; -import { startNetworkSyncing } from './controller-integration'; -import * as SyncModule from './sync-mutations'; +import { + createMockNetworkConfiguration, + createMockRemoteNetworkConfiguration, +} from './__fixtures__/mockNetwork'; +import { + performMainNetworkSync, + startNetworkSyncing, +} from './controller-integration'; +import * as ControllerIntegrationModule from './controller-integration'; +import * as ServicesModule from './services'; +import * as SyncAllModule from './sync-all'; +import * as SyncMutationsModule from './sync-mutations'; jest.mock('loglevel', () => { const actual = jest.requireActual('loglevel'); @@ -30,121 +40,347 @@ const storageOpts: UserStorageBaseOptions = { storageKey: MOCK_STORAGE_KEY, }; -type ExternalEvents = NotNamespacedBy< - 'UserStorageController', - AllowedEvents['type'] ->; -const getEvents = (): ExternalEvents[] => [ - 'NetworkController:networkAdded', - 'NetworkController:networkChanged', - 'NetworkController:networkDeleted', -]; - -const testMatrix = [ - { - event: 'NetworkController:networkAdded' as const, - arrangeSyncFnMock: () => - jest.spyOn(SyncModule, 'addNetwork').mockResolvedValue(), - }, - { - event: 'NetworkController:networkChanged' as const, - arrangeSyncFnMock: () => - jest.spyOn(SyncModule, 'updateNetwork').mockResolvedValue(), - }, - { - event: 'NetworkController:networkDeleted' as const, - arrangeSyncFnMock: () => - jest.spyOn(SyncModule, 'deleteNetwork').mockResolvedValue(), - }, -]; - -describe.each(testMatrix)( - 'network-syncing/controller-integration - $event', - ({ event, arrangeSyncFnMock }) => { - it(`should successfully sync when ${event} is emitted`, async () => { - const syncFnMock = arrangeSyncFnMock(); - const { baseMessenger, messenger, getStorageConfig } = arrangeMocks(); - startNetworkSyncing({ messenger, getStorageConfig }); - baseMessenger.publish(event, createMockNetworkConfiguration()); - - await waitFor(() => { - expect(getStorageConfig).toHaveBeenCalled(); - expect(syncFnMock).toHaveBeenCalled(); - }); +describe('network-syncing/controller-integration - startNetworkSyncing()', () => { + it(`should successfully sync when NetworkController:networkRemoved is emitted`, async () => { + const { baseMessenger, props, deleteNetworkMock } = arrangeMocks(); + startNetworkSyncing(props); + baseMessenger.publish( + 'NetworkController:networkRemoved', + createMockNetworkConfiguration(), + ); + + await waitFor(() => { + expect(props.getStorageConfig).toHaveBeenCalled(); + expect(deleteNetworkMock).toHaveBeenCalled(); }); + }); - it('should silently fail is unable to authenticate or get storage key', async () => { - const syncFnMock = arrangeSyncFnMock(); - const { baseMessenger, messenger, getStorageConfig } = arrangeMocks(); - getStorageConfig.mockRejectedValue(new Error('Mock Error')); - startNetworkSyncing({ messenger, getStorageConfig }); - baseMessenger.publish(event, createMockNetworkConfiguration()); + it('should silently fail is unable to authenticate or get storage key', async () => { + const { baseMessenger, props, deleteNetworkMock } = arrangeMocks(); + props.getStorageConfig.mockRejectedValue(new Error('Mock Error')); + startNetworkSyncing(props); + baseMessenger.publish( + 'NetworkController:networkRemoved', + createMockNetworkConfiguration(), + ); - expect(getStorageConfig).toHaveBeenCalled(); - expect(syncFnMock).not.toHaveBeenCalled(); + await waitFor(() => { + expect(props.getStorageConfig).toHaveBeenCalled(); + expect(deleteNetworkMock).not.toHaveBeenCalled(); }); + }); + + it('should silently fail if unable to get storage config', async () => { + const { baseMessenger, props, deleteNetworkMock } = arrangeMocks(); + props.getStorageConfig.mockResolvedValue(null); + startNetworkSyncing(props); + baseMessenger.publish( + 'NetworkController:networkRemoved', + createMockNetworkConfiguration(), + ); - it(`should emit a warning if controller messenger is missing the ${event} event`, async () => { - const { baseMessenger, getStorageConfig } = arrangeMocks(); + await waitFor(() => { + expect(props.getStorageConfig).toHaveBeenCalled(); + expect(deleteNetworkMock).not.toHaveBeenCalled(); + }); + }); - const eventsWithoutNetworkAdded = getEvents().filter((e) => e !== event); - const messenger = mockUserStorageMessenger( - baseMessenger, - eventsWithoutNetworkAdded, - ); + it(`should emit a warning if controller messenger is missing the NetworkController:networkRemoved event`, async () => { + // arrange without setting event permissions + const { props } = arrangeMocks(); + const { messenger } = mockUserStorageMessenger( + createCustomUserStorageMessenger({ overrideEvents: [] }), + ); - startNetworkSyncing({ messenger, getStorageConfig }); + await waitFor(() => { + startNetworkSyncing({ ...props, messenger }); expect(warnMock).toHaveBeenCalled(); }); - }, -); - -/** - * Test Utility - arrange mocks and parameters - * @returns the mocks and parameters used when testing `startNetworkSyncing()` - */ -function arrangeMocks() { - const baseMessenger = mockBaseMessenger(); - const messenger = mockUserStorageMessenger(baseMessenger); - const getStorageConfigMock = jest.fn().mockResolvedValue(storageOpts); + }); - return { - getStorageConfig: getStorageConfigMock, - baseMessenger, - messenger, - }; -} - -/** - * Test Utility - creates a base messenger so we can invoke/publish events - * @returns Base messenger for publishing events - */ -function mockBaseMessenger() { - const baseMessenger = new ControllerMessenger< - AllowedActions, - AllowedEvents - >(); - - return baseMessenger; -} - -/** - * Test Utility - creates a UserStorageMessenger to simulate the messenger used inside the UserStorageController - * @param baseMessenger - base messenger to restrict - * @param eventsOverride - provide optional override events - * @returns UserStorageMessenger - */ -function mockUserStorageMessenger( - baseMessenger: ReturnType, - eventsOverride?: ExternalEvents[], -) { - const allowedEvents = eventsOverride ?? getEvents(); - - const messenger = baseMessenger.getRestricted({ - name: 'UserStorageController', - allowedActions: [], - allowedEvents, + it('should not remove networks if main sync is in progress', async () => { + const { baseMessenger, props, deleteNetworkMock } = arrangeMocks(); + + // TODO - replace with jest.replaceProperty once we upgrade jest. + Object.defineProperty( + ControllerIntegrationModule, + 'isMainNetworkSyncInProgress', + { value: true }, + ); + + startNetworkSyncing(props); + + baseMessenger.publish( + 'NetworkController:networkRemoved', + createMockNetworkConfiguration(), + ); + + expect(props.getStorageConfig).not.toHaveBeenCalled(); + expect(deleteNetworkMock).not.toHaveBeenCalled(); + + // Reset this property + Object.defineProperty( + ControllerIntegrationModule, + 'isMainNetworkSyncInProgress', + { value: false }, + ); + }); + + it('should not remove networks if the mutation sync is blocked (e.g. main sync has not happened before)', async () => { + const { props, baseMessenger, deleteNetworkMock } = arrangeMocks(); + const mockIsBlocked = jest.fn(() => true); + startNetworkSyncing({ ...props, isMutationSyncBlocked: mockIsBlocked }); + + baseMessenger.publish( + 'NetworkController:networkRemoved', + createMockNetworkConfiguration(), + ); + + expect(mockIsBlocked).toHaveBeenCalled(); + expect(props.getStorageConfig).not.toHaveBeenCalled(); + expect(deleteNetworkMock).not.toHaveBeenCalled(); + }); + + /** + * Test Utility - arrange mocks and parameters + * @returns the mocks and parameters used when testing `startNetworkSyncing()` + */ + function arrangeMocks() { + const messengerMocks = mockUserStorageMessenger(); + const getStorageConfigMock = jest.fn().mockResolvedValue(storageOpts); + const deleteNetworkMock = jest + .spyOn(SyncMutationsModule, 'deleteNetwork') + .mockResolvedValue(); + + return { + props: { + getStorageConfig: getStorageConfigMock, + messenger: messengerMocks.messenger, + isMutationSyncBlocked: () => false, + }, + deleteNetworkMock, + baseMessenger: messengerMocks.baseMessenger, + }; + } +}); + +describe('network-syncing/controller-integration - performMainSync()', () => { + it('should do nothing if unable to get storage config', async () => { + const { getStorageConfig, messenger, mockCalls } = arrangeMocks(); + getStorageConfig.mockResolvedValue(null); + + await performMainNetworkSync({ messenger, getStorageConfig }); + expect(getStorageConfig).toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerGetState).not.toHaveBeenCalled(); + }); + + it('should do nothing if unable to calculate networks to update', async () => { + const { messenger, getStorageConfig, mockSync, mockServices, mockCalls } = + arrangeMocks(); + mockSync.findNetworksToUpdate.mockReturnValue(undefined); + + await performMainNetworkSync({ messenger, getStorageConfig }); + expect(mockServices.mockBatchUpdateNetworks).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerAddNetwork).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerUpdateNetwork).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerRemoveNetwork).not.toHaveBeenCalled(); + }); + + it('should update remote networks if there are local networks to add', async () => { + const { messenger, getStorageConfig, mockSync, mockServices, mockCalls } = + arrangeMocks(); + mockSync.findNetworksToUpdate.mockReturnValue({ + remoteNetworksToUpdate: [createMockRemoteNetworkConfiguration()], + missingLocalNetworks: [], + localNetworksToUpdate: [], + localNetworksToRemove: [], + }); + + await performMainNetworkSync({ + messenger, + getStorageConfig, + }); + + expect(mockServices.mockBatchUpdateNetworks).toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerAddNetwork).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerUpdateNetwork).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerRemoveNetwork).not.toHaveBeenCalled(); + }); + + it('should add missing local networks', async () => { + const { messenger, getStorageConfig, mockSync, mockServices, mockCalls } = + arrangeMocks(); + mockSync.findNetworksToUpdate.mockReturnValue({ + remoteNetworksToUpdate: [], + missingLocalNetworks: [createMockNetworkConfiguration()], + localNetworksToUpdate: [], + localNetworksToRemove: [], + }); + + const mockAddCallback = jest.fn(); + await performMainNetworkSync({ + messenger, + getStorageConfig, + onNetworkAdded: mockAddCallback, + }); + + expect(mockServices.mockBatchUpdateNetworks).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerAddNetwork).toHaveBeenCalled(); + expect(mockAddCallback).toHaveBeenCalledTimes(1); + expect(mockCalls.mockNetworkControllerUpdateNetwork).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerRemoveNetwork).not.toHaveBeenCalled(); }); - return messenger; -} + it('should not add missing local networks if there is no available space', async () => { + const { messenger, getStorageConfig, mockSync, mockServices, mockCalls } = + arrangeMocks(); + mockSync.findNetworksToUpdate.mockReturnValue({ + remoteNetworksToUpdate: [], + missingLocalNetworks: [createMockNetworkConfiguration()], + localNetworksToUpdate: [], + localNetworksToRemove: [], + }); + + const mockAddCallback = jest.fn(); + await performMainNetworkSync({ + messenger, + getStorageConfig, + onNetworkAdded: mockAddCallback, + maxNetworksToAdd: 0, // mocking that there is no available space + }); + + expect(mockServices.mockBatchUpdateNetworks).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerAddNetwork).not.toHaveBeenCalled(); + expect(mockAddCallback).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerUpdateNetwork).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerRemoveNetwork).not.toHaveBeenCalled(); + }); + + it('should update local networks', async () => { + const { messenger, getStorageConfig, mockSync, mockServices, mockCalls } = + arrangeMocks(); + mockSync.findNetworksToUpdate.mockReturnValue({ + remoteNetworksToUpdate: [], + missingLocalNetworks: [], + localNetworksToUpdate: [createMockNetworkConfiguration()], + localNetworksToRemove: [], + }); + + const mockUpdateCallback = jest.fn(); + await performMainNetworkSync({ + messenger, + getStorageConfig, + onNetworkUpdated: mockUpdateCallback, + }); + + expect(mockServices.mockBatchUpdateNetworks).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerAddNetwork).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerUpdateNetwork).toHaveBeenCalled(); + expect(mockUpdateCallback).toHaveBeenCalledTimes(1); + expect(mockCalls.mockNetworkControllerRemoveNetwork).not.toHaveBeenCalled(); + }); + + it('should remove local networks', async () => { + const { messenger, getStorageConfig, mockSync, mockServices, mockCalls } = + arrangeMocks(); + mockSync.findNetworksToUpdate.mockReturnValue({ + remoteNetworksToUpdate: [], + missingLocalNetworks: [], + localNetworksToUpdate: [], + localNetworksToRemove: [createMockNetworkConfiguration()], + }); + + const mockRemoveCallback = jest.fn(); + await performMainNetworkSync({ + messenger, + getStorageConfig, + onNetworkRemoved: mockRemoveCallback, + }); + expect(mockServices.mockBatchUpdateNetworks).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerAddNetwork).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerUpdateNetwork).not.toHaveBeenCalled(); + expect(mockCalls.mockNetworkControllerRemoveNetwork).toHaveBeenCalled(); + expect(mockRemoveCallback).toHaveBeenCalledTimes(1); + }); + + it('should handle multiple networks to update', async () => { + const { messenger, getStorageConfig, mockSync, mockServices, mockCalls } = + arrangeMocks(); + mockSync.findNetworksToUpdate.mockReturnValue({ + remoteNetworksToUpdate: [ + createMockRemoteNetworkConfiguration(), + createMockRemoteNetworkConfiguration(), + ], + missingLocalNetworks: [ + createMockNetworkConfiguration(), + createMockNetworkConfiguration(), + ], + localNetworksToUpdate: [ + createMockNetworkConfiguration(), + createMockNetworkConfiguration(), + ], + localNetworksToRemove: [ + createMockNetworkConfiguration(), + createMockNetworkConfiguration(), + ], + }); + + await performMainNetworkSync({ messenger, getStorageConfig }); + expect(mockServices.mockBatchUpdateNetworks).toHaveBeenCalledTimes(1); + expect(mockCalls.mockNetworkControllerAddNetwork).toHaveBeenCalledTimes(2); + expect(mockCalls.mockNetworkControllerUpdateNetwork).toHaveBeenCalledTimes( + 2, + ); + expect(mockCalls.mockNetworkControllerRemoveNetwork).toHaveBeenCalledTimes( + 2, + ); + }); + + /** + * Jest Mock Utility - create suite of mocks for tests + * @returns mocks for tests + */ + function arrangeMocks() { + const messengerMocks = mockUserStorageMessenger(); + const getStorageConfigMock = jest + .fn, []>() + .mockResolvedValue(storageOpts); + + return { + baseMessenger: messengerMocks.baseMessenger, + messenger: messengerMocks.messenger, + getStorageConfig: getStorageConfigMock, + mockCalls: { + mockNetworkControllerGetState: + messengerMocks.mockNetworkControllerGetState.mockReturnValue({ + networkConfigurationsByChainId: { + '0x1337': createMockNetworkConfiguration(), + }, + selectedNetworkClientId: '1111-1111-1111', + networksMetadata: {}, + }), + mockNetworkControllerAddNetwork: + messengerMocks.mockNetworkControllerAddNetwork, + mockNetworkControllerRemoveNetwork: + messengerMocks.mockNetworkControllerRemoveNetwork, + mockNetworkControllerUpdateNetwork: + messengerMocks.mockNetworkControllerUpdateNetwork.mockResolvedValue( + createMockNetworkConfiguration(), + ), + }, + mockServices: { + mockGetAllRemoveNetworks: jest + .spyOn(ServicesModule, 'getAllRemoteNetworks') + .mockResolvedValue([]), + mockBatchUpdateNetworks: jest + .spyOn(ServicesModule, 'batchUpsertRemoteNetworks') + .mockResolvedValue(), + }, + mockSync: { + findNetworksToUpdate: jest + .spyOn(SyncAllModule, 'findNetworksToUpdate') + .mockReturnValue(undefined), + }, + }; + } +}); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.ts index 901046a2951..63df3b5721b 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.ts @@ -1,29 +1,71 @@ +import type { NetworkConfiguration } from '@metamask/network-controller'; import log from 'loglevel'; import type { UserStorageBaseOptions } from '../services'; import type { UserStorageControllerMessenger } from '../UserStorageController'; -import { addNetwork, deleteNetwork, updateNetwork } from './sync-mutations'; +import { getBoundedNetworksToAdd } from './add-network-utils'; +import { getAllRemoteNetworks } from './services'; +import { findNetworksToUpdate } from './sync-all'; +import { batchUpdateNetworks, deleteNetwork } from './sync-mutations'; +import { createUpdateNetworkProps } from './update-network-utils'; -type SetupNetworkSyncingProps = { +type StartNetworkSyncingProps = { messenger: UserStorageControllerMessenger; - getStorageConfig: () => Promise; + getStorageConfig: () => Promise; + isMutationSyncBlocked: () => boolean; }; +type PerformMainNetworkSyncProps = { + messenger: UserStorageControllerMessenger; + getStorageConfig: () => Promise; + maxNetworksToAdd?: number; + onNetworkAdded?: (chainId: string) => void; + onNetworkUpdated?: (chainId: string) => void; + onNetworkRemoved?: (chainId: string) => void; +}; + +/** + * Global in-mem cache to signify that the network syncing is in progress + * Ensures that listeners do not fire during main sync (prevent double requests) + */ +// Exported to help testing +// eslint-disable-next-line import/no-mutable-exports +export let isMainNetworkSyncInProgress = false; + /** * Initialize and setup events to listen to for network syncing + * We will be listening to: + * - Remove Event, to indicate that we need to remote network from remote + * + * We will not be listening to: + * - Add/Update events are not required, as we can sync these during the main sync + * * @param props - parameters used for initializing and enabling network syncing */ -export function startNetworkSyncing(props: SetupNetworkSyncingProps) { - const { messenger, getStorageConfig } = props; - +export function startNetworkSyncing(props: StartNetworkSyncingProps) { + const { messenger, getStorageConfig, isMutationSyncBlocked } = props; try { messenger.subscribe( - 'NetworkController:networkAdded', + 'NetworkController:networkRemoved', // eslint-disable-next-line @typescript-eslint/no-misused-promises async (networkConfiguration) => { try { + // If blocked (e.g. we have not yet performed a main-sync), then we should not perform any mutations + if (isMutationSyncBlocked()) { + return; + } + + // As main sync is in progress, it will already local and remote networks + // So no need to re-process again. + if (isMainNetworkSyncInProgress) { + return; + } + const opts = await getStorageConfig(); - await addNetwork(networkConfiguration, opts); + if (!opts) { + return; + } + await deleteNetwork(networkConfiguration, opts); } catch { // Silently fail sync } @@ -32,38 +74,193 @@ export function startNetworkSyncing(props: SetupNetworkSyncingProps) { } catch (e) { log.warn('NetworkSyncing, event subscription failed', e); } +} + +/** + * method that will dispatch the `NetworkController:updateNetwork` action. + * transforms and corrects the network configuration (and RPCs) we pass through. + * @param props - properties + * @param props.messenger - messenger to call the action + * @param props.originalNetworkConfiguration - original network config (from network controller state) + * @param props.newNetworkConfiguration - new network config (from remote) + * @param props.selectedNetworkClientId - currently selected network client id + */ +export const dispatchUpdateNetwork = async (props: { + messenger: UserStorageControllerMessenger; + originalNetworkConfiguration: NetworkConfiguration; + newNetworkConfiguration: NetworkConfiguration; + selectedNetworkClientId: string; +}) => { + const { + messenger, + originalNetworkConfiguration, + newNetworkConfiguration, + selectedNetworkClientId, + } = props; + const { updateNetworkFields, newSelectedRpcEndpointIndex } = + createUpdateNetworkProps({ + originalNetworkConfiguration, + newNetworkConfiguration, + selectedNetworkClientId, + }); + + await messenger.call( + 'NetworkController:updateNetwork', + updateNetworkFields.chainId, + updateNetworkFields, + { replacementSelectedRpcEndpointIndex: newSelectedRpcEndpointIndex }, + ); +}; + +/** + * Action to perform the main network sync. + * It will fetch local networks and remote networks, then determines which networks (local and remote) to add/update + * @param props - parameters used for this main sync + */ +export async function performMainNetworkSync( + props: PerformMainNetworkSyncProps, +) { + const { + messenger, + getStorageConfig, + maxNetworksToAdd, + onNetworkAdded, + onNetworkRemoved, + onNetworkUpdated, + } = props; + + // Edge-Case, we do not want to re-run the main-sync if it already is in progress + /* istanbul ignore if - this is not testable */ + if (isMainNetworkSyncInProgress) { + return; + } + + isMainNetworkSyncInProgress = true; try { - messenger.subscribe( - 'NetworkController:networkDeleted', - // eslint-disable-next-line @typescript-eslint/no-misused-promises - async (networkConfiguration) => { + const opts = await getStorageConfig(); + if (!opts) { + return; + } + + const networkControllerState = messenger.call('NetworkController:getState'); + const localNetworks = Object.values( + networkControllerState.networkConfigurationsByChainId ?? {}, + ); + + const remoteNetworks = await getAllRemoteNetworks(opts); + const networkChanges = findNetworksToUpdate({ + localNetworks, + remoteNetworks, + }); + + log.debug('performMainNetworkSync() - Network Syncing Started', { + localNetworks, + remoteNetworks, + networkChanges, + }); + + // Update Remote + if ( + networkChanges?.remoteNetworksToUpdate && + networkChanges.remoteNetworksToUpdate.length > 0 + ) { + await batchUpdateNetworks(networkChanges?.remoteNetworksToUpdate, opts); + } + + // Add missing local networks + const boundedNetworkedToAdd = + networkChanges?.missingLocalNetworks && + getBoundedNetworksToAdd( + localNetworks, + networkChanges.missingLocalNetworks, + maxNetworksToAdd, + ); + if (boundedNetworkedToAdd && boundedNetworkedToAdd.length > 0) { + const errors: unknown[] = []; + boundedNetworkedToAdd.forEach((n) => { try { - const opts = await getStorageConfig(); - await deleteNetwork(networkConfiguration, opts); - } catch { - // Silently fail sync + messenger.call('NetworkController:addNetwork', n); + onNetworkAdded?.(n.chainId); + } catch (e) { + /* istanbul ignore next - allocates logs, do not need to test */ + errors.push(e); + // Silently fail, we can try this again on next main sync } - }, - ); - } catch (e) { - log.warn('NetworkSyncing, event subscription failed', e); - } + }); - try { - messenger.subscribe( - 'NetworkController:networkChanged', - // eslint-disable-next-line @typescript-eslint/no-misused-promises - async (networkConfiguration) => { + /* istanbul ignore if - only logs errors, not useful to test */ + if (errors.length > 0) { + log.error( + 'performMainNetworkSync() - NetworkController:addNetwork failures', + errors, + ); + } + } + + // Update local networks + if ( + networkChanges?.localNetworksToUpdate && + networkChanges.localNetworksToUpdate.length > 0 + ) { + const errors: unknown[] = []; + for (const n of networkChanges.localNetworksToUpdate) { try { - const opts = await getStorageConfig(); - await updateNetwork(networkConfiguration, opts); - } catch { - // Silently fail sync + await dispatchUpdateNetwork({ + messenger, + originalNetworkConfiguration: + networkControllerState.networkConfigurationsByChainId[n.chainId], + newNetworkConfiguration: n, + selectedNetworkClientId: + networkControllerState.selectedNetworkClientId, + }); + onNetworkUpdated?.(n.chainId); + } catch (e) { + /* istanbul ignore next - allocates logs, do not need to test */ + errors.push(e); + // Silently fail, we can try this again on next main sync } - }, - ); + } + + /* istanbul ignore if - only logs errors, not useful to test */ + if (errors.length > 0) { + log.error( + 'performMainNetworkSync() - NetworkController:updateNetwork failed', + errors, + ); + } + } + + // Remove local networks + if ( + networkChanges?.localNetworksToRemove && + networkChanges.localNetworksToRemove.length > 0 + ) { + const errors: unknown[] = []; + networkChanges.localNetworksToRemove.forEach((n) => { + try { + messenger.call('NetworkController:removeNetwork', n.chainId); + onNetworkRemoved?.(n.chainId); + } catch (e) { + /* istanbul ignore next - allocates logs, do not need to test */ + errors.push(e); + // Silently fail, we can try this again on next main sync + } + }); + + /* istanbul ignore if - only logs errors, not useful to test */ + if (errors.length > 0) { + log.error( + 'performMainNetworkSync() - NetworkController:removeNetwork failed', + errors, + ); + } + } } catch (e) { - log.warn('NetworkSyncing, event subscription failed', e); + /* istanbul ignore next - only logs errors, not useful to test */ + log.error('performMainNetworkSync() failed', e); + // Silently fail sync + } finally { + isMainNetworkSyncInProgress = false; } } diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.update-network.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.update-network.test.ts new file mode 100644 index 00000000000..c545f1affd2 --- /dev/null +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.update-network.test.ts @@ -0,0 +1,265 @@ +import { ControllerMessenger } from '@metamask/base-controller'; +import type { + NetworkState, + NetworkControllerActions, + NetworkConfiguration, +} from '@metamask/network-controller'; +import { + NetworkController, + NetworkStatus, + RpcEndpointType, +} from '@metamask/network-controller'; +import nock from 'nock'; + +import type { UserStorageControllerMessenger } from '..'; +import type { RPCEndpoint } from './__fixtures__/mockNetwork'; +import { + createMockCustomRpcEndpoint, + createMockInfuraRpcEndpoint, + createMockNetworkConfiguration, +} from './__fixtures__/mockNetwork'; +import { dispatchUpdateNetwork } from './controller-integration'; + +const createNetworkControllerState = ( + rpcs: RPCEndpoint[] = [createMockInfuraRpcEndpoint()], +): NetworkState => { + const mockNetworkConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + mockNetworkConfig.rpcEndpoints = rpcs; + + const state: NetworkState = { + selectedNetworkClientId: 'mainnet', + networkConfigurationsByChainId: { + '0x1': mockNetworkConfig, + }, + networksMetadata: {}, + }; + + rpcs.forEach((r) => { + state.networksMetadata[r.networkClientId] = { + EIPS: { + '1559': true, + }, + status: NetworkStatus.Available, + }; + }); + + return state; +}; + +const createNetworkConfigurationWithRpcs = (rpcs: RPCEndpoint[]) => { + const config = createMockNetworkConfiguration({ chainId: '0x1' }); + config.rpcEndpoints = rpcs; + return config; +}; + +describe('network-syncing/controller-integration - dispatchUpdateNetwork()', () => { + beforeEach(() => { + nock('https://mainnet.infura.io').post('/v3/TEST_ID').reply(200, { + jsonrpc: '2.0', + id: 1, + result: {}, + }); + }); + + afterAll(() => { + nock.cleanAll(); + }); + + const setupTest = ({ + initialRpcs, + newRpcs, + selectedNetworkClientId, + }: { + initialRpcs: RPCEndpoint[]; + newRpcs: RPCEndpoint[]; + selectedNetworkClientId?: string; + }) => { + const initialState = createNetworkControllerState(initialRpcs); + if (selectedNetworkClientId) { + initialState.selectedNetworkClientId = selectedNetworkClientId; + } + + const newNetworkConfiguration = createNetworkConfigurationWithRpcs(newRpcs); + + return { initialState, newNetworkConfiguration }; + }; + + const arrangeNetworkController = (networkState: NetworkState) => { + const baseMessenger = new ControllerMessenger< + NetworkControllerActions, + never + >(); + const networkControllerMessenger = baseMessenger.getRestricted({ + name: 'NetworkController', + allowedActions: [], + allowedEvents: [], + }); + + const networkController = new NetworkController({ + messenger: networkControllerMessenger, + state: networkState, + infuraProjectId: 'TEST_ID', + }); + + return { networkController, baseMessenger }; + }; + + const act = async ( + props: Pick< + ReturnType, + 'networkController' | 'baseMessenger' + > & { + newNetworkConfiguration: NetworkConfiguration; + }, + ) => { + const { baseMessenger, networkController, newNetworkConfiguration } = props; + + await dispatchUpdateNetwork({ + messenger: baseMessenger as unknown as UserStorageControllerMessenger, + originalNetworkConfiguration: + networkController.state.networkConfigurationsByChainId['0x1'], + selectedNetworkClientId: networkController.state.selectedNetworkClientId, + newNetworkConfiguration, + }); + + return { + rpcEndpoints: + networkController.state.networkConfigurationsByChainId['0x1'] + .rpcEndpoints, + newSelectedNetworkClientId: + networkController.state.selectedNetworkClientId, + }; + }; + + it('should append missing Infura networks', async () => { + // Arrange + const { initialState, newNetworkConfiguration } = setupTest({ + initialRpcs: [createMockInfuraRpcEndpoint()], + newRpcs: [], + }); + const arrange = arrangeNetworkController(initialState); + + // Act + const result = await act({ ...arrange, newNetworkConfiguration }); + + // Assert - we keep the infura endpoint and it is not overwritten + expect(result.rpcEndpoints).toHaveLength(1); + expect(result.rpcEndpoints[0].type).toBe(RpcEndpointType.Infura); + }); + + it('should add new remote RPCs (from a different device)', async () => { + // Arrange + const { initialState, newNetworkConfiguration } = setupTest({ + initialRpcs: [createMockInfuraRpcEndpoint()], + newRpcs: [ + createMockInfuraRpcEndpoint(), + createMockCustomRpcEndpoint({ + networkClientId: 'EXT_DEVICE_1', + url: 'https://mock.network', + }), + ], + }); + const arrange = arrangeNetworkController(initialState); + + // Act + const result = await act({ + ...arrange, + newNetworkConfiguration, + }); + + // Assert + expect(result.rpcEndpoints).toHaveLength(2); + expect(result.rpcEndpoints[1]).toStrictEqual( + expect.objectContaining({ + networkClientId: expect.any(String), // this was added, so is a new random uuid + url: 'https://mock.network', + }), + ); + expect(result.rpcEndpoints[1].networkClientId).not.toBe('EXT_DEVICE_1'); + }); + + it('should overwrite (remove and add) rpcs from remote (a different device) and update selected network if necessary', async () => { + // Arrange + const { initialState, newNetworkConfiguration } = setupTest({ + initialRpcs: [ + createMockInfuraRpcEndpoint(), + createMockCustomRpcEndpoint({ + networkClientId: 'DEVICE_1', + url: 'https://mock.network', + }), + ], + // Remote does not have https://mock.network, but does have https://mock.network/2 + newRpcs: [ + createMockInfuraRpcEndpoint(), + createMockCustomRpcEndpoint({ + networkClientId: 'EXT_DEVICE_2', + url: 'https://mock.network/2', + }), + ], + // We have selected DEVICE_1 + selectedNetworkClientId: 'DEVICE_1', + }); + const arrange = arrangeNetworkController(initialState); + + // Act + const result = await act({ + ...arrange, + newNetworkConfiguration, + }); + + // Assert + expect(result.rpcEndpoints).toHaveLength(2); + expect(result.rpcEndpoints[0].type).toBe(RpcEndpointType.Infura); // Infura RPC is kept + expect(result.rpcEndpoints[1]).toStrictEqual( + expect.objectContaining({ + // New RPC was added + networkClientId: expect.any(String), + url: 'https://mock.network/2', + }), + ); + expect( + result.rpcEndpoints.some((r) => r.networkClientId === 'DEVICE_1'), + ).toBe(false); // Old RPC was removed + expect(result.newSelectedNetworkClientId).toBe('mainnet'); // We also change to the next available RPC to select + }); + + it('should keep the selected network if it is still present', async () => { + // Arrange + const { initialState, newNetworkConfiguration } = setupTest({ + initialRpcs: [ + createMockInfuraRpcEndpoint(), + createMockCustomRpcEndpoint({ + networkClientId: 'DEVICE_1', + url: 'https://mock.network', + }), + ], + newRpcs: [ + createMockInfuraRpcEndpoint(), + createMockCustomRpcEndpoint({ + networkClientId: 'DEVICE_1', // We keep DEVICE_1 + url: 'https://mock.network', + name: 'Custom Name', + }), + ], + selectedNetworkClientId: 'DEVICE_1', + }); + const arrange = arrangeNetworkController(initialState); + + // Act + const result = await act({ + ...arrange, + newNetworkConfiguration, + }); + + // Assert + expect(result.rpcEndpoints).toHaveLength(2); + expect(result.rpcEndpoints[1]).toStrictEqual( + expect.objectContaining({ + networkClientId: 'DEVICE_1', + url: 'https://mock.network', + name: 'Custom Name', + }), + ); + expect(result.newSelectedNetworkClientId).toBe('DEVICE_1'); // selected rpc has not changed + }); +}); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.test.ts index b9915116caf..a77fa2a1d9f 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.test.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.test.ts @@ -4,6 +4,7 @@ import { createMockAllFeatureEntriesResponse, } from '../__fixtures__'; import { + mockEndpointBatchUpsertUserStorage, mockEndpointGetUserStorageAllFeatureEntries, mockEndpointUpsertUserStorage, } from '../__fixtures__/mockServices'; @@ -115,10 +116,6 @@ describe('network-syncing/services - upsertRemoteNetwork()', () => { }); }); -/** - * TODO - the batch endpoint has not been made in the backend yet. - * Mock endpoints will need to be updated in future - */ describe('network-syncing/services - batchUpsertRemoteNetworks()', () => { const arrangeMocks = () => { const mockNetworks = [ @@ -126,21 +123,16 @@ describe('network-syncing/services - batchUpsertRemoteNetworks()', () => { createMockRemoteNetworkConfiguration({ chainId: '0x1338' }), ]; - const mockAPI = (key: string) => - mockEndpointUpsertUserStorage(`networks.${key}`); - return { storageOps: storageOpts, mockNetworks, - mockUpsertAPI1: mockAPI('0x1337'), - mockUpsertAPI2: mockAPI('0x1338'), + mockBatchUpsertAPI: mockEndpointBatchUpsertUserStorage('networks'), }; }; it('should call upsert storage API with mock network', async () => { - const { mockNetworks, mockUpsertAPI1, mockUpsertAPI2 } = arrangeMocks(); + const { mockNetworks, mockBatchUpsertAPI } = arrangeMocks(); await batchUpsertRemoteNetworks(mockNetworks, storageOpts); - expect(mockUpsertAPI1.isDone()).toBe(true); - expect(mockUpsertAPI2.isDone()).toBe(true); + expect(mockBatchUpsertAPI.isDone()).toBe(true); }); }); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.ts index 2dab5e89af3..5d48ba14127 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/services.ts @@ -1,6 +1,7 @@ import { USER_STORAGE_FEATURE_NAMES } from '../../../shared/storage-schema'; import type { UserStorageBaseOptions } from '../services'; import { + batchUpsertUserStorage, getUserStorageAllFeatureEntries, upsertUserStorage, } from '../services'; @@ -72,8 +73,14 @@ export async function batchUpsertRemoteNetworks( networks: RemoteNetworkConfiguration[], opts: UserStorageBaseOptions, ): Promise { - // TODO - this has not yet been provided by the backend team - // we will replace this with a batch endpoint in near future - const promises = networks.map((n) => upsertRemoteNetwork(n, opts)); - await Promise.allSettled(promises); + const networkPathAndValues = networks.map((n) => { + const path = n.chainId; + const data = JSON.stringify(n); + return [path, data] as [string, string]; + }); + + await batchUpsertUserStorage(networkPathAndValues, { + path: 'networks', + ...opts, + }); } diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-all.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-all.test.ts index 9702f641adb..7dba3649345 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-all.test.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-all.test.ts @@ -1,16 +1,17 @@ +import type { NetworkConfiguration } from '@metamask/network-controller'; + import { createMockNetworkConfiguration, createMockRemoteNetworkConfiguration, } from './__fixtures__/mockNetwork'; import { checkWhichNetworkIsLatest, - findNetworksToUpdate, getDataStructures, getMissingNetworkLists, - getNewLocalNetworks, getUpdatedNetworkLists, + findNetworksToUpdate, } from './sync-all'; -import type { NetworkConfiguration, RemoteNetworkConfiguration } from './types'; +import type { RemoteNetworkConfiguration } from './types'; /** * This is not used externally, but meant to check logic is consistent @@ -205,75 +206,22 @@ describe('getUpdatedNetworkLists()', () => { }); }); -/** - * This is not used externally, but meant to check logic is consistent - */ -describe('getNewLocalNetworks()', () => { - it('should append original list with missing networks', () => { - const originalList = arrangeLocalNetworks(['1', '2', '3']); - const missingNetworks = arrangeLocalNetworks(['4']); - - const result = getNewLocalNetworks({ - originalList, - missingLocalNetworks: missingNetworks, - localNetworksToRemove: [], - localNetworksToUpdate: [], - }); - - expect(result).toHaveLength(4); - expect(result.map((n) => n.chainId)).toStrictEqual([ - '0x1', - '0x2', - '0x3', - '0x4', - ]); - }); - - it('should update original list if there are networks that need updating', () => { - const originalList = arrangeLocalNetworks(['1', '2', '3']); - const updatedNetwork = createMockNetworkConfiguration({ - chainId: '0x1', - name: 'Updated Name', - }); - - const result = getNewLocalNetworks({ - originalList, - missingLocalNetworks: [], - localNetworksToRemove: [], - localNetworksToUpdate: [updatedNetwork], - }); - - expect(result).toHaveLength(3); - expect(result.find((n) => n.chainId === '0x1')?.name).toBe('Updated Name'); - }); - - it('should remote a network from the original list if there are networks that need to be removed', () => { - const originalList = arrangeLocalNetworks(['1', '2', '3']); - const deletedNetwork = createMockNetworkConfiguration({ chainId: '0x1' }); - - const result = getNewLocalNetworks({ - originalList, - missingLocalNetworks: [], - localNetworksToRemove: [deletedNetwork], - localNetworksToUpdate: [], - }); - - expect(result).toHaveLength(2); - expect(result.find((n) => n.chainId === '0x1')).toBeUndefined(); - }); -}); - describe('findNetworksToUpdate()', () => { it('should add missing networks to remote and local', () => { const localNetworks = arrangeLocalNetworks(['1']); const remoteNetworks = arrangeRemoteNetworks(['2']); const result = findNetworksToUpdate({ localNetworks, remoteNetworks }); - expect(result?.newLocalNetworks).toHaveLength(2); - expect(result?.newLocalNetworks.map((n) => n.chainId)).toStrictEqual([ - '0x1', - '0x2', - ]); + + // Only 1 network needs to be added to local + expect(result?.missingLocalNetworks).toHaveLength(1); + expect(result?.missingLocalNetworks?.[0]?.chainId).toBe('0x2'); + + // No networks are to be removed locally + expect(result?.localNetworksToRemove).toStrictEqual([]); + + // No networks are to be updated locally + expect(result?.localNetworksToUpdate).toStrictEqual([]); // Only 1 network needs to be updated expect(result?.remoteNetworksToUpdate).toHaveLength(1); @@ -302,38 +250,43 @@ describe('findNetworksToUpdate()', () => { }); const result = findNetworksToUpdate({ localNetworks, remoteNetworks }); - const newLocalIds = result?.newLocalNetworks?.map((n) => n.chainId) ?? []; + + // Assert - No local networks to add or remove + expect(result?.missingLocalNetworks).toStrictEqual([]); + expect(result?.localNetworksToRemove).toStrictEqual([]); + + // Assert - Local and Remote networks to update + const updateLocalIds = + result?.localNetworksToUpdate?.map((n) => n.chainId) ?? []; const updateRemoteIds = result?.remoteNetworksToUpdate?.map((n) => n.chainId) ?? []; - // Assert - Test Matrix combinations were all tested + + // Check Test Matrix combinations were all tested let testCount = 0; testMatrix.forEach(({ actual }, idx) => { const chainId = `0x${idx}` as const; if (actual === 'Do Nothing') { testCount += 1; - // Combined Local Networks will include this - // Updated Remote Networks will not include this, as it is not a network that needs updating on remote + // No lists are updated if nothing changes // eslint-disable-next-line jest/no-conditional-expect expect([ - newLocalIds.includes(chainId), + updateLocalIds.includes(chainId), updateRemoteIds.includes(chainId), - ]).toStrictEqual([true, false]); + ]).toStrictEqual([false, false]); } else if (actual === 'Local Wins') { testCount += 1; - // Combined Local Networks will include this - // Updated Remote Networks will include this, as we need to update remote + // Only remote is updated if local wins // eslint-disable-next-line jest/no-conditional-expect expect([ - newLocalIds.includes(chainId), + updateLocalIds.includes(chainId), updateRemoteIds.includes(chainId), - ]).toStrictEqual([true, true]); + ]).toStrictEqual([false, true]); } else if (actual === 'Remote Wins') { testCount += 1; - // Combined Local Networks will include this - // Updated Remote Networks will not include this, as it is not a network that needs updating on remote + // Only local is updated if remote wins // eslint-disable-next-line jest/no-conditional-expect expect([ - newLocalIds.includes(chainId), + updateLocalIds.includes(chainId), updateRemoteIds.includes(chainId), ]).toStrictEqual([true, false]); } @@ -349,11 +302,17 @@ describe('findNetworksToUpdate()', () => { remoteNetworks[1].d = true; const result = findNetworksToUpdate({ localNetworks, remoteNetworks }); - // Combined Local List is updated - expect(result?.newLocalNetworks).toHaveLength(1); - expect( - result?.newLocalNetworks.find((n) => n.chainId === '0x2'), - ).toBeUndefined(); + + // Assert no remote networks need updating + expect(result?.remoteNetworksToUpdate).toStrictEqual([]); + + // Assert no local networks need to be updated or added + expect(result?.localNetworksToUpdate).toStrictEqual([]); + expect(result?.missingLocalNetworks).toStrictEqual([]); + + // Assert that a network needs to be removed locally (network 0x2) + expect(result?.localNetworksToRemove).toHaveLength(1); + expect(result?.localNetworksToRemove?.[0]?.chainId).toBe('0x2'); // Remote List does not have any networks that need updating expect(result?.remoteNetworksToUpdate).toHaveLength(0); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-all.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-all.ts index f3cd7da156f..40042e21df3 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-all.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-all.ts @@ -1,7 +1,8 @@ +import type { NetworkConfiguration } from '@metamask/network-controller'; + import { setDifference, setIntersection } from '../utils'; import { toRemoteNetworkConfiguration, - type NetworkConfiguration, type RemoteNetworkConfiguration, toNetworkConfiguration, } from './types'; @@ -81,6 +82,8 @@ export const checkWhichNetworkIsLatest = ( : 'Remote Wins'; } + // Unreachable statement + /* istanbul ignore next */ return 'Do Nothing'; }; @@ -141,6 +144,9 @@ export const getUpdatedNetworkLists = ( const localNetwork = localMap.get(chain); const remoteNetwork = remoteMap.get(chain); if (!localNetwork || !remoteNetwork) { + // This should be unreachable as we know the Maps created will have the values + // This is to satisfy types + /* istanbul ignore next */ return; } @@ -173,35 +179,6 @@ export const getUpdatedNetworkLists = ( }; }; -export const getNewLocalNetworks = (props: { - originalList: NetworkConfiguration[]; - missingLocalNetworks: NetworkConfiguration[]; - localNetworksToUpdate: NetworkConfiguration[]; - localNetworksToRemove: NetworkConfiguration[]; -}) => { - let newList = [...props.originalList]; - newList.push(...props.missingLocalNetworks); - const updateMap = createMap(props.localNetworksToUpdate); - const remoteMap = createMap(props.localNetworksToRemove); - - newList = newList - .map((n) => { - if (remoteMap.has(n.chainId)) { - return undefined; - } - - const updatedNetwork = updateMap.get(n.chainId); - if (updatedNetwork) { - return updatedNetwork; - } - - return n; - }) - .filter((n): n is NetworkConfiguration => n !== undefined); - - return newList; -}; - export const findNetworksToUpdate = (props: FindNetworksToUpdateProps) => { try { const { localNetworks, remoteNetworks } = props; @@ -221,21 +198,17 @@ export const findNetworksToUpdate = (props: FindNetworksToUpdateProps) => { ...updatedNetworks.remoteNetworksToUpdate, ]; - // List of new local networks - const newLocalNetworks = getNewLocalNetworks({ - originalList: localNetworks, + return { + remoteNetworksToUpdate, missingLocalNetworks: missingNetworks.missingLocalNetworks, localNetworksToRemove: updatedNetworks.localNetworksToRemove, localNetworksToUpdate: updatedNetworks.localNetworksToUpdate, - }); - - return { - remoteNetworksToUpdate, - newLocalNetworks, }; } catch { // Unable to perform sync, silently fail } + // Unreachable statement + /* istanbul ignore next */ return undefined; }; diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.test.ts index c40101fec2f..76e1b780c42 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.test.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.test.ts @@ -2,7 +2,10 @@ import type { NetworkConfiguration } from '@metamask/network-controller'; import { USER_STORAGE_FEATURE_NAMES } from '../../../shared/storage-schema'; import { MOCK_STORAGE_KEY } from '../__fixtures__'; -import { mockEndpointUpsertUserStorage } from '../__fixtures__/mockServices'; +import { + mockEndpointBatchUpsertUserStorage, + mockEndpointUpsertUserStorage, +} from '../__fixtures__/mockServices'; import type { UserStorageBaseOptions } from '../services'; import { createMockNetworkConfiguration } from './__fixtures__/mockNetwork'; import { @@ -63,10 +66,6 @@ describe('network-syncing/sync - updateNetwork() / addNetwork() / deleteNetwork( ); }); -/** - * TODO - the batch endpoint has not been made in the backend yet. - * Mock endpoints will need to be updated in future - */ describe('network-syncing/sync - batchUpdateNetworks()', () => { const arrangeMocks = () => { const mockNetworks = [ @@ -74,25 +73,20 @@ describe('network-syncing/sync - batchUpdateNetworks()', () => { createMockNetworkConfiguration({ chainId: '0x1338' }), ]; - const mockAPI = (key: string) => - mockEndpointUpsertUserStorage(`networks.${key}`); - return { storageOps: storageOpts, mockNetworks, - mockUpsertAPI1: mockAPI('0x1337'), - mockUpsertAPI2: mockAPI('0x1338'), + mockBatchUpsertAPI: mockEndpointBatchUpsertUserStorage('networks'), }; }; it('should call upsert storage API with mock network', async () => { - const { mockNetworks, mockUpsertAPI1, mockUpsertAPI2 } = arrangeMocks(); + const { mockNetworks, mockBatchUpsertAPI } = arrangeMocks(); // Example where we can batch normal adds/updates with deletes await batchUpdateNetworks( [mockNetworks[0], { ...mockNetworks[1], deleted: true }], storageOpts, ); - expect(mockUpsertAPI1.isDone()).toBe(true); - expect(mockUpsertAPI2.isDone()).toBe(true); + expect(mockBatchUpsertAPI.isDone()).toBe(true); }); }); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.ts index 841a459dcd5..f11f4e19e31 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/sync-mutations.ts @@ -1,6 +1,8 @@ +import type { NetworkConfiguration } from '@metamask/network-controller'; + import type { UserStorageBaseOptions } from '../services'; import { batchUpsertRemoteNetworks, upsertRemoteNetwork } from './services'; -import type { NetworkConfiguration, RemoteNetworkConfiguration } from './types'; +import type { RemoteNetworkConfiguration } from './types'; export const updateNetwork = async ( network: NetworkConfiguration, @@ -21,7 +23,7 @@ export const deleteNetwork = async ( v: '1', ...network, d: true, - lastUpdatedAt: network.lastUpdatedAt ?? Date.now(), // Ensures that a deleted entry has a date field + lastUpdatedAt: Date.now(), // Ensures that a deleted entry has a date field }, opts, ); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/types.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/types.ts index 5eb0f9ce55b..321a8a6046a 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/types.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/types.ts @@ -1,9 +1,4 @@ -import type { NetworkConfiguration as _NetworkConfiguration } from '@metamask/network-controller'; - -// TODO - replace shim once we update NetworkController type -export type NetworkConfiguration = _NetworkConfiguration & { - lastUpdatedAt?: number; -}; +import type { NetworkConfiguration } from '@metamask/network-controller'; export type RemoteNetworkConfiguration = NetworkConfiguration & { /** diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/update-network-utils.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/update-network-utils.test.ts new file mode 100644 index 00000000000..1e3f68197a0 --- /dev/null +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/update-network-utils.test.ts @@ -0,0 +1,287 @@ +import type { RPCEndpoint } from './__fixtures__/mockNetwork'; +import { + createMockCustomRpcEndpoint, + createMockInfuraRpcEndpoint, + createMockNetworkConfiguration, +} from './__fixtures__/mockNetwork'; +import { + appendMissingInfuraNetworks, + createUpdateNetworkProps, + getMappedNetworkConfiguration, + getNewRPCIndex, +} from './update-network-utils'; + +describe('getMappedNetworkConfiguration() tests', () => { + const arrangeRPCs = (clientIds: string[]) => + clientIds.map((id, idx) => + createMockCustomRpcEndpoint({ + networkClientId: id, + url: `https://mock.rpc/${idx}`, + }), + ); + + const createConfigs = ( + originalClientIds: string[], + newClientIds: string[], + ) => { + const originalConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + originalConfig.rpcEndpoints = arrangeRPCs(originalClientIds); + + const newConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + newConfig.rpcEndpoints = arrangeRPCs(newClientIds); + + return { originalConfig, newConfig }; + }; + + it('should map existing RPCs to the clients networkClientId', () => { + const { originalConfig, newConfig } = createConfigs( + ['DEVICE_1', 'DEVICE_2'], + ['EXT_DEVICE_1', 'EXT_DEVICE_2'], + ); + + const result = getMappedNetworkConfiguration({ + originalNetworkConfiguration: originalConfig, + newNetworkConfiguration: newConfig, + }); + + // We have mapped both existing networks to use the original clientIds + expect(result.rpcEndpoints.map((r) => r.networkClientId)).toStrictEqual([ + 'DEVICE_1', + 'DEVICE_2', + ]); + }); + + it('should map new RPCs to no networkClientId (so the NetworkController can append them correctly)', () => { + const { originalConfig, newConfig } = createConfigs( + ['DEVICE_1', 'DEVICE_2'], + ['EXT_DEVICE_1', 'EXT_DEVICE_2', 'EXT_DEVICE_3'], + ); + + const result = getMappedNetworkConfiguration({ + originalNetworkConfiguration: originalConfig, + newNetworkConfiguration: newConfig, + }); + + // We have mapped both existing networks to use the original clientIds + // We have also mapped the new RPC to 'undefined'/no networkClientId + expect(result.rpcEndpoints.map((r) => r.networkClientId)).toStrictEqual([ + 'DEVICE_1', + 'DEVICE_2', + undefined, + ]); + }); +}); + +describe('appendMissingInfuraNetworks() tests', () => { + const createConfigs = ( + originalRpcEndpoints: RPCEndpoint[], + newRpcEndpoints: RPCEndpoint[], + ) => { + const originalConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + originalConfig.rpcEndpoints = originalRpcEndpoints; + + const newConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + newConfig.rpcEndpoints = newRpcEndpoints; + + return { originalConfig, newConfig }; + }; + + it('should append missing Infura networks (as we do not want to remove Infura RPCs)', () => { + const infuraRpc = createMockInfuraRpcEndpoint(); + const { originalConfig, newConfig } = createConfigs([infuraRpc], []); + + const result = appendMissingInfuraNetworks({ + originalNetworkConfiguration: originalConfig, + updateNetworkConfiguration: newConfig, + }); + + expect(result.rpcEndpoints).toHaveLength(1); + expect(result.rpcEndpoints).toStrictEqual([infuraRpc]); + }); + + it('should not append if there are no Infura RPCs to add', () => { + const infuraRpc = createMockInfuraRpcEndpoint(); + const { originalConfig, newConfig } = createConfigs([], [infuraRpc]); + + const result = appendMissingInfuraNetworks({ + originalNetworkConfiguration: originalConfig, + updateNetworkConfiguration: newConfig, + }); + + expect(result.rpcEndpoints).toHaveLength(1); // no additional RPCs were added + }); + + it('should not append if the new config already has all the Infura RPCs', () => { + const infuraRpc = createMockInfuraRpcEndpoint(); + const { originalConfig, newConfig } = createConfigs( + [infuraRpc], + [infuraRpc], + ); + + const result = appendMissingInfuraNetworks({ + originalNetworkConfiguration: originalConfig, + updateNetworkConfiguration: newConfig, + }); + + expect(result.rpcEndpoints).toHaveLength(1); // no additional RPCs were added + }); +}); + +describe('getNewRPCIndex() tests', () => { + const arrangeRPCs = (clientIds: string[]) => + clientIds.map((id) => + createMockCustomRpcEndpoint({ + networkClientId: id, + url: `https://mock.rpc/${id}`, + }), + ); + + const createConfigs = ( + originalClientIds: string[], + newClientIds: string[], + ) => { + const originalConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + originalConfig.rpcEndpoints = arrangeRPCs(originalClientIds); + + const newConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + newConfig.rpcEndpoints = arrangeRPCs(newClientIds); + + return { originalConfig, newConfig }; + }; + + it('should return the index of a new RPC if the selected RPC is removed', () => { + const { originalConfig, newConfig } = createConfigs( + ['DEVICE_1', 'DEVICE_2'], + ['DEVICE_2'], + ); + + const selectedNetworkClientId = 'DEVICE_1'; + + const result = getNewRPCIndex({ + originalNetworkConfiguration: originalConfig, + updateNetworkConfiguration: newConfig, + selectedNetworkClientId, + }); + + expect(result).toBe(0); // The new index should be the first available RPC + }); + + it('should return the same index if RPC ordering is unchanged', () => { + const { originalConfig, newConfig } = createConfigs( + ['DEVICE_1', 'DEVICE_2'], + ['DEVICE_1', 'DEVICE_2'], + ); + + const selectedNetworkClientId = 'DEVICE_2'; + + const result = getNewRPCIndex({ + originalNetworkConfiguration: originalConfig, + updateNetworkConfiguration: newConfig, + selectedNetworkClientId, + }); + + expect(result).toBe(1); // The index should remain the same + }); + + it('should return new index if the RPC ordering changed', () => { + const { originalConfig, newConfig } = createConfigs( + ['DEVICE_1', 'DEVICE_2'], + ['DEVICE_0', 'DEVICE_1', 'DEVICE_2'], + ); + + const selectedNetworkClientId = 'DEVICE_2'; + + const result = getNewRPCIndex({ + originalNetworkConfiguration: originalConfig, + updateNetworkConfiguration: newConfig, + selectedNetworkClientId, + }); + + expect(result).toBe(2); // The index has changed + }); + + it('should return undefined if the selected RPC is not in the original or new list', () => { + const { originalConfig, newConfig } = createConfigs( + ['DEVICE_1', 'DEVICE_2'], + ['DEVICE_1', 'DEVICE_2'], + ); + + const selectedNetworkClientId = 'DEVICE_5'; // this is a networkClientId from a different configuration + + const result = getNewRPCIndex({ + originalNetworkConfiguration: originalConfig, + updateNetworkConfiguration: newConfig, + selectedNetworkClientId, + }); + + expect(result).toBeUndefined(); // No matching RPC found + }); +}); + +describe('createUpdateNetworkProps() tests', () => { + const arrangeRPCs = (clientIds: string[]) => + clientIds.map((id) => + createMockCustomRpcEndpoint({ + networkClientId: id, + url: `https://mock.rpc/${id}`, + }), + ); + + const createConfigs = ( + originalClientIds: string[], + newClientIds: string[], + ) => { + const originalConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + originalConfig.rpcEndpoints = arrangeRPCs(originalClientIds); + + const newConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + newConfig.rpcEndpoints = arrangeRPCs(newClientIds); + + return { originalConfig, newConfig }; + }; + + it('should map new RPCs without networkClientId and keep existing ones', () => { + const { originalConfig, newConfig } = createConfigs( + ['DEVICE_1', 'DEVICE_2'], + ['DEVICE_1', 'DEVICE_2', 'DEVICE_3'], + ); + + const selectedNetworkClientId = 'DEVICE_1'; + + const result = createUpdateNetworkProps({ + originalNetworkConfiguration: originalConfig, + newNetworkConfiguration: newConfig, + selectedNetworkClientId, + }); + + expect( + result.updateNetworkFields.rpcEndpoints.map((r) => r.networkClientId), + ).toStrictEqual(['DEVICE_1', 'DEVICE_2', undefined]); + expect(result.newSelectedRpcEndpointIndex).toBe(0); // the index for `DEVICE_1` + }); + + it('should append missing Infura networks', () => { + const originalConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + const infuraRpc = createMockInfuraRpcEndpoint(); + const customRpcs = arrangeRPCs(['DEVICE_1']); + originalConfig.rpcEndpoints.push(infuraRpc); + originalConfig.rpcEndpoints.push(...customRpcs); + + const newConfig = createMockNetworkConfiguration({ chainId: '0x1' }); + newConfig.rpcEndpoints = customRpcs; + + const selectedNetworkClientId = 'DEVICE_1'; + + const result = createUpdateNetworkProps({ + originalNetworkConfiguration: originalConfig, + newNetworkConfiguration: newConfig, + selectedNetworkClientId, + }); + + expect(result.updateNetworkFields.rpcEndpoints).toHaveLength(2); + expect( + result.updateNetworkFields.rpcEndpoints.map((r) => r.networkClientId), + ).toStrictEqual([infuraRpc.networkClientId, 'DEVICE_1']); + expect(result.newSelectedRpcEndpointIndex).toBe(1); // DEVICE_1 has a new index + }); +}); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/update-network-utils.ts b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/update-network-utils.ts new file mode 100644 index 00000000000..925974872eb --- /dev/null +++ b/packages/profile-sync-controller/src/controllers/user-storage/network-syncing/update-network-utils.ts @@ -0,0 +1,198 @@ +import { + RpcEndpointType, + type UpdateNetworkFields, + type NetworkConfiguration, +} from '@metamask/network-controller'; + +import { setDifference } from '../utils'; + +/** + * Will map the network configuration we want to update into something valid that `updateNetwork` accepts + * Exported for testability + * + * @param props - properties + * @param props.originalNetworkConfiguration - original network configuration we will override + * @param props.newNetworkConfiguration - new network configuration + * @returns NetworkConfiguration to dispatch to `NetworkController:updateNetwork` + */ +export const getMappedNetworkConfiguration = (props: { + originalNetworkConfiguration: NetworkConfiguration; + newNetworkConfiguration: NetworkConfiguration; +}): UpdateNetworkFields => { + const { originalNetworkConfiguration, newNetworkConfiguration } = props; + + // Map of URL <> clientId (url is unique) + const originalRPCUrlMap = new Map( + originalNetworkConfiguration.rpcEndpoints.map( + (r) => [r.url, r.networkClientId] as const, + ), + ); + + const updateNetworkConfig = newNetworkConfiguration as UpdateNetworkFields; + + updateNetworkConfig.rpcEndpoints = updateNetworkConfig.rpcEndpoints.map( + (r) => { + const originalRPCClientId = originalRPCUrlMap.get(r.url); + + // This is an existing RPC, so use the clients networkClientId + if (originalRPCClientId) { + r.networkClientId = originalRPCClientId; + return r; + } + + // This is a new RPC, so remove the remote networkClientId + r.networkClientId = undefined; + return r; + }, + ); + + return updateNetworkConfig; +}; + +/** + * Will insert any missing infura RPCs, as we cannot remove infura RPC + * Exported for testability + * @param props - properties + * @param props.originalNetworkConfiguration - original network configuration + * @param props.updateNetworkConfiguration - the updated network configuration to use when dispatching `NetworkController:updateNetwork` + * @returns mutates and returns the updateNetworkConfiguration + */ +export const appendMissingInfuraNetworks = (props: { + originalNetworkConfiguration: NetworkConfiguration; + updateNetworkConfiguration: UpdateNetworkFields; +}) => { + const { originalNetworkConfiguration, updateNetworkConfiguration } = props; + + // Ensure we have not removed any infura networks (and add them back if they were removed) + const origInfuraRPCMap = new Map( + originalNetworkConfiguration.rpcEndpoints + .filter((r) => r.type === RpcEndpointType.Infura) + .map((r) => [r.networkClientId, r] as const), + ); + const newInfuraRPCMap = new Map( + updateNetworkConfiguration.rpcEndpoints + .filter((r) => r.type === RpcEndpointType.Infura && r.networkClientId) + .map((r) => [r.networkClientId as string, r]), + ); + const missingOrigInfuraRPCs = setDifference( + new Set(origInfuraRPCMap.keys()), + new Set(newInfuraRPCMap.keys()), + ); + + if (missingOrigInfuraRPCs.size > 0) { + const missingRPCs: UpdateNetworkFields['rpcEndpoints'] = []; + missingOrigInfuraRPCs.forEach((clientId) => { + missingRPCs.push( + origInfuraRPCMap.get( + clientId, + ) as UpdateNetworkFields['rpcEndpoints'][number], + ); + }); + + updateNetworkConfiguration.rpcEndpoints.unshift(...missingRPCs); + } + + return updateNetworkConfiguration; +}; + +/** + * The `NetworkController:updateNetwork` method will require us to pass in a `replacementSelectedRpcEndpointIndex` if the selected RPC is removed or modified + * @param props - properties + * @param props.originalNetworkConfiguration - the original network configuration + * @param props.updateNetworkConfiguration - the new network configuration we will use to update + * @param props.selectedNetworkClientId - the NetworkController's selected network id. + * @returns the new RPC index if it needs modification + */ +export const getNewRPCIndex = (props: { + originalNetworkConfiguration: NetworkConfiguration; + updateNetworkConfiguration: UpdateNetworkFields; + selectedNetworkClientId: string; +}) => { + const { + originalNetworkConfiguration, + updateNetworkConfiguration, + selectedNetworkClientId, + } = props; + + const isRPCInNewList = updateNetworkConfiguration.rpcEndpoints.some( + (r) => r.networkClientId === selectedNetworkClientId, + ); + const isRPCInOldList = originalNetworkConfiguration.rpcEndpoints.some( + (r) => r.networkClientId === selectedNetworkClientId, + ); + + const getAnyRPCIndex = () => + Math.max( + updateNetworkConfiguration.rpcEndpoints.findIndex((r) => + Boolean(r.networkClientId), + ), + 0, + ); + + // We have removed the selected RPC, so we must point to a new RPC index + if (isRPCInOldList && !isRPCInNewList) { + // Try finding an existing index, or default to first RPC. + const newIndex = getAnyRPCIndex(); + return newIndex; + } + + // We have updated the selected RPC, so we must point to the same RPC index (or a new one) + if (isRPCInOldList && isRPCInNewList) { + const existingIndex = updateNetworkConfiguration.rpcEndpoints.findIndex( + (r) => r.networkClientId === selectedNetworkClientId, + ); + /* istanbul ignore next - the `getAnyRPCIndex` should not be reachable since this is an existing network */ + return existingIndex !== -1 ? existingIndex : getAnyRPCIndex(); + } + + return undefined; +}; + +/** + * create the correct `NetworkController:updateNetwork` parameters + * @param props - properties + * @param props.originalNetworkConfiguration - original config + * @param props.newNetworkConfiguration - new config (from remote) + * @param props.selectedNetworkClientId - the current selected network client id + * @returns parameters to be used for `NetworkController:updateNetwork` call + */ +export const createUpdateNetworkProps = (props: { + originalNetworkConfiguration: NetworkConfiguration; + newNetworkConfiguration: NetworkConfiguration; + selectedNetworkClientId: string; +}) => { + const { + originalNetworkConfiguration, + newNetworkConfiguration, + selectedNetworkClientId, + } = props; + + // The `NetworkController:updateNetwork` has a strict set of rules to follow + // New RPCs that we are adding must not have a networkClientId + // Existing RPCs must point to the correct networkClientId (so we must convert and use this client clientIds set) + // Removing RPCs are omitted from the list + // We cannot remove infura RPCs - so ensure that they stay populated + // If we are removing a selected RPC - then we need to provide `replacementSelectedRpcEndpointIndex` to an index in the new list + // If we are updating a selected RPC - then we need to provide `replacementSelectedRpcEndpointIndex` to the index in the new list + + const mappedNetworkConfiguration = getMappedNetworkConfiguration({ + originalNetworkConfiguration, + newNetworkConfiguration, + }); + + appendMissingInfuraNetworks({ + originalNetworkConfiguration, + updateNetworkConfiguration: mappedNetworkConfiguration, + }); + + const updatedRPCIndex = getNewRPCIndex({ + originalNetworkConfiguration, + updateNetworkConfiguration: mappedNetworkConfiguration, + selectedNetworkClientId, + }); + + return { + updateNetworkFields: mappedNetworkConfiguration, + newSelectedRpcEndpointIndex: updatedRPCIndex, + }; +}; diff --git a/packages/profile-sync-controller/src/controllers/user-storage/services.test.ts b/packages/profile-sync-controller/src/controllers/user-storage/services.test.ts index 7fda37ed15c..2c839b143a8 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/services.test.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/services.test.ts @@ -260,6 +260,19 @@ describe('user-storage/services.ts - batchUpsertUserStorage() tests', () => { ); mockUpsertUserStorage.done(); }); + + it('does nothing if empty data is provided', async () => { + const mockUpsertUserStorage = + mockEndpointBatchUpsertUserStorage('accounts_v2'); + + await batchUpsertUserStorage([], { + bearerToken: 'MOCK_BEARER_TOKEN', + path: 'accounts_v2', + storageKey: MOCK_STORAGE_KEY, + }); + + expect(mockUpsertUserStorage.isDone()).toBe(false); + }); }); describe('user-storage/services.ts - deleteUserStorage() tests', () => { @@ -431,4 +444,17 @@ describe('user-storage/services.ts - batchDeleteUserStorage() tests', () => { ); mockDeleteUserStorage.done(); }); + + it('does nothing if empty data is provided', async () => { + const mockDeleteUserStorage = + mockEndpointBatchDeleteUserStorage('accounts_v2'); + + await batchDeleteUserStorage([], { + bearerToken: 'MOCK_BEARER_TOKEN', + path: 'accounts_v2', + storageKey: MOCK_STORAGE_KEY, + }); + + expect(mockDeleteUserStorage.isDone()).toBe(false); + }); }); diff --git a/packages/profile-sync-controller/src/controllers/user-storage/services.ts b/packages/profile-sync-controller/src/controllers/user-storage/services.ts index 6680d8ca730..5dcb48b51a4 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/services.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/services.ts @@ -82,6 +82,7 @@ export async function getUserStorage( await userStorageResponse.json(); const encryptedData = userStorage?.Data ?? null; + /* istanbul ignore if - this is an edge case where our endpoint returns invalid JSON payload */ if (!encryptedData) { return null; } @@ -138,6 +139,7 @@ export async function getUserStorageAllFeatureEntries( const decryptedData: string[] = []; for (const entry of userStorage) { + /* istanbul ignore if - unreachable if statement, but kept as edge case */ if (!entry.Data) { continue; }