fix: accounts controller type error (#4331)

## Explanation

This PR is a temp fix for the type error `Type instantiation is
excessively deep and possibly infinite.` when updating the state.

## References

Related to: MetaMask/utils#168

## Changelog

### `@metamask/accounts-controller`

- **FIXED**: Type error during state update

## 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
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

packages/accounts-controller/src/AccountsController.test.ts

@@ -172,9 +172,7 @@ function createExpectedInternalAccount({
       name,
       keyring: { type: keyringType },
       importTime: expect.any(Number),
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore
-      lastSelected: undefined,
+      lastSelected: expect.any(Number),
     },
   };
 
@@ -739,17 +737,15 @@ describe('AccountsController', () => {
 
         const accounts = accountsController.listAccounts();
 
-        expect(accounts).toStrictEqual([
+        expect(accounts.map(setLastSelectedAsAny)).toStrictEqual([
           mockAccount,
           mockAccount2WithCustomName,
-          setLastSelectedAsAny(
-            createExpectedInternalAccount({
-              id: 'mock-id3',
-              name: 'Account 3',
-              address: mockAccount3.address,
-              keyringType: KeyringTypes.hd,
-            }),
-          ),
+          createExpectedInternalAccount({
+            id: 'mock-id3',
+            name: 'Account 3',
+            address: mockAccount3.address,
+            keyringType: KeyringTypes.hd,
+          }),
         ]);
       });
 
@@ -1222,7 +1218,7 @@ describe('AccountsController', () => {
         metadata: {
           ...mockSnapAccount.metadata,
           name: 'Snap Account 1',
-          lastSelected: undefined,
+          lastSelected: expect.any(Number),
           importTime: expect.any(Number),
         },
       };
@@ -1232,7 +1228,7 @@ describe('AccountsController', () => {
         metadata: {
           ...mockSnapAccount2.metadata,
           name: 'Snap Account 2',
-          lastSelected: undefined,
+          lastSelected: expect.any(Number),
           importTime: expect.any(Number),
         },
       };
@@ -1241,7 +1237,9 @@ describe('AccountsController', () => {
 
       await accountsController.updateAccounts();
 
-      expect(accountsController.listAccounts()).toStrictEqual(expectedAccounts);
+      expect(
+        accountsController.listAccounts().map(setLastSelectedAsAny),
+      ).toStrictEqual(expectedAccounts);
     });
 
     it('should return an empty array if the snap keyring is not defined', async () => {
@@ -1485,7 +1483,9 @@ describe('AccountsController', () => {
 
       await accountsController.updateAccounts();
 
-      expect(accountsController.listAccounts()).toStrictEqual(expectedAccounts);
+      expect(
+        accountsController.listAccounts().map(setLastSelectedAsAny),
+      ).toStrictEqual(expectedAccounts);
     });
 
     it('should throw an error if the keyring type is unknown', async () => {

packages/accounts-controller/src/AccountsController.ts

@@ -25,6 +25,7 @@ import type { Keyring, Json } from '@metamask/utils';
 import type { Draft } from 'immer';
 
 import {
+  deepCloneDraft,
   getUUIDFromAddressOfNormalAccount,
   isNormalKeyringType,
   keyringTypeToName,
@@ -312,7 +313,12 @@ export class AccountsController extends BaseController<
         ...account,
         metadata: { ...account.metadata, name: accountName },
       };
-      currentState.internalAccounts.accounts[accountId] = internalAccount;
+      // FIXME: deep clone of old state to get around Type instantiation is excessively deep and possibly infinite.
+      const newState = deepCloneDraft(currentState);
+
+      newState.internalAccounts.accounts[accountId] = internalAccount;
+
+      return newState;
     });
   }
 
@@ -357,19 +363,24 @@ export class AccountsController extends BaseController<
           importTime:
             this.#populateExistingMetadata(existingAccount?.id, 'importTime') ??
             Date.now(),
-          lastSelected: this.#populateExistingMetadata(
-            existingAccount?.id,
-            'lastSelected',
-          ),
+          lastSelected:
+            this.#populateExistingMetadata(
+              existingAccount?.id,
+              'lastSelected',
+            ) ?? 0,
         },
       };
 
       return internalAccountMap;
     }, {} as Record<string, InternalAccount>);
 
     this.update((currentState: Draft<AccountsControllerState>) => {
-      (currentState as AccountsControllerState).internalAccounts.accounts =
-        accounts;
+      // FIXME: deep clone of old state to get around Type instantiation is excessively deep and possibly infinite.
+      const newState = deepCloneDraft(currentState);
+
+      newState.internalAccounts.accounts = accounts;
+
+      return newState;
     });
   }
 
@@ -381,8 +392,12 @@ export class AccountsController extends BaseController<
   loadBackup(backup: AccountsControllerState): void {
     if (backup.internalAccounts) {
       this.update((currentState: Draft<AccountsControllerState>) => {
-        (currentState as AccountsControllerState).internalAccounts =
-          backup.internalAccounts;
+        // FIXME: deep clone of old state to get around Type instantiation is excessively deep and possibly infinite.
+        const newState = deepCloneDraft(currentState);
+
+        newState.internalAccounts = backup.internalAccounts;
+
+        return newState;
       });
     }
   }
@@ -483,7 +498,7 @@ export class AccountsController extends BaseController<
           name: this.#populateExistingMetadata(id, 'name') ?? '',
           importTime:
             this.#populateExistingMetadata(id, 'importTime') ?? Date.now(),
-          lastSelected: this.#populateExistingMetadata(id, 'lastSelected'),
+          lastSelected: this.#populateExistingMetadata(id, 'lastSelected') ?? 0,
           keyring: {
             type: (keyring as Keyring<Json>).type,
           },
@@ -765,9 +780,10 @@ export class AccountsController extends BaseController<
     );
 
     this.update((currentState: Draft<AccountsControllerState>) => {
-      (currentState as AccountsControllerState).internalAccounts.accounts[
-        newAccount.id
-      ] = {
+      // FIXME: deep clone of old state to get around Type instantiation is excessively deep and possibly infinite.
+      const newState = deepCloneDraft(currentState);
+
+      newState.internalAccounts.accounts[newAccount.id] = {
         ...newAccount,
         metadata: {
           ...newAccount.metadata,
@@ -776,6 +792,8 @@ export class AccountsController extends BaseController<
           lastSelected: Date.now(),
         },
       };
+
+      return newState;
     });
 
     this.setSelectedAccount(newAccount.id);

packages/accounts-controller/src/utils.ts

@@ -1,9 +1,13 @@
 import { toBuffer } from '@ethereumjs/util';
 import { isCustodyKeyring, KeyringTypes } from '@metamask/keyring-controller';
+import { deepClone } from '@metamask/snaps-utils';
 import { sha256 } from 'ethereum-cryptography/sha256';
+import type { Draft } from 'immer';
 import type { V4Options } from 'uuid';
 import { v4 as uuid } from 'uuid';
 
+import type { AccountsControllerState } from './AccountsController';
+
 /**
  * Returns the name of the keyring type.
  *
@@ -79,3 +83,19 @@ export function isNormalKeyringType(keyringType: KeyringTypes): boolean {
   // adapted later on if we have new kind of keyrings!
   return keyringType !== KeyringTypes.snap;
 }
+
+/**
+ * WARNING: To be removed once type issue is fixed. https://github.com/MetaMask/utils/issues/168
+ *
+ * Creates a deep clone of the given object.
+ * This is to get around error `Type instantiation is excessively deep and possibly infinite.`
+ *
+ * @param obj - The object to be cloned.
+ * @returns The deep clone of the object.
+ */
+export function deepCloneDraft(
+  obj: Draft<AccountsControllerState>,
+): AccountsControllerState {
+  // We use unknown here because the type inference when using structured clone leads to the same type error.
+  return deepClone(obj) as unknown as AccountsControllerState;
+}