From d8ce2c7805cbcc37a22d81c49f9f90f8810dfaa5 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Wed, 15 Jan 2025 07:12:50 +0100 Subject: [PATCH] fix: account syncing was not working after upgrading from a previous version (#29701) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Account syncing was depending on `_addAccountsWithBalance` to be finished before being triggered. But `_addAccountsWithBalance` was only triggered after onboarding. So people upgrading from a previous version of the extension were not able to use account syncing since `_addAccountsWithBalance` was triggered during their original onboarding, at a time where account syncing was not available. Additionally, the state value `isAccountSyncingReadyToBeDispatched` from `UserStorageController` was not persisted. So people were not able to use account syncing after upgrading their extension version, even coming from a version that had account syncing enabled and working. This PR fixes that, by adding a migration that sets `isAccountSyncingReadyToBeDispatched` to true if `completedOnboarding` is true. Also, bumping `@metamask/profile-sync-controller` to version `v4.1.0` will ensure that `isAccountSyncingReadyToBeDispatched` is persisted. (https://github.com/MetaMask/core/pull/5147) [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29701?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/29679 ## **Manual testing steps** 1. Install MetaMask v12.9.3 2. Complete onboarding 3. Upgrade to this PR version 4. Verify that account syncing is working ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [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-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [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/scripts/migrations/137.test.ts | 66 ++++++++++++++++++++++++++ app/scripts/migrations/137.ts | 75 ++++++++++++++++++++++++++++++ app/scripts/migrations/index.js | 1 + package.json | 2 +- yarn.lock | 10 ++-- 5 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 app/scripts/migrations/137.test.ts create mode 100644 app/scripts/migrations/137.ts diff --git a/app/scripts/migrations/137.test.ts b/app/scripts/migrations/137.test.ts new file mode 100644 index 000000000000..20e7fec205cb --- /dev/null +++ b/app/scripts/migrations/137.test.ts @@ -0,0 +1,66 @@ +import { migrate, version, VersionedData } from './137'; + +const oldVersion = 136; + +describe(`migration #${version}`, () => { + it('updates the version metadata', async () => { + const oldStorage: VersionedData = { + meta: { version: oldVersion }, + data: {}, + }; + const newStorage = await migrate(oldStorage); + expect(newStorage.meta).toStrictEqual({ version }); + }); + + describe(`migration #${version}`, () => { + it('sets isAccountSyncingReadyToBeDispatched to true if completedOnboarding is true', async () => { + const oldStorage: VersionedData = { + meta: { version: oldVersion }, + data: { + OnboardingController: { + completedOnboarding: true, + }, + UserStorageController: { + isAccountSyncingReadyToBeDispatched: false, + }, + }, + }; + const expectedData = { + OnboardingController: { + completedOnboarding: true, + }, + UserStorageController: { + isAccountSyncingReadyToBeDispatched: true, + }, + }; + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(expectedData); + }); + + it('sets isAccountSyncingReadyToBeDispatched to false if completedOnboarding is false', async () => { + const oldStorage: VersionedData = { + meta: { version: oldVersion }, + data: { + OnboardingController: { + completedOnboarding: false, + }, + UserStorageController: { + isAccountSyncingReadyToBeDispatched: true, + }, + }, + }; + const expectedData = { + OnboardingController: { + completedOnboarding: false, + }, + UserStorageController: { + isAccountSyncingReadyToBeDispatched: false, + }, + }; + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(expectedData); + }); + }); +}); diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts new file mode 100644 index 000000000000..2e45bd6db502 --- /dev/null +++ b/app/scripts/migrations/137.ts @@ -0,0 +1,75 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; + +export type VersionedData = { + meta: { + version: number; + }; + data: { + OnboardingController?: { + completedOnboarding?: boolean; + }; + UserStorageController?: { + isAccountSyncingReadyToBeDispatched?: boolean; + }; + }; +}; + +export const version = 137; + +function transformState(state: VersionedData['data']) { + if ( + !hasProperty(state, 'OnboardingController') || + !isObject(state.OnboardingController) + ) { + global.sentry?.captureException?.( + new Error( + `Invalid OnboardingController state: ${typeof state.OnboardingController}`, + ), + ); + return state; + } + + if ( + !hasProperty(state, 'UserStorageController') || + !isObject(state.UserStorageController) + ) { + global.sentry?.captureException?.( + new Error( + `Invalid UserStorageController state: ${typeof state.UserStorageController}`, + ), + ); + return state; + } + + const { OnboardingController } = state; + + const currentCompletedOnboardingStatus = + OnboardingController.completedOnboarding; + + if (currentCompletedOnboardingStatus) { + state.UserStorageController.isAccountSyncingReadyToBeDispatched = true; + } else { + state.UserStorageController.isAccountSyncingReadyToBeDispatched = false; + } + + return state; +} + +/** + * This migration sets isAccountSyncingReadyToBeDispatched to true if completedOnboarding is true + * + * @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist. + * @param originalVersionedData.meta - State metadata. + * @param originalVersionedData.meta.version - The current state version. + * @param originalVersionedData.data - The persisted MetaMask state, keyed by controller. + * @returns Updated versioned MetaMask extension state. + */ +export async function migrate( + originalVersionedData: VersionedData, +): Promise { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + transformState(versionedData.data); + return versionedData; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 797b06d85cb5..67fb73bdaae6 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -160,6 +160,7 @@ const migrations = [ require('./134'), require('./135'), require('./136'), + require('./137'), ]; export default migrations; diff --git a/package.json b/package.json index 367957e39cc4..632665e1b805 100644 --- a/package.json +++ b/package.json @@ -337,7 +337,7 @@ "@metamask/post-message-stream": "^8.0.0", "@metamask/ppom-validator": "0.36.0", "@metamask/preinstalled-example-snap": "^0.3.0", - "@metamask/profile-sync-controller": "^4.0.1", + "@metamask/profile-sync-controller": "^4.1.0", "@metamask/providers": "^18.2.0", "@metamask/queued-request-controller": "^7.0.1", "@metamask/rate-limit-controller": "^6.0.0", diff --git a/yarn.lock b/yarn.lock index a9f73bebe17d..8d47d88e4930 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6066,9 +6066,9 @@ __metadata: languageName: node linkType: hard -"@metamask/profile-sync-controller@npm:^4.0.1": - version: 4.0.1 - resolution: "@metamask/profile-sync-controller@npm:4.0.1" +"@metamask/profile-sync-controller@npm:^4.1.0": + version: 4.1.0 + resolution: "@metamask/profile-sync-controller@npm:4.1.0" dependencies: "@metamask/base-controller": "npm:^7.1.1" "@metamask/keyring-api": "npm:^13.0.0" @@ -6088,7 +6088,7 @@ __metadata: "@metamask/providers": ^18.1.0 "@metamask/snaps-controllers": ^9.10.0 webextension-polyfill: ^0.10.0 || ^0.11.0 || ^0.12.0 - checksum: 10/613cc0968a701b4f509557c059d7e4aad63ee1c9b8405f2428039520c7afdb476c317078388d3186b325bf47fd6740aac72c21eeac63a08a9af65e08b8222235 + checksum: 10/bdddc4386c4497a55dff7a4ce0e484d61ccfb19081086ac992200cb5955bb53113d43683492480b04a54c016eebadebea37f460b13d4ae8c9f4d8573609e9893 languageName: node linkType: hard @@ -26671,7 +26671,7 @@ __metadata: "@metamask/ppom-validator": "npm:0.36.0" "@metamask/preferences-controller": "npm:^15.0.1" "@metamask/preinstalled-example-snap": "npm:^0.3.0" - "@metamask/profile-sync-controller": "npm:^4.0.1" + "@metamask/profile-sync-controller": "npm:^4.1.0" "@metamask/providers": "npm:^18.2.0" "@metamask/queued-request-controller": "npm:^7.0.1" "@metamask/rate-limit-controller": "npm:^6.0.0"