fix: Reapply migration 52 to fix undefined selectedAccount (#12115)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR fixes #11866
where it ensures that selectedAccount in the AccountsController is
defined.

## **Related issues**

Fixes: #11866

## **Manual testing steps**

1. Run `main` and create a wallet with 3 accounts, select account 1
2. Kill the app and apply a migration that sets `selectedAccount` in the
AccountsController to undefined, re-run yarn watch and re-open app
3. Notice that the app is stuck on the splash screen
4. Kill the app and run this branch, which applies the migration that
fixes the issue. Notice that the app is now accessible again

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings]  -->
<img width="900" alt="Screenshot 2024-10-30 at 4 42 44 PM"
src="https://github.com/user-attachments/assets/394f1b4c-741c-4c7f-993e-b0120e16588e">

<img width="961" alt="Screenshot 2024-10-30 at 4 43 07 PM"
src="https://github.com/user-attachments/assets/37c685b2-925a-45c5-a7c4-7fadec8cba6f">

### **After**

<!-- [screenshots/recordings] -->
App is accessible again
<img width="444" alt="Screenshot 2024-10-30 at 10 11 24 PM"
src="https://github.com/user-attachments/assets/cc0cb9ef-2a5b-4804-a9e8-eac93cc4212d">


## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

app/store/migrations/052.test.ts

@@ -83,24 +83,6 @@ describe('Migration #52', () => {
       expectedError:
         "FATAL ERROR: Migration 52: Invalid AccountsController state error: internalAccounts.accounts is not an object, type: 'object'",
     },
-    {
-      state: merge({}, initialRootState, {
-        engine: {
-          backgroundState: {
-            AccountsController: {
-              internalAccounts: {
-                accounts: {},
-                selectedAccount: null,
-              },
-            },
-          },
-        },
-      }),
-      scenario:
-        'AccountsController internalAccounts selectedAccount is not a string',
-      expectedError:
-        "FATAL ERROR: Migration 52: Invalid AccountsController state error: internalAccounts.selectedAccount is not a string, type: 'object'",
-    },
   ];
 
   for (const { scenario, state, expectedError } of invalidStates) {
@@ -145,6 +127,25 @@ describe('Migration #52', () => {
     ).toEqual(expectedUuid);
   });
 
+  it('updates the selectedAccount if it is undefined', () => {
+    const invalidState = merge({}, oldState, {
+      engine: {
+        backgroundState: {
+          AccountsController: {
+            internalAccounts: {},
+          },
+        },
+      },
+    });
+
+    const newState = migration(invalidState) as typeof invalidState;
+
+    expect(
+      newState.engine.backgroundState.AccountsController.internalAccounts
+        .selectedAccount,
+    ).toEqual(expectedUuid);
+  });
+
   it('does not change the state if there are no accounts', () => {
     const emptyAccountsState = merge({}, oldState, {
       engine: {

app/store/migrations/052.ts

@@ -52,25 +52,6 @@ function isAccountsControllerState(
     return false;
   }
 
-  if (!hasProperty(state.internalAccounts, 'selectedAccount')) {
-    captureException(
-      new Error(
-        'FATAL ERROR: Migration 52: Invalid AccountsController state error: missing internalAccounts.selectedAccount',
-      ),
-    );
-    return false;
-  }
-
-  if (typeof state.internalAccounts.selectedAccount !== 'string') {
-    captureException(
-      new Error(
-        `FATAL ERROR: Migration 52: Invalid AccountsController state error: internalAccounts.selectedAccount is not a string, type: '${typeof state
-          .internalAccounts.selectedAccount}'`,
-      ),
-    );
-    return false;
-  }
-
   return true;
 }
 
@@ -89,7 +70,10 @@ export default function migrate(state: unknown) {
   const { accounts, selectedAccount } =
     accountsControllerState.internalAccounts;
 
-  if (Object.values(accounts).length > 0 && !accounts[selectedAccount]) {
+  if (
+    Object.values(accounts).length > 0 &&
+    (!selectedAccount || (selectedAccount && !accounts[selectedAccount]))
+  ) {
     accountsControllerState.internalAccounts.selectedAccount =
       Object.values(accounts)[0].id;
   }

app/store/migrations/057.test.ts

@@ -0,0 +1,197 @@
+import migration from './057';
+import { merge } from 'lodash';
+import initialRootState from '../../util/test/initial-root-state';
+import { captureException } from '@sentry/react-native';
+import {
+  expectedUuid,
+  expectedUuid2,
+  internalAccount1,
+  internalAccount2,
+} from '../../util/test/accountsControllerTestUtils';
+
+const oldState = {
+  engine: {
+    backgroundState: {
+      AccountsController: {
+        internalAccounts: {
+          accounts: {
+            [expectedUuid]: internalAccount1,
+            [expectedUuid2]: internalAccount2,
+          },
+          selectedAccount: expectedUuid,
+        },
+      },
+    },
+  },
+};
+
+jest.mock('@sentry/react-native', () => ({
+  captureException: jest.fn(),
+}));
+const mockedCaptureException = jest.mocked(captureException);
+
+describe('Migration #57', () => {
+  beforeEach(() => {
+    jest.restoreAllMocks();
+    jest.resetAllMocks();
+  });
+
+  const invalidStates = [
+    {
+      state: merge({}, initialRootState, {
+        engine: null,
+      }),
+      scenario: 'engine state is invalid',
+      expectedError:
+        "FATAL ERROR: Migration 57: Invalid engine state error: 'object'",
+    },
+    {
+      state: merge({}, initialRootState, {
+        engine: {
+          backgroundState: null,
+        },
+      }),
+      scenario: 'backgroundState is invalid',
+      expectedError:
+        "FATAL ERROR: Migration 57: Invalid engine backgroundState error: 'object'",
+    },
+    {
+      state: merge({}, initialRootState, {
+        engine: {
+          backgroundState: {
+            AccountsController: { internalAccounts: null },
+          },
+        },
+      }),
+      scenario: 'AccountsController internalAccounts state is invalid',
+      expectedError:
+        "FATAL ERROR: Migration 57: Invalid AccountsController state error: internalAccounts is not an object, type: 'object'",
+    },
+    {
+      state: merge({}, initialRootState, {
+        engine: {
+          backgroundState: {
+            AccountsController: {
+              internalAccounts: {
+                accounts: null,
+              },
+            },
+          },
+        },
+      }),
+      scenario: 'AccountsController internalAccounts accounts state is invalid',
+      expectedError:
+        "FATAL ERROR: Migration 57: Invalid AccountsController state error: internalAccounts.accounts is not an object, type: 'object'",
+    },
+  ];
+
+  for (const { scenario, state, expectedError } of invalidStates) {
+    it(`captures exception if ${scenario}`, () => {
+      const newState = migration(state);
+
+      expect(newState).toStrictEqual(state);
+      expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error));
+      expect(mockedCaptureException.mock.calls[0][0].message).toBe(
+        expectedError,
+      );
+    });
+  }
+
+  it('does not change the selectedAccount if it is valid', () => {
+    const newState = migration(oldState) as typeof oldState;
+
+    expect(
+      newState.engine.backgroundState.AccountsController.internalAccounts
+        .selectedAccount,
+    ).toEqual(expectedUuid);
+  });
+
+  it('updates the selectedAccount if it is invalid', () => {
+    const invalidState = merge({}, oldState, {
+      engine: {
+        backgroundState: {
+          AccountsController: {
+            internalAccounts: {
+              selectedAccount: 'invalid-uuid',
+            },
+          },
+        },
+      },
+    });
+
+    const newState = migration(invalidState) as typeof invalidState;
+
+    expect(
+      newState.engine.backgroundState.AccountsController.internalAccounts
+        .selectedAccount,
+    ).toEqual(expectedUuid);
+  });
+
+  it('updates the selectedAccount if it is undefined', () => {
+    const invalidState = merge({}, oldState, {
+      engine: {
+        backgroundState: {
+          AccountsController: {
+            internalAccounts: {},
+          },
+        },
+      },
+    });
+
+    const newState = migration(invalidState) as typeof invalidState;
+
+    expect(
+      newState.engine.backgroundState.AccountsController.internalAccounts
+        .selectedAccount,
+    ).toEqual(expectedUuid);
+  });
+
+  it('does not change the state if there are no accounts', () => {
+    const emptyAccountsState = merge({}, oldState, {
+      engine: {
+        backgroundState: {
+          AccountsController: {
+            internalAccounts: {
+              accounts: {},
+              selectedAccount: 'some-uuid',
+            },
+          },
+        },
+      },
+    });
+
+    const newState = migration(emptyAccountsState) as typeof emptyAccountsState;
+
+    expect(newState).toStrictEqual(emptyAccountsState);
+  });
+
+  it('updates the selectedAccount to the first account if current selection is invalid', () => {
+    const invalidSelectedState = merge({}, oldState, {
+      engine: {
+        backgroundState: {
+          AccountsController: {
+            internalAccounts: {
+              selectedAccount: 'non-existent-uuid',
+            },
+          },
+        },
+      },
+    });
+
+    const newState = migration(
+      invalidSelectedState,
+    ) as typeof invalidSelectedState;
+
+    expect(
+      newState.engine.backgroundState.AccountsController.internalAccounts
+        .selectedAccount,
+    ).toEqual(expectedUuid);
+  });
+
+  it('does not modify the state if the selectedAccount is valid', () => {
+    const validState = merge({}, oldState);
+    const newState = migration(validState) as typeof validState;
+
+    expect(newState).toStrictEqual(validState);
+  });
+});

app/store/migrations/057.ts

@@ -0,0 +1,85 @@
+import { captureException } from '@sentry/react-native';
+import { hasProperty, isObject } from '@metamask/utils';
+import { ensureValidState } from './util';
+import { AccountsControllerState } from '@metamask/accounts-controller';
+
+function isAccountsControllerState(
+  state: unknown,
+): state is AccountsControllerState {
+  if (!isObject(state)) {
+    captureException(
+      new Error(
+        `FATAL ERROR: Migration 57: Invalid AccountsController state error: AccountsController state is not an object, type: '${typeof state}'`,
+      ),
+    );
+    return false;
+  }
+
+  if (!hasProperty(state, 'internalAccounts')) {
+    captureException(
+      new Error(
+        'FATAL ERROR: Migration 57: Invalid AccountsController state error: missing internalAccounts',
+      ),
+    );
+    return false;
+  }
+
+  if (!isObject(state.internalAccounts)) {
+    captureException(
+      new Error(
+        `FATAL ERROR: Migration 57: Invalid AccountsController state error: internalAccounts is not an object, type: '${typeof state.internalAccounts}'`,
+      ),
+    );
+    return false;
+  }
+
+  if (!hasProperty(state.internalAccounts, 'accounts')) {
+    captureException(
+      new Error(
+        'FATAL ERROR: Migration 57: Invalid AccountsController state error: missing internalAccounts.accounts',
+      ),
+    );
+    return false;
+  }
+
+  if (!isObject(state.internalAccounts.accounts)) {
+    captureException(
+      new Error(
+        `FATAL ERROR: Migration 57: Invalid AccountsController state error: internalAccounts.accounts is not an object, type: '${typeof state
+          .internalAccounts.accounts}'`,
+      ),
+    );
+    return false;
+  }
+
+  return true;
+}
+
+/**
+ * Migration for ensuring that selectedAccount on the AccountsController is defined
+ */
+export default function migrate(state: unknown) {
+  if (!ensureValidState(state, 57)) {
+    return state;
+  }
+
+  const accountsControllerState =
+    state.engine.backgroundState.AccountsController;
+
+  if (!isAccountsControllerState(accountsControllerState)) {
+    return state;
+  }
+
+  const { accounts, selectedAccount } =
+    accountsControllerState.internalAccounts;
+
+  if (
+    Object.values(accounts).length > 0 &&
+    (!selectedAccount || (selectedAccount && !accounts[selectedAccount]))
+  ) {
+    accountsControllerState.internalAccounts.selectedAccount =
+      Object.values(accounts)[0].id;
+  }
+
+  return state;
+}

app/store/migrations/index.ts

@@ -57,6 +57,7 @@ import migration53 from './053';
 import migration54 from './054';
 import migration55 from './055';
 import migration56 from './056';
+import migration57 from './057';
 
 type MigrationFunction = (state: unknown) => unknown;
 type AsyncMigrationFunction = (state: unknown) => Promise<unknown>;
@@ -126,6 +127,7 @@ export const migrationList: MigrationsList = {
   54: migration54,
   55: migration55,
   56: migration56,
+  57: migration57,
 };
 
 // Enable both synchronous and asynchronous migrations