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 5af28ab924..0039566652 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts @@ -18,7 +18,11 @@ import type { KeyringControllerUnlockEvent, KeyringControllerAddNewAccountAction, } from '@metamask/keyring-controller'; -import type { NetworkConfiguration } from '@metamask/network-controller'; +import type { + NetworkConfiguration, + NetworkController, + NetworkControllerGetStateAction, +} from '@metamask/network-controller'; import type { HandleSnapRequest } from '@metamask/snaps-controllers'; import { createSnapSignMessageRequest } from '../authentication/auth-snap-requests'; @@ -35,7 +39,10 @@ import { mapInternalAccountToUserStorageAccount, } from './accounts/user-storage'; import { createSHA256Hash } from './encryption'; -import { startNetworkSyncing } from './network-syncing/controller-integration'; +import { + performMainNetworkSync, + startNetworkSyncing, +} from './network-syncing/controller-integration'; import type { UserStoragePathWithFeatureAndKey, UserStoragePathWithFeatureOnly, @@ -46,26 +53,32 @@ import { upsertUserStorage, } from './services'; -// TODO: add external NetworkController event -// Need to listen for when a network gets added +// TODO - replace shimmed interface with actual interfaces once merged +// Waiting on #4698 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'; +type NetworkControllerNetworkUpdatedEvent = { + type: 'NetworkController:networkUpdated'; payload: [networkConfiguration: NetworkConfiguration]; }; - -// TODO: add external NetworkController event -// Need to listen for when a network gets deleted -type NetworkControllerNetworkDeletedEvent = { - type: 'NetworkController:networkDeleted'; +type NetworkControllerNetworkRemovedEvent = { + type: 'NetworkController:networkRemoved'; payload: [networkConfiguration: NetworkConfiguration]; }; +type NetworkControllerAddNetworkAction = { + type: 'NetworkController:addNetwork'; + handler: NetworkController['addNetwork']; +}; +type NetworkControllerUpdateNetworkAction = { + type: 'NetworkController:updateNetwork'; + handler: NetworkController['updateNetwork']; +}; +type NetworkControllerRemoveNetworkAction = { + type: 'NetworkController:removeNetwork'; + handler: NetworkController['removeNetwork']; +}; // TODO: fix external dependencies export declare type NotificationServicesControllerDisableNotificationServices = @@ -173,10 +186,15 @@ export type AllowedActions = // Metamask Notifications | NotificationServicesControllerDisableNotificationServices | NotificationServicesControllerSelectIsNotificationServicesEnabled - // Account syncing + // Account Syncing | AccountsControllerListAccountsAction | AccountsControllerUpdateAccountMetadataAction - | KeyringControllerAddNewAccountAction; + | KeyringControllerAddNewAccountAction + // Network Syncing + | NetworkControllerGetStateAction + | NetworkControllerAddNetworkAction + | NetworkControllerUpdateNetworkAction + | NetworkControllerRemoveNetworkAction; // Messenger events export type UserStorageControllerStateChangeEvent = ControllerStateChangeEvent< @@ -207,8 +225,8 @@ export type AllowedEvents = | AccountsControllerAccountRenamedEvent // Network Syncing Events | NetworkControllerNetworkAddedEvent - | NetworkControllerNetworkChangedEvent - | NetworkControllerNetworkDeletedEvent; + | NetworkControllerNetworkUpdatedEvent + | NetworkControllerNetworkRemovedEvent; // Messenger export type UserStorageControllerMessenger = RestrictedControllerMessenger< @@ -232,6 +250,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( @@ -260,17 +285,12 @@ export default class UserStorageController extends BaseController< }; #accounts = { - // This is replaced with the actual value in the constructor - // We will remove this once the feature will be released - isAccountSyncingEnabled: false, isAccountSyncingInProgress: false, canSync: () => { try { this.#assertProfileSyncingEnabled(); - return ( - this.#accounts.isAccountSyncingEnabled && this.#auth.isAuthEnabled() - ); + return this.#env.isAccountSyncingEnabled && this.#auth.isAuthEnabled(); } catch { return false; } @@ -406,9 +426,8 @@ export default class UserStorageController extends BaseController< state: { ...defaultState, ...state }, }); - this.#accounts.isAccountSyncingEnabled = Boolean( - env?.isAccountSyncingEnabled, - ); + this.#env.isAccountSyncingEnabled = Boolean(env?.isAccountSyncingEnabled); + this.#env.isNetworkSyncingEnabled = Boolean(env?.isNetworkSyncingEnabled); this.getMetaMetricsState = getMetaMetricsState; this.#keyringController.setupLockedStateSubscriptions(); @@ -417,18 +436,10 @@ 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, }); } } @@ -479,6 +490,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); @@ -868,4 +893,15 @@ export default class UserStorageController extends BaseController< ); } } + + async syncNetworks() { + if (!this.#env.isNetworkSyncingEnabled) { + return; + } + + await performMainNetworkSync({ + messenger: this.messagingSystem, + getStorageConfig: this.#getStorageOptions, + }); + } } 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 901046a295..1e8fde240d 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 @@ -2,28 +2,50 @@ import log from 'loglevel'; import type { UserStorageBaseOptions } from '../services'; import type { UserStorageControllerMessenger } from '../UserStorageController'; -import { addNetwork, deleteNetwork, updateNetwork } from './sync-mutations'; +import { getAllRemoteNetworks } from './services'; +import { findNetworksToUpdate } from './sync-all'; +import { batchUpdateNetworks, deleteNetwork } from './sync-mutations'; -type SetupNetworkSyncingProps = { +type NetworkSyncingProps = { messenger: UserStorageControllerMessenger; - getStorageConfig: () => Promise; + getStorageConfig: () => Promise; }; +/** + * 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) + */ +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) { +export function startNetworkSyncing(props: NetworkSyncingProps) { const { messenger, getStorageConfig } = props; - try { messenger.subscribe( - 'NetworkController:networkAdded', + 'NetworkController:networkRemoved', // eslint-disable-next-line @typescript-eslint/no-misused-promises async (networkConfiguration) => { try { + // 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 +54,75 @@ export function startNetworkSyncing(props: SetupNetworkSyncingProps) { } catch (e) { log.warn('NetworkSyncing, event subscription failed', e); } +} +/** + * 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: NetworkSyncingProps) { + const { messenger, getStorageConfig } = props; + 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 localNetworks = Object.values( + messenger.call('NetworkController:getState') + .networkConfigurationsByChainId ?? {}, + ); + + const remoteNetworks = await getAllRemoteNetworks(opts); + + const networksToUpdate = findNetworksToUpdate({ + localNetworks, + remoteNetworks, + }); + + // Update Remote + if (networksToUpdate?.remoteNetworksToUpdate) { + await batchUpdateNetworks(networksToUpdate?.remoteNetworksToUpdate, opts); + } + + // Add missing local networks + if (networksToUpdate?.missingLocalNetworks) { + networksToUpdate.missingLocalNetworks.forEach((n) => { try { - const opts = await getStorageConfig(); - await deleteNetwork(networkConfiguration, opts); + messenger.call('NetworkController:addNetwork', n); } catch { - // Silently fail sync + // 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) => { + // Update local networks + if (networksToUpdate?.localNetworksToUpdate) { + const promises = networksToUpdate.localNetworksToUpdate.map(async (n) => { try { - const opts = await getStorageConfig(); - await updateNetwork(networkConfiguration, opts); + await messenger.call('NetworkController:updateNetwork', n.chainId, n); } catch { - // Silently fail sync + // Silently fail, we can try this again on next main sync } - }, - ); - } catch (e) { - log.warn('NetworkSyncing, event subscription failed', e); + }); + await Promise.all(promises); + } + + // Remove local networks + if (networksToUpdate?.localNetworksToRemove) { + networksToUpdate.localNetworksToRemove.forEach((n) => { + try { + messenger.call('NetworkController:removeNetwork', n.chainId); + } catch { + // Silently fail, we can try this again on next main sync + } + }); + } + } catch { + // Silently fail sync + } finally { + isMainNetworkSyncInProgress = false; } } 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 9702f641ad..efd314fee7 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 @@ -4,11 +4,10 @@ import { } from './__fixtures__/mockNetwork'; import { checkWhichNetworkIsLatest, - findNetworksToUpdate, getDataStructures, getMissingNetworkLists, - getNewLocalNetworks, getUpdatedNetworkLists, + findNetworksToUpdate, } from './sync-all'; import type { NetworkConfiguration, RemoteNetworkConfiguration } from './types'; @@ -205,75 +204,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 +248,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 +300,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 f3cd7da156..4f07912d03 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 @@ -173,35 +173,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,17 +192,11 @@ 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