From 19adab683196588d2d862a03c7e7caa73f418b8e Mon Sep 17 00:00:00 2001 From: Sylva Elendu Date: Tue, 27 Feb 2024 15:35:30 +0100 Subject: [PATCH 1/2] fix: migration to enable Blockaid by default (#8727) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrating from a previous version to v7.17.0 should enable Blockaid by default in the same way it's done with 7.17.0 fresh installs. Fixes: #8669 1. Install v7.16 2. Update to v7.17 3. Check settings --> see Blockaid is enabled - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [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. - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". - [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/031.test.ts | 92 ++++++++++++++++++++++++++++++++ app/store/migrations/031.ts | 54 +++++++++++++++++++ app/store/migrations/index.ts | 2 + 3 files changed, 148 insertions(+) create mode 100644 app/store/migrations/031.test.ts create mode 100644 app/store/migrations/031.ts diff --git a/app/store/migrations/031.test.ts b/app/store/migrations/031.test.ts new file mode 100644 index 00000000000..0388d65ac13 --- /dev/null +++ b/app/store/migrations/031.test.ts @@ -0,0 +1,92 @@ +import migrate from './031'; +import { merge } from 'lodash'; +import { captureException } from '@sentry/react-native'; +import initialRootState from '../../util/test/initial-root-state'; + +const expectedState = { + engine: { + backgroundState: { + PreferencesController: { + securityAlertsEnabled: true, + }, + }, + }, +}; + +jest.mock('@sentry/react-native', () => ({ + captureException: jest.fn(), +})); +const mockedCaptureException = jest.mocked(captureException); + +describe('Migration #31', () => { + beforeEach(() => { + jest.restoreAllMocks(); + jest.resetAllMocks(); + }); + + const invalidStates = [ + { + state: null, + errorMessage: "Migration 31: Invalid root state: 'object'", + scenario: 'state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: null, + }), + errorMessage: "Migration 31: Invalid root engine state: 'object'", + scenario: 'engine state is invalid', + }, + { + state: merge({}, initialRootState, { + engine: { + backgroundState: null, + }, + }), + errorMessage: + "Migration 31: Invalid root engine backgroundState: 'object'", + scenario: 'backgroundState is invalid', + }, + ]; + + for (const { errorMessage, scenario, state } of invalidStates) { + it(`should capture exception if ${scenario}`, () => { + const newState = migrate(state); + + expect(newState).toStrictEqual(state); + expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); + expect(mockedCaptureException.mock.calls[0][0].message).toBe( + errorMessage, + ); + }); + } + + it('should not change anything if security alert is already enabled', () => { + const oldState = { + engine: { + backgroundState: { + PreferencesController: { + securityAlertsEnabled: true, + }, + }, + }, + }; + + const migratedState = migrate(oldState); + expect(migratedState).toStrictEqual(expectedState); + }); + + it('should enable security alert if it is not enabled', () => { + const oldState = { + engine: { + backgroundState: { + PreferencesController: { + securityAlertsEnabled: false, + }, + }, + }, + }; + const migratedState = migrate(oldState); + expect(migratedState).toStrictEqual(expectedState); + }); +}); diff --git a/app/store/migrations/031.ts b/app/store/migrations/031.ts new file mode 100644 index 00000000000..7deb2216deb --- /dev/null +++ b/app/store/migrations/031.ts @@ -0,0 +1,54 @@ +import { PreferencesState } from '@metamask/preferences-controller'; +import { captureException } from '@sentry/react-native'; +import { isObject } from '@metamask/utils'; + +/** + * Enable security alerts by default. + * @param {any} state - Redux state. + * @returns Migrated Redux state. + */ +export default function migrate(state: unknown) { + if (!isObject(state)) { + captureException( + new Error(`Migration 31: Invalid root state: '${typeof state}'`), + ); + return state; + } + + if (!isObject(state.engine)) { + captureException( + new Error( + `Migration 31: Invalid root engine state: '${typeof state.engine}'`, + ), + ); + return state; + } + + if (!isObject(state.engine.backgroundState)) { + captureException( + new Error( + `Migration 31: Invalid root engine backgroundState: '${typeof state + .engine.backgroundState}'`, + ), + ); + return state; + } + + const preferencesControllerState = state.engine.backgroundState + .PreferencesController as PreferencesState; + + if (!isObject(preferencesControllerState)) { + captureException( + new Error( + `Migration 31: Invalid PreferencesController state: '${typeof preferencesControllerState}'`, + ), + ); + return state; + } + + if (!preferencesControllerState.securityAlertsEnabled) { + preferencesControllerState.securityAlertsEnabled = true; + } + + return state; +} diff --git a/app/store/migrations/index.ts b/app/store/migrations/index.ts index 9e599fa3e50..306d3ac88cb 100644 --- a/app/store/migrations/index.ts +++ b/app/store/migrations/index.ts @@ -30,6 +30,7 @@ import migration26 from './026'; import migration27 from './027'; import migration28 from './028'; import migration29 from './029'; +import migration31 from './031'; // We do not keep track of the old state // We create this type for better readability @@ -66,6 +67,7 @@ export const migrations: MigrationManifest = { 27: migration27, 28: migration28 as unknown as (state: PersistedState) => PersistedState, 29: migration29 as unknown as (state: OldState) => PersistedState, + 31: migration31 as unknown as (state: OldState) => PersistedState, }; // The latest (i.e. highest) version number. From cdf411306ba3e022dd2fd3652e0bc3524ad3bad0 Mon Sep 17 00:00:00 2001 From: Sylva Elendu Date: Thu, 29 Feb 2024 03:59:48 +0100 Subject: [PATCH 2/2] update migration value to 30 --- app/store/migrations/{031.test.ts => 030.test.ts} | 10 +++++----- app/store/migrations/{031.ts => 030.ts} | 8 ++++---- app/store/migrations/index.ts | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) rename app/store/migrations/{031.test.ts => 030.test.ts} (89%) rename app/store/migrations/{031.ts => 030.ts} (81%) diff --git a/app/store/migrations/031.test.ts b/app/store/migrations/030.test.ts similarity index 89% rename from app/store/migrations/031.test.ts rename to app/store/migrations/030.test.ts index 0388d65ac13..8f404f0a8f6 100644 --- a/app/store/migrations/031.test.ts +++ b/app/store/migrations/030.test.ts @@ -1,4 +1,4 @@ -import migrate from './031'; +import migrate from './030'; import { merge } from 'lodash'; import { captureException } from '@sentry/react-native'; import initialRootState from '../../util/test/initial-root-state'; @@ -18,7 +18,7 @@ jest.mock('@sentry/react-native', () => ({ })); const mockedCaptureException = jest.mocked(captureException); -describe('Migration #31', () => { +describe('Migration #30', () => { beforeEach(() => { jest.restoreAllMocks(); jest.resetAllMocks(); @@ -27,14 +27,14 @@ describe('Migration #31', () => { const invalidStates = [ { state: null, - errorMessage: "Migration 31: Invalid root state: 'object'", + errorMessage: "Migration 30: Invalid root state: 'object'", scenario: 'state is invalid', }, { state: merge({}, initialRootState, { engine: null, }), - errorMessage: "Migration 31: Invalid root engine state: 'object'", + errorMessage: "Migration 30: Invalid root engine state: 'object'", scenario: 'engine state is invalid', }, { @@ -44,7 +44,7 @@ describe('Migration #31', () => { }, }), errorMessage: - "Migration 31: Invalid root engine backgroundState: 'object'", + "Migration 30: Invalid root engine backgroundState: 'object'", scenario: 'backgroundState is invalid', }, ]; diff --git a/app/store/migrations/031.ts b/app/store/migrations/030.ts similarity index 81% rename from app/store/migrations/031.ts rename to app/store/migrations/030.ts index 7deb2216deb..dab6dcd4c01 100644 --- a/app/store/migrations/031.ts +++ b/app/store/migrations/030.ts @@ -10,7 +10,7 @@ import { isObject } from '@metamask/utils'; export default function migrate(state: unknown) { if (!isObject(state)) { captureException( - new Error(`Migration 31: Invalid root state: '${typeof state}'`), + new Error(`Migration 30: Invalid root state: '${typeof state}'`), ); return state; } @@ -18,7 +18,7 @@ export default function migrate(state: unknown) { if (!isObject(state.engine)) { captureException( new Error( - `Migration 31: Invalid root engine state: '${typeof state.engine}'`, + `Migration 30: Invalid root engine state: '${typeof state.engine}'`, ), ); return state; @@ -27,7 +27,7 @@ export default function migrate(state: unknown) { if (!isObject(state.engine.backgroundState)) { captureException( new Error( - `Migration 31: Invalid root engine backgroundState: '${typeof state + `Migration 30: Invalid root engine backgroundState: '${typeof state .engine.backgroundState}'`, ), ); @@ -40,7 +40,7 @@ export default function migrate(state: unknown) { if (!isObject(preferencesControllerState)) { captureException( new Error( - `Migration 31: Invalid PreferencesController state: '${typeof preferencesControllerState}'`, + `Migration 30: Invalid PreferencesController state: '${typeof preferencesControllerState}'`, ), ); return state; diff --git a/app/store/migrations/index.ts b/app/store/migrations/index.ts index 306d3ac88cb..de194ea95db 100644 --- a/app/store/migrations/index.ts +++ b/app/store/migrations/index.ts @@ -30,7 +30,7 @@ import migration26 from './026'; import migration27 from './027'; import migration28 from './028'; import migration29 from './029'; -import migration31 from './031'; +import migration30 from './030'; // We do not keep track of the old state // We create this type for better readability @@ -67,7 +67,7 @@ export const migrations: MigrationManifest = { 27: migration27, 28: migration28 as unknown as (state: PersistedState) => PersistedState, 29: migration29 as unknown as (state: OldState) => PersistedState, - 31: migration31 as unknown as (state: OldState) => PersistedState, + 30: migration30 as unknown as (state: OldState) => PersistedState, }; // The latest (i.e. highest) version number.