diff --git a/app/store/migrations/059.test.ts b/app/store/migrations/059.test.ts new file mode 100644 index 00000000000..11161ff0388 --- /dev/null +++ b/app/store/migrations/059.test.ts @@ -0,0 +1,164 @@ +import migrate from './059'; +import { merge } from 'lodash'; +import { captureException } from '@sentry/react-native'; +import initialRootState from '../../util/test/initial-root-state'; +import mockedEngine from '../../core/__mocks__/MockedEngine'; +import { + expectedUuid, + expectedUuid2, + internalAccount1, + internalAccount2, +} from '../../util/test/accountsControllerTestUtils'; + +jest.mock('@sentry/react-native', () => ({ + captureException: jest.fn(), +})); +const mockedCaptureException = jest.mocked(captureException); + +jest.mock('../../core/Engine', () => ({ + init: () => mockedEngine.init(), +})); + +describe('Migration #59 - Fix crasher related to undefined selectedAccount on AccountsController', () => { + beforeEach(() => { + jest.restoreAllMocks(); + jest.resetAllMocks(); + }); + + const invalidStates = [ + { + state: null, + errorMessage: "FATAL ERROR: Migration 59: Invalid state error: 'object'", + scenario: 'state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: null, + }), + errorMessage: + "FATAL ERROR: Migration 59: Invalid engine state error: 'object'", + scenario: 'engine state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: { + backgroundState: null, + }, + }), + errorMessage: + "FATAL ERROR: Migration 59: Invalid engine backgroundState error: 'object'", + scenario: 'backgroundState is invalid', + }, + ]; + + for (const { errorMessage, scenario, state } of invalidStates) { + it(`should capture exception if ${scenario}`, async () => { + const newState = await migrate(state); + + expect(newState).toStrictEqual(state); + expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); + expect(mockedCaptureException.mock.calls[0][0].message).toBe( + errorMessage, + ); + }); + } + + it('should set selectedAccount to empty string if it is undefined', async () => { + const oldState = { + engine: { + backgroundState: { + AccountsController: { + internalAccounts: { + accounts: {}, + selectedAccount: undefined, + }, + }, + }, + }, + }; + + const expectedState = { + engine: { + backgroundState: { + AccountsController: { + internalAccounts: { + accounts: {}, + selectedAccount: '', + }, + }, + }, + }, + }; + + const migratedState = await migrate(oldState); + expect(migratedState).toStrictEqual(expectedState); + }); + + it('should set selectedAccount to the id of the first account if accounts exist', async () => { + const oldState = { + engine: { + backgroundState: { + AccountsController: { + internalAccounts: { + accounts: { + [expectedUuid]: internalAccount1, + [expectedUuid2]: internalAccount2, + }, + selectedAccount: undefined, + }, + }, + }, + }, + }; + + const expectedState = { + engine: { + backgroundState: { + AccountsController: { + internalAccounts: { + accounts: { + [expectedUuid]: internalAccount1, + [expectedUuid2]: internalAccount2, + }, + selectedAccount: expectedUuid, + }, + }, + }, + }, + }; + + const migratedState = await migrate(oldState); + expect(migratedState).toStrictEqual(expectedState); + }); + + it('should leave selectedAccount alone if it is not undefined', async () => { + const oldState = { + engine: { + backgroundState: { + AccountsController: { + internalAccounts: { + accounts: {}, + selectedAccount: '0x1', + }, + }, + }, + }, + }; + + const expectedState = { + engine: { + backgroundState: { + AccountsController: { + internalAccounts: { + accounts: {}, + selectedAccount: '0x1', + }, + }, + }, + }, + }; + + const migratedState = await migrate(oldState); + expect(migratedState).toStrictEqual(expectedState); + }); +}); diff --git a/app/store/migrations/059.ts b/app/store/migrations/059.ts new file mode 100644 index 00000000000..5415d882dde --- /dev/null +++ b/app/store/migrations/059.ts @@ -0,0 +1,67 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { ensureValidState } from './util'; +import Logger from '../../util/Logger'; + +/** + * Migration for checking if selectedAccount on AccountsController is undefined + * If it is, set the field to be an empty string. + * An undefined value causes the AccountsController to throw an Error + * Fix: The AccountsController automatically sets the selectedAddress if it is an empty string + * Fixes issue: View fullstack trace in https://github.com/MetaMask/metamask-mobile/issues/11488 + * + * @param state + * @returns + */ +export default function migrate(state: unknown) { + if (!ensureValidState(state, 59)) { + // Increment the migration number as appropriate + return state; + } + + // Check if selectedAccount is undefined + if ( + hasProperty(state.engine.backgroundState, 'AccountsController') && + isObject(state.engine.backgroundState.AccountsController) && + hasProperty( + state.engine.backgroundState.AccountsController, + 'internalAccounts', + ) && + isObject( + state.engine.backgroundState.AccountsController.internalAccounts, + ) && + hasProperty( + state.engine.backgroundState.AccountsController.internalAccounts, + 'selectedAccount', + ) && + state.engine.backgroundState.AccountsController.internalAccounts + .selectedAccount === undefined + ) { + // Try to set the selectedAccount to be the first account id + const internalAccounts = + state.engine.backgroundState.AccountsController.internalAccounts; + if ( + hasProperty(internalAccounts, 'accounts') && + isObject(internalAccounts.accounts) && + Object.keys(internalAccounts.accounts).length > 0 + ) { + const firstAccount = Object.values(internalAccounts.accounts)[0]; + if (isObject(firstAccount) && hasProperty(firstAccount, 'id')) { + Logger.log( + `Migration 59: Setting selectedAccount to the id of the first account.`, + ); + state.engine.backgroundState.AccountsController.internalAccounts.selectedAccount = + firstAccount.id; + } + } else { + // Fallback to setting selectedAccount to empty string. AccountsController automatically reconciles the field + Logger.log( + `Migration 59: AccountController's selectedAccount is undefined. Setting it to empty string.`, + ); + state.engine.backgroundState.AccountsController.internalAccounts.selectedAccount = + ''; + } + } + + // Return the modified state + return state; +} diff --git a/app/store/migrations/index.ts b/app/store/migrations/index.ts index 91c8ba0a5d5..52ac3f83fc8 100644 --- a/app/store/migrations/index.ts +++ b/app/store/migrations/index.ts @@ -59,6 +59,7 @@ import migration55 from './055'; import migration56 from './056'; import migration57 from './057'; import migration58 from './058'; +import migration59 from './059'; type MigrationFunction = (state: unknown) => unknown; type AsyncMigrationFunction = (state: unknown) => Promise; @@ -130,6 +131,7 @@ export const migrationList: MigrationsList = { 56: migration56, 57: migration57, 58: migration58, + 59: migration59, }; // Enable both synchronous and asynchronous migrations