fix: Reapply migration 52 to fix undefined selectedAccount

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