Skip to content

Commit 4e45b22

Browse files
authored
cherry-pick: Fix migration 120.3 error caused by invalid state (#26485) (#26517)
This cherry-picks #26485 into v12.0.6. Original description: ## **Description** For a small number of users, migration 120.3 is failing due to invalid `TransactionController.transactions` state. We aren't sure yet how this is happening, but we can resolve the problem by updating the migration to delete this invalid state if we detect it. No other state relies on this state, and if it's invalid it won't work properly anyway. The migration has been renamed from 120.3 to 120.6 so that it will be re-run for any users who encountered this migration on v12.0.1 already. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26517?quickstart=1) ## **Related issues** Fixes #26423 ## **Manual testing steps** * Create a dev build from v12.0.0 * Install the dev build from the `dist/chrome` directory and proceed through onboarding * Run this command in the background console: ``` chrome.storage.local.get( null, (state) => { state.data.TransactionController.transactions = {}; chrome.storage.local.set(state, () => chrome.runtime.reload()); } ); ``` * Disable the extension * Switch to v12.0.1 and create a dev build * Enable and reload the extension * You should see in the console that migration 120.3 has failed * Disable the extension * Switch to this branch and create a dev build * Enable and reload the extension * You should see in the console that migration 120.6 has run without error * You can run `chrome.storage.local.get(console.log)` to check that the transactions state has been removed. ## **Screenshots/Recordings** N/A ## **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/develop/.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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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.
1 parent 692f6cd commit 4e45b22

File tree

3 files changed

+16
-13
lines changed

3 files changed

+16
-13
lines changed

app/scripts/migrations/120.3.test.ts renamed to app/scripts/migrations/120.6.test.ts

+8-9
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import { cloneDeep } from 'lodash';
2-
import { migrate, version } from './120.3';
2+
import { migrate, version } from './120.6';
33

44
const sentryCaptureExceptionMock = jest.fn();
55

66
global.sentry = {
77
captureException: sentryCaptureExceptionMock,
88
};
99

10-
const oldVersion = 120.2;
10+
const oldVersion = 120.5;
1111

12-
describe('migration #120.3', () => {
12+
describe('migration #120.6', () => {
1313
afterEach(() => {
1414
jest.resetAllMocks();
1515
});
@@ -72,7 +72,7 @@ describe('migration #120.3', () => {
7272
expect(newStorage.data).toStrictEqual(oldStorageDataClone);
7373
});
7474

75-
it('reports error and returns state unchanged if transactions property is invalid', async () => {
75+
it('removes transactions property if it is invalid', async () => {
7676
const oldStorage = {
7777
meta: { version: oldVersion },
7878
data: {
@@ -82,14 +82,13 @@ describe('migration #120.3', () => {
8282
},
8383
},
8484
};
85-
const oldStorageDataClone = cloneDeep(oldStorage.data);
8685

8786
const newStorage = await migrate(oldStorage);
8887

89-
expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(
90-
`Migration ${version}: Invalid TransactionController transactions state of type 'string'`,
91-
);
92-
expect(newStorage.data).toStrictEqual(oldStorageDataClone);
88+
expect(newStorage.data).toStrictEqual({
89+
PreferencesController: {},
90+
TransactionController: {},
91+
});
9392
});
9493

9594
it('reports error and returns state unchanged if there is an invalid transaction', async () => {

app/scripts/migrations/120.3.ts renamed to app/scripts/migrations/120.6.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ type VersionedData = {
77
data: Record<string, unknown>;
88
};
99

10-
export const version = 120.3;
10+
export const version = 120.6;
1111

1212
const MAX_TRANSACTION_HISTORY_LENGTH = 100;
1313

@@ -52,9 +52,12 @@ function transformState(state: Record<string, unknown>): void {
5252
);
5353
return;
5454
} else if (!Array.isArray(transactionControllerState.transactions)) {
55-
global.sentry?.captureException(
56-
`Migration ${version}: Invalid TransactionController transactions state of type '${typeof transactionControllerState.transactions}'`,
55+
log.error(
56+
new Error(
57+
`Migration ${version}: Invalid TransactionController transactions state of type '${typeof transactionControllerState.transactions}'`,
58+
),
5759
);
60+
delete transactionControllerState.transactions;
5861
return;
5962
}
6063

app/scripts/migrations/index.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,10 @@ const migrations = [
133133
require('./120'),
134134
require('./120.1'),
135135
require('./120.2'),
136-
require('./120.3'),
136+
// require('./120.3'), Renamed to 120.6, do not re-use this number
137137
require('./120.4'),
138138
require('./120.5'),
139+
require('./120.6'),
139140
];
140141

141142
export default migrations;

0 commit comments

Comments
 (0)