refactor: use internalAccount instead of account address for granul…

…ar updates (#4693) This PR replaces the parameter for `UserStorageController` `saveInternalAccountToUserStorage` from an account address to an `InternalAccount`. This helps because we're listening to `AccountsController` events and they are passing an `InternalAccount` as their return value. By using this return value directly, we remove some unnecessary logic & queries. ## Explanation ## References ## Changelog ### `@metamask/profile-sync-controller` - **CHANGED**: use `InternalAccount` instead of account address for `saveInternalAccountToUserStorage` parameter ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
MetaMask · Sep 12, 2024 · 908215f · 908215f
1 parent 6f716b6
commit 908215f
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 36 deletions.
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
@@ -1104,7 +1104,7 @@ describe('user-storage/user-storage-controller - saveInternalAccountToUserStorag
     });
 
     await controller.saveInternalAccountToUserStorage(
-      MOCK_INTERNAL_ACCOUNTS.ONE[0].address,
+      MOCK_INTERNAL_ACCOUNTS.ONE[0] as InternalAccount,
     );
 
     expect(
@@ -1132,7 +1132,7 @@ describe('user-storage/user-storage-controller - saveInternalAccountToUserStorag
     });
 
     await controller.saveInternalAccountToUserStorage(
-      MOCK_INTERNAL_ACCOUNTS.ONE[0].address,
+      MOCK_INTERNAL_ACCOUNTS.ONE[0] as InternalAccount,
     );
 
     expect(mockAPI.isDone()).toBe(false);
@@ -1158,7 +1158,7 @@ describe('user-storage/user-storage-controller - saveInternalAccountToUserStorag
     });
 
     await controller.saveInternalAccountToUserStorage(
-      MOCK_INTERNAL_ACCOUNTS.ONE[0].address,
+      MOCK_INTERNAL_ACCOUNTS.ONE[0] as InternalAccount,
     );
 
     expect(mockAPI.isDone()).toBe(true);
@@ -1186,7 +1186,7 @@ describe('user-storage/user-storage-controller - saveInternalAccountToUserStorag
 
     await expect(
       controller.saveInternalAccountToUserStorage(
-        MOCK_INTERNAL_ACCOUNTS.ONE[0].address,
+        MOCK_INTERNAL_ACCOUNTS.ONE[0] as InternalAccount,
       ),
     ).rejects.toThrow(expect.any(Error));
   });
@@ -1212,7 +1212,7 @@ describe('user-storage/user-storage-controller - saveInternalAccountToUserStorag
     );
 
     expect(mockSaveInternalAccountToUserStorage).toHaveBeenCalledWith(
-      MOCK_INTERNAL_ACCOUNTS.ONE[0].address,
+      MOCK_INTERNAL_ACCOUNTS.ONE[0],
     );
   });
 
@@ -1237,7 +1237,7 @@ describe('user-storage/user-storage-controller - saveInternalAccountToUserStorag
     );
 
     expect(mockSaveInternalAccountToUserStorage).toHaveBeenCalledWith(
-      MOCK_INTERNAL_ACCOUNTS.ONE[0].address,
+      MOCK_INTERNAL_ACCOUNTS.ONE[0],
     );
   });
 });
@@ -1274,7 +1274,6 @@ function mockUserStorageMessenger(options?: {
       'NotificationServicesController:selectIsNotificationServicesEnabled',
       'AccountsController:listAccounts',
       'AccountsController:updateAccountMetadata',
-      'AccountsController:getAccountByAddress',
       'KeyringController:addNewAccount',
     ],
     allowedEvents: [

diff --git a/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts b/packages/profile-sync-controller/src/controllers/user-storage/UserStorageController.ts
@@ -1,7 +1,6 @@
 import type {
   AccountsControllerListAccountsAction,
   AccountsControllerUpdateAccountMetadataAction,
-  AccountsControllerGetAccountByAddressAction,
   AccountsControllerAccountRenamedEvent,
   AccountsControllerAccountAddedEvent,
 } from '@metamask/accounts-controller';
@@ -176,7 +175,6 @@ export type AllowedActions =
   | NotificationServicesControllerSelectIsNotificationServicesEnabled
   // Account syncing
   | AccountsControllerListAccountsAction
-  | AccountsControllerGetAccountByAddressAction
   | AccountsControllerUpdateAccountMetadataAction
   | KeyringControllerAddNewAccountAction;
 
@@ -285,7 +283,7 @@ export default class UserStorageController extends BaseController<
           if (this.#accounts.isAccountSyncingInProgress) {
             return;
           }
-          await this.saveInternalAccountToUserStorage(account.address);
+          await this.saveInternalAccountToUserStorage(account);
         },
       );
 
@@ -296,16 +294,10 @@ export default class UserStorageController extends BaseController<
           if (this.#accounts.isAccountSyncingInProgress) {
             return;
           }
-          await this.saveInternalAccountToUserStorage(account.address);
+          await this.saveInternalAccountToUserStorage(account);
         },
       );
     },
-    getInternalAccountByAddress: async (address: string) => {
-      return this.messagingSystem.call(
-        'AccountsController:getAccountByAddress',
-        address,
-      );
-    },
     getInternalAccountsList: async (): Promise<InternalAccount[]> => {
       return this.messagingSystem.call('AccountsController:listAccounts');
     },
@@ -320,21 +312,15 @@ export default class UserStorageController extends BaseController<
         null
       );
     },
-    saveInternalAccountToUserStorage: async (address: string) => {
-      const internalAccount = await this.#accounts.getInternalAccountByAddress(
-        address,
-      );
-
-      if (!internalAccount) {
-        return;
-      }
-
+    saveInternalAccountToUserStorage: async (
+      internalAccount: InternalAccount,
+    ) => {
       // Map the internal account to the user storage account schema
       const mappedAccount =
         mapInternalAccountToUserStorageAccount(internalAccount);
 
       await this.performSetStorage(
-        `accounts.${address}`,
+        `accounts.${internalAccount.address}`,
         JSON.stringify(mappedAccount),
       );
     },
@@ -782,7 +768,7 @@ export default class UserStorageController extends BaseController<
 
         if (!userStorageAccount) {
           await this.#accounts.saveInternalAccountToUserStorage(
-            internalAccount.address,
+            internalAccount,
           );
           continue;
         }
@@ -812,7 +798,7 @@ export default class UserStorageController extends BaseController<
         // Internal account has custom name but user storage account has default name
         if (isUserStorageAccountNameDefault) {
           await this.#accounts.saveInternalAccountToUserStorage(
-            internalAccount.address,
+            internalAccount,
           );
           continue;
         }
@@ -829,7 +815,7 @@ export default class UserStorageController extends BaseController<
 
             if (isInternalAccountNameNewer) {
               await this.#accounts.saveInternalAccountToUserStorage(
-                internalAccount.address,
+                internalAccount,
               );
               continue;
             }
@@ -847,7 +833,7 @@ export default class UserStorageController extends BaseController<
           continue;
         } else if (internalAccount.metadata.nameLastUpdatedAt !== undefined) {
           await this.#accounts.saveInternalAccountToUserStorage(
-            internalAccount.address,
+            internalAccount,
           );
           continue;
         }
@@ -864,17 +850,17 @@ export default class UserStorageController extends BaseController<
 
   /**
    * Saves an individual internal account to the user storage.
-   * @param address - The address of the internal account to save
+   * @param internalAccount - The internal account to save
    */
-  async saveInternalAccountToUserStorage(address: string): Promise<void> {
+  async saveInternalAccountToUserStorage(
+    internalAccount: InternalAccount,
+  ): Promise<void> {
     if (!this.#accounts.canSync()) {
       return;
     }
 
     try {
-      this.#assertProfileSyncingEnabled();
-
-      await this.#accounts.saveInternalAccountToUserStorage(address);
+      await this.#accounts.saveInternalAccountToUserStorage(internalAccount);
     } catch (e) {
       const errorMessage = e instanceof Error ? e.message : JSON.stringify(e);
       throw new Error(