From 9bbabd4868fa7da70d8c2065e38ea8d702fcde14 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Wed, 16 Aug 2023 12:22:38 -0230 Subject: [PATCH] Capture app and migration version (#20458) * Add AppMetadataController so current and previous application and migration version can be captured in sentry * Add currentAppVersion, previousAppVersion, previousMigrationVersion, currentMigrationVersion to SENTRY_OBJECT * Update app/scripts/controllers/app-metadata.ts Co-authored-by: Mark Stacey * Update app/scripts/controllers/app-metadata.ts Co-authored-by: Mark Stacey * Update app/scripts/controllers/app-metadata.ts Co-authored-by: Mark Stacey * Fix types * Add tests for app-metadata.test.ts * Lint fixes * Modify loadStateFromPersistence to return the whole versionData object, so that the migration version can be passed to the metamask-controller on instantiation * Remove reference to implementation details in test descriptions in app/scripts/controllers/app-metadata.test.ts * Reset all mocks afterEach in AppMetadataController * Refactor AppMetadataController to be passed version instead of calling platform.version directly (for ease of unit testing the MetaMask Controller) * Make maybeUpdateAppVersion and maybeUpdateMigrationVersion private, and remove unit tests of those specific functions --------- Co-authored-by: Mark Stacey --- app/scripts/background.js | 9 +- app/scripts/controllers/app-metadata.test.ts | 104 +++++++++++++++++++ app/scripts/controllers/app-metadata.ts | 99 ++++++++++++++++++ app/scripts/lib/setupSentry.js | 4 + app/scripts/metamask-controller.js | 11 ++ 5 files changed, 225 insertions(+), 2 deletions(-) create mode 100644 app/scripts/controllers/app-metadata.test.ts create mode 100644 app/scripts/controllers/app-metadata.ts diff --git a/app/scripts/background.js b/app/scripts/background.js index 5eac5d1d6..264964cca 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -264,7 +264,8 @@ browser.runtime.onConnectExternal.addListener(async (...args) => { */ async function initialize() { try { - const initState = await loadStateFromPersistence(); + const initData = await loadStateFromPersistence(); + const initState = initData.data; const initLangCode = await getFirstPreferredLangCode(); ///: BEGIN:ONLY_INCLUDE_IN(desktop) @@ -287,6 +288,7 @@ async function initialize() { initLangCode, {}, isFirstMetaMaskControllerSetup, + initData.meta, ); if (!isManifestV3) { await loadPhishingWarningPage(); @@ -417,7 +419,7 @@ export async function loadStateFromPersistence() { localStore.set(versionedData.data); // return just the data - return versionedData.data; + return versionedData; } /** @@ -430,12 +432,14 @@ export async function loadStateFromPersistence() { * @param {string} initLangCode - The region code for the language preferred by the current user. * @param {object} overrides - object with callbacks that are allowed to override the setup controller logic (usefull for desktop app) * @param isFirstMetaMaskControllerSetup + * @param {object} stateMetadata - Metadata about the initial state and migrations, including the most recent migration version */ export function setupController( initState, initLangCode, overrides, isFirstMetaMaskControllerSetup, + stateMetadata, ) { // // MetaMask Controller @@ -462,6 +466,7 @@ export function setupController( localStore, overrides, isFirstMetaMaskControllerSetup, + currentMigrationVersion: stateMetadata.version, }); setupEnsIpfsResolver({ diff --git a/app/scripts/controllers/app-metadata.test.ts b/app/scripts/controllers/app-metadata.test.ts new file mode 100644 index 000000000..890811ee2 --- /dev/null +++ b/app/scripts/controllers/app-metadata.test.ts @@ -0,0 +1,104 @@ +import assert from 'assert'; +import AppMetadataController from './app-metadata'; + +const EXPECTED_DEFAULT_STATE = { + currentAppVersion: '', + previousAppVersion: '', + previousMigrationVersion: 0, + currentMigrationVersion: 0, +}; + +describe('AppMetadataController', () => { + describe('constructor', () => { + it('accepts initial state and does not modify it if currentMigrationVersion and platform.getVersion() match respective values in state', async () => { + const initState = { + currentAppVersion: '1', + previousAppVersion: '1', + previousMigrationVersion: 1, + currentMigrationVersion: 1, + }; + const appMetadataController = new AppMetadataController({ + state: initState, + currentMigrationVersion: 1, + currentAppVersion: '1', + }); + assert.deepStrictEqual(appMetadataController.store.getState(), initState); + }); + + it('sets default state and does not modify it', async () => { + const appMetadataController = new AppMetadataController({ + state: {}, + }); + assert.deepStrictEqual( + appMetadataController.store.getState(), + EXPECTED_DEFAULT_STATE, + ); + }); + + it('sets default state and does not modify it if options version parameters match respective default values', async () => { + const appMetadataController = new AppMetadataController({ + state: {}, + currentMigrationVersion: 0, + currentAppVersion: '', + }); + assert.deepStrictEqual( + appMetadataController.store.getState(), + EXPECTED_DEFAULT_STATE, + ); + }); + + it('updates the currentAppVersion state property if options.currentAppVersion does not match the default value', async () => { + const appMetadataController = new AppMetadataController({ + state: {}, + currentMigrationVersion: 0, + currentAppVersion: '1', + }); + assert.deepStrictEqual(appMetadataController.store.getState(), { + ...EXPECTED_DEFAULT_STATE, + currentAppVersion: '1', + }); + }); + + it('updates the currentAppVersion and previousAppVersion state properties if options.currentAppVersion, currentAppVersion and previousAppVersion are all different', async () => { + const appMetadataController = new AppMetadataController({ + state: { + currentAppVersion: '2', + previousAppVersion: '1', + }, + currentAppVersion: '3', + currentMigrationVersion: 0, + }); + assert.deepStrictEqual(appMetadataController.store.getState(), { + ...EXPECTED_DEFAULT_STATE, + currentAppVersion: '3', + previousAppVersion: '2', + }); + }); + + it('updates the currentMigrationVersion state property if the currentMigrationVersion param does not match the default value', async () => { + const appMetadataController = new AppMetadataController({ + state: {}, + currentMigrationVersion: 1, + }); + assert.deepStrictEqual(appMetadataController.store.getState(), { + ...EXPECTED_DEFAULT_STATE, + currentMigrationVersion: 1, + }); + }); + + it('updates the currentMigrationVersion and previousMigrationVersion state properties if the currentMigrationVersion param, the currentMigrationVersion state property and the previousMigrationVersion state property are all different', async () => { + const appMetadataController = new AppMetadataController({ + state: { + currentMigrationVersion: 2, + previousMigrationVersion: 1, + }, + currentMigrationVersion: 3, + }); + assert.deepStrictEqual(appMetadataController.store.getState(), { + ...EXPECTED_DEFAULT_STATE, + currentMigrationVersion: 3, + previousMigrationVersion: 2, + }); + }); + }); +}); diff --git a/app/scripts/controllers/app-metadata.ts b/app/scripts/controllers/app-metadata.ts new file mode 100644 index 000000000..0d745730d --- /dev/null +++ b/app/scripts/controllers/app-metadata.ts @@ -0,0 +1,99 @@ +import EventEmitter from 'events'; +import { ObservableStore } from '@metamask/obs-store'; + +/** + * The state of the AppMetadataController + */ +export type AppMetadataControllerState = { + currentAppVersion: string; + previousAppVersion: string; + previousMigrationVersion: number; + currentMigrationVersion: number; +}; + +/** + * The options that NetworkController takes. + */ +export type AppMetadataControllerOptions = { + currentMigrationVersion?: number; + currentAppVersion?: string; + state?: Partial; +}; + +const defaultState: AppMetadataControllerState = { + currentAppVersion: '', + previousAppVersion: '', + previousMigrationVersion: 0, + currentMigrationVersion: 0, +}; + +/** + * The AppMetadata controller stores metadata about the current extension instance, + * including the currently and previously installed versions, and the most recently + * run migration. + * + */ +export default class AppMetadataController extends EventEmitter { + /** + * Observable store containing controller data. + */ + store: ObservableStore; + + /** + * Constructs a AppMetadata controller. + * + * @param options - the controller options + * @param options.state - Initial controller state. + * @param options.currentMigrationVersion + * @param options.currentAppVersion + */ + constructor({ + currentAppVersion = '', + currentMigrationVersion = 0, + state = {}, + }: AppMetadataControllerOptions) { + super(); + + this.store = new ObservableStore({ + ...defaultState, + ...state, + }); + + this.#maybeUpdateAppVersion(currentAppVersion); + + this.#maybeUpdateMigrationVersion(currentMigrationVersion); + } + + /** + * Updates the currentAppVersion in state, and sets the previousAppVersion to the old currentAppVersion. + * + * @param maybeNewAppVersion + */ + #maybeUpdateAppVersion(maybeNewAppVersion: string): void { + const oldCurrentAppVersion = this.store.getState().currentAppVersion; + + if (maybeNewAppVersion !== oldCurrentAppVersion) { + this.store.updateState({ + currentAppVersion: maybeNewAppVersion, + previousAppVersion: oldCurrentAppVersion, + }); + } + } + + /** + * Updates the migrationVersion in state. + * + * @param maybeNewMigrationVersion + */ + #maybeUpdateMigrationVersion(maybeNewMigrationVersion: number): void { + const oldCurrentMigrationVersion = + this.store.getState().currentMigrationVersion; + + if (maybeNewMigrationVersion !== oldCurrentMigrationVersion) { + this.store.updateState({ + previousMigrationVersion: oldCurrentMigrationVersion, + currentMigrationVersion: maybeNewMigrationVersion, + }); + } + } +} diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index 8cdc65223..26fd5f01d 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -35,9 +35,11 @@ export const SENTRY_STATE = { connectedStatusPopoverHasBeenShown: true, conversionDate: true, conversionRate: true, + currentAppVersion: true, currentBlockGasLimit: true, currentCurrency: true, currentLocale: true, + currentMigrationVersion: true, customNonceValue: true, defaultHomeActiveTabName: true, desktopEnabled: true, @@ -56,6 +58,8 @@ export const SENTRY_STATE = { nextNonce: true, participateInMetaMetrics: true, preferences: true, + previousAppVersion: true, + previousMigrationVersion: true, providerConfig: { nickname: true, ticker: true, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e0f7f476c..7c0afce37 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -198,6 +198,7 @@ import createMetaRPCHandler from './lib/createMetaRPCHandler'; import { previousValueComparator } from './lib/util'; import createMetamaskMiddleware from './lib/createMetamaskMiddleware'; import EncryptionPublicKeyController from './controllers/encryption-public-key'; +import AppMetadataController from './controllers/app-metadata'; import { CaveatMutatorFactories, @@ -267,6 +268,8 @@ export default class MetamaskController extends EventEmitter { // instance of a class that wraps the extension's storage local API. this.localStoreApiWrapper = opts.localStore; + this.currentMigrationVersion = opts.currentMigrationVersion; + // observable state store this.store = new ComposableObservableStore({ state: initState, @@ -287,6 +290,12 @@ export default class MetamaskController extends EventEmitter { } }); + this.appMetadataController = new AppMetadataController({ + state: initState.AppMetadataController, + currentMigrationVersion: this.currentMigrationVersion, + currentAppVersion: version, + }); + // next, we will initialize the controllers // controller initialization order matters @@ -1629,6 +1638,7 @@ export default class MetamaskController extends EventEmitter { this.store.updateStructure({ AppStateController: this.appStateController.store, + AppMetadataController: this.appMetadataController.store, TransactionController: this.txController.store, KeyringController: this.keyringController.store, PreferencesController: this.preferencesController.store, @@ -1676,6 +1686,7 @@ export default class MetamaskController extends EventEmitter { this.memStore = new ComposableObservableStore({ config: { AppStateController: this.appStateController.store, + AppMetadataController: this.appMetadataController.store, NetworkController: this.networkController, CachedBalancesController: this.cachedBalancesController.store, KeyringController: this.keyringController.memStore,