Skip to content

Commit

Permalink
fix: account syncing was not working after upgrading from a previous …
Browse files Browse the repository at this point in the history
…version (#29701)

## **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.
(MetaMask/core#5147)

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29701?quickstart=1)

## **Related issues**

Fixes: #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**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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.
  • Loading branch information
mathieuartu authored Jan 15, 2025
1 parent 31915a6 commit d8ce2c7
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 6 deletions.
66 changes: 66 additions & 0 deletions app/scripts/migrations/137.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
75 changes: 75 additions & 0 deletions app/scripts/migrations/137.ts
Original file line number Diff line number Diff line change
@@ -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<VersionedData> {
const versionedData = cloneDeep(originalVersionedData);
versionedData.meta.version = version;
transformState(versionedData.data);
return versionedData;
}
1 change: 1 addition & 0 deletions app/scripts/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ const migrations = [
require('./134'),
require('./135'),
require('./136'),
require('./137'),
];

export default migrations;
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 5 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit d8ce2c7

Please sign in to comment.