feat: add max size bounds for networks to add

this prevents users from adding thousands of networks, which could cause issues for the application

Author: Prithpal-Sooriya

packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts

@@ -175,6 +175,7 @@ type ControllerConfig = {
   };
 
   networkSyncing?: {
+    maxNumberOfNetworksToAdd?: number;
     /**
      * Callback that fires when network sync adds a network
      * This is used for analytics.
@@ -1176,6 +1177,7 @@ export default class UserStorageController extends BaseController<
     await performMainNetworkSync({
       messenger: this.messagingSystem,
       getStorageConfig: () => this.#getStorageOptions(),
+      maxNetworksToAdd: this.#config?.networkSyncing?.maxNumberOfNetworksToAdd,
       onNetworkAdded: (cId) =>
         this.#config?.networkSyncing?.onNetworkAdded?.(profileId, cId),
       onNetworkUpdated: (cId) =>

packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.test.ts

@@ -14,6 +14,8 @@ import {
 import {
   performMainNetworkSync,
   startNetworkSyncing,
+  calculateAvailableSpaceToAdd,
+  getBoundedNetworksToAdd,
 } from './controller-integration';
 import * as ControllerIntegrationModule from './controller-integration';
 import * as ServicesModule from './services';
@@ -40,6 +42,69 @@ const storageOpts: UserStorageBaseOptions = {
   storageKey: MOCK_STORAGE_KEY,
 };
 
+describe('network-syncing/controller-integration - 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('network-syncing/controller-integration - 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;
+    });
+  }
+});
+
 describe('network-syncing/controller-integration - startNetworkSyncing()', () => {
   it(`should successfully sync when NetworkController:networkRemoved is emitted`, async () => {
     const { baseMessenger, props, deleteNetworkMock } = arrangeMocks();
@@ -237,6 +302,33 @@ describe('network-syncing/controller-integration - performMainSync()', () => {
     expect(mockCalls.mockNetworkControllerRemoveNetwork).not.toHaveBeenCalled();
   });
 
+  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.mockNetworkControllerDangerouslySetNetworkConfiguration,
+    ).not.toHaveBeenCalled();
+    expect(mockCalls.mockNetworkControllerRemoveNetwork).not.toHaveBeenCalled();
+  });
+
   it('should update local networks', async () => {
     const { messenger, getStorageConfig, mockSync, mockServices, mockCalls } =
       arrangeMocks();

packages/profile-sync-controller/src/controllers/user-storage/network-syncing/controller-integration.ts

@@ -5,6 +5,7 @@ import type { UserStorageControllerMessenger } from '../UserStorageController';
 import { getAllRemoteNetworks } from './services';
 import { findNetworksToUpdate } from './sync-all';
 import { batchUpdateNetworks, deleteNetwork } from './sync-mutations';
+import type { NetworkConfiguration } from './types';
 
 type StartNetworkSyncingProps = {
   messenger: UserStorageControllerMessenger;
@@ -15,11 +16,14 @@ type StartNetworkSyncingProps = {
 type PerformMainNetworkSyncProps = {
   messenger: UserStorageControllerMessenger;
   getStorageConfig: () => Promise<UserStorageBaseOptions | null>;
+  maxNetworksToAdd?: number;
   onNetworkAdded?: (chainId: string) => void;
   onNetworkUpdated?: (chainId: string) => void;
   onNetworkRemoved?: (chainId: string) => void;
 };
 
+export const MAX_NETWORKS_SIZE = 50;
+
 /**
  * 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)
@@ -72,6 +76,50 @@ export function startNetworkSyncing(props: StartNetworkSyncingProps) {
   }
 }
 
+/**
+ * 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);
+};
+
 /**
  * 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
@@ -80,7 +128,14 @@ export function startNetworkSyncing(props: StartNetworkSyncingProps) {
 export async function performMainNetworkSync(
   props: PerformMainNetworkSyncProps,
 ) {
-  const { messenger, getStorageConfig } = props;
+  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 */
@@ -121,15 +176,19 @@ export async function performMainNetworkSync(
     }
 
     // Add missing local networks
-    if (
+    const boundedNetworkedToAdd =
       networkChanges?.missingLocalNetworks &&
-      networkChanges.missingLocalNetworks.length > 0
-    ) {
+      getBoundedNetworksToAdd(
+        localNetworks,
+        networkChanges.missingLocalNetworks,
+        maxNetworksToAdd,
+      );
+    if (boundedNetworkedToAdd && boundedNetworkedToAdd.length > 0) {
       const errors: unknown[] = [];
-      networkChanges.missingLocalNetworks.forEach((n) => {
+      boundedNetworkedToAdd.forEach((n) => {
         try {
           messenger.call('NetworkController:addNetwork', n);
-          props.onNetworkAdded?.(n.chainId);
+          onNetworkAdded?.(n.chainId);
         } catch (e) {
           /* istanbul ignore next - allocates logs, do not need to test */
           errors.push(e);
@@ -158,7 +217,7 @@ export async function performMainNetworkSync(
             'NetworkController:dangerouslySetNetworkConfiguration',
             n,
           );
-          props.onNetworkUpdated?.(n.chainId);
+          onNetworkUpdated?.(n.chainId);
         } catch (e) {
           /* istanbul ignore next - allocates logs, do not need to test */
           errors.push(e);
@@ -184,7 +243,7 @@ export async function performMainNetworkSync(
       networkChanges.localNetworksToRemove.forEach((n) => {
         try {
           messenger.call('NetworkController:removeNetwork', n.chainId);
-          props.onNetworkRemoved?.(n.chainId);
+          onNetworkRemoved?.(n.chainId);
         } catch (e) {
           /* istanbul ignore next - allocates logs, do not need to test */
           errors.push(e);