diff --git a/app/store/migrations/052.test.ts b/app/store/migrations/052.test.ts index 19b7a26c03a..907673d06c9 100644 --- a/app/store/migrations/052.test.ts +++ b/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: { diff --git a/app/store/migrations/052.ts b/app/store/migrations/052.ts index f11b942c321..fb11a846aba 100644 --- a/app/store/migrations/052.ts +++ b/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; } diff --git a/app/store/migrations/057.test.ts b/app/store/migrations/057.test.ts new file mode 100644 index 00000000000..5419e6e80ce --- /dev/null +++ b/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); + }); +}); diff --git a/app/store/migrations/057.ts b/app/store/migrations/057.ts new file mode 100644 index 00000000000..78c0ed9bf66 --- /dev/null +++ b/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; +} diff --git a/app/store/migrations/index.ts b/app/store/migrations/index.ts index 4f8e9e30ee9..3a684dead60 100644 --- a/app/store/migrations/index.ts +++ b/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; @@ -126,6 +127,7 @@ export const migrationList: MigrationsList = { 54: migration54, 55: migration55, 56: migration56, + 57: migration57, }; // Enable both synchronous and asynchronous migrations