From f01e8379fa150981845691e43ab436de61c348d8 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 18 Jul 2023 16:50:03 -0230 Subject: [PATCH 01/31] v10.34.1 --- CHANGELOG.md | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00d3852f0..3ffcc03cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.34.1] + ## [10.34.0] ### Added - Add a security quiz to the SRP reveal ([#19283](https://github.com/MetaMask/metamask-extension/pull/19283)) @@ -3853,7 +3855,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Uncategorized - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.0...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.1...HEAD +[10.34.1]: https://github.com/MetaMask/metamask-extension/compare/v10.34.0...v10.34.1 [10.34.0]: https://github.com/MetaMask/metamask-extension/compare/v10.33.1...v10.34.0 [10.33.1]: https://github.com/MetaMask/metamask-extension/compare/v10.33.0...v10.33.1 [10.33.0]: https://github.com/MetaMask/metamask-extension/compare/v10.32.0...v10.33.0 diff --git a/package.json b/package.json index a830bca0c..6fe998e48 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "10.34.0", + "version": "10.34.1", "private": true, "repository": { "type": "git", From fbae250b0a3316387bac845c21642280850bd6d5 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Tue, 18 Jul 2023 20:05:06 -0230 Subject: [PATCH 02/31] Fix invalid state persistence error (#20080) * Fix invalid state persistence error We have been seeing Sentry errors showing that state persistence has been failing for some users that have invalid `NetworkController` state. This has been fixed by updating to `@metamask/base-controller@v3.2.0`, which is more tolerant of unexpected state properties. * Update LavaMoat policies --------- Co-authored-by: MetaMask Bot --- lavamoat/browserify/beta/policy.json | 4 ++++ lavamoat/browserify/desktop/policy.json | 8 ++++++++ lavamoat/browserify/flask/policy.json | 8 ++++++++ lavamoat/browserify/main/policy.json | 4 ++++ lavamoat/browserify/mmi/policy.json | 4 ++++ package.json | 2 +- yarn.lock | 21 +++++++++++---------- 7 files changed, 40 insertions(+), 11 deletions(-) diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 8231a9ec8..9b750dc06 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -1527,6 +1527,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1769,6 +1770,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1911,6 +1913,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1946,6 +1949,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, diff --git a/lavamoat/browserify/desktop/policy.json b/lavamoat/browserify/desktop/policy.json index 7428fea81..75d7032be 100644 --- a/lavamoat/browserify/desktop/policy.json +++ b/lavamoat/browserify/desktop/policy.json @@ -1274,6 +1274,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1291,6 +1292,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1653,6 +1655,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1992,6 +1995,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2198,6 +2202,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2367,6 +2372,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2404,6 +2410,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2437,6 +2444,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 7428fea81..75d7032be 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -1274,6 +1274,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1291,6 +1292,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1653,6 +1655,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1992,6 +1995,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2198,6 +2202,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2367,6 +2372,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2404,6 +2410,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2437,6 +2444,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 8231a9ec8..9b750dc06 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -1527,6 +1527,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1769,6 +1770,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1911,6 +1913,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1946,6 +1949,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index 9dabc001b..938c0d76e 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -1748,6 +1748,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1990,6 +1991,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2132,6 +2134,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2167,6 +2170,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, diff --git a/package.json b/package.json index 6fe998e48..e80cdb33a 100644 --- a/package.json +++ b/package.json @@ -227,7 +227,7 @@ "@metamask/announcement-controller": "^4.0.0", "@metamask/approval-controller": "^3.3.0", "@metamask/assets-controllers": "^9.2.0", - "@metamask/base-controller": "^3.1.0", + "@metamask/base-controller": "^3.2.0", "@metamask/browser-passworder": "^4.1.0", "@metamask/contract-metadata": "^2.3.1", "@metamask/controller-utils": "^4.2.0", diff --git a/yarn.lock b/yarn.lock index a47b620aa..da39e1257 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3940,13 +3940,13 @@ __metadata: languageName: node linkType: hard -"@metamask/base-controller@npm:^3.0.0, @metamask/base-controller@npm:^3.1.0": - version: 3.1.0 - resolution: "@metamask/base-controller@npm:3.1.0" +"@metamask/base-controller@npm:^3.0.0, @metamask/base-controller@npm:^3.1.0, @metamask/base-controller@npm:^3.2.0": + version: 3.2.0 + resolution: "@metamask/base-controller@npm:3.2.0" dependencies: - "@metamask/utils": ^5.0.2 + "@metamask/utils": ^6.2.0 immer: ^9.0.6 - checksum: fc1597a099e6d28bd089df936ca349d6c38c2e1b0f0737385cba30c34a5239241519eb172d77c70f8db2604f4dc5724f6893affe42bdd104cef98f9cfd6f1db8 + checksum: 3be6f2594309c013e07f83c4bb8271e1e99f02b6ff829c18b5e7218fbab4e6a9e03bcb49056704ce47f84ae2f38b1bc1c10284ec538aad56ed7b554ef2d3e189 languageName: node linkType: hard @@ -5005,16 +5005,17 @@ __metadata: languageName: node linkType: hard -"@metamask/utils@npm:^6.0.0, @metamask/utils@npm:^6.0.1, @metamask/utils@npm:^6.1.0": - version: 6.1.0 - resolution: "@metamask/utils@npm:6.1.0" +"@metamask/utils@npm:^6.0.0, @metamask/utils@npm:^6.0.1, @metamask/utils@npm:^6.1.0, @metamask/utils@npm:^6.2.0": + version: 6.2.0 + resolution: "@metamask/utils@npm:6.2.0" dependencies: "@ethereumjs/tx": ^4.1.2 + "@noble/hashes": ^1.3.1 "@types/debug": ^4.1.7 debug: ^4.3.4 semver: ^7.3.8 superstruct: ^1.0.3 - checksum: d4eac3ce3c08674b8e9ef838d1661a5025690c6f266c26ebdb8e8d0da11fce786e54c326b5d9c6d33b262f37e7057e31d6545a3715613bd0a5bfa10e7755643a + checksum: 0bc675358ecc09b3bc04da613d73666295d7afa51ff6b8554801585966900b24b8545bd93b8b2e9a17db867ebe421fe884baf3558ec4ca3199fa65504f677c1b languageName: node linkType: hard @@ -24474,7 +24475,7 @@ __metadata: "@metamask/approval-controller": ^3.3.0 "@metamask/assets-controllers": ^9.2.0 "@metamask/auto-changelog": ^2.1.0 - "@metamask/base-controller": ^3.1.0 + "@metamask/base-controller": ^3.2.0 "@metamask/browser-passworder": ^4.1.0 "@metamask/contract-metadata": ^2.3.1 "@metamask/controller-utils": ^4.2.0 From 374656a3d15e43f25f73874bacb85ce679532f8d Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Fri, 21 Jul 2023 16:33:25 -0230 Subject: [PATCH 03/31] Fix sentry sourcemaps (#20122) * Update sentry/cli to 2.19.4 * Ensure sentry files are loaded and referenced with a valid url * Temp to eliminate errors in sentry (should be split into other PRs) --- app/scripts/lib/setupSentry.js | 2 +- development/build/index.js | 2 ++ development/sentry-upload-artifacts.sh | 2 +- lavamoat/build-system/policy.json | 16 ++++++++-------- package.json | 2 +- yarn.lock | 16 ++++++++-------- 6 files changed, 21 insertions(+), 19 deletions(-) diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index a383f8221..bac3c79ce 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -352,6 +352,6 @@ function toMetamaskUrl(origUrl) { if (!filePath) { return origUrl; } - const metamaskUrl = `metamask${filePath}`; + const metamaskUrl = `/metamask${filePath}`; return metamaskUrl; } diff --git a/development/build/index.js b/development/build/index.js index 84ee91040..92a6ea678 100755 --- a/development/build/index.js +++ b/development/build/index.js @@ -99,6 +99,8 @@ async function defineAndRunBuildTasks() { 'navigator', 'harden', 'console', + 'WeakSet', + 'Event', 'Image', // Used by browser to generate notifications // globals chromedriver needs to function /cdc_[a-zA-Z0-9]+_[a-zA-Z]+/iu, diff --git a/development/sentry-upload-artifacts.sh b/development/sentry-upload-artifacts.sh index 9d2fd32b4..e70989123 100755 --- a/development/sentry-upload-artifacts.sh +++ b/development/sentry-upload-artifacts.sh @@ -31,7 +31,7 @@ function upload_sourcemaps { local release="${1}"; shift local dist_directory="${1}"; shift - sentry-cli releases files "${release}" upload-sourcemaps "${dist_directory}"/chrome/*.js "${dist_directory}"/sourcemaps/ --rewrite --url-prefix 'metamask' + sentry-cli releases files "${release}" upload-sourcemaps "${dist_directory}"/chrome/*.js "${dist_directory}"/sourcemaps/ --rewrite --url-prefix '/metamask' } function main { diff --git a/lavamoat/build-system/policy.json b/lavamoat/build-system/policy.json index a615563a8..98fd8942c 100644 --- a/lavamoat/build-system/policy.json +++ b/lavamoat/build-system/policy.json @@ -1124,13 +1124,6 @@ "@metamask/jazzicon>color>color-convert>color-name": true } }, - "@sentry/cli>mkdirp": { - "builtin": { - "fs": true, - "path.dirname": true, - "path.resolve": true - } - }, "@storybook/addon-knobs>qs": { "packages": { "string.prototype.matchall>side-channel": true @@ -8153,7 +8146,14 @@ "path.dirname": true }, "packages": { - "@sentry/cli>mkdirp": true + "stylelint>file-entry-cache>flat-cache>write>mkdirp": true + } + }, + "stylelint>file-entry-cache>flat-cache>write>mkdirp": { + "builtin": { + "fs": true, + "path.dirname": true, + "path.resolve": true } }, "stylelint>global-modules": { diff --git a/package.json b/package.json index e80cdb33a..ac81c669a 100644 --- a/package.json +++ b/package.json @@ -382,7 +382,7 @@ "@metamask/forwarder": "^1.1.0", "@metamask/phishing-warning": "^2.1.0", "@metamask/test-dapp": "^7.0.1", - "@sentry/cli": "^1.58.0", + "@sentry/cli": "^2.19.4", "@storybook/addon-a11y": "^7.0.11", "@storybook/addon-actions": "^7.0.11", "@storybook/addon-essentials": "^7.0.11", diff --git a/yarn.lock b/yarn.lock index da39e1257..4b43e388f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5597,18 +5597,18 @@ __metadata: languageName: node linkType: hard -"@sentry/cli@npm:^1.58.0": - version: 1.58.0 - resolution: "@sentry/cli@npm:1.58.0" +"@sentry/cli@npm:^2.19.4": + version: 2.19.4 + resolution: "@sentry/cli@npm:2.19.4" dependencies: https-proxy-agent: ^5.0.0 - mkdirp: ^0.5.5 - node-fetch: ^2.6.0 + node-fetch: ^2.6.7 progress: ^2.0.3 proxy-from-env: ^1.1.0 + which: ^2.0.2 bin: sentry-cli: bin/sentry-cli - checksum: fc781bbffcf5cd970bb023168421ad89bca4184c2ddfbfddde92f4f5333c8b9075e9e16a8a4b192ecc3b197ac97062715e7b350c306ccc538fc01b955b06c3bb + checksum: 1f2442857a5eec2bc6f872a633d88fc2f11ed7f434db36627a034d904390f4cbbb4dccc33c571a8815e423cd36b863c72621298d49a1541b28370c7f7308f0dc languageName: node linkType: hard @@ -24530,7 +24530,7 @@ __metadata: "@reduxjs/toolkit": ^1.6.2 "@segment/loosely-validate-event": ^2.0.0 "@sentry/browser": ^7.53.0 - "@sentry/cli": ^1.58.0 + "@sentry/cli": ^2.19.4 "@sentry/integrations": ^7.53.0 "@sentry/types": ^7.53.0 "@sentry/utils": ^7.53.0 @@ -26117,7 +26117,7 @@ __metadata: languageName: node linkType: hard -"node-fetch@npm:^2, node-fetch@npm:^2.6.0, node-fetch@npm:^2.6.1, node-fetch@npm:^2.6.11, node-fetch@npm:^2.6.7, node-fetch@npm:~2.6.1": +"node-fetch@npm:^2, node-fetch@npm:^2.6.1, node-fetch@npm:^2.6.11, node-fetch@npm:^2.6.7, node-fetch@npm:~2.6.1": version: 2.6.11 resolution: "node-fetch@npm:2.6.11" dependencies: From b1fb8204f3b5d9fdd88a5cef98d42bc926a536bb Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 25 Jul 2023 21:17:57 -0230 Subject: [PATCH 04/31] Migration 89: ensure providerConfig in state has an id property (#20181) * Migration 89: ensure providerConfig in state has an id property * Exit transformState function early if providerConfig already has an id * Update migrations/index.js * Code cleanup --- app/scripts/migrations/089.test.ts | 224 +++++++++++++++++++++++++++++ app/scripts/migrations/089.ts | 71 +++++++++ app/scripts/migrations/index.js | 2 + 3 files changed, 297 insertions(+) create mode 100644 app/scripts/migrations/089.test.ts create mode 100644 app/scripts/migrations/089.ts diff --git a/app/scripts/migrations/089.test.ts b/app/scripts/migrations/089.test.ts new file mode 100644 index 000000000..00868ff74 --- /dev/null +++ b/app/scripts/migrations/089.test.ts @@ -0,0 +1,224 @@ +import { migrate, version } from './089'; + +jest.mock('uuid', () => { + const actual = jest.requireActual('uuid'); + + return { + ...actual, + v4: jest.fn(), + }; +}); + +describe('migration #89', () => { + it('should update the version metadata', async () => { + const oldStorage = { + meta: { + version: 88, + }, + data: {}, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.meta).toStrictEqual({ + version, + }); + }); + + it('should return state unaltered if there is no network controller state', async () => { + const oldData = { + other: 'data', + }; + const oldStorage = { + meta: { + version: 88, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should return state unaltered if there is no network controller providerConfig state', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + }, + }, + }, + }; + const oldStorage = { + meta: { + version: 88, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should return state unaltered if the providerConfig already has an id', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + }, + }, + providerConfig: { + id: 'test', + }, + }, + }; + const oldStorage = { + meta: { + version: 88, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should return state unaltered if there is no network config with the same rpcUrl and the providerConfig', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + rpcUrl: 'http://foo.bar', + }, + }, + providerConfig: { + rpcUrl: 'http://baz.buzz', + }, + }, + }; + const oldStorage = { + meta: { + version: 88, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should update the provider config to have the id of a network config with the same rpcUrl', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + rpcUrl: 'http://foo.bar', + id: 'test', + }, + }, + providerConfig: { + rpcUrl: 'http://foo.bar', + }, + }, + }; + const oldStorage = { + meta: { + version: 88, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual({ + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + rpcUrl: 'http://foo.bar', + id: 'test', + }, + }, + providerConfig: { + rpcUrl: 'http://foo.bar', + id: 'test', + }, + }, + }); + }); + + it('should update the provider config to have the id of a network config with the same rpcUrl, even if there are other networks with the same chainId', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + rpcUrl: 'http://fizz.buzz', + id: 'FAILEDtest', + chainId: 1, + }, + id2: { + foo: 'bar', + rpcUrl: 'http://foo.bar', + id: 'PASSEDtest', + }, + id3: { + foo: 'bar', + rpcUrl: 'http://baz.buzz', + id: 'FAILEDtest', + chainId: 1, + }, + }, + providerConfig: { + rpcUrl: 'http://foo.bar', + chainId: 1, + }, + }, + }; + const oldStorage = { + meta: { + version: 88, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual({ + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + rpcUrl: 'http://fizz.buzz', + id: 'FAILEDtest', + chainId: 1, + }, + id2: { + foo: 'bar', + rpcUrl: 'http://foo.bar', + id: 'PASSEDtest', + }, + id3: { + foo: 'bar', + rpcUrl: 'http://baz.buzz', + id: 'FAILEDtest', + chainId: 1, + }, + }, + providerConfig: { + rpcUrl: 'http://foo.bar', + id: 'PASSEDtest', + chainId: 1, + }, + }, + }); + }); +}); diff --git a/app/scripts/migrations/089.ts b/app/scripts/migrations/089.ts new file mode 100644 index 000000000..cc1bfa4dc --- /dev/null +++ b/app/scripts/migrations/089.ts @@ -0,0 +1,71 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; + +export const version = 89; + +/** + * Add an `id` to the `providerConfig` object. + * + * @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: { + meta: { version: number }; + data: Record; +}) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + versionedData.data = transformState(versionedData.data); + return versionedData; +} + +function transformState(state: Record) { + if ( + hasProperty(state, 'NetworkController') && + isObject(state.NetworkController) && + hasProperty(state.NetworkController, 'providerConfig') && + isObject(state.NetworkController.providerConfig) + ) { + const { networkConfigurations, providerConfig } = state.NetworkController; + + if (!isObject(networkConfigurations)) { + return state; + } + + if (providerConfig.id) { + return state; + } + + let newProviderConfigId; + + for (const networkConfigurationId of Object.keys(networkConfigurations)) { + const networkConfiguration = + networkConfigurations[networkConfigurationId]; + if (!isObject(networkConfiguration)) { + return state; + } + if (networkConfiguration.rpcUrl === providerConfig.rpcUrl) { + newProviderConfigId = networkConfiguration.id; + break; + } + } + + if (!newProviderConfigId) { + return state; + } + + state.NetworkController.providerConfig = { + ...providerConfig, + id: newProviderConfigId, + }; + + return { + ...state, + NetworkController: state.NetworkController, + }; + } + return state; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 813f5e799..429ef7959 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -92,6 +92,7 @@ import * as m085 from './085'; import * as m086 from './086'; import * as m087 from './087'; import * as m088 from './088'; +import * as m089 from './089'; const migrations = [ m002, @@ -181,6 +182,7 @@ const migrations = [ m086, m087, m088, + m089, ]; export default migrations; From 40314a84c9a1670a893abf633e562a1264a8f6b5 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 25 Jul 2023 21:23:42 -0230 Subject: [PATCH 05/31] Update changelog for v10.34.1 --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ffcc03cc..212f65a2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ## [10.34.1] +### Fixed +- Fix bug that could cause a failure in the persistence of network related data ([#20080](https://github.com/MetaMask/metamask-extension/pull/20080)) +- Fix ([#20080](https://github.com/MetaMask/metamask-extension/pull/20080)) ## [10.34.0] ### Added From 9ee7459d0fbd868d4fafa65e91a8b4655fa930bd Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 25 Jul 2023 23:44:54 -0230 Subject: [PATCH 06/31] Update yarn.lock for v10.34.1, which updated sentry/cli -> node-fetch dep version, but doesn't include b7da0a9 --- yarn.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/yarn.lock b/yarn.lock index 4b43e388f..83b49a9a7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -26117,9 +26117,9 @@ __metadata: languageName: node linkType: hard -"node-fetch@npm:^2, node-fetch@npm:^2.6.1, node-fetch@npm:^2.6.11, node-fetch@npm:^2.6.7, node-fetch@npm:~2.6.1": - version: 2.6.11 - resolution: "node-fetch@npm:2.6.11" +"node-fetch@npm:^2, node-fetch@npm:^2.6.0, node-fetch@npm:^2.6.1, node-fetch@npm:^2.6.11, node-fetch@npm:^2.6.7, node-fetch@npm:~2.6.1": + version: 2.6.12 + resolution: "node-fetch@npm:2.6.12" dependencies: whatwg-url: ^5.0.0 peerDependencies: @@ -26127,7 +26127,7 @@ __metadata: peerDependenciesMeta: encoding: optional: true - checksum: 249d0666a9497553384d46b5ab296ba223521ac88fed4d8a17d6ee6c2efb0fc890f3e8091cafe7f9fba8151a5b8d925db2671543b3409a56c3cd522b468b47b3 + checksum: 3bc1655203d47ee8e313c0d96664b9673a3d4dd8002740318e9d27d14ef306693a4b2ef8d6525775056fd912a19e23f3ac0d7111ad8925877b7567b29a625592 languageName: node linkType: hard From d8c49d9a99b68d464b58070e92857ee7b85acc0b Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Wed, 26 Jul 2023 17:32:27 -0230 Subject: [PATCH 07/31] v10.34.2 --- CHANGELOG.md | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 212f65a2b..52aa68b23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.34.2] + ## [10.34.1] ### Fixed - Fix bug that could cause a failure in the persistence of network related data ([#20080](https://github.com/MetaMask/metamask-extension/pull/20080)) @@ -3858,7 +3860,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Uncategorized - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.1...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.2...HEAD +[10.34.1]: https://github.com/MetaMask/metamask-extension/compare/v10.34.1...v10.34.2 [10.34.1]: https://github.com/MetaMask/metamask-extension/compare/v10.34.0...v10.34.1 [10.34.0]: https://github.com/MetaMask/metamask-extension/compare/v10.33.1...v10.34.0 [10.33.1]: https://github.com/MetaMask/metamask-extension/compare/v10.33.0...v10.33.1 diff --git a/package.json b/package.json index ac81c669a..0200efb5c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "10.34.1", + "version": "10.34.2", "private": true, "repository": { "type": "git", From 78a0587c9702e9895f6e6dd7d1c6d2421a43787e Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 18 Jul 2023 17:01:07 -0500 Subject: [PATCH 08/31] Fix #20006 - Add Address Details and View on Explorer to Global Menu (#20013) * Fix #20006 - Add Address Details and View on Explorer to Global Menu * Fix tests --- test/e2e/tests/permissions.spec.js | 2 +- .../account-list-item-menu.js | 93 +++--------------- .../account-list-item-menu.stories.js | 4 - .../account-list-item-menu.test.js | 19 +--- .../account-list-item/account-list-item.js | 8 +- .../multichain/global-menu/global-menu.js | 26 +++-- .../global-menu/global-menu.test.js | 2 + ui/components/multichain/index.js | 1 + .../menu-items/account-details-menu-item.js | 54 +++++++++++ .../account-details-menu-item.test.js | 38 ++++++++ ui/components/multichain/menu-items/index.js | 2 + .../menu-items/view-explorer-menu-item.js | 97 +++++++++++++++++++ .../view-explorer-menu-item.test.js | 29 ++++++ 13 files changed, 261 insertions(+), 114 deletions(-) create mode 100644 ui/components/multichain/menu-items/account-details-menu-item.js create mode 100644 ui/components/multichain/menu-items/account-details-menu-item.test.js create mode 100644 ui/components/multichain/menu-items/index.js create mode 100644 ui/components/multichain/menu-items/view-explorer-menu-item.js create mode 100644 ui/components/multichain/menu-items/view-explorer-menu-item.test.js diff --git a/test/e2e/tests/permissions.spec.js b/test/e2e/tests/permissions.spec.js index 648e0552e..be4fa78e7 100644 --- a/test/e2e/tests/permissions.spec.js +++ b/test/e2e/tests/permissions.spec.js @@ -55,7 +55,7 @@ describe('Permissions', function () { await driver.clickElement( '[data-testid="account-options-menu-button"]', ); - await driver.clickElement('.menu-item'); + await driver.clickElement('.menu-item:nth-of-type(3)'); await driver.findElement({ text: 'Connected sites', diff --git a/ui/components/multichain/account-list-item-menu/account-list-item-menu.js b/ui/components/multichain/account-list-item-menu/account-list-item-menu.js index cb763097d..ac5a7ee51 100644 --- a/ui/components/multichain/account-list-item-menu/account-list-item-menu.js +++ b/ui/components/multichain/account-list-item-menu/account-list-item-menu.js @@ -1,16 +1,12 @@ import React, { useContext, useRef, useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { useHistory } from 'react-router-dom'; import PropTypes from 'prop-types'; -import { getAccountLink } from '@metamask/etherscan-link'; ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) import { mmiActionsFactory } from '../../../store/institutional/institution-background'; ///: END:ONLY_INCLUDE_IN import { MetaMetricsContext } from '../../../contexts/metametrics'; import { useI18nContext } from '../../../hooks/useI18nContext'; import { - getRpcPrefsForCurrentProvider, - getBlockExplorerLinkText, getCurrentChainId, getHardwareWalletType, getAccountTypeForKeyring, @@ -22,7 +18,6 @@ import { import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; ///: END:ONLY_INCLUDE_IN import { findKeyringForAddress } from '../../../ducks/metamask/metamask'; -import { NETWORKS_ROUTE } from '../../../helpers/constants/routes'; import { MenuItem } from '../../ui/menu'; import { Text, @@ -34,17 +29,17 @@ import { } from '../../component-library'; import { MetaMetricsEventCategory, - MetaMetricsEventLinkType, MetaMetricsEventName, } from '../../../../shared/constants/metametrics'; -import { getURLHostName } from '../../../helpers/utils/util'; -import { setAccountDetailsAddress, showModal } from '../../../store/actions'; +import { showModal } from '../../../store/actions'; import { TextVariant } from '../../../helpers/constants/design-system'; import { formatAccountType } from '../../../helpers/utils/metrics'; +import { AccountDetailsMenuItem, ViewExplorerMenuItem } from '..'; + +const METRICS_LOCATION = 'Account Options'; export const AccountListItemMenu = ({ anchorElement, - blockExplorerUrlSubTitle, onClose, closeMenu, isRemovable, @@ -54,11 +49,8 @@ export const AccountListItemMenu = ({ const t = useI18nContext(); const trackEvent = useContext(MetaMetricsContext); const dispatch = useDispatch(); - const history = useHistory(); const chainId = useSelector(getCurrentChainId); - const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider); - const addressLink = getAccountLink(identity.address, chainId, rpcPrefs); const deviceName = useSelector(getHardwareWalletType); @@ -67,28 +59,6 @@ export const AccountListItemMenu = ({ ); const accountType = formatAccountType(getAccountTypeForKeyring(keyring)); - const blockExplorerLinkText = useSelector(getBlockExplorerLinkText); - const openBlockExplorer = () => { - trackEvent({ - event: MetaMetricsEventName.ExternalLinkClicked, - category: MetaMetricsEventCategory.Navigation, - properties: { - link_type: MetaMetricsEventLinkType.AccountTracker, - location: 'Account Options', - url_domain: getURLHostName(addressLink), - }, - }); - - global.platform.openTab({ - url: addressLink, - }); - onClose(); - }; - - const routeToAddBlockExplorerUrl = () => { - history.push(`${NETWORKS_ROUTE}#blockExplorerUrl`); - }; - ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) const isCustodial = keyring?.type ? /Custody/u.test(keyring.type) : false; const accounts = useSelector(getMetaMaskAccountsOrdered); @@ -158,46 +128,17 @@ export const AccountListItemMenu = ({ >
- { - blockExplorerLinkText.firstPart === 'addBlockExplorer' - ? routeToAddBlockExplorerUrl() - : openBlockExplorer(); - - trackEvent({ - event: MetaMetricsEventName.BlockExplorerLinkClicked, - category: MetaMetricsEventCategory.Accounts, - properties: { - location: 'Account Options', - chain_id: chainId, - }, - }); - }} - subtitle={blockExplorerUrlSubTitle || null} - iconName={IconName.Export} - data-testid="account-list-menu-open-explorer" - > - {t('viewOnExplorer')} - - { - dispatch(setAccountDetailsAddress(identity.address)); - trackEvent({ - event: MetaMetricsEventName.NavAccountDetailsOpened, - category: MetaMetricsEventCategory.Navigation, - properties: { - location: 'Account Options', - }, - }); - onClose(); - closeMenu?.(); - }} - iconName={IconName.ScanBarcode} - data-testid="account-list-menu-details" - > - {t('accountDetails')} - + + {isRemovable ? ( { }; describe('AccountListItem', () => { - it('renders the URL for explorer', () => { - const blockExplorerDomain = 'etherscan.io'; - const { getByText, getByTestId } = render({ - blockExplorerUrlSubTitle: blockExplorerDomain, - }); - expect(getByText(blockExplorerDomain)).toBeInTheDocument(); - - Object.defineProperty(global, 'platform', { - value: { - openTab: jest.fn(), - }, - }); - const openExplorerTabSpy = jest.spyOn(global.platform, 'openTab'); - fireEvent.click(getByTestId('account-list-menu-open-explorer')); - expect(openExplorerTabSpy).toHaveBeenCalled(); - }); - it('renders remove icon with isRemovable', () => { const { getByTestId } = render({ isRemovable: true }); expect(getByTestId('account-list-menu-remove')).toBeInTheDocument(); diff --git a/ui/components/multichain/account-list-item/account-list-item.js b/ui/components/multichain/account-list-item/account-list-item.js index 5066578fd..3b70a4a3d 100644 --- a/ui/components/multichain/account-list-item/account-list-item.js +++ b/ui/components/multichain/account-list-item/account-list-item.js @@ -4,8 +4,7 @@ import classnames from 'classnames'; import { useSelector } from 'react-redux'; import { useI18nContext } from '../../../hooks/useI18nContext'; -import { getRpcPrefsForCurrentProvider } from '../../../selectors'; -import { getURLHostName, shortenAddress } from '../../../helpers/utils/util'; +import { shortenAddress } from '../../../helpers/utils/util'; import { AccountListItemMenu } from '..'; import { @@ -92,10 +91,6 @@ export const AccountListItem = ({ ); const label = getLabel(keyring, t); - const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider); - const { blockExplorerUrl } = rpcPrefs; - const blockExplorerUrlSubTitle = getURLHostName(blockExplorerUrl); - const trackEvent = useContext(MetaMetricsContext); return ( @@ -250,7 +245,6 @@ export const AccountListItem = ({ /> setAccountOptionsMenuOpen(false)} isOpen={accountOptionsMenuOpen} diff --git a/ui/components/multichain/global-menu/global-menu.js b/ui/components/multichain/global-menu/global-menu.js index 50ff45780..0e213c24e 100644 --- a/ui/components/multichain/global-menu/global-menu.js +++ b/ui/components/multichain/global-menu/global-menu.js @@ -41,6 +41,7 @@ import { ///: END:ONLY_INCLUDE_IN import { getMetaMetricsId, + getSelectedAddress, ///: BEGIN:ONLY_INCLUDE_IN(snaps) getUnreadNotificationsCount, ///: END:ONLY_INCLUDE_IN @@ -56,6 +57,9 @@ import { TextVariant, } from '../../../helpers/constants/design-system'; ///: END:ONLY_INCLUDE_IN +import { AccountDetailsMenuItem, ViewExplorerMenuItem } from '..'; + +const METRICS_LOCATION = 'Global Menu'; export const GlobalMenu = ({ closeMenu, anchorElement }) => { const t = useI18nContext(); @@ -63,6 +67,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { const trackEvent = useContext(MetaMetricsContext); const history = useHistory(); const metaMetricsId = useSelector(getMetaMetricsId); + const address = useSelector(getSelectedAddress); const hasUnapprovedTransactions = useSelector( (state) => Object.keys(state.metamask.unapprovedTxs).length > 0, @@ -85,6 +90,15 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { return ( + + { event: MetaMetricsEventName.NavConnectedSitesOpened, category: MetaMetricsEventCategory.Navigation, properties: { - location: 'Global Menu', + location: METRICS_LOCATION, }, }); closeMenu(); @@ -140,7 +154,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { event: MetaMetricsEventName.PortfolioLinkClicked, properties: { url: portfolioUrl, - location: 'Global Menu', + location: METRICS_LOCATION, }, }, { @@ -167,7 +181,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { event: MetaMetricsEventName.AppWindowExpanded, category: MetaMetricsEventCategory.Navigation, properties: { - location: 'Global Menu', + location: METRICS_LOCATION, }, }); closeMenu(); @@ -225,7 +239,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { event: MetaMetricsEventName.SupportLinkClicked, properties: { url: supportLink, - location: 'Global Menu', + location: METRICS_LOCATION, }, }, { @@ -249,7 +263,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { category: MetaMetricsEventCategory.Navigation, event: MetaMetricsEventName.NavSettingsOpened, properties: { - location: 'Global Menu', + location: METRICS_LOCATION, }, }); closeMenu(); @@ -267,7 +281,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { category: MetaMetricsEventCategory.Navigation, event: MetaMetricsEventName.AppLocked, properties: { - location: 'Global Menu', + location: METRICS_LOCATION, }, }); closeMenu(); diff --git a/ui/components/multichain/global-menu/global-menu.test.js b/ui/components/multichain/global-menu/global-menu.test.js index a2fbb8c5f..0f635c968 100644 --- a/ui/components/multichain/global-menu/global-menu.test.js +++ b/ui/components/multichain/global-menu/global-menu.test.js @@ -18,8 +18,10 @@ const render = (metamaskStateChanges = {}) => { }; const mockLockMetaMask = jest.fn(); +const mockSetAccountDetailsAddress = jest.fn(); jest.mock('../../../store/actions', () => ({ lockMetamask: () => mockLockMetaMask, + setAccountDetailsAddress: () => mockSetAccountDetailsAddress, })); describe('AccountListItem', () => { diff --git a/ui/components/multichain/index.js b/ui/components/multichain/index.js index fa535a66e..3400dd850 100644 --- a/ui/components/multichain/index.js +++ b/ui/components/multichain/index.js @@ -15,3 +15,4 @@ export { ProductTour } from './product-tour-popover'; export { AccountDetails } from './account-details'; export { CreateAccount } from './create-account'; export { ImportAccount } from './import-account'; +export { AccountDetailsMenuItem, ViewExplorerMenuItem } from './menu-items'; diff --git a/ui/components/multichain/menu-items/account-details-menu-item.js b/ui/components/multichain/menu-items/account-details-menu-item.js new file mode 100644 index 000000000..a5526e9cb --- /dev/null +++ b/ui/components/multichain/menu-items/account-details-menu-item.js @@ -0,0 +1,54 @@ +import React, { useContext } from 'react'; +import PropTypes from 'prop-types'; +import { useDispatch } from 'react-redux'; + +import { setAccountDetailsAddress } from '../../../store/actions'; + +import { MenuItem } from '../../ui/menu'; +import { useI18nContext } from '../../../hooks/useI18nContext'; +import { MetaMetricsContext } from '../../../contexts/metametrics'; +import { + MetaMetricsEventCategory, + MetaMetricsEventName, +} from '../../../../shared/constants/metametrics'; +import { IconName, Text } from '../../component-library'; + +export const AccountDetailsMenuItem = ({ + metricsLocation, + closeMenu, + address, + textProps, +}) => { + const t = useI18nContext(); + const dispatch = useDispatch(); + const trackEvent = useContext(MetaMetricsContext); + + const LABEL = t('accountDetails'); + + return ( + { + dispatch(setAccountDetailsAddress(address)); + trackEvent({ + event: MetaMetricsEventName.NavAccountDetailsOpened, + category: MetaMetricsEventCategory.Navigation, + properties: { + location: metricsLocation, + }, + }); + closeMenu?.(); + }} + iconName={IconName.ScanBarcode} + data-testid="account-list-menu-details" + > + {textProps ? {LABEL} : LABEL} + + ); +}; + +AccountDetailsMenuItem.propTypes = { + metricsLocation: PropTypes.string.isRequired, + closeMenu: PropTypes.func, + address: PropTypes.string.isRequired, + textProps: PropTypes.object, +}; diff --git a/ui/components/multichain/menu-items/account-details-menu-item.test.js b/ui/components/multichain/menu-items/account-details-menu-item.test.js new file mode 100644 index 000000000..f13c27d63 --- /dev/null +++ b/ui/components/multichain/menu-items/account-details-menu-item.test.js @@ -0,0 +1,38 @@ +import React from 'react'; +import { renderWithProvider, fireEvent } from '../../../../test/jest'; +import configureStore from '../../../store/store'; +import mockState from '../../../../test/data/mock-state.json'; +import * as actions from '../../../store/actions'; +import { AccountDetailsMenuItem } from '.'; + +const render = () => { + const store = configureStore(mockState); + return renderWithProvider( + , + store, + ); +}; + +jest.mock('../../../store/actions', () => ({ + ...jest.requireActual('../../../store/actions.ts'), + setAccountDetailsAddress: jest.fn().mockReturnValue({ type: 'TYPE' }), +})); + +describe('AccountDetailsMenuItem', () => { + it('opens the Account Details modal with the correct address', () => { + global.platform = { openTab: jest.fn() }; + + const { getByText, getByTestId } = render(); + expect(getByText('Account details')).toBeInTheDocument(); + + fireEvent.click(getByTestId('account-list-menu-details')); + + expect(actions.setAccountDetailsAddress).toHaveBeenCalledWith( + mockState.metamask.selectedAddress, + ); + }); +}); diff --git a/ui/components/multichain/menu-items/index.js b/ui/components/multichain/menu-items/index.js new file mode 100644 index 000000000..f1c9632d5 --- /dev/null +++ b/ui/components/multichain/menu-items/index.js @@ -0,0 +1,2 @@ +export { AccountDetailsMenuItem } from './account-details-menu-item'; +export { ViewExplorerMenuItem } from './view-explorer-menu-item'; diff --git a/ui/components/multichain/menu-items/view-explorer-menu-item.js b/ui/components/multichain/menu-items/view-explorer-menu-item.js new file mode 100644 index 000000000..534090be7 --- /dev/null +++ b/ui/components/multichain/menu-items/view-explorer-menu-item.js @@ -0,0 +1,97 @@ +import React, { useContext } from 'react'; +import PropTypes from 'prop-types'; +import { useSelector } from 'react-redux'; +import { useHistory } from 'react-router-dom'; + +import { getAccountLink } from '@metamask/etherscan-link'; + +import { MenuItem } from '../../ui/menu'; +import { useI18nContext } from '../../../hooks/useI18nContext'; +import { MetaMetricsContext } from '../../../contexts/metametrics'; +import { + MetaMetricsEventCategory, + MetaMetricsEventLinkType, + MetaMetricsEventName, +} from '../../../../shared/constants/metametrics'; +import { IconName, Text } from '../../component-library'; +import { + getBlockExplorerLinkText, + getCurrentChainId, + getRpcPrefsForCurrentProvider, + getSelectedAddress, +} from '../../../selectors'; +import { getURLHostName } from '../../../helpers/utils/util'; +import { NETWORKS_ROUTE } from '../../../helpers/constants/routes'; + +export const ViewExplorerMenuItem = ({ + metricsLocation, + closeMenu, + textProps, +}) => { + const t = useI18nContext(); + const trackEvent = useContext(MetaMetricsContext); + const history = useHistory(); + + const currentAddress = useSelector(getSelectedAddress); + const chainId = useSelector(getCurrentChainId); + const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider); + const addressLink = getAccountLink(currentAddress, chainId, rpcPrefs); + + const { blockExplorerUrl } = rpcPrefs; + const blockExplorerUrlSubTitle = getURLHostName(blockExplorerUrl); + const blockExplorerLinkText = useSelector(getBlockExplorerLinkText); + const openBlockExplorer = () => { + trackEvent({ + event: MetaMetricsEventName.ExternalLinkClicked, + category: MetaMetricsEventCategory.Navigation, + properties: { + link_type: MetaMetricsEventLinkType.AccountTracker, + location: metricsLocation, + url_domain: getURLHostName(addressLink), + }, + }); + + global.platform.openTab({ + url: addressLink, + }); + closeMenu(); + }; + + const routeToAddBlockExplorerUrl = () => { + history.push(`${NETWORKS_ROUTE}#blockExplorerUrl`); + }; + + const LABEL = t('viewOnExplorer'); + + return ( + { + blockExplorerLinkText.firstPart === 'addBlockExplorer' + ? routeToAddBlockExplorerUrl() + : openBlockExplorer(); + + trackEvent({ + event: MetaMetricsEventName.BlockExplorerLinkClicked, + category: MetaMetricsEventCategory.Accounts, + properties: { + location: metricsLocation, + chain_id: chainId, + }, + }); + + closeMenu?.(); + }} + subtitle={blockExplorerUrlSubTitle || null} + iconName={IconName.Export} + data-testid="account-list-menu-open-explorer" + > + {textProps ? {LABEL} : LABEL} + + ); +}; + +ViewExplorerMenuItem.propTypes = { + metricsLocation: PropTypes.string.isRequired, + closeMenu: PropTypes.func, + textProps: PropTypes.object, +}; diff --git a/ui/components/multichain/menu-items/view-explorer-menu-item.test.js b/ui/components/multichain/menu-items/view-explorer-menu-item.test.js new file mode 100644 index 000000000..58fd8b115 --- /dev/null +++ b/ui/components/multichain/menu-items/view-explorer-menu-item.test.js @@ -0,0 +1,29 @@ +import React from 'react'; +import { renderWithProvider, fireEvent } from '../../../../test/jest'; +import configureStore from '../../../store/store'; +import mockState from '../../../../test/data/mock-state.json'; +import { ViewExplorerMenuItem } from '.'; + +const render = () => { + const store = configureStore(mockState); + return renderWithProvider( + , + store, + ); +}; + +describe('ViewExplorerMenuItem', () => { + it('renders "View on explorer"', () => { + global.platform = { openTab: jest.fn() }; + + const { getByText, getByTestId } = render(); + expect(getByText('View on explorer')).toBeInTheDocument(); + + const openExplorerTabSpy = jest.spyOn(global.platform, 'openTab'); + fireEvent.click(getByTestId('account-list-menu-open-explorer')); + expect(openExplorerTabSpy).toHaveBeenCalled(); + }); +}); From f14a0ddb949938fc94063068ff829565ace03429 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Tue, 25 Jul 2023 11:59:22 -0500 Subject: [PATCH 09/31] UX: Ensure block explorer link is for desired account (#20144) --- .../account-list-item-menu/account-list-item-menu.js | 1 + ui/components/multichain/global-menu/global-menu.js | 1 + .../multichain/menu-items/view-explorer-menu-item.js | 6 +++--- .../multichain/menu-items/view-explorer-menu-item.test.js | 1 + 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ui/components/multichain/account-list-item-menu/account-list-item-menu.js b/ui/components/multichain/account-list-item-menu/account-list-item-menu.js index ac5a7ee51..f9483c2c2 100644 --- a/ui/components/multichain/account-list-item-menu/account-list-item-menu.js +++ b/ui/components/multichain/account-list-item-menu/account-list-item-menu.js @@ -138,6 +138,7 @@ export const AccountListItemMenu = ({ metricsLocation={METRICS_LOCATION} closeMenu={closeMenu} textProps={{ variant: TextVariant.bodySm }} + address={identity.address} /> {isRemovable ? ( { { const t = useI18nContext(); const trackEvent = useContext(MetaMetricsContext); const history = useHistory(); - const currentAddress = useSelector(getSelectedAddress); const chainId = useSelector(getCurrentChainId); const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider); - const addressLink = getAccountLink(currentAddress, chainId, rpcPrefs); + const addressLink = getAccountLink(address, chainId, rpcPrefs); const { blockExplorerUrl } = rpcPrefs; const blockExplorerUrlSubTitle = getURLHostName(blockExplorerUrl); @@ -94,4 +93,5 @@ ViewExplorerMenuItem.propTypes = { metricsLocation: PropTypes.string.isRequired, closeMenu: PropTypes.func, textProps: PropTypes.object, + address: PropTypes.string.isRequired, }; diff --git a/ui/components/multichain/menu-items/view-explorer-menu-item.test.js b/ui/components/multichain/menu-items/view-explorer-menu-item.test.js index 58fd8b115..447635fd9 100644 --- a/ui/components/multichain/menu-items/view-explorer-menu-item.test.js +++ b/ui/components/multichain/menu-items/view-explorer-menu-item.test.js @@ -10,6 +10,7 @@ const render = () => { , store, ); From 41bab4a6e14682388f4021f2f51bc74bddcbe80e Mon Sep 17 00:00:00 2001 From: David Walsh Date: Fri, 21 Jul 2023 18:57:59 -0500 Subject: [PATCH 10/31] UX: Show Checksum Addresses in Account Menu --- .../__snapshots__/account-list-item.test.js.snap | 2 +- .../multichain/account-list-item/account-list-item.js | 3 ++- .../multichain/account-list-item/account-list-item.test.js | 3 ++- .../multichain/menu-items/view-explorer-menu-item.js | 7 ++++++- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ui/components/multichain/account-list-item/__snapshots__/account-list-item.test.js.snap b/ui/components/multichain/account-list-item/__snapshots__/account-list-item.test.js.snap index 2afc7f4ed..d85798afc 100644 --- a/ui/components/multichain/account-list-item/__snapshots__/account-list-item.test.js.snap +++ b/ui/components/multichain/account-list-item/__snapshots__/account-list-item.test.js.snap @@ -103,7 +103,7 @@ exports[`AccountListItem renders AccountListItem component and shows account nam

- 0x0dc...e7bc + 0x0DC...E7bc

) : null} - {shortenAddress(identity.address)} + {shortenAddress(toChecksumHexAddress(identity.address))} { const { container } = render(); expect(screen.getByText(identity.name)).toBeInTheDocument(); expect( - screen.getByText(shortenAddress(identity.address)), + screen.getByText(shortenAddress(toChecksumHexAddress(identity.address))), ).toBeInTheDocument(); expect(document.querySelector('[title="0.006 ETH"]')).toBeInTheDocument(); diff --git a/ui/components/multichain/menu-items/view-explorer-menu-item.js b/ui/components/multichain/menu-items/view-explorer-menu-item.js index ca99996e8..60baab1a6 100644 --- a/ui/components/multichain/menu-items/view-explorer-menu-item.js +++ b/ui/components/multichain/menu-items/view-explorer-menu-item.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import { useSelector } from 'react-redux'; import { useHistory } from 'react-router-dom'; +import { toChecksumHexAddress } from '@metamask/controller-utils'; import { getAccountLink } from '@metamask/etherscan-link'; import { MenuItem } from '../../ui/menu'; @@ -34,7 +35,11 @@ export const ViewExplorerMenuItem = ({ const chainId = useSelector(getCurrentChainId); const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider); - const addressLink = getAccountLink(address, chainId, rpcPrefs); + const addressLink = getAccountLink( + toChecksumHexAddress(address), + chainId, + rpcPrefs, + ); const { blockExplorerUrl } = rpcPrefs; const blockExplorerUrlSubTitle = getURLHostName(blockExplorerUrl); From 5db22ceb848502337173e7983f8f49d78d0951d5 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Wed, 26 Jul 2023 14:38:19 -0500 Subject: [PATCH 11/31] UX: Ensure currently selected account displays when Account Menu opens (#20166) * UX: Ensure currently selected account displays when Account Menu opens * Jest tests --- .../account-list-item/account-list-item.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ui/components/multichain/account-list-item/account-list-item.js b/ui/components/multichain/account-list-item/account-list-item.js index df7f80f1e..a49e09543 100644 --- a/ui/components/multichain/account-list-item/account-list-item.js +++ b/ui/components/multichain/account-list-item/account-list-item.js @@ -1,4 +1,4 @@ -import React, { useState, useContext } from 'react'; +import React, { useState, useContext, useEffect, useRef } from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; @@ -87,6 +87,15 @@ export const AccountListItem = ({ setAccountListItemMenuElement(ref); }; + // If this is the selected item in the Account menu, + // scroll the item into view + const itemRef = useRef(null); + useEffect(() => { + if (selected) { + itemRef.current?.scrollIntoView?.(); + } + }, [itemRef, selected]); + const keyring = useSelector((state) => findKeyringForAddress(state, identity.address), ); @@ -103,6 +112,7 @@ export const AccountListItem = ({ 'multichain-account-list-item--selected': selected, 'multichain-account-list-item--connected': Boolean(connectedAvatar), })} + ref={itemRef} onClick={() => { // Without this check, the account will be selected after // the account options menu closes From 51cad751de2228f71ca53602e7ecdb782b0cffb5 Mon Sep 17 00:00:00 2001 From: Nicholas Ellul Date: Thu, 27 Jul 2023 13:33:36 -0400 Subject: [PATCH 12/31] Add improved downloading logic when exporting state logs (#19872) * Add improved downloading logic when exporting state logs * Make test for state logs download only apply to firefox * Remove eslint override * Add file extension to test * Move make jest global.Blob accessible to window --- test/e2e/tests/backup-restore.spec.js | 8 ++ test/e2e/tests/state-logs.spec.js | 5 + ui/helpers/utils/export-utils.js | 94 +++++++++++++++++-- ui/helpers/utils/export-utils.test.js | 68 ++++++++++++++ .../advanced-tab/advanced-tab.component.js | 13 ++- 5 files changed, 179 insertions(+), 9 deletions(-) create mode 100644 ui/helpers/utils/export-utils.test.js diff --git a/test/e2e/tests/backup-restore.spec.js b/test/e2e/tests/backup-restore.spec.js index d52cf1fa6..dfba955c6 100644 --- a/test/e2e/tests/backup-restore.spec.js +++ b/test/e2e/tests/backup-restore.spec.js @@ -56,6 +56,10 @@ describe('Backup and Restore', function () { ], }; it('should backup the account settings', async function () { + if (process.env.SELENIUM_BROWSER === 'chrome') { + // Chrome shows OS level download prompt which can't be dismissed by Selenium + this.skip(); + } await withFixtures( { fixtures: new FixtureBuilder().build(), @@ -97,6 +101,10 @@ describe('Backup and Restore', function () { }); it('should restore the account settings', async function () { + if (process.env.SELENIUM_BROWSER === 'chrome') { + // Chrome shows OS level download prompt which can't be dismissed by Selenium + this.skip(); + } await withFixtures( { fixtures: new FixtureBuilder().build(), diff --git a/test/e2e/tests/state-logs.spec.js b/test/e2e/tests/state-logs.spec.js index f08a38fd1..8cccbcf91 100644 --- a/test/e2e/tests/state-logs.spec.js +++ b/test/e2e/tests/state-logs.spec.js @@ -30,7 +30,12 @@ describe('State logs', function () { }, ], }; + it('should download state logs for the account', async function () { + if (process.env.SELENIUM_BROWSER === 'chrome') { + // Chrome shows OS level download prompt which can't be dismissed by Selenium + this.skip(); + } await withFixtures( { fixtures: new FixtureBuilder().build(), diff --git a/ui/helpers/utils/export-utils.js b/ui/helpers/utils/export-utils.js index d9f65e946..993d2c90a 100644 --- a/ui/helpers/utils/export-utils.js +++ b/ui/helpers/utils/export-utils.js @@ -1,11 +1,93 @@ -import { getRandomFileName } from './util'; +/** + * @enum { string } + */ +export const ExportableContentType = { + JSON: 'application/json', + TXT: 'text/plain', +}; -export function exportAsFile(filename, data, type = 'text/csv') { +/** + * @enum { string } + */ +const ExtensionForContentType = { + [ExportableContentType.JSON]: '.json', + [ExportableContentType.TXT]: '.txt', +}; + +/** + * Export data as a file. + * + * @param {string} filename - The name of the file to export. + * @param {string} data - The data to export. + * @param {ExportableContentType} contentType - The content type of the file to export. + */ +export async function exportAsFile(filename, data, contentType) { + if (!ExtensionForContentType[contentType]) { + throw new Error(`Unsupported file type: ${contentType}`); + } + + if (supportsShowSaveFilePicker()) { + // Preferred method for downloads + await saveFileUsingFilePicker(filename, data, contentType); + } else { + saveFileUsingDataUri(filename, data, contentType); + } +} +/** + * Notes if the browser supports the File System Access API. + * + * @returns {boolean} + */ +function supportsShowSaveFilePicker() { + return ( + typeof window !== 'undefined' && + typeof window.showSaveFilePicker !== 'undefined' && + typeof window.Blob !== 'undefined' + ); +} + +/** + * Saves a file using the File System Access API. + * + * @param {string} filename - The name of the file to export. + * @param {string} data - The data to export. + * @param {ExportableContentType} contentType - The content type of the file to export. + * @returns {Promise} + */ +async function saveFileUsingFilePicker(filename, data, contentType) { + const blob = new window.Blob([data], { contentType }); + const fileExtension = ExtensionForContentType[contentType]; + + const handle = await window.showSaveFilePicker({ + suggestedName: filename, + types: [ + { + description: filename, + accept: { + [contentType]: [fileExtension], + }, + }, + ], + }); + + const writable = await handle.createWritable(); + await writable.write(blob); + await writable.close(); +} + +/** + * Saves a file using a data URI. + * This is a fallback for browsers that do not support the File System Access API. + * This method is less preferred because it requires the entire file to be encoded in a data URI. + * + * @param {string} filename - The name of the file to export. + * @param {string} data - The data to export. + * @param {ExportableContentType} contentType - The content type of the file to export. + */ +function saveFileUsingDataUri(filename, data, contentType) { const b64 = Buffer.from(data, 'utf8').toString('base64'); - // eslint-disable-next-line no-param-reassign - filename = filename || getRandomFileName(); - const elem = window.document.createElement('a'); - elem.href = `data:${type};Base64,${b64}`; + const elem = document.createElement('a'); + elem.href = `data:${contentType};Base64,${b64}`; elem.download = filename; document.body.appendChild(elem); elem.click(); diff --git a/ui/helpers/utils/export-utils.test.js b/ui/helpers/utils/export-utils.test.js new file mode 100644 index 000000000..a5ac3c2aa --- /dev/null +++ b/ui/helpers/utils/export-utils.test.js @@ -0,0 +1,68 @@ +import { exportAsFile, ExportableContentType } from './export-utils'; + +describe('exportAsFile', () => { + let windowSpy; + + beforeEach(() => { + windowSpy = jest.spyOn(window, 'window', 'get'); + }); + + afterEach(() => { + windowSpy.mockRestore(); + }); + + describe('when showSaveFilePicker is supported', () => { + it('uses .json file extension when content type is JSON', async () => { + const showSaveFilePicker = mockShowSaveFilePicker(); + const filename = 'test.json'; + const data = '{file: "content"}'; + windowSpy.mockImplementation(() => ({ + showSaveFilePicker, + Blob: global.Blob, + })); + + await exportAsFile(filename, data, ExportableContentType.JSON); + + expect(showSaveFilePicker).toHaveBeenCalledWith({ + suggestedName: filename, + types: [ + { + description: filename, + accept: { 'application/json': ['.json'] }, + }, + ], + }); + }); + + it('uses .txt file extension when content type is TXT', async () => { + const showSaveFilePicker = mockShowSaveFilePicker(); + const filename = 'test.txt'; + const data = 'file content'; + + windowSpy.mockImplementation(() => ({ + showSaveFilePicker, + Blob: global.Blob, + })); + + await exportAsFile(filename, data, ExportableContentType.TXT); + + expect(showSaveFilePicker).toHaveBeenCalledWith({ + suggestedName: filename, + types: [ + { + description: filename, + accept: { 'text/plain': ['.txt'] }, + }, + ], + }); + }); + }); +}); + +function mockShowSaveFilePicker() { + return jest.fn().mockResolvedValueOnce({ + createWritable: jest + .fn() + .mockResolvedValueOnce({ write: jest.fn(), close: jest.fn() }), + }); +} diff --git a/ui/pages/settings/advanced-tab/advanced-tab.component.js b/ui/pages/settings/advanced-tab/advanced-tab.component.js index 95c5250a9..230ea8a30 100644 --- a/ui/pages/settings/advanced-tab/advanced-tab.component.js +++ b/ui/pages/settings/advanced-tab/advanced-tab.component.js @@ -23,7 +23,10 @@ import { MetaMetricsEventName, } from '../../../../shared/constants/metametrics'; import { DEFAULT_AUTO_LOCK_TIME_LIMIT } from '../../../../shared/constants/preferences'; -import { exportAsFile } from '../../../helpers/utils/export-utils'; +import { + exportAsFile, + ExportableContentType, +} from '../../../helpers/utils/export-utils'; import ActionableMessage from '../../../components/ui/actionable-message'; import ZENDESK_URLS from '../../../helpers/constants/zendesk-url'; import { BannerAlert } from '../../../components/component-library'; @@ -150,7 +153,7 @@ export default class AdvancedTab extends PureComponent { backupUserData = async () => { const { fileName, data } = await this.props.backupUserData(); - exportAsFile(fileName, data); + exportAsFile(fileName, data, ExportableContentType.JSON); this.context.trackEvent({ event: 'User Data Exported', @@ -185,7 +188,11 @@ export default class AdvancedTab extends PureComponent { if (err) { displayWarning(t('stateLogError')); } else { - exportAsFile(`${t('stateLogFileName')}.json`, result); + exportAsFile( + `${t('stateLogFileName')}.json`, + result, + ExportableContentType.JSON, + ); } }); }} From 3c65b3fa7f01854dc764dec6364f8d3332be3e75 Mon Sep 17 00:00:00 2001 From: Howard Braham Date: Thu, 13 Jul 2023 13:49:13 -0700 Subject: [PATCH 13/31] fix(clipboard): Increase DEFAULT copy to clipboard time (#20008) --- ui/hooks/useCopyToClipboard.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/hooks/useCopyToClipboard.js b/ui/hooks/useCopyToClipboard.js index 8f6384ac0..2aead9976 100644 --- a/ui/hooks/useCopyToClipboard.js +++ b/ui/hooks/useCopyToClipboard.js @@ -1,6 +1,6 @@ import { useState, useCallback } from 'react'; import copyToClipboard from 'copy-to-clipboard'; -import { SECOND } from '../../shared/constants/time'; +import { MINUTE } from '../../shared/constants/time'; import { useTimeout } from './useTimeout'; /** @@ -9,7 +9,7 @@ import { useTimeout } from './useTimeout'; * @param {number} [delay=3000] - delay in ms * @returns {[boolean, Function]} */ -const DEFAULT_DELAY = SECOND * 3; +const DEFAULT_DELAY = MINUTE; export function useCopyToClipboard(delay = DEFAULT_DELAY) { const [copied, setCopied] = useState(false); From f1af52cbdfde4e5a9ff315fe74961f58717300e6 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Fri, 28 Jul 2023 18:18:22 -0500 Subject: [PATCH 14/31] Fix #20162 - Add Whats New for Global Menu (#20244) --- app/_locales/en/messages.json | 9 + app/images/global-menu-block-explorer.svg | 827 ++++++++++++++++++++++ shared/notifications/index.js | 19 + test/e2e/fixture-builder.js | 5 + 4 files changed, 860 insertions(+) create mode 100644 app/images/global-menu-block-explorer.svg diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 5d9b0ba06..2da978ac5 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -2722,6 +2722,15 @@ "notifications21Title": { "message": "Introducing new and refreshed Swaps!" }, + "notifications22ActionText": { + "message": "Got it" + }, + "notifications22Description": { + "message": "💡 Just click the global menu or account menu to find them!" + }, + "notifications22Title": { + "message": "Looking for your account details or the block explorer URL?" + }, "notifications3ActionText": { "message": "Read more", "description": "The 'call to action' on the button, or link, of the 'Stay secure' notification. Upon clicking, users will be taken to a page about security on the metamask support website." diff --git a/app/images/global-menu-block-explorer.svg b/app/images/global-menu-block-explorer.svg new file mode 100644 index 000000000..728459030 --- /dev/null +++ b/app/images/global-menu-block-explorer.svg @@ -0,0 +1,827 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/shared/notifications/index.js b/shared/notifications/index.js index a5a689f3a..bc74e3222 100644 --- a/shared/notifications/index.js +++ b/shared/notifications/index.js @@ -114,6 +114,14 @@ export const UI_NOTIFICATIONS = { width: '100%', }, }, + 22: { + id: 22, + date: null, + image: { + src: 'images/global-menu-block-explorer.svg', + width: '100%', + }, + }, }; export const getTranslatedUINotifications = (t, locale) => { @@ -313,5 +321,16 @@ export const getTranslatedUINotifications = (t, locale) => { ) : '', }, + 22: { + ...UI_NOTIFICATIONS[22], + title: t('notifications22Title'), + description: t('notifications22Description'), + actionText: t('notifications22ActionText'), + date: UI_NOTIFICATIONS[22].date + ? new Intl.DateTimeFormat(formattedLocale).format( + new Date(UI_NOTIFICATIONS[22].date), + ) + : '', + }, }; }; diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js index bbbcd49cf..ed3e3aaac 100644 --- a/test/e2e/fixture-builder.js +++ b/test/e2e/fixture-builder.js @@ -141,6 +141,11 @@ function defaultFixture() { id: 21, isShown: true, }, + 22: { + date: null, + id: 22, + isShown: true, + }, }, }, AppStateController: { From db92bef002e770f31d4f55c55fd58b6272d2a52e Mon Sep 17 00:00:00 2001 From: Nicholas Ellul Date: Mon, 31 Jul 2023 08:48:48 -0400 Subject: [PATCH 15/31] Update @metamask/phishing-controller to v4.0.0 (#18840) * Update phishing controller to v4.0.0 * Move phishing e2e test utilities into its own helper.js * Update phishing detection e2e test * Update MetaMask Controller test mocks * Update mv3 phishing tests * Fix test for 500 error on warning page * Allow for directories in test folder * Update migration number * Linting fixes * Remove fail on console error * Separate mocks from helpers * Have migration delete PhishingController state entirely * Remove phishing detection directory * Only delete the listState in migration * Bump migration version --- .../metamask-controller.actions.test.js | 44 ++++- app/scripts/metamask-controller.test.js | 43 ++++- app/scripts/migrations/090.test.js | 109 +++++++++++ app/scripts/migrations/090.ts | 37 ++++ app/scripts/migrations/index.js | 2 + package.json | 2 +- test/e2e/helpers.js | 81 --------- test/e2e/mock-e2e.js | 32 +--- .../mv3/phishing-warning-sw-restart.spec.js | 17 +- test/e2e/run-all.js | 18 +- test/e2e/tests/phishing-controller/helpers.js | 25 +++ test/e2e/tests/phishing-controller/mocks.js | 172 ++++++++++++++++++ .../phishing-detection.spec.js | 163 +++++++++++------ yarn.lock | 14 +- 14 files changed, 565 insertions(+), 194 deletions(-) create mode 100644 app/scripts/migrations/090.test.js create mode 100644 app/scripts/migrations/090.ts create mode 100644 test/e2e/tests/phishing-controller/helpers.js create mode 100644 test/e2e/tests/phishing-controller/mocks.js rename test/e2e/tests/{ => phishing-controller}/phishing-detection.spec.js (69%) diff --git a/app/scripts/metamask-controller.actions.test.js b/app/scripts/metamask-controller.actions.test.js index c1ea740f8..5e7c771a1 100644 --- a/app/scripts/metamask-controller.actions.test.js +++ b/app/scripts/metamask-controller.actions.test.js @@ -1,7 +1,14 @@ import { strict as assert } from 'assert'; import sinon from 'sinon'; import proxyquire from 'proxyquire'; - +import { + ListNames, + METAMASK_STALELIST_URL, + METAMASK_HOTLIST_DIFF_URL, + PHISHING_CONFIG_BASE_URL, + METAMASK_STALELIST_FILE, + METAMASK_HOTLIST_DIFF_FILE, +} from '@metamask/phishing-controller'; import { ApprovalRequestNotFoundError } from '@metamask/approval-controller'; import { PermissionsRequestNotFoundError } from '@metamask/permission-controller'; import nock from 'nock'; @@ -59,21 +66,28 @@ describe('MetaMaskController', function () { }); beforeEach(function () { - nock('https://static.metafi.codefi.network') + nock(PHISHING_CONFIG_BASE_URL) .persist() - .get('/api/v1/lists/stalelist.json') + .get(METAMASK_STALELIST_FILE) .reply( 200, JSON.stringify({ version: 2, tolerance: 2, - fuzzylist: [], - allowlist: [], - blocklist: ['127.0.0.1'], - lastUpdated: 0, + lastUpdated: 1, + eth_phishing_detect_config: { + fuzzylist: [], + allowlist: [], + blocklist: ['127.0.0.1'], + name: ListNames.MetaMask, + }, + phishfort_hotlist: { + blocklist: [], + name: ListNames.Phishfort, + }, }), ) - .get('/api/v1/lists/hotlist.json') + .get(METAMASK_HOTLIST_DIFF_FILE) .reply( 200, JSON.stringify([ @@ -110,6 +124,20 @@ describe('MetaMaskController', function () { await ganacheServer.quit(); }); + describe('Phishing Detection Mock', function () { + it('should be updated to use v1 of the API', function () { + // Update the fixture above if this test fails + assert.equal( + METAMASK_STALELIST_URL, + 'https://phishing-detection.metafi.codefi.network/v1/stalelist', + ); + assert.equal( + METAMASK_HOTLIST_DIFF_URL, + 'https://phishing-detection.metafi.codefi.network/v1/diffsSince', + ); + }); + }); + describe('#addNewAccount', function () { it('two parallel calls with same accountCount give same result', async function () { await metamaskController.createNewVaultAndKeychain('test@123'); diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 32d7b3c2f..d641699b9 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -7,6 +7,14 @@ import EthQuery from 'eth-query'; import proxyquire from 'proxyquire'; import browser from 'webextension-polyfill'; import { wordlist as englishWordlist } from '@metamask/scure-bip39/dist/wordlists/english'; +import { + ListNames, + METAMASK_STALELIST_URL, + METAMASK_HOTLIST_DIFF_URL, + PHISHING_CONFIG_BASE_URL, + METAMASK_STALELIST_FILE, + METAMASK_HOTLIST_DIFF_FILE, +} from '@metamask/phishing-controller'; import { TransactionStatus } from '../../shared/constants/transaction'; import createTxMeta from '../../test/lib/createTxMeta'; import { NETWORK_TYPES } from '../../shared/constants/network'; @@ -185,21 +193,28 @@ describe('MetaMaskController', function () { .persist() .get(/.*/u) .reply(200, '{"JPY":12415.9}'); - nock('https://static.metafi.codefi.network') + nock(PHISHING_CONFIG_BASE_URL) .persist() - .get('/api/v1/lists/stalelist.json') + .get(METAMASK_STALELIST_FILE) .reply( 200, JSON.stringify({ version: 2, tolerance: 2, - fuzzylist: [], - allowlist: [], - blocklist: ['127.0.0.1'], - lastUpdated: 0, + lastUpdated: 1, + eth_phishing_detect_config: { + fuzzylist: [], + allowlist: [], + blocklist: ['127.0.0.1'], + name: ListNames.MetaMask, + }, + phishfort_hotlist: { + blocklist: [], + name: ListNames.Phishfort, + }, }), ) - .get('/api/v1/lists/hotlist.json') + .get(METAMASK_HOTLIST_DIFF_FILE) .reply( 200, JSON.stringify([ @@ -223,6 +238,20 @@ describe('MetaMaskController', function () { await ganacheServer.quit(); }); + describe('Phishing Detection Mock', function () { + it('should be updated to use v1 of the API', function () { + // Update the fixture above if this test fails + assert.equal( + METAMASK_STALELIST_URL, + 'https://phishing-detection.metafi.codefi.network/v1/stalelist', + ); + assert.equal( + METAMASK_HOTLIST_DIFF_URL, + 'https://phishing-detection.metafi.codefi.network/v1/diffsSince', + ); + }); + }); + describe('MetaMaskController Behaviour', function () { let metamaskController; diff --git a/app/scripts/migrations/090.test.js b/app/scripts/migrations/090.test.js new file mode 100644 index 000000000..6a28c60f2 --- /dev/null +++ b/app/scripts/migrations/090.test.js @@ -0,0 +1,109 @@ +import { migrate, version } from './090'; + +const PREVIOUS_VERSION = version - 1; + +describe('migration #90', () => { + it('updates the version metadata', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: {}, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.meta).toStrictEqual({ + version, + }); + }); + + it('does not change the state if the phishing controller state does not exist', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { test: '123' }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does not change the state if the phishing controller state is invalid', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { PhishingController: 'this is not valid' }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('does not change the state if the listState property does not exist', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { + PhishingController: { test: 123 }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); + + it('deletes the "listState" property', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { PhishingController: { listState: {} } }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data.PhishingController.listState).toBeUndefined(); + }); + + it('deletes the listState if present', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { PhishingController: { listState: { test: 123 } } }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + PhishingController: {}, + }); + }); + + it('does not delete the allowlist if present', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: { + PhishingController: { + whitelist: ['foobar.com'], + listState: { test: 123 }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + PhishingController: { whitelist: ['foobar.com'] }, + }); + }); +}); diff --git a/app/scripts/migrations/090.ts b/app/scripts/migrations/090.ts new file mode 100644 index 000000000..e45ec05e4 --- /dev/null +++ b/app/scripts/migrations/090.ts @@ -0,0 +1,37 @@ +import { cloneDeep } from 'lodash'; +import { hasProperty, isObject } from '@metamask/utils'; + +export const version = 90; + +/** + * Explain the purpose of the migration here. + * + * @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: { + meta: { version: number }; + data: Record; +}) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + versionedData.data = transformState(versionedData.data); + return versionedData; +} + +function transformState(state: Record) { + if ( + !hasProperty(state, 'PhishingController') || + !isObject(state.PhishingController) || + !hasProperty(state.PhishingController, 'listState') + ) { + return state; + } + + delete state.PhishingController.listState; + + return state; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 429ef7959..adbe48252 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -93,6 +93,7 @@ import * as m086 from './086'; import * as m087 from './087'; import * as m088 from './088'; import * as m089 from './089'; +import * as m090 from './090'; const migrations = [ m002, @@ -183,6 +184,7 @@ const migrations = [ m087, m088, m089, + m090, ]; export default migrations; diff --git a/package.json b/package.json index 0200efb5c..1b5a74ff5 100644 --- a/package.json +++ b/package.json @@ -250,7 +250,7 @@ "@metamask/notification-controller": "^3.0.0", "@metamask/obs-store": "^8.1.0", "@metamask/permission-controller": "^4.0.0", - "@metamask/phishing-controller": "^3.0.0", + "@metamask/phishing-controller": "^4.0.0", "@metamask/post-message-stream": "^6.0.0", "@metamask/providers": "^11.1.0", "@metamask/rate-limit-controller": "^3.0.0", diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index d6a4bb2ab..f6e553b49 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -497,85 +497,6 @@ const openDapp = async (driver, contract = null, dappURL = DAPP_URL) => { ? await driver.openNewPage(`${dappURL}/?contract=${contract}`) : await driver.openNewPage(dappURL); }; -const STALELIST_URL = - 'https://static.metafi.codefi.network/api/v1/lists/stalelist.json'; - -const emptyHtmlPage = ` - - - - title - - - Empty page - -`; - -/** - * Setup fetch mocks for the phishing detection feature. - * - * The mock configuration will show that "127.0.0.1" is blocked. The dynamic lookup on the warning - * page can be customized, so that we can test both the MetaMask and PhishFort block cases. - * - * @param {import('mockttp').Mockttp} mockServer - The mock server. - * @param {object} metamaskPhishingConfigResponse - The response for the dynamic phishing - * configuration lookup performed by the warning page. - */ -async function setupPhishingDetectionMocks( - mockServer, - metamaskPhishingConfigResponse, -) { - await mockServer.forGet(STALELIST_URL).thenCallback(() => { - return { - statusCode: 200, - json: { - version: 2, - tolerance: 2, - fuzzylist: [], - allowlist: [], - blocklist: ['127.0.0.1'], - lastUpdated: 0, - }, - }; - }); - - await mockServer - .forGet('https://github.com/MetaMask/eth-phishing-detect/issues/new') - .thenCallback(() => { - return { - statusCode: 200, - body: emptyHtmlPage, - }; - }); - await mockServer - .forGet('https://github.com/phishfort/phishfort-lists/issues/new') - .thenCallback(() => { - return { - statusCode: 200, - body: emptyHtmlPage, - }; - }); - - await mockServer - .forGet( - 'https://raw.githubusercontent.com/MetaMask/eth-phishing-detect/master/src/config.json', - ) - .thenCallback(() => metamaskPhishingConfigResponse); -} - -function mockPhishingDetection(mockServer) { - setupPhishingDetectionMocks(mockServer, { - statusCode: 200, - json: { - version: 2, - tolerance: 2, - fuzzylist: [], - whitelist: [], - blacklist: ['127.0.0.1'], - lastUpdated: 0, - }, - }); -} const PRIVATE_KEY = '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC'; @@ -791,8 +712,6 @@ module.exports = { importWrongSRPOnboardingFlow, testSRPDropdownIterations, openDapp, - mockPhishingDetection, - setupPhishingDetectionMocks, defaultGanacheOptions, sendTransaction, findAnotherAccountFromAccountList, diff --git a/test/e2e/mock-e2e.js b/test/e2e/mock-e2e.js index 29035f46c..20e70d84b 100644 --- a/test/e2e/mock-e2e.js +++ b/test/e2e/mock-e2e.js @@ -4,21 +4,9 @@ const blacklistedHosts = [ 'mainnet.infura.io', 'sepolia.infura.io', ]; - -const HOTLIST_URL = - 'https://static.metafi.codefi.network/api/v1/lists/hotlist.json'; -const STALELIST_URL = - 'https://static.metafi.codefi.network/api/v1/lists/stalelist.json'; - -const emptyHotlist = []; -const emptyStalelist = { - version: 2, - tolerance: 2, - fuzzylist: [], - allowlist: [], - blocklist: [], - lastUpdated: 0, -}; +const { + mockEmptyStalelistAndHotlist, +} = require('./tests/phishing-controller/mocks'); async function setupMocking(server, testSpecificMock) { await server.forAnyRequest().thenPassThrough({ @@ -374,19 +362,7 @@ async function setupMocking(server, testSpecificMock) { }; }); - await server.forGet(STALELIST_URL).thenCallback(() => { - return { - statusCode: 200, - json: emptyStalelist, - }; - }); - - await server.forGet(HOTLIST_URL).thenCallback(() => { - return { - statusCode: 200, - json: emptyHotlist, - }; - }); + await mockEmptyStalelistAndHotlist(server); await server .forPost('https://customnetwork.com/api/customRPC') diff --git a/test/e2e/mv3/phishing-warning-sw-restart.spec.js b/test/e2e/mv3/phishing-warning-sw-restart.spec.js index 2e10f35b2..8de3ad7e0 100644 --- a/test/e2e/mv3/phishing-warning-sw-restart.spec.js +++ b/test/e2e/mv3/phishing-warning-sw-restart.spec.js @@ -1,7 +1,7 @@ const { strict: assert } = require('assert'); +const FixtureBuilder = require('../fixture-builder'); const { withFixtures, - mockPhishingDetection, openDapp, defaultGanacheOptions, assertAccountBalanceForDOM, @@ -11,7 +11,11 @@ const { unlockWallet, terminateServiceWorker, } = require('../helpers'); -const FixtureBuilder = require('../fixture-builder'); + +const { + setupPhishingDetectionMocks, + BlockProvider, +} = require('../tests/phishing-controller/helpers'); describe('Phishing warning page', function () { const driverOptions = { openDevToolsForTabs: true }; @@ -21,12 +25,17 @@ describe('Phishing warning page', function () { await withFixtures( { - dapp: true, fixtures: new FixtureBuilder().build(), ganacheOptions: defaultGanacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, driverOptions, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + }, + dapp: true, }, async ({ driver, ganacheServer }) => { await driver.navigate(); diff --git a/test/e2e/run-all.js b/test/e2e/run-all.js index d045233e5..799177b77 100644 --- a/test/e2e/run-all.js +++ b/test/e2e/run-all.js @@ -6,10 +6,20 @@ const { runInShell } = require('../../development/lib/run-command'); const { exitWithError } = require('../../development/lib/exit-with-error'); const getTestPathsForTestDir = async (testDir) => { - const testFilenames = await fs.readdir(testDir); - const testPaths = testFilenames.map((filename) => - path.join(testDir, filename), - ); + const testFilenames = await fs.readdir(testDir, { withFileTypes: true }); + const testPaths = []; + + for (const itemInDirectory of testFilenames) { + const fullPath = path.join(testDir, itemInDirectory.name); + + if (itemInDirectory.isDirectory()) { + const subDirPaths = await getTestPathsForTestDir(fullPath); + testPaths.push(...subDirPaths); + } else if (fullPath.endsWith('.spec.js')) { + testPaths.push(fullPath); + } + } + return testPaths; }; diff --git a/test/e2e/tests/phishing-controller/helpers.js b/test/e2e/tests/phishing-controller/helpers.js new file mode 100644 index 000000000..a00d5ddb3 --- /dev/null +++ b/test/e2e/tests/phishing-controller/helpers.js @@ -0,0 +1,25 @@ +const { + METAMASK_STALELIST_URL, + METAMASK_HOTLIST_DIFF_URL, + ListNames, +} = require('@metamask/phishing-controller'); + +/** + * The block provider names. + * + * @enum {BlockProvider} + * @readonly + * @property {string} MetaMask - The name of the MetaMask block provider. + * @property {string} PhishFort - The name of the PhishFort block provider. + */ +const BlockProvider = { + MetaMask: 'metamask', + PhishFort: 'phishfort', +}; + +module.exports = { + METAMASK_HOTLIST_DIFF_URL, + METAMASK_STALELIST_URL, + BlockProvider, + ListNames, +}; diff --git a/test/e2e/tests/phishing-controller/mocks.js b/test/e2e/tests/phishing-controller/mocks.js new file mode 100644 index 000000000..f15e4b848 --- /dev/null +++ b/test/e2e/tests/phishing-controller/mocks.js @@ -0,0 +1,172 @@ +const { + METAMASK_STALELIST_URL, + METAMASK_HOTLIST_DIFF_URL, + ListNames, + BlockProvider, +} = require('./helpers'); + +// last updated must not be 0 +const lastUpdated = 1; +const defaultHotlist = { data: [] }; +const defaultStalelist = { + version: 2, + tolerance: 2, + lastUpdated, + eth_phishing_detect_config: { + fuzzylist: [], + allowlist: [], + blocklist: [], + name: ListNames.MetaMask, + }, + phishfort_hotlist: { + blocklist: [], + name: ListNames.Phishfort, + }, +}; + +const emptyHtmlPage = (blockProvider) => ` + + + + title + + + Empty page by ${blockProvider} + +`; + +/** + * Setup fetch mocks for the phishing detection feature. + * + * The mock configuration will show that "127.0.0.1" is blocked. The dynamic lookup on the warning + * page can be customized, so that we can test both the MetaMask and PhishFort block cases. + * + * @param {import('mockttp').Mockttp} mockServer - The mock server. + * @param {object} mockPhishingConfigResponseConfig - The response for the dynamic phishing + * @param {number} mockPhishingConfigResponseConfig.statusCode - The status code for the response. + * @param {string[]} mockPhishingConfigResponseConfig.blocklist - The blocklist for the response. + * @param {BlockProvider} mockPhishingConfigResponseConfig.blockProvider - The name of the provider who blocked the page. + * configuration lookup performed by the warning page. + */ +async function setupPhishingDetectionMocks( + mockServer, + { + statusCode = 200, + blocklist = ['127.0.0.1'], + blockProvider = BlockProvider.MetaMask, + }, +) { + const blockProviderConfig = resolveProviderConfigName(blockProvider); + + const response = + statusCode >= 400 + ? { statusCode } + : { + statusCode, + json: { + data: { + ...defaultStalelist, + [blockProviderConfig]: { + ...defaultStalelist[blockProviderConfig], + blocklist, + }, + }, + }, + }; + + await mockServer.forGet(METAMASK_STALELIST_URL).thenCallback(() => { + return response; + }); + + await mockServer + .forGet(`${METAMASK_HOTLIST_DIFF_URL}/${lastUpdated}`) + .thenCallback(() => { + return { + statusCode: 200, + json: defaultHotlist, + }; + }); + + await mockServer + .forGet('https://github.com/MetaMask/eth-phishing-detect/issues/new') + .thenCallback(() => { + return { + statusCode: 200, + body: emptyHtmlPage(blockProvider), + }; + }); + + await mockServer + .forGet('https://github.com/phishfort/phishfort-lists/issues/new') + .thenCallback(() => { + return { + statusCode: 200, + body: emptyHtmlPage(blockProvider), + }; + }); +} + +/** + * Mocks the request made from the phishing warning page to check eth-phishing-detect + * + * @param {*} mockServer + * @param {*} metamaskPhishingConfigResponse + */ +async function mockConfigLookupOnWarningPage( + mockServer, + metamaskPhishingConfigResponse, +) { + await mockServer + .forGet( + 'https://raw.githubusercontent.com/MetaMask/eth-phishing-detect/master/src/config.json', + ) + .thenCallback(() => metamaskPhishingConfigResponse); +} + +/** + * Setup fallback mocks for default behaviour of the phishing detection feature. + * + * This sets up default mocks for a mockttp server when included in test/e2e/mock-e2e.js + * + * @param {import('mockttp').Mockttp} mockServer - The mock server. + */ + +async function mockEmptyStalelistAndHotlist(mockServer) { + await mockServer.forGet(METAMASK_STALELIST_URL).thenCallback(() => { + return { + statusCode: 200, + json: { ...defaultStalelist }, + }; + }); + + await mockServer + .forGet(`${METAMASK_HOTLIST_DIFF_URL}/${lastUpdated}`) + .thenCallback(() => { + return { + statusCode: 200, + json: defaultHotlist, + }; + }); +} + +/** + * + * @param {BlockProvider} providerName - The name of the provider who issued the block. + * @returns {string} The name of the phishing config in the response. + */ +function resolveProviderConfigName(providerName) { + switch (providerName.toLowerCase()) { + case BlockProvider.MetaMask: + return 'eth_phishing_detect_config'; + case BlockProvider.PhishFort: + return 'phishfort_hotlist'; + default: + throw new Error('Provider name must either be metamask or phishfort'); + } +} + +module.exports = { + setupPhishingDetectionMocks, + mockEmptyStalelistAndHotlist, + mockConfigLookupOnWarningPage, +}; diff --git a/test/e2e/tests/phishing-detection.spec.js b/test/e2e/tests/phishing-controller/phishing-detection.spec.js similarity index 69% rename from test/e2e/tests/phishing-detection.spec.js rename to test/e2e/tests/phishing-controller/phishing-detection.spec.js index 735d2bfb2..2e29a2411 100644 --- a/test/e2e/tests/phishing-detection.spec.js +++ b/test/e2e/tests/phishing-controller/phishing-detection.spec.js @@ -1,12 +1,17 @@ const { strict: assert } = require('assert'); + +const { convertToHexValue, withFixtures, openDapp } = require('../../helpers'); +const FixtureBuilder = require('../../fixture-builder'); +const { + METAMASK_HOTLIST_DIFF_URL, + METAMASK_STALELIST_URL, + BlockProvider, +} = require('./helpers'); + const { - convertToHexValue, - withFixtures, - openDapp, setupPhishingDetectionMocks, - mockPhishingDetection, -} = require('../helpers'); -const FixtureBuilder = require('../fixture-builder'); + mockConfigLookupOnWarningPage, +} = require('./mocks'); describe('Phishing Detection', function () { const ganacheOptions = { @@ -19,13 +24,32 @@ describe('Phishing Detection', function () { ], }; + describe('Phishing Detection Mock', function () { + it('should be updated to use v1 of the API', function () { + // Update the fixture in phishing-controller/mocks.js if this test fails + assert.equal( + METAMASK_STALELIST_URL, + 'https://phishing-detection.metafi.codefi.network/v1/stalelist', + ); + assert.equal( + METAMASK_HOTLIST_DIFF_URL, + 'https://phishing-detection.metafi.codefi.network/v1/diffsSince', + ); + }); + }); + it('should display the MetaMask Phishing Detection page and take the user to the blocked page if they continue', async function () { await withFixtures( { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + }, dapp: true, failOnConsoleError: false, }, @@ -44,12 +68,20 @@ describe('Phishing Detection', function () { }); it('should display the MetaMask Phishing Detection page in an iframe and take the user to the blocked page if they continue', async function () { + const DAPP_WITH_IFRAMED_PAGE_ON_BLOCKLIST = 'http://localhost:8080/'; + const IFRAMED_HOSTNAME = '127.0.0.1'; + await withFixtures( { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: [IFRAMED_HOSTNAME], + }); + }, dapp: true, dappPaths: ['mock-page-with-iframe'], dappOptions: { @@ -61,7 +93,7 @@ describe('Phishing Detection', function () { await driver.navigate(); await driver.fill('#password', 'correct horse battery staple'); await driver.press('#password', driver.Key.ENTER); - await driver.openNewPage('http://localhost:8080/'); + await driver.openNewPage(DAPP_WITH_IFRAMED_PAGE_ON_BLOCKLIST); const iframe = await driver.findElement('iframe'); @@ -85,7 +117,12 @@ describe('Phishing Detection', function () { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + }, dapp: true, dappPaths: ['mock-page-with-disallowed-iframe'], dappOptions: { @@ -125,7 +162,11 @@ describe('Phishing Detection', function () { ganacheOptions, title: this.test.title, testSpecificMock: (mockServer) => { - setupPhishingDetectionMocks(mockServer, { statusCode: 500 }); + setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + mockConfigLookupOnWarningPage(mockServer, { statusCode: 500 }); }, dapp: true, failOnConsoleError: false, @@ -139,7 +180,9 @@ describe('Phishing Detection', function () { await driver.clickElement({ text: 'report a detection problem.' }); // wait for page to load before checking URL. - await driver.findElement({ text: 'Empty page' }); + await driver.findElement({ + text: `Empty page by ${BlockProvider.MetaMask}`, + }); assert.equal( await driver.getCurrentUrl(), `https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20127.0.0.1&body=http%3A%2F%2F127.0.0.1%3A8080%2F`, @@ -149,50 +192,18 @@ describe('Phishing Detection', function () { }); it('should navigate the user to eth-phishing-detect to dispute a block from MetaMask', async function () { + // Must be site on actual eth-phishing-detect blocklist + const phishingSite = new URL('https://test.metamask-phishing.io'); + await withFixtures( { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, - dapp: true, - failOnConsoleError: false, - }, - async ({ driver }) => { - await driver.navigate(); - await driver.fill('#password', 'correct horse battery staple'); - await driver.press('#password', driver.Key.ENTER); - await openDapp(driver); - - await driver.clickElement({ text: 'report a detection problem.' }); - - // wait for page to load before checking URL. - await driver.findElement({ text: 'Empty page' }); - assert.equal( - await driver.getCurrentUrl(), - `https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20127.0.0.1&body=http%3A%2F%2F127.0.0.1%3A8080%2F`, - ); - }, - ); - }); - - it('should navigate the user to PhishFort to dispute a block from MetaMask', async function () { - await withFixtures( - { - fixtures: new FixtureBuilder().build(), - ganacheOptions, - title: this.test.title, - testSpecificMock: (mockServer) => { - setupPhishingDetectionMocks(mockServer, { - statusCode: 200, - json: { - version: 2, - tolerance: 2, - fuzzylist: [], - whitelist: [], - blacklist: [], - lastUpdated: 0, - }, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: [phishingSite.hostname], }); }, dapp: true, @@ -202,12 +213,51 @@ describe('Phishing Detection', function () { await driver.navigate(); await driver.fill('#password', 'correct horse battery staple'); await driver.press('#password', driver.Key.ENTER); - await openDapp(driver); + await driver.openNewPage(phishingSite.href); await driver.clickElement({ text: 'report a detection problem.' }); // wait for page to load before checking URL. - await driver.findElement({ text: 'Empty page' }); + await driver.findElement({ + text: `Empty page by ${BlockProvider.MetaMask}`, + }); + assert.equal( + await driver.getCurrentUrl(), + `https://github.com/MetaMask/eth-phishing-detect/issues/new?title=[Legitimate%20Site%20Blocked]%20${encodeURIComponent( + phishingSite.hostname, + )}&body=${encodeURIComponent(phishingSite.href)}`, + ); + }, + ); + }); + + it('should navigate the user to PhishFort to dispute a Phishfort Block', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions, + title: this.test.title, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.PhishFort, + blocklist: ['127.0.0.1'], + }); + }, + dapp: true, + failOnConsoleError: false, + }, + async ({ driver }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + await driver.openNewPage('http://127.0.0.1:8080'); + + await driver.clickElement({ text: 'report a detection problem.' }); + + // wait for page to load before checking URL. + await driver.findElement({ + text: `Empty page by ${BlockProvider.PhishFort}`, + }); assert.equal( await driver.getCurrentUrl(), `https://github.com/phishfort/phishfort-lists/issues/new?title=[Legitimate%20Site%20Blocked]%20127.0.0.1&body=http%3A%2F%2F127.0.0.1%3A8080%2F`, @@ -222,7 +272,12 @@ describe('Phishing Detection', function () { fixtures: new FixtureBuilder().build(), ganacheOptions, title: this.test.title, - testSpecificMock: mockPhishingDetection, + testSpecificMock: async (mockServer) => { + return setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + blocklist: ['127.0.0.1'], + }); + }, dapp: true, dappPaths: ['mock-page-with-disallowed-iframe'], dappOptions: { diff --git a/yarn.lock b/yarn.lock index 83b49a9a7..144acb323 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3992,7 +3992,7 @@ __metadata: languageName: node linkType: hard -"@metamask/controller-utils@npm:^3.0.0, @metamask/controller-utils@npm:^3.4.0": +"@metamask/controller-utils@npm:^3.0.0, @metamask/controller-utils@npm:^3.1.0, @metamask/controller-utils@npm:^3.4.0": version: 3.4.0 resolution: "@metamask/controller-utils@npm:3.4.0" dependencies: @@ -4511,16 +4511,16 @@ __metadata: languageName: node linkType: hard -"@metamask/phishing-controller@npm:^3.0.0": - version: 3.0.0 - resolution: "@metamask/phishing-controller@npm:3.0.0" +"@metamask/phishing-controller@npm:^4.0.0": + version: 4.0.0 + resolution: "@metamask/phishing-controller@npm:4.0.0" dependencies: "@metamask/base-controller": ^2.0.0 - "@metamask/controller-utils": ^3.0.0 + "@metamask/controller-utils": ^3.1.0 "@types/punycode": ^2.1.0 eth-phishing-detect: ^1.2.0 punycode: ^2.1.1 - checksum: b0b9a86cba1928f0fd22a2aed196d75dc19a5e56547efe1b533d7ae06eaaf9432a6ee5004a8fd477f52310b50c2f3635a1e70ac83e3670f4cc6a1f488a674d73 + checksum: 15de581f7bec21d75531167275c68d7bbeae7fdaad02268749ba0a71c4d3ccb53718d963d6583e90c337407f65b7fcc9a89eb76c6f731802c2668a8425d5df89 languageName: node linkType: hard @@ -24504,7 +24504,7 @@ __metadata: "@metamask/notification-controller": ^3.0.0 "@metamask/obs-store": ^8.1.0 "@metamask/permission-controller": ^4.0.0 - "@metamask/phishing-controller": ^3.0.0 + "@metamask/phishing-controller": ^4.0.0 "@metamask/phishing-warning": ^2.1.0 "@metamask/post-message-stream": ^6.0.0 "@metamask/providers": ^11.1.0 From d09f375b2ce575ad86aa770c82ab61dc00fa3f8c Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Mon, 31 Jul 2023 19:43:51 -0230 Subject: [PATCH 16/31] Fix migration 77 (#20276) * Handle the case where tokensChainsCache data is undefined in migration 77 * Delete parts of state that should have been removed in migrations 82,84,86 and 88 * Create 077-supplements.md * Update 077-supplements.md * Update 077-supplements/*.js code comments * Fix types and jsdoc * Type/lint fix * Cleanup * Add 'should set data to an empty object if it is null' test case to 077.test.js * Update app/scripts/migrations/077.test.js Co-authored-by: Mark Stacey * Modify deletion criteria so that all decimal chain id proprties are deleted in migration 88 supplement * Readme.md * Update app/scripts/migrations/077.test.js Co-authored-by: Mark Stacey * Update app/scripts/migrations/077.test.js Co-authored-by: Mark Stacey * Update app/scripts/migrations/077.test.js Co-authored-by: Mark Stacey * Lint fix * Only delete decimal chain id keyed-entries in migration 88 supplement if there are hexadecimal keyed entries as well * Remove redundant test * Add 'does not delete' cases for nftcontroller related tests in 077.test.js --------- Co-authored-by: Mark Stacey --- .../077-supplements/077-supplement-for-082.ts | 25 + .../077-supplements/077-supplement-for-084.ts | 24 + .../077-supplements/077-supplement-for-086.ts | 23 + .../077-supplements/077-supplement-for-088.ts | 152 ++ .../077-supplements/077-supplements.md | 100 ++ app/scripts/migrations/077.js | 18 +- app/scripts/migrations/077.test.js | 1247 +++++++++++++++++ 7 files changed, 1585 insertions(+), 4 deletions(-) create mode 100644 app/scripts/migrations/077-supplements/077-supplement-for-082.ts create mode 100644 app/scripts/migrations/077-supplements/077-supplement-for-084.ts create mode 100644 app/scripts/migrations/077-supplements/077-supplement-for-086.ts create mode 100644 app/scripts/migrations/077-supplements/077-supplement-for-088.ts create mode 100644 app/scripts/migrations/077-supplements/077-supplements.md diff --git a/app/scripts/migrations/077-supplements/077-supplement-for-082.ts b/app/scripts/migrations/077-supplements/077-supplement-for-082.ts new file mode 100644 index 000000000..149b2ab0c --- /dev/null +++ b/app/scripts/migrations/077-supplements/077-supplement-for-082.ts @@ -0,0 +1,25 @@ +import { hasProperty, isObject } from '@metamask/utils'; + +/** + * Deletes frequentRpcListDetail if networkConfigurations exists, on the NetworkController state. + * Further explanation in ./077-supplements.md + * + * @param state - The persisted MetaMask state, keyed by controller. + * @returns Updated versioned MetaMask extension state. + */ + +export default function transformState077For082( + state: Record, +) { + if ( + hasProperty(state, 'PreferencesController') && + isObject(state.PreferencesController) && + hasProperty(state.PreferencesController, 'frequentRpcListDetail') && + isObject(state.NetworkController) && + hasProperty(state.NetworkController, 'networkConfigurations') + ) { + delete state.PreferencesController.frequentRpcListDetail; + } + + return { ...state }; +} diff --git a/app/scripts/migrations/077-supplements/077-supplement-for-084.ts b/app/scripts/migrations/077-supplements/077-supplement-for-084.ts new file mode 100644 index 000000000..397efec01 --- /dev/null +++ b/app/scripts/migrations/077-supplements/077-supplement-for-084.ts @@ -0,0 +1,24 @@ +import { hasProperty, isObject } from '@metamask/utils'; + +/** + * Deletes network if networkId exists, on the NetworkController state. + * Further explanation in ./077-supplements.md + * + * @param state - The persisted MetaMask state, keyed by controller. + * @returns Updated versioned MetaMask extension state. + */ + +export default function transformState077For084( + state: Record, +) { + if ( + hasProperty(state, 'NetworkController') && + isObject(state.NetworkController) && + hasProperty(state.NetworkController, 'network') && + hasProperty(state.NetworkController, 'networkId') + ) { + delete state.NetworkController.network; + } + + return { ...state }; +} diff --git a/app/scripts/migrations/077-supplements/077-supplement-for-086.ts b/app/scripts/migrations/077-supplements/077-supplement-for-086.ts new file mode 100644 index 000000000..bad44820e --- /dev/null +++ b/app/scripts/migrations/077-supplements/077-supplement-for-086.ts @@ -0,0 +1,23 @@ +import { hasProperty, isObject } from '@metamask/utils'; + +/** + * Prior to token detection v2 the data property in tokensChainsCache was an array, + * in v2 we changes that to an object. In this migration we are converting the data as array to object. + * + * @param state - The persisted MetaMask state, keyed by controller. + * @returns Updated versioned MetaMask extension state. + */ +export default function transformState077For086( + state: Record, +) { + if ( + hasProperty(state, 'NetworkController') && + isObject(state.NetworkController) && + hasProperty(state.NetworkController, 'provider') && + hasProperty(state.NetworkController, 'providerConfig') + ) { + delete state.NetworkController.provider; + } + + return { ...state }; +} diff --git a/app/scripts/migrations/077-supplements/077-supplement-for-088.ts b/app/scripts/migrations/077-supplements/077-supplement-for-088.ts new file mode 100644 index 000000000..4d430865a --- /dev/null +++ b/app/scripts/migrations/077-supplements/077-supplement-for-088.ts @@ -0,0 +1,152 @@ +import { hasProperty, isObject, isStrictHexString } from '@metamask/utils'; + +/** + * Deletes properties of `NftController.allNftContracts`, `NftController.allNfts`, + * `TokenListController.tokensChainsCache`, `TokensController.allTokens`, + * `TokensController.allIgnoredTokens` and `TokensController.allDetectedTokens` if + * their keyed by decimal number chainId and another hexadecimal chainId property + * exists within the same object. + * Further explanation in ./077-supplements.md + * + * @param state - The persisted MetaMask state, keyed by controller. + * @returns Updated versioned MetaMask extension state. + */ +export default function transformState077For086( + state: Record, +): Record { + if (hasProperty(state, 'NftController') && isObject(state.NftController)) { + const nftControllerState = state.NftController; + + // Migrate NftController.allNftContracts + if ( + hasProperty(nftControllerState, 'allNftContracts') && + isObject(nftControllerState.allNftContracts) + ) { + const { allNftContracts } = nftControllerState; + + if ( + Object.keys(allNftContracts).every((address) => + isObject(allNftContracts[address]), + ) + ) { + Object.keys(allNftContracts).forEach((address) => { + const nftContractsByChainId = allNftContracts[address]; + if ( + isObject(nftContractsByChainId) && + anyKeysAreHex(nftContractsByChainId) + ) { + for (const chainId of Object.keys(nftContractsByChainId)) { + if (!isStrictHexString(chainId)) { + delete nftContractsByChainId[chainId]; + } + } + } + }); + } + } + + // Migrate NftController.allNfts + if ( + hasProperty(nftControllerState, 'allNfts') && + isObject(nftControllerState.allNfts) + ) { + const { allNfts } = nftControllerState; + + if (Object.keys(allNfts).every((address) => isObject(allNfts[address]))) { + Object.keys(allNfts).forEach((address) => { + const nftsByChainId = allNfts[address]; + if (isObject(nftsByChainId) && anyKeysAreHex(nftsByChainId)) { + for (const chainId of Object.keys(nftsByChainId)) { + if (!isStrictHexString(chainId)) { + delete nftsByChainId[chainId]; + } + } + } + }); + } + } + + state.NftController = nftControllerState; + } + + if ( + hasProperty(state, 'TokenListController') && + isObject(state.TokenListController) + ) { + const tokenListControllerState = state.TokenListController; + + // Migrate TokenListController.tokensChainsCache + if ( + hasProperty(tokenListControllerState, 'tokensChainsCache') && + isObject(tokenListControllerState.tokensChainsCache) && + anyKeysAreHex(tokenListControllerState.tokensChainsCache) + ) { + for (const chainId of Object.keys( + tokenListControllerState.tokensChainsCache, + )) { + if (!isStrictHexString(chainId)) { + delete tokenListControllerState.tokensChainsCache[chainId]; + } + } + } + } + + if ( + hasProperty(state, 'TokensController') && + isObject(state.TokensController) + ) { + const tokensControllerState = state.TokensController; + + // Migrate TokensController.allTokens + if ( + hasProperty(tokensControllerState, 'allTokens') && + isObject(tokensControllerState.allTokens) && + anyKeysAreHex(tokensControllerState.allTokens) + ) { + const { allTokens } = tokensControllerState; + + for (const chainId of Object.keys(allTokens)) { + if (!isStrictHexString(chainId)) { + delete tokensControllerState.allTokens[chainId]; + } + } + } + + // Migrate TokensController.allIgnoredTokens + if ( + hasProperty(tokensControllerState, 'allIgnoredTokens') && + isObject(tokensControllerState.allIgnoredTokens) && + anyKeysAreHex(tokensControllerState.allIgnoredTokens) + ) { + const { allIgnoredTokens } = tokensControllerState; + + for (const chainId of Object.keys(allIgnoredTokens)) { + if (!isStrictHexString(chainId)) { + delete tokensControllerState.allIgnoredTokens[chainId]; + } + } + } + + // Migrate TokensController.allDetectedTokens + if ( + hasProperty(tokensControllerState, 'allDetectedTokens') && + isObject(tokensControllerState.allDetectedTokens) && + anyKeysAreHex(tokensControllerState.allDetectedTokens) + ) { + const { allDetectedTokens } = tokensControllerState; + + for (const chainId of Object.keys(allDetectedTokens)) { + if (!isStrictHexString(chainId)) { + delete tokensControllerState.allDetectedTokens[chainId]; + } + } + } + + state.TokensController = tokensControllerState; + } + return state; +} + +function anyKeysAreHex(obj: Record) { + return Object.keys(obj).some((chainId) => isStrictHexString(chainId)); +} diff --git a/app/scripts/migrations/077-supplements/077-supplements.md b/app/scripts/migrations/077-supplements/077-supplements.md new file mode 100644 index 000000000..ca40d9852 --- /dev/null +++ b/app/scripts/migrations/077-supplements/077-supplements.md @@ -0,0 +1,100 @@ +# 077 Supplements + +As of the time this file was first PR'd, we had not yet had to do what was done in this PR, which is to fix an old migration and also supplement it with state transformations +to handle problems that could be introduced by future migrations. + +The document explains the need for these new state transformations and the rationale behind each. It also explains why other state transformations were not included. + +## Background + +As of release 10.34.0, we started having a `No metadata found for 'previousProviderStore'` error thrown from the `deriveStateFromMetadata` function in `BaseControllerV2.js`. +This was occuring when there was data on the NetworkController state for which the NetworkController + BaseController expect metadata, but no metadata exists. In particular, +`previousProviderStore` was on the NetworkController state when it should not have been. + +`previousProviderStore` should not have been on the NetworkController state because of migration 85, which explictly deletes it. + +We discovered that for some users, that migration had failed to run because of an error in an earlier migration: `TypeError#1: MetaMask Migration Error #77: Cannot convert undefined or null to object`. +This error was thrown from this line https://github.com/MetaMask/metamask-extension/commit/8f18e04b97af02e5a8a72e3e4872aac66595d1d8#diff-9e76a7c60c1e37cd949f729222338b23ab743e44938ccf63a4a6dab7d84ed8bcR38 + +So the `data` property of the objects within `TokenListController.tokensChainsCache` could be undefined, and migration 77 didn't handle that case. It could be undefined because of the way the assets controller +code was as of the core controller libraries 14.0.2 release https://github.com/MetaMask/core/blame/19f7bf3b9fd8abe6cef9cb1ac1fe831d9f651ae0/src/assets/TokenListController.ts#L270 (the `safelyExecute` call there +will return undefined if the network call failed) + +For users who were in that situation, where a `TokenListController.tokensChainsCache[chainId].data` property was undefined, some significant problems would occur after updating to v10.24.0, which is the +release where migration 77 was shipped. In particular, migration 77 would fail, and all subsequent migrations would not run. The most plain case of this would be a user who was on v10.23.0 +with `TokenListController.tokensChainsCache[chainId].data === undefined`. Then suppose they didn't update until v10.34.0. None of migrations 77-89 would run. Leaving their state in a form that doesn't match +with assumptions throughout our codebase. Various bugs could be caused, but the sentry error describe above is the worst case, where MetaMask simply could not be opened and users would hit the error screen. + +To correct this situation we had to fix migration 77. Once we do that, all users who were in this situation (and then upgraded to the version which included the fixes for migration 77) would have all migrations +from 77 upwards run for the first time. This could be problematic for users who had used the wallet on versions 10.24.0-10.34.0, where our controllers would be writing data to state under the assumption that +the migrations had run. + +As such, we had to also add code to migration 77 to avoid new errors being introduced by the migrations running on code that had been modified by controllers on versions 10.24.0 to 10.34.0 + +## Introducing migration 77 supplements + +To correct the aforementioned problems with the data, new state transformation functions were added to this directory, to be run in migration 77 after the pre-existing migration 77 code had run. +Each file in this directory exports a state transformation function which is then imported in the migration 77 file and applied to state in sequence, after the state transformation function in +077.js itself has run and returns state. These have been split into their own files for each of use, and so that they could be grouped with this documentation. + +## The chosen supplements + +We added supplements for migrations 82, 84, 86 and 88 for the following reasons and with the following effects -> + +**Migration 82** + +Attempts to convert `frequentRpcListDetail` - an array of objects with network related data - to a `networkConfigurations` object, with the objects that were in the array keyed by a unique id. +If this migration had not run, then (prior to v10.34.0) a user would still have been able to use MetaMask, but the data they had in `frequentRpcListDetail` would now be ignored by application code, +and subsequent network data would between written to and modified in state in the `networkConfigurations` object. If migration 82 was later run (after fixing migration 77), the old `frequentRpcListDetail` +data could overwrite the existing `networkConfigurations` data, and the user could lose `networkConfigurations` data that had been written to their state since migration 82 had first failed to run. + +To fix this, the migration 82 supplement deletes `frequentRpcListDetail` if the `networkConfigurations` object exists. Users in such a scenario will have network data in `networkConfigurations` that +they have been using, while the `frequentRpcListDetail` data would not have been seen for some time. So the best thing to do for them is delete their old data and preserve the data they have most recently +used. + +**Migration 84** + +Replaces the `NetworkController.network` property with a `networkId` and `networkStatus` property. If this migration had not run, the NetworkController would have a `network` property and +`networkId` and `networkStatus` properties. If migration 84 later ran on this state, the old (and forgotten) `network` property could cause the `networkId` and `networkStatus` to be overwritten, +affecting the state the user's experience was currently depending on. + +The fix in the migration 84 supplement is to delete the `NetworkController.network` property if the `NetworkId` property already exists. + +**Migration 86** + +Renamed the `NetworkController.provider` property to `providerConfig`. If this migration had not run, the NetworkController would have a `provider` property and +a `providerConfig` property. If migration 86 later ran on this state, the old (and forgotten) `provider` property could cause the `providerConfig` property to be overwritten, +affecting the state the user's experience was currently depending on. + +The fix in the migration 86 supplement is to delete the `NetworkController.provider` property if the `providerConfig` property already exists. + +**Migration 88** + +Attempted to change the keys of multiple parts of state related to tokens. In particular, `NftController.allNftContracts`, `NftController.allNfts`, `TokenListController.tokensChainsCache`, `TokensController.allTokens`, `TokensController.allIgnoredTokens` and `TokensController.allDetectedTokens`. All of these objects were keyed by chainId in decimal number form. The migration's +purpose was to change those decimal chain ID keys to hexadecimal. If migration 77 failed, and then the user added or modified tokens, they could have duplicates within these parts of state: +some with decimal keys and others with an equivalent hexadecimal key. If the data pointed to by those keys was modified at all, and the migration 88 was later run, the most recent data (under +the hexadecimal key) could be overwritten by the old data under the decimal key. + +The migration 88 supplement fixes this by deleting the properties with decimal keys if an equivalent hexadecimal key exists. + +## Migrations that were not supplemented + +**Migration 78** was not supplemented because it only deletes data; it does not overwrite data. It's failure to run will have left rogue data in state, but that will be removed when it is run after the migration +77 fix. + +**Migration 79** was not supplemented because it only deletes data; it does not overwrite data. + +**Migration 80** was not supplemented because it only deletes data; it does not overwrite data. + +**Migration 81** was not supplemented because it modifies data that could only be in state on a flask build. The bug that caused the undefined data in tokenlistcontroller state was present on v14.0.2 and v14.1.0 of +the controllers, but fixed in v14.2.0 of the controllers. By the time flask was released to prod, controllers was at v25.0 + +**Migration 83** just builds on migration 82. No additional fix is needed for 83 given that we have the 82 supplement. + +**Migration 85** was not supplemented because it only deletes data; it does not overwrite data. + +**Migration 87** was not supplemented because it only deletes data; it does not overwrite data. + +**Migration 89** just builds on migration 82 and 84. No additional fix is needed for 89 given that we have the 82 and 84 supplement. + +**Migration 90** was not supplemented because it only deletes data; it does not overwrite data. diff --git a/app/scripts/migrations/077.js b/app/scripts/migrations/077.js index 141cbb142..5a5d4f1ac 100644 --- a/app/scripts/migrations/077.js +++ b/app/scripts/migrations/077.js @@ -1,4 +1,8 @@ import { cloneDeep } from 'lodash'; +import transformState077For082 from './077-supplements/077-supplement-for-082'; +import transformState077For084 from './077-supplements/077-supplement-for-084'; +import transformState077For086 from './077-supplements/077-supplement-for-086'; +import transformState077For088 from './077-supplements/077-supplement-for-088'; const version = 77; @@ -12,7 +16,13 @@ export default { const versionedData = cloneDeep(originalVersionedData); versionedData.meta.version = version; const state = versionedData.data; - const newState = transformState(state); + let newState = transformState(state); + + newState = transformState077For082(newState); + newState = transformState077For084(newState); + newState = transformState077For086(newState); + newState = transformState077For088(newState); + versionedData.data = newState; return versionedData; }, @@ -27,7 +37,7 @@ function transformState(state) { let dataObject; // eslint-disable-next-line for (const chainId in tokensChainsCache) { - dataCache = tokensChainsCache[chainId].data; + dataCache = tokensChainsCache[chainId].data || {}; dataObject = {}; // if the data is array conver that to object if (Array.isArray(dataCache)) { @@ -35,8 +45,8 @@ function transformState(state) { dataObject[token.address] = token; } } else if ( - Object.keys(dataCache)[0].toLowerCase() !== - dataCache[Object.keys(dataCache)[0]].address.toLowerCase() + Object.keys(dataCache)[0]?.toLowerCase() !== + dataCache[Object.keys(dataCache)[0]]?.address?.toLowerCase() ) { // for the users who already updated to the recent version // and the dataCache is already an object keyed with 0,1,2,3 etc diff --git a/app/scripts/migrations/077.test.js b/app/scripts/migrations/077.test.js index 1c16420ec..53efb5cd5 100644 --- a/app/scripts/migrations/077.test.js +++ b/app/scripts/migrations/077.test.js @@ -82,6 +82,100 @@ describe('migration #77', () => { }, }); }); + it('should set data to an empty object if it is undefined', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: undefined, + }, + }, + }, + }, + }; + const newStorage = await migration77.migrate(oldStorage); + expect(newStorage).toStrictEqual({ + meta: { + version: 77, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: {}, + }, + }, + }, + }, + }); + }); + it('should set data to an empty object if it is null', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: null, + }, + }, + }, + }, + }; + const newStorage = await migration77.migrate(oldStorage); + expect(newStorage).toStrictEqual({ + meta: { + version: 77, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: {}, + }, + }, + }, + }, + }); + }); it('should change the data from array to object for a multiple networks', async () => { const oldStorage = { meta: { @@ -319,4 +413,1157 @@ describe('migration #77', () => { }, }); }); + + describe('migration #77 supplements', () => { + describe('state transformation to ahead of migration 82', () => { + it('should delete frequentRpcListDetail from the PreferencesController state, if the user already has networkConfigurations in NetworkController state, without interferring with the rest of the migration', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: [ + { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + ], + }, + }, + }, + PreferencesController: { + frequentRpcListDetail: ['foobar'], + fizz: 'buzz', + }, + NetworkController: { + networkConfigurations: { foo: 'bar' }, + }, + }, + }; + const newStorage = await migration77.migrate(oldStorage); + expect(newStorage).toStrictEqual({ + meta: { + version: 77, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + '0xc00e94cb662c3520282e6f5717214004a7f26888': { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + }, + }, + }, + }, + PreferencesController: { + fizz: 'buzz', + }, + NetworkController: { + networkConfigurations: { foo: 'bar' }, + }, + }, + }); + }); + + it('should not delete frequentRpcListDetail from the PreferencesController state if there are no networkConfigurations in NetworkController state', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + PreferencesController: { + frequentRpcListDetail: ['foobar'], + fizz: 'buzz', + }, + NetworkController: { + foobar: { foo: 'bar' }, + }, + }, + }; + const newStorage = await migration77.migrate(oldStorage); + expect(newStorage).toStrictEqual({ + meta: { + version: 77, + }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + PreferencesController: { + frequentRpcListDetail: ['foobar'], + fizz: 'buzz', + }, + NetworkController: { + foobar: { foo: 'bar' }, + }, + }, + }); + }); + }); + + describe('state transformation to ahead of migration 84', () => { + it('should delete `network` from the NetworkController state, if the user already has `networkId` in NetworkController state, without interferring with the rest of the migration', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: [ + { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + ], + }, + }, + }, + NetworkController: { + network: 'foobar', + networkId: 'fizzbuzz', + }, + }, + }; + const newStorage = await migration77.migrate(oldStorage); + expect(newStorage).toStrictEqual({ + meta: { + version: 77, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + '0xc00e94cb662c3520282e6f5717214004a7f26888': { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + }, + }, + }, + }, + NetworkController: { + networkId: 'fizzbuzz', + }, + }, + }); + }); + + it('should not delete `network` from the NetworkController state, if there is no `networkId` in NetworkController state', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + PreferencesController: { + frequentRpcListDetail: ['foobar'], + fizz: 'buzz', + }, + NetworkController: { + network: 'foobar', + foobar: { foo: 'bar' }, + }, + }, + }; + const newStorage = await migration77.migrate(oldStorage); + expect(newStorage).toStrictEqual({ + meta: { + version: 77, + }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + PreferencesController: { + frequentRpcListDetail: ['foobar'], + fizz: 'buzz', + }, + NetworkController: { + network: 'foobar', + foobar: { foo: 'bar' }, + }, + }, + }); + }); + }); + + describe('state transformation to ahead of migration 86', () => { + it('should delete `provider` from the NetworkController state, if the user already has `providerConfig` in NetworkController state, without interferring with the rest of the migration', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: [ + { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + ], + }, + }, + }, + NetworkController: { + provider: { foo: 'bar ' }, + providerConfig: { fizz: 'buzz' }, + }, + }, + }; + const newStorage = await migration77.migrate(oldStorage); + expect(newStorage).toStrictEqual({ + meta: { + version: 77, + }, + data: { + TokenListController: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + }, + tokensChainsCache: { + 1: { + timestamp: 1234, + data: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + '0xc00e94cb662c3520282e6f5717214004a7f26888': { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + }, + }, + }, + }, + NetworkController: { + providerConfig: { fizz: 'buzz' }, + }, + }, + }); + }); + + it('should not delete `provider` from the NetworkController state, if there is no `providerConfig` in NetworkController state', async () => { + const oldStorage = { + meta: { + version: 76, + }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + PreferencesController: { + frequentRpcListDetail: ['foobar'], + fizz: 'buzz', + }, + NetworkController: { + provider: { foo: 'bar ' }, + }, + }, + }; + const newStorage = await migration77.migrate(oldStorage); + expect(newStorage).toStrictEqual({ + meta: { + version: 77, + }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + PreferencesController: { + frequentRpcListDetail: ['foobar'], + fizz: 'buzz', + }, + NetworkController: { + provider: { foo: 'bar ' }, + }, + }, + }); + }); + }); + + describe('state transformation to ahead of migration 88', () => { + it('deletes entries in NftController.allNftContracts that have decimal chain ID keys only if any chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 76 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + NftController: { + allNftContracts: { + '0x111': { + 16: [ + { + name: 'Contract 1', + address: '0xaaa', + }, + ], + '0x20': [ + { + name: 'Contract 2', + address: '0xbbb', + }, + ], + 32: [ + { + name: 'Contract 2', + address: '0xbbb', + }, + ], + }, + '0x222': { + 64: [ + { + name: 'Contract 3', + address: '0xccc', + }, + ], + '0x40': [ + { + name: 'Contract 3', + address: '0xccc', + }, + ], + 128: [ + { + name: 'Contract 4', + address: '0xddd', + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + NftController: { + allNftContracts: { + '0x111': { + '0x20': [ + { + name: 'Contract 2', + address: '0xbbb', + }, + ], + }, + '0x222': { + '0x40': [ + { + name: 'Contract 3', + address: '0xccc', + }, + ], + }, + }, + }, + }); + }); + + it('does not delete entries in NftController.allNftContracts that have decimal chain ID keys if no other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 76 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + NftController: { + allNftContracts: { + '0x333': { + 256: [ + { + name: 'Contract 3', + address: '0xccc', + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + NftController: { + allNftContracts: { + '0x333': { + 256: [ + { + name: 'Contract 3', + address: '0xccc', + }, + ], + }, + }, + }, + }); + }); + + it('deletes entries in NftController.allNfts that have decimal chain ID keys only if any chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 76 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + NftController: { + allNfts: { + '0x111': { + 16: [ + { + name: 'NFT 1', + description: 'Description for NFT 1', + image: 'nft1.jpg', + standard: 'ERC721', + tokenId: '1', + address: '0xaaa', + }, + ], + 32: [ + { + name: 'NFT 2', + description: 'Description for NFT 2', + image: 'nft2.jpg', + standard: 'ERC721', + tokenId: '2', + address: '0xbbb', + }, + ], + '0x20': [ + { + name: 'NFT 2', + description: 'Description for NFT 2', + image: 'nft2.jpg', + standard: 'ERC721', + tokenId: '2', + address: '0xbbb', + }, + ], + }, + '0x222': { + 64: [ + { + name: 'NFT 3', + description: 'Description for NFT 3', + image: 'nft3.jpg', + standard: 'ERC721', + tokenId: '3', + address: '0xccc', + }, + ], + '0x40': [ + { + name: 'NFT 3', + description: 'Description for NFT 3', + image: 'nft3.jpg', + standard: 'ERC721', + tokenId: '3', + address: '0xccc', + }, + ], + 128: [ + { + name: 'NFT 4', + description: 'Description for NFT 4', + image: 'nft4.jpg', + standard: 'ERC721', + tokenId: '4', + address: '0xddd', + }, + ], + }, + '0x333': { + 256: [ + { + name: 'NFT 3', + description: 'Description for NFT 3', + image: 'nft3.jpg', + standard: 'ERC721', + tokenId: '3', + address: '0xccc', + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + NftController: { + allNfts: { + '0x111': { + '0x20': [ + { + name: 'NFT 2', + description: 'Description for NFT 2', + image: 'nft2.jpg', + standard: 'ERC721', + tokenId: '2', + address: '0xbbb', + }, + ], + }, + '0x222': { + '0x40': [ + { + name: 'NFT 3', + description: 'Description for NFT 3', + image: 'nft3.jpg', + standard: 'ERC721', + tokenId: '3', + address: '0xccc', + }, + ], + }, + '0x333': { + 256: [ + { + name: 'NFT 3', + description: 'Description for NFT 3', + image: 'nft3.jpg', + standard: 'ERC721', + tokenId: '3', + address: '0xccc', + }, + ], + }, + }, + }, + }); + }); + + it('does not delete entries in NftController.allNfts that have decimal chain ID keys if no other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 76 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + NftController: { + allNfts: { + '0x333': { + 256: [ + { + name: 'NFT 3', + description: 'Description for NFT 3', + image: 'nft3.jpg', + standard: 'ERC721', + tokenId: '3', + address: '0xccc', + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + NftController: { + allNfts: { + '0x333': { + 256: [ + { + name: 'NFT 3', + description: 'Description for NFT 3', + image: 'nft3.jpg', + standard: 'ERC721', + tokenId: '3', + address: '0xccc', + }, + ], + }, + }, + }, + }); + }); + + it('deletes entries in TokenListController.tokensChainsCache that have decimal chain ID keys only if any other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 76 }, + data: { + TokenListController: { + tokensChainsCache: { + 16: { + timestamp: 111111, + data: [ + { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + ], + }, + '0x10': { + timestamp: 111111, + data: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + '0xc00e94cb662c3520282e6f5717214004a7f26888': { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + }, + }, + 32: { + timestamp: 222222, + data: [ + { + address: '0x3ee2200efb3400fabb9aacf31297cbdd1d435d47', + symbol: 'ADA', + decimals: 18, + }, + { + address: '0x928e55dab735aa8260af3cedada18b5f70c72f1b', + symbol: 'FRONT', + decimals: 18, + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: { + '0x10': { + timestamp: 111111, + data: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + '0xc00e94cb662c3520282e6f5717214004a7f26888': { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + }, + }, + }, + }, + }); + }); + it('does not delete entries in TokenListController.tokensChainsCache that have decimal chain ID keys if no other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 76 }, + data: { + TokenListController: { + tokensChainsCache: { + 16: { + timestamp: 111111, + data: [ + { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + ], + }, + 32: { + timestamp: 222222, + data: [ + { + address: '0x3ee2200efb3400fabb9aacf31297cbdd1d435d47', + symbol: 'ADA', + decimals: 18, + }, + { + address: '0x928e55dab735aa8260af3cedada18b5f70c72f1b', + symbol: 'FRONT', + decimals: 18, + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: { + 16: { + timestamp: 111111, + data: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + }, + '0xc00e94cb662c3520282e6f5717214004a7f26888': { + address: '0xc00e94cb662c3520282e6f5717214004a7f26888', + symbol: 'COMP', + decimals: 18, + }, + }, + }, + 32: { + timestamp: 222222, + data: { + '0x3ee2200efb3400fabb9aacf31297cbdd1d435d47': { + address: '0x3ee2200efb3400fabb9aacf31297cbdd1d435d47', + symbol: 'ADA', + decimals: 18, + }, + '0x928e55dab735aa8260af3cedada18b5f70c72f1b': { + address: '0x928e55dab735aa8260af3cedada18b5f70c72f1b', + symbol: 'FRONT', + decimals: 18, + }, + }, + }, + }, + }, + }); + }); + it('deletes entries in TokensController.allTokens that have decimal chain IDs only if any other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 76 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allTokens: { + 16: { + '0x111': [ + { + address: '0xaaa', + decimals: 1, + symbol: 'TEST1', + }, + ], + }, + '0x10': { + '0x111': [ + { + address: '0xaaa', + decimals: 1, + symbol: 'TEST1', + }, + ], + }, + 32: { + '0x222': [ + { + address: '0xbbb', + decimals: 1, + symbol: 'TEST2', + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allTokens: { + '0x10': { + '0x111': [ + { + address: '0xaaa', + decimals: 1, + symbol: 'TEST1', + }, + ], + }, + }, + }, + }); + }); + + it('does not delete entries in TokensController.allTokens that have decimal chain IDs if no other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 76 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allTokens: { + 16: { + '0x111': [ + { + address: '0xaaa', + decimals: 1, + symbol: 'TEST1', + }, + ], + }, + 32: { + '0x222': [ + { + address: '0xbbb', + decimals: 1, + symbol: 'TEST2', + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allTokens: { + 16: { + '0x111': [ + { + address: '0xaaa', + decimals: 1, + symbol: 'TEST1', + }, + ], + }, + 32: { + '0x222': [ + { + address: '0xbbb', + decimals: 1, + symbol: 'TEST2', + }, + ], + }, + }, + }, + }); + }); + + it('deletes entries in TokensController.allIgnoredTokens that have decimal chain IDs only if any other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allIgnoredTokens: { + 16: { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + '0x10': { + '0x1': { + '0x222': ['0xbbb'], + }, + }, + 32: { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allIgnoredTokens: { + '0x10': { + '0x1': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }); + }); + + it('does not delete entries in TokensController.allIgnoredTokens that have decimal chain IDs if no other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allIgnoredTokens: { + 16: { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + 32: { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allIgnoredTokens: { + 16: { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + 32: { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }); + }); + + it('deletes entries in TokensController.allDetectedTokens that have decimal chain IDs only if any other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allDetectedTokens: { + 16: { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + '0x10': { + '0x1': { + '0x222': ['0xbbb'], + }, + }, + 32: { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allDetectedTokens: { + '0x10': { + '0x1': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }); + }); + + it('does not delete entries in TokensController.allDetectedTokens that have decimal chain IDs if no other chain ID keys are hex', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allDetectedTokens: { + 16: { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + 32: { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }, + }; + + const newStorage = await migration77.migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: {}, + }, + TokensController: { + allDetectedTokens: { + 16: { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + 32: { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }); + }); + }); + }); }); From 6f0caf4d3f269fc3da3307904162f4120566a916 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Mon, 31 Jul 2023 20:35:50 -0230 Subject: [PATCH 17/31] Capture Sentry errors prior to initialization (#20265) (#20330) * Capture Sentry errors prior to initialization Sentry errors captured before/during the wallet initialization are currently not captured because we don't have the controller state yet to determine whether the user has consented. The Sentry setup has been updated to check the persisted state for whether the user has consented, as a fallback in case the controller state hasn't been initialized yet. This ensures that we capture errors during initialization if the user has opted in. * Always await async check for whether the user has opted in * Remove unused import * Update JSDoc return type * Remove unused driver method * Fix metametrics controller unit tests * Fix e2e tests * Fix e2e test on Firefox * Start session upon install rather than toggle Co-authored-by: Mark Stacey --- app/scripts/background.js | 4 + app/scripts/lib/sentry-filter-events.ts | 8 +- app/scripts/lib/setup-persisted-state-hook.js | 10 + app/scripts/lib/setupSentry.js | 20 +- app/scripts/ui.js | 4 + test/e2e/tests/errors.spec.js | 199 ++++++++++++++---- 6 files changed, 195 insertions(+), 50 deletions(-) create mode 100644 app/scripts/lib/setup-persisted-state-hook.js diff --git a/app/scripts/background.js b/app/scripts/background.js index e3416b796..44d3870ad 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -2,6 +2,10 @@ * @file The entry point for the web extension singleton process. */ +// This import sets up a global function required for Sentry to function. +// It must be run first in case an error is thrown later during initialization. +import './lib/setup-persisted-state-hook'; + import EventEmitter from 'events'; import endOfStream from 'end-of-stream'; import pump from 'pump'; diff --git a/app/scripts/lib/sentry-filter-events.ts b/app/scripts/lib/sentry-filter-events.ts index 050f0bcd2..50d2f4c77 100644 --- a/app/scripts/lib/sentry-filter-events.ts +++ b/app/scripts/lib/sentry-filter-events.ts @@ -29,7 +29,7 @@ export class FilterEvents implements Integration { * @returns `true` if MetaMask's state has been initialized, and MetaMetrics * is enabled, `false` otherwise. */ - private getMetaMetricsEnabled: () => boolean; + private getMetaMetricsEnabled: () => Promise; /** * @param options - Constructor options. @@ -40,7 +40,7 @@ export class FilterEvents implements Integration { constructor({ getMetaMetricsEnabled, }: { - getMetaMetricsEnabled: () => boolean; + getMetaMetricsEnabled: () => Promise; }) { this.getMetaMetricsEnabled = getMetaMetricsEnabled; } @@ -56,13 +56,13 @@ export class FilterEvents implements Integration { addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub, ): void { - addGlobalEventProcessor((currentEvent: SentryEvent) => { + addGlobalEventProcessor(async (currentEvent: SentryEvent) => { // Sentry integrations use the Sentry hub to get "this" references, for // reasons I don't fully understand. // eslint-disable-next-line consistent-this const self = getCurrentHub().getIntegration(FilterEvents); if (self) { - if (!self.getMetaMetricsEnabled()) { + if (!(await self.getMetaMetricsEnabled())) { logger.warn(`Event dropped due to MetaMetrics setting.`); return null; } diff --git a/app/scripts/lib/setup-persisted-state-hook.js b/app/scripts/lib/setup-persisted-state-hook.js new file mode 100644 index 000000000..9b29fad26 --- /dev/null +++ b/app/scripts/lib/setup-persisted-state-hook.js @@ -0,0 +1,10 @@ +import LocalStore from './local-store'; +import ReadOnlyNetworkStore from './network-store'; + +const localStore = process.env.IN_TEST + ? new ReadOnlyNetworkStore() + : new LocalStore(); + +globalThis.stateHooks.getPersistedState = async function () { + return await localStore.get(); +}; diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index bac3c79ce..dd0eb7bd6 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -118,16 +118,20 @@ export default function setupSentry({ release, getState }) { * @returns `true` if MetaMask's state has been initialized, and MetaMetrics * is enabled, `false` otherwise. */ - function getMetaMetricsEnabled() { - if (getState) { - const appState = getState(); - if (!appState?.store?.metamask?.participateInMetaMetrics) { - return false; - } - } else { + async function getMetaMetricsEnabled() { + const appState = getState(); + if (Object.keys(appState) > 0) { + return Boolean(appState?.store?.metamask?.participateInMetaMetrics); + } + try { + const persistedState = await globalThis.stateHooks.getPersistedState(); + return Boolean( + persistedState?.data?.MetaMetricsController?.participateInMetaMetrics, + ); + } catch (error) { + console.error(error); return false; } - return true; } Sentry.init({ diff --git a/app/scripts/ui.js b/app/scripts/ui.js index 148ce0397..565e8187a 100644 --- a/app/scripts/ui.js +++ b/app/scripts/ui.js @@ -4,6 +4,10 @@ import '@formatjs/intl-relativetimeformat/polyfill'; // dev only, "react-devtools" import is skipped in prod builds import 'react-devtools'; +// This import sets up a global function required for Sentry to function. +// It must be run first in case an error is thrown later during initialization. +import './lib/setup-persisted-state-hook'; + import PortStream from 'extension-port-stream'; import browser from 'webextension-polyfill'; diff --git a/test/e2e/tests/errors.spec.js b/test/e2e/tests/errors.spec.js index 194578558..960135215 100644 --- a/test/e2e/tests/errors.spec.js +++ b/test/e2e/tests/errors.spec.js @@ -1,9 +1,26 @@ const { strict: assert } = require('assert'); +const { Browser } = require('selenium-webdriver'); const { convertToHexValue, withFixtures } = require('../helpers'); const FixtureBuilder = require('../fixture-builder'); describe('Sentry errors', function () { - async function mockSentry(mockServer) { + const migrationError = + process.env.SELENIUM_BROWSER === Browser.CHROME + ? `Cannot read properties of undefined (reading 'version')` + : 'meta is undefined'; + async function mockSentryMigratorError(mockServer) { + return await mockServer + .forPost('https://sentry.io/api/0000000/envelope/') + .withBodyIncluding(migrationError) + .thenCallback(() => { + return { + statusCode: 200, + json: {}, + }; + }); + } + + async function mockSentryTestError(mockServer) { return await mockServer .forPost('https://sentry.io/api/0000000/envelope/') .withBodyIncluding('Test Error') @@ -23,43 +40,149 @@ describe('Sentry errors', function () { }, ], }; - it('should send error events', async function () { - await withFixtures( - { - fixtures: new FixtureBuilder() - .withMetaMetricsController({ - metaMetricsId: 'fake-metrics-id', - participateInMetaMetrics: true, - }) - .build(), - ganacheOptions, - title: this.test.title, - failOnConsoleError: false, - testSpecificMock: mockSentry, - }, - async ({ driver, mockedEndpoint }) => { - await driver.navigate(); - await driver.fill('#password', 'correct horse battery staple'); - await driver.press('#password', driver.Key.ENTER); - // Trigger error - driver.executeScript('window.stateHooks.throwTestError()'); - // Wait for Sentry request - await driver.wait(async () => { + + describe('before initialization', function () { + it('should NOT send error events when participateInMetaMetrics is false', async function () { + await withFixtures( + { + fixtures: { + ...new FixtureBuilder() + .withMetaMetricsController({ + metaMetricsId: null, + participateInMetaMetrics: false, + }) + .build(), + // Intentionally corrupt state to trigger migration error during initialization + meta: undefined, + }, + ganacheOptions, + title: this.test.title, + failOnConsoleError: false, + testSpecificMock: mockSentryMigratorError, + }, + async ({ driver, mockedEndpoint }) => { + await driver.navigate(); + + // Wait for Sentry request + await driver.delay(3000); const isPending = await mockedEndpoint.isPending(); - return isPending === false; - }, 10000); - const [mockedRequest] = await mockedEndpoint.getSeenRequests(); - const mockTextBody = mockedRequest.body.text.split('\n'); - const mockJsonBody = JSON.parse(mockTextBody[2]); - const { level, extra } = mockJsonBody; - const [{ type, value }] = mockJsonBody.exception.values; - const { participateInMetaMetrics } = extra.appState.store.metamask; - // Verify request - assert.equal(type, 'TestError'); - assert.equal(value, 'Test Error'); - assert.equal(level, 'error'); - assert.equal(participateInMetaMetrics, true); - }, - ); + assert.ok( + isPending, + 'A request to sentry was sent when it should not have been', + ); + }, + ); + }); + it('should send error events', async function () { + await withFixtures( + { + fixtures: { + ...new FixtureBuilder() + .withMetaMetricsController({ + metaMetricsId: 'fake-metrics-id', + participateInMetaMetrics: true, + }) + .build(), + // Intentionally corrupt state to trigger migration error during initialization + meta: undefined, + }, + ganacheOptions, + title: this.test.title, + failOnConsoleError: false, + testSpecificMock: mockSentryMigratorError, + }, + async ({ driver, mockedEndpoint }) => { + await driver.navigate(); + + // Wait for Sentry request + await driver.wait(async () => { + const isPending = await mockedEndpoint.isPending(); + return isPending === false; + }, 3000); + + const [mockedRequest] = await mockedEndpoint.getSeenRequests(); + const mockTextBody = mockedRequest.body.text.split('\n'); + const mockJsonBody = JSON.parse(mockTextBody[2]); + const { level } = mockJsonBody; + const [{ type, value }] = mockJsonBody.exception.values; + // Verify request + assert.equal(type, 'TypeError'); + assert(value.includes(migrationError)); + assert.equal(level, 'error'); + }, + ); + }); + }); + + describe('after initialization', function () { + it('should NOT send error events when participateInMetaMetrics is false', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder() + .withMetaMetricsController({ + metaMetricsId: null, + participateInMetaMetrics: false, + }) + .build(), + ganacheOptions, + title: this.test.title, + failOnConsoleError: false, + testSpecificMock: mockSentryTestError, + }, + async ({ driver, mockedEndpoint }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + // Trigger error + driver.executeScript('window.stateHooks.throwTestError()'); + driver.delay(3000); + // Wait for Sentry request + const isPending = await mockedEndpoint.isPending(); + assert.ok( + isPending, + 'A request to sentry was sent when it should not have been', + ); + }, + ); + }); + it('should send error events', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder() + .withMetaMetricsController({ + metaMetricsId: 'fake-metrics-id', + participateInMetaMetrics: true, + }) + .build(), + ganacheOptions, + title: this.test.title, + failOnConsoleError: false, + testSpecificMock: mockSentryTestError, + }, + async ({ driver, mockedEndpoint }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + // Trigger error + driver.executeScript('window.stateHooks.throwTestError()'); + // Wait for Sentry request + await driver.wait(async () => { + const isPending = await mockedEndpoint.isPending(); + return isPending === false; + }, 10000); + const [mockedRequest] = await mockedEndpoint.getSeenRequests(); + const mockTextBody = mockedRequest.body.text.split('\n'); + const mockJsonBody = JSON.parse(mockTextBody[2]); + const { level, extra } = mockJsonBody; + const [{ type, value }] = mockJsonBody.exception.values; + const { participateInMetaMetrics } = extra.appState.store.metamask; + // Verify request + assert.equal(type, 'TestError'); + assert.equal(value, 'Test Error'); + assert.equal(level, 'error'); + assert.equal(participateInMetaMetrics, true); + }, + ); + }); }); }); From 1ed8f99fdfd8e345b3b8bfbc7224a7f584052545 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Mon, 31 Jul 2023 22:26:40 -0230 Subject: [PATCH 18/31] Remove fallback phishing warning configuration (#20327) * Remove fallback phishing warning configuration The package `@metamask/phishing-controller` has been updated from v4 v6. The only breaking changes are a minimum Node.js version bump, and the removal of the fallback phishing configuration. The fallback phishing configuration was resulting in MetaMask being incorrectly flagged as malware, and the stale config was causing problems for sites that had been blocked in the past but have since been unblocked. This should substantially reduce the bundle size as well. * Update LavaMoat policies * Update test state to include example blocked site --------- Co-authored-by: MetaMask Bot --- app/scripts/metamask-controller.test.js | 22 +++++++++++-- lavamoat/browserify/beta/policy.json | 41 ++++++++++--------------- lavamoat/browserify/desktop/policy.json | 41 ++++++++++--------------- lavamoat/browserify/flask/policy.json | 41 ++++++++++--------------- lavamoat/browserify/main/policy.json | 41 ++++++++++--------------- lavamoat/browserify/mmi/policy.json | 41 ++++++++++--------------- package.json | 2 +- yarn.lock | 39 ++++++++++++++--------- 8 files changed, 124 insertions(+), 144 deletions(-) diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index d641699b9..2db150de3 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -177,6 +177,18 @@ const firstTimeState = { }, }, }, + PhishingController: { + phishingLists: [ + { + allowlist: [], + blocklist: ['test.metamask-phishing.io'], + fuzzylist: [], + tolerance: 0, + version: 0, + name: 'MetaMask', + }, + ], + }, }; const noop = () => undefined; @@ -205,7 +217,7 @@ describe('MetaMaskController', function () { eth_phishing_detect_config: { fuzzylist: [], allowlist: [], - blocklist: ['127.0.0.1'], + blocklist: ['test.metamask-phishing.io'], name: ListNames.MetaMask, }, phishfort_hotlist: { @@ -218,7 +230,11 @@ describe('MetaMaskController', function () { .reply( 200, JSON.stringify([ - { url: '127.0.0.1', targetList: 'blocklist', timestamp: 0 }, + { + url: 'test.metamask-phishing.io', + targetList: 'blocklist', + timestamp: 0, + }, ]), ); @@ -960,7 +976,7 @@ describe('MetaMaskController', function () { it('sets up phishing stream for untrusted communication', async function () { const phishingMessageSender = { - url: 'http://myethereumwalletntw.com', + url: 'http://test.metamask-phishing.io', tab: {}, }; diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index 9b750dc06..defef7b1a 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -884,8 +884,8 @@ "setTimeout": true }, "packages": { + "@metamask/controller-utils>@metamask/utils": true, "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, "browserify>buffer": true, "eslint>fast-deep-equal": true, "eth-ens-namehash": true, @@ -893,6 +893,19 @@ "ethjs>ethjs-unit": true } }, + "@metamask/controller-utils>@metamask/utils": { + "globals": { + "TextDecoder": true, + "TextEncoder": true + }, + "packages": { + "@metamask/key-tree>@noble/hashes": true, + "browserify>buffer": true, + "nock>debug": true, + "semver": true, + "superstruct": true + } + }, "@metamask/controller-utils>@spruceid/siwe-parser": { "globals": { "console.error": true, @@ -1712,34 +1725,12 @@ "fetch": true }, "packages": { - "@metamask/phishing-controller>@metamask/base-controller": true, - "@metamask/phishing-controller>@metamask/controller-utils": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, "@metamask/phishing-warning>eth-phishing-detect": true, "punycode": true } }, - "@metamask/phishing-controller>@metamask/base-controller": { - "packages": { - "immer": true - } - }, - "@metamask/phishing-controller>@metamask/controller-utils": { - "globals": { - "URL": true, - "console.error": true, - "fetch": true, - "setTimeout": true - }, - "packages": { - "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, - "browserify>buffer": true, - "eslint>fast-deep-equal": true, - "eth-ens-namehash": true, - "ethereumjs-util": true, - "ethjs>ethjs-unit": true - } - }, "@metamask/phishing-warning>eth-phishing-detect": { "packages": { "eslint>optionator>fast-levenshtein": true diff --git a/lavamoat/browserify/desktop/policy.json b/lavamoat/browserify/desktop/policy.json index 75d7032be..43c731fcc 100644 --- a/lavamoat/browserify/desktop/policy.json +++ b/lavamoat/browserify/desktop/policy.json @@ -884,8 +884,8 @@ "setTimeout": true }, "packages": { + "@metamask/controller-utils>@metamask/utils": true, "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, "browserify>buffer": true, "eslint>fast-deep-equal": true, "eth-ens-namehash": true, @@ -893,6 +893,19 @@ "ethjs>ethjs-unit": true } }, + "@metamask/controller-utils>@metamask/utils": { + "globals": { + "TextDecoder": true, + "TextEncoder": true + }, + "packages": { + "@metamask/key-tree>@noble/hashes": true, + "browserify>buffer": true, + "nock>debug": true, + "semver": true, + "superstruct": true + } + }, "@metamask/controller-utils>@spruceid/siwe-parser": { "globals": { "console.error": true, @@ -1847,34 +1860,12 @@ "fetch": true }, "packages": { - "@metamask/phishing-controller>@metamask/base-controller": true, - "@metamask/phishing-controller>@metamask/controller-utils": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, "@metamask/phishing-warning>eth-phishing-detect": true, "punycode": true } }, - "@metamask/phishing-controller>@metamask/base-controller": { - "packages": { - "immer": true - } - }, - "@metamask/phishing-controller>@metamask/controller-utils": { - "globals": { - "URL": true, - "console.error": true, - "fetch": true, - "setTimeout": true - }, - "packages": { - "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, - "browserify>buffer": true, - "eslint>fast-deep-equal": true, - "eth-ens-namehash": true, - "ethereumjs-util": true, - "ethjs>ethjs-unit": true - } - }, "@metamask/phishing-warning>eth-phishing-detect": { "packages": { "eslint>optionator>fast-levenshtein": true diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index 75d7032be..43c731fcc 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -884,8 +884,8 @@ "setTimeout": true }, "packages": { + "@metamask/controller-utils>@metamask/utils": true, "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, "browserify>buffer": true, "eslint>fast-deep-equal": true, "eth-ens-namehash": true, @@ -893,6 +893,19 @@ "ethjs>ethjs-unit": true } }, + "@metamask/controller-utils>@metamask/utils": { + "globals": { + "TextDecoder": true, + "TextEncoder": true + }, + "packages": { + "@metamask/key-tree>@noble/hashes": true, + "browserify>buffer": true, + "nock>debug": true, + "semver": true, + "superstruct": true + } + }, "@metamask/controller-utils>@spruceid/siwe-parser": { "globals": { "console.error": true, @@ -1847,34 +1860,12 @@ "fetch": true }, "packages": { - "@metamask/phishing-controller>@metamask/base-controller": true, - "@metamask/phishing-controller>@metamask/controller-utils": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, "@metamask/phishing-warning>eth-phishing-detect": true, "punycode": true } }, - "@metamask/phishing-controller>@metamask/base-controller": { - "packages": { - "immer": true - } - }, - "@metamask/phishing-controller>@metamask/controller-utils": { - "globals": { - "URL": true, - "console.error": true, - "fetch": true, - "setTimeout": true - }, - "packages": { - "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, - "browserify>buffer": true, - "eslint>fast-deep-equal": true, - "eth-ens-namehash": true, - "ethereumjs-util": true, - "ethjs>ethjs-unit": true - } - }, "@metamask/phishing-warning>eth-phishing-detect": { "packages": { "eslint>optionator>fast-levenshtein": true diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index 9b750dc06..defef7b1a 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -884,8 +884,8 @@ "setTimeout": true }, "packages": { + "@metamask/controller-utils>@metamask/utils": true, "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, "browserify>buffer": true, "eslint>fast-deep-equal": true, "eth-ens-namehash": true, @@ -893,6 +893,19 @@ "ethjs>ethjs-unit": true } }, + "@metamask/controller-utils>@metamask/utils": { + "globals": { + "TextDecoder": true, + "TextEncoder": true + }, + "packages": { + "@metamask/key-tree>@noble/hashes": true, + "browserify>buffer": true, + "nock>debug": true, + "semver": true, + "superstruct": true + } + }, "@metamask/controller-utils>@spruceid/siwe-parser": { "globals": { "console.error": true, @@ -1712,34 +1725,12 @@ "fetch": true }, "packages": { - "@metamask/phishing-controller>@metamask/base-controller": true, - "@metamask/phishing-controller>@metamask/controller-utils": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, "@metamask/phishing-warning>eth-phishing-detect": true, "punycode": true } }, - "@metamask/phishing-controller>@metamask/base-controller": { - "packages": { - "immer": true - } - }, - "@metamask/phishing-controller>@metamask/controller-utils": { - "globals": { - "URL": true, - "console.error": true, - "fetch": true, - "setTimeout": true - }, - "packages": { - "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, - "browserify>buffer": true, - "eslint>fast-deep-equal": true, - "eth-ens-namehash": true, - "ethereumjs-util": true, - "ethjs>ethjs-unit": true - } - }, "@metamask/phishing-warning>eth-phishing-detect": { "packages": { "eslint>optionator>fast-levenshtein": true diff --git a/lavamoat/browserify/mmi/policy.json b/lavamoat/browserify/mmi/policy.json index 938c0d76e..41ecf2d75 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -1105,8 +1105,8 @@ "setTimeout": true }, "packages": { + "@metamask/controller-utils>@metamask/utils": true, "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, "browserify>buffer": true, "eslint>fast-deep-equal": true, "eth-ens-namehash": true, @@ -1114,6 +1114,19 @@ "ethjs>ethjs-unit": true } }, + "@metamask/controller-utils>@metamask/utils": { + "globals": { + "TextDecoder": true, + "TextEncoder": true + }, + "packages": { + "@metamask/key-tree>@noble/hashes": true, + "browserify>buffer": true, + "nock>debug": true, + "semver": true, + "superstruct": true + } + }, "@metamask/controller-utils>@spruceid/siwe-parser": { "globals": { "console.error": true, @@ -1933,34 +1946,12 @@ "fetch": true }, "packages": { - "@metamask/phishing-controller>@metamask/base-controller": true, - "@metamask/phishing-controller>@metamask/controller-utils": true, + "@metamask/base-controller": true, + "@metamask/controller-utils": true, "@metamask/phishing-warning>eth-phishing-detect": true, "punycode": true } }, - "@metamask/phishing-controller>@metamask/base-controller": { - "packages": { - "immer": true - } - }, - "@metamask/phishing-controller>@metamask/controller-utils": { - "globals": { - "URL": true, - "console.error": true, - "fetch": true, - "setTimeout": true - }, - "packages": { - "@metamask/controller-utils>@spruceid/siwe-parser": true, - "@metamask/utils": true, - "browserify>buffer": true, - "eslint>fast-deep-equal": true, - "eth-ens-namehash": true, - "ethereumjs-util": true, - "ethjs>ethjs-unit": true - } - }, "@metamask/phishing-warning>eth-phishing-detect": { "packages": { "eslint>optionator>fast-levenshtein": true diff --git a/package.json b/package.json index 1b5a74ff5..93c082075 100644 --- a/package.json +++ b/package.json @@ -250,7 +250,7 @@ "@metamask/notification-controller": "^3.0.0", "@metamask/obs-store": "^8.1.0", "@metamask/permission-controller": "^4.0.0", - "@metamask/phishing-controller": "^4.0.0", + "@metamask/phishing-controller": "^6.0.0", "@metamask/post-message-stream": "^6.0.0", "@metamask/providers": "^11.1.0", "@metamask/rate-limit-controller": "^3.0.0", diff --git a/yarn.lock b/yarn.lock index 144acb323..476f1f709 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3992,7 +3992,7 @@ __metadata: languageName: node linkType: hard -"@metamask/controller-utils@npm:^3.0.0, @metamask/controller-utils@npm:^3.1.0, @metamask/controller-utils@npm:^3.4.0": +"@metamask/controller-utils@npm:^3.0.0, @metamask/controller-utils@npm:^3.4.0": version: 3.4.0 resolution: "@metamask/controller-utils@npm:3.4.0" dependencies: @@ -4007,20 +4007,19 @@ __metadata: languageName: node linkType: hard -"@metamask/controller-utils@npm:^4.0.0, @metamask/controller-utils@npm:^4.0.1, @metamask/controller-utils@npm:^4.1.0, @metamask/controller-utils@npm:^4.2.0": - version: 4.2.0 - resolution: "@metamask/controller-utils@npm:4.2.0" +"@metamask/controller-utils@npm:^4.0.0, @metamask/controller-utils@npm:^4.0.1, @metamask/controller-utils@npm:^4.1.0, @metamask/controller-utils@npm:^4.2.0, @metamask/controller-utils@npm:^4.3.0": + version: 4.3.1 + resolution: "@metamask/controller-utils@npm:4.3.1" dependencies: - "@metamask/utils": ^5.0.2 + "@metamask/eth-query": ^3.0.1 + "@metamask/utils": ^6.2.0 "@spruceid/siwe-parser": 1.1.3 - babel-runtime: ^6.26.0 eth-ens-namehash: ^2.0.8 - eth-query: ^2.1.2 eth-rpc-errors: ^4.0.2 ethereumjs-util: ^7.0.10 ethjs-unit: ^0.1.6 fast-deep-equal: ^3.1.3 - checksum: e71779577c37038e6e605a43ef6b9c1af82e0b3887a72c01f48ae1e4e2005116fc9d09c8b690139478c04dd2929e227642c5fd80cfbc81814d667c415c714228 + checksum: 5bb471df560a12fba1b7fa147fe0332e06b527637c04facff1774b1279dd388b4cf1d74340469adb13551c08cc156f204d90e36599ad69b54716b11e5842b348 languageName: node linkType: hard @@ -4186,6 +4185,16 @@ __metadata: languageName: node linkType: hard +"@metamask/eth-query@npm:^3.0.1": + version: 3.0.1 + resolution: "@metamask/eth-query@npm:3.0.1" + dependencies: + json-rpc-random-id: ^1.0.0 + xtend: ^4.0.1 + checksum: b9a323dff67328eace7d54fc8b0bc4dd763bf15760870656cbd5aad5380d1ee4489fb5c59506290d5f77cf55e74e530ee97b52702a329f1090ec03a6158434b7 + languageName: node + linkType: hard + "@metamask/eth-sig-util@npm:5.0.2": version: 5.0.2 resolution: "@metamask/eth-sig-util@npm:5.0.2" @@ -4511,16 +4520,16 @@ __metadata: languageName: node linkType: hard -"@metamask/phishing-controller@npm:^4.0.0": - version: 4.0.0 - resolution: "@metamask/phishing-controller@npm:4.0.0" +"@metamask/phishing-controller@npm:^6.0.0": + version: 6.0.0 + resolution: "@metamask/phishing-controller@npm:6.0.0" dependencies: - "@metamask/base-controller": ^2.0.0 - "@metamask/controller-utils": ^3.1.0 + "@metamask/base-controller": ^3.2.0 + "@metamask/controller-utils": ^4.3.0 "@types/punycode": ^2.1.0 eth-phishing-detect: ^1.2.0 punycode: ^2.1.1 - checksum: 15de581f7bec21d75531167275c68d7bbeae7fdaad02268749ba0a71c4d3ccb53718d963d6583e90c337407f65b7fcc9a89eb76c6f731802c2668a8425d5df89 + checksum: 13a85865cef1515f6d0ee1cd02da37e5e6b98c493676e3a80195294725b717aa17651a0c24d2e841f790bbd22ae16911cc16bab7846da8266f4ee03007a17f4e languageName: node linkType: hard @@ -24504,7 +24513,7 @@ __metadata: "@metamask/notification-controller": ^3.0.0 "@metamask/obs-store": ^8.1.0 "@metamask/permission-controller": ^4.0.0 - "@metamask/phishing-controller": ^4.0.0 + "@metamask/phishing-controller": ^6.0.0 "@metamask/phishing-warning": ^2.1.0 "@metamask/post-message-stream": ^6.0.0 "@metamask/providers": ^11.1.0 From b9f647f478cc64b1de5c2bb8c83cb33efdff8cd9 Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Mon, 31 Jul 2023 18:44:59 -0700 Subject: [PATCH 19/31] [skip-e2e] Update changelog for v10.34.2 (#20238) --- CHANGELOG.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52aa68b23..a95f4c6e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ## [10.34.2] +### Added +- Add Address Details and View on Explorer to Global Menu ([#20013](https://github.com/MetaMask/metamask-extension/pull/20013)) + +## Changed +- Increase copy clipboard time ([#20008](https://github.com/MetaMask/metamask-extension/pull/20008)) +- Show checksum addresses on account list menu ([#20217](https://github.com/MetaMask/metamask-extension/pull/20217/commits/41bab4a6e14682388f4021f2f51bc74bddcbe80e)) +- Scroll to selected account when opening account list menu ([#20166](https://github.com/MetaMask/metamask-extension/pull/20166)) ## [10.34.1] ### Fixed @@ -3861,7 +3868,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added the ability to restore accounts from seed words. [Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.2...HEAD -[10.34.1]: https://github.com/MetaMask/metamask-extension/compare/v10.34.1...v10.34.2 +[10.34.2]: https://github.com/MetaMask/metamask-extension/compare/v10.34.1...v10.34.2 [10.34.1]: https://github.com/MetaMask/metamask-extension/compare/v10.34.0...v10.34.1 [10.34.0]: https://github.com/MetaMask/metamask-extension/compare/v10.33.1...v10.34.0 [10.33.1]: https://github.com/MetaMask/metamask-extension/compare/v10.33.0...v10.33.1 From 75377cf890710cfc5bb45931666f86c543431ca3 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 1 Aug 2023 19:52:18 -0230 Subject: [PATCH 20/31] Update CHANGELOG.md Co-authored-by: Mark Stacey --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a95f4c6e2..f5745a9d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Increase copy clipboard time ([#20008](https://github.com/MetaMask/metamask-extension/pull/20008)) - Show checksum addresses on account list menu ([#20217](https://github.com/MetaMask/metamask-extension/pull/20217/commits/41bab4a6e14682388f4021f2f51bc74bddcbe80e)) - Scroll to selected account when opening account list menu ([#20166](https://github.com/MetaMask/metamask-extension/pull/20166)) +- Remove fallback phishing warning configuration ([#20327](https://github.com/MetaMask/metamask-extension/pull/20327)) + - The phishing warning feature will no longer function if the wallet is unable to receive configuration updates. Previously a fallback config was used in this case, but we found that it was too outdated to be helpful and it caused many problems for users. ## [10.34.1] ### Fixed From ab967487da9bf947c9204fb802203d39b8e9c93d Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 1 Aug 2023 19:55:43 -0230 Subject: [PATCH 21/31] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5745a9d3..97a336d0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Remove fallback phishing warning configuration ([#20327](https://github.com/MetaMask/metamask-extension/pull/20327)) - The phishing warning feature will no longer function if the wallet is unable to receive configuration updates. Previously a fallback config was used in this case, but we found that it was too outdated to be helpful and it caused many problems for users. +### Fixed +- Fixed bug that could cause loss of network or token data for users upgrading from old versions ([#20276](https://github.com/MetaMask/metamask-extension/pull/20276)) +- Fix crash on open of MetaMask for some users that have old network data in state ([#20345](https://github.com/MetaMask/metamask-extension/pull/20345)) + ## [10.34.1] ### Fixed - Fix bug that could cause a failure in the persistence of network related data ([#20080](https://github.com/MetaMask/metamask-extension/pull/20080)) From 5ed415d807f3fa0e3c527dcd9114e35fdbabdef6 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 1 Aug 2023 20:24:02 -0230 Subject: [PATCH 22/31] Fix migration 88 to handle the case where chainId keys can be undefined (#20345) * Fix migration 88 to handle the case where chainId keys can be undefined * Add migration 91 to delete network configurations that have no chainId * Lint fix * Update migration number * Update migration 91 description * Update version numbers in 091.test.js * Fix 088.test.ts typescript problem * Fix 088.test.ts typescript problem * Update app/scripts/migrations/091.ts Co-authored-by: Mark Stacey * Change app/scripts/migrations/091.test.js to typescript * clone oldstorage for test result comparisons in 091.test.js * Lint fix * Add missing test case --------- Co-authored-by: Mark Stacey --- app/scripts/migrations/088.test.ts | 349 +++++++++++++++++++++++++++++ app/scripts/migrations/088.ts | 45 +++- app/scripts/migrations/091.test.ts | 150 +++++++++++++ app/scripts/migrations/091.ts | 55 +++++ app/scripts/migrations/index.js | 2 + 5 files changed, 599 insertions(+), 2 deletions(-) create mode 100644 app/scripts/migrations/091.test.ts create mode 100644 app/scripts/migrations/091.ts diff --git a/app/scripts/migrations/088.test.ts b/app/scripts/migrations/088.test.ts index 2677cc3b3..a33c30eff 100644 --- a/app/scripts/migrations/088.test.ts +++ b/app/scripts/migrations/088.test.ts @@ -156,6 +156,65 @@ describe('migration #88', () => { }); }); + it('deletes undefined-keyed properties from state of NftController.allNftContracts', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + NftController: { + allNftContracts: { + '0x111': { + '16': [ + { + name: 'Contract 1', + address: '0xaaa', + }, + ], + undefined: [ + { + name: 'Contract 2', + address: '0xbbb', + }, + ], + }, + '0x222': { + '64': [ + { + name: 'Contract 3', + address: '0xccc', + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + NftController: { + allNftContracts: { + '0x111': { + '0x10': [ + { + name: 'Contract 1', + address: '0xaaa', + }, + ], + }, + '0x222': { + '0x40': [ + { + name: 'Contract 3', + address: '0xccc', + }, + ], + }, + }, + }, + }); + }); + it('does not convert chain IDs in NftController.allNftContracts which are already hex strings', async () => { const oldStorage = { meta: { version: 87 }, @@ -395,6 +454,85 @@ describe('migration #88', () => { }); }); + it('deletes undefined-keyed properties from state of NftController.allNfts', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + NftController: { + allNfts: { + '0x111': { + '16': [ + { + name: 'NFT 1', + description: 'Description for NFT 1', + image: 'nft1.jpg', + standard: 'ERC721', + tokenId: '1', + address: '0xaaa', + }, + ], + undefined: [ + { + name: 'NFT 2', + description: 'Description for NFT 2', + image: 'nft2.jpg', + standard: 'ERC721', + tokenId: '2', + address: '0xbbb', + }, + ], + }, + '0x222': { + '64': [ + { + name: 'NFT 3', + description: 'Description for NFT 3', + image: 'nft3.jpg', + standard: 'ERC721', + tokenId: '3', + address: '0xccc', + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + NftController: { + allNfts: { + '0x111': { + '0x10': [ + { + name: 'NFT 1', + description: 'Description for NFT 1', + image: 'nft1.jpg', + standard: 'ERC721', + tokenId: '1', + address: '0xaaa', + }, + ], + }, + '0x222': { + '0x40': [ + { + name: 'NFT 3', + description: 'Description for NFT 3', + image: 'nft3.jpg', + standard: 'ERC721', + tokenId: '3', + address: '0xccc', + }, + ], + }, + }, + }, + }); + }); + it('does not convert chain IDs in NftController.allNfts which are already hex strings', async () => { const oldStorage = { meta: { version: 87 }, @@ -627,6 +765,69 @@ describe('migration #88', () => { }); }); + it('deletes undefined-keyed properties from state of TokenListController.tokensChainsCache', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + TokenListController: { + tokensChainsCache: { + '16': { + timestamp: 111111, + data: { + '0x111': { + address: '0x111', + symbol: 'TEST1', + decimals: 1, + occurrences: 1, + name: 'Token 1', + iconUrl: 'https://url/to/token1.png', + aggregators: [], + }, + }, + }, + undefined: { + timestamp: 222222, + data: { + '0x222': { + address: '0x222', + symbol: 'TEST2', + decimals: 1, + occurrences: 1, + name: 'Token 2', + iconUrl: 'https://url/to/token2.png', + aggregators: [], + }, + }, + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokenListController: { + tokensChainsCache: { + '0x10': { + timestamp: 111111, + data: { + '0x111': { + address: '0x111', + symbol: 'TEST1', + decimals: 1, + occurrences: 1, + name: 'Token 1', + iconUrl: 'https://url/to/token1.png', + aggregators: [], + }, + }, + }, + }, + }, + }); + }); + it('does not convert chain IDs in TokenListController.tokensChainsCache which are already hex strings', async () => { const oldStorage = { meta: { version: 87 }, @@ -807,6 +1008,72 @@ describe('migration #88', () => { }); }); + it('deletes undefined keyed properties from TokensController.allTokens', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + TokensController: { + allTokens: { + '16': { + '0x111': [ + { + address: '0xaaa', + decimals: 1, + symbol: 'TEST1', + }, + ], + }, + '32': { + '0x222': [ + { + address: '0xbbb', + decimals: 1, + symbol: 'TEST2', + }, + ], + }, + undefined: { + '0x333': [ + { + address: '0xbbb', + decimals: 1, + symbol: 'TEST2', + }, + ], + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokensController: { + allTokens: { + '0x10': { + '0x111': [ + { + address: '0xaaa', + decimals: 1, + symbol: 'TEST1', + }, + ], + }, + '0x20': { + '0x222': [ + { + address: '0xbbb', + decimals: 1, + symbol: 'TEST2', + }, + ], + }, + }, + }, + }); + }); + it('does not convert chain IDs in TokensController.allTokens which are already hex strings', async () => { const oldStorage = { meta: { version: 87 }, @@ -937,6 +1204,52 @@ describe('migration #88', () => { }); }); + it('deletes undefined-keyed properties from TokensController.allIgnoredTokens', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + TokensController: { + allIgnoredTokens: { + '16': { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + '32': { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + undefined: { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokensController: { + allIgnoredTokens: { + '0x10': { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + '0x20': { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }); + }); + it('does not convert chain IDs in TokensController.allIgnoredTokens which are already hex strings', async () => { const oldStorage = { meta: { version: 87 }, @@ -1051,6 +1364,42 @@ describe('migration #88', () => { }); }); + it('deletes undefined-keyed properties from TokensController.allDetectedTokens', async () => { + const oldStorage = { + meta: { version: 87 }, + data: { + TokensController: { + allDetectedTokens: { + '16': { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + undefined: { + '0x2': { + '0x222': ['0xbbb'], + }, + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual({ + TokensController: { + allDetectedTokens: { + '0x10': { + '0x1': { + '0x111': ['0xaaa'], + }, + }, + }, + }, + }); + }); + it('does not convert chain IDs in TokensController.allDetectedTokens which are already hex strings', async () => { const oldStorage = { meta: { version: 87 }, diff --git a/app/scripts/migrations/088.ts b/app/scripts/migrations/088.ts index 3233a3d1c..a4b874b2f 100644 --- a/app/scripts/migrations/088.ts +++ b/app/scripts/migrations/088.ts @@ -16,8 +16,11 @@ export const version = 88; * by a hex chain ID rather than a decimal chain ID. * - Rebuilds `tokensChainsCache` in TokenListController to be keyed by a hex * chain ID rather than a decimal chain ID. - * - Rebuilds `allTokens` and `allIgnoredTokens` in TokensController to be keyed - * by a hex chain ID rather than a decimal chain ID. + * - Rebuilds `allTokens`, `allDetectedTokens`, and `allIgnoredTokens` in + * TokensController to be keyed by a hex chain ID rather than a decimal chain ID. + * - removes any entries in `allNftContracts`, `allNfts`, `tokensChainsCache`, + * `allTokens`, `allIgnoredTokens` or `allDetectedTokens` that are keyed by the + * string 'undefined' * * @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist. * @param originalVersionedData.meta - State metadata. @@ -54,6 +57,12 @@ function migrateData(state: Record): void { const nftContractsByChainId = allNftContracts[address]; if (isObject(nftContractsByChainId)) { + for (const chainId of Object.keys(nftContractsByChainId)) { + if (chainId === 'undefined' || chainId === undefined) { + delete nftContractsByChainId[chainId]; + } + } + allNftContracts[address] = mapKeys( nftContractsByChainId, (_, chainId: string) => toHex(chainId), @@ -75,6 +84,12 @@ function migrateData(state: Record): void { const nftsByChainId = allNfts[address]; if (isObject(nftsByChainId)) { + for (const chainId of Object.keys(nftsByChainId)) { + if (chainId === 'undefined' || chainId === undefined) { + delete nftsByChainId[chainId]; + } + } + allNfts[address] = mapKeys(nftsByChainId, (_, chainId: string) => toHex(chainId), ); @@ -97,6 +112,14 @@ function migrateData(state: Record): void { hasProperty(tokenListControllerState, 'tokensChainsCache') && isObject(tokenListControllerState.tokensChainsCache) ) { + for (const chainId of Object.keys( + tokenListControllerState.tokensChainsCache, + )) { + if (chainId === 'undefined' || chainId === undefined) { + delete tokenListControllerState.tokensChainsCache[chainId]; + } + } + tokenListControllerState.tokensChainsCache = mapKeys( tokenListControllerState.tokensChainsCache, (_, chainId: string) => toHex(chainId), @@ -117,6 +140,12 @@ function migrateData(state: Record): void { ) { const { allTokens } = tokensControllerState; + for (const chainId of Object.keys(allTokens)) { + if (chainId === 'undefined' || chainId === undefined) { + delete allTokens[chainId]; + } + } + tokensControllerState.allTokens = mapKeys( allTokens, (_, chainId: string) => toHex(chainId), @@ -130,6 +159,12 @@ function migrateData(state: Record): void { ) { const { allIgnoredTokens } = tokensControllerState; + for (const chainId of Object.keys(allIgnoredTokens)) { + if (chainId === 'undefined' || chainId === undefined) { + delete allIgnoredTokens[chainId]; + } + } + tokensControllerState.allIgnoredTokens = mapKeys( allIgnoredTokens, (_, chainId: string) => toHex(chainId), @@ -143,6 +178,12 @@ function migrateData(state: Record): void { ) { const { allDetectedTokens } = tokensControllerState; + for (const chainId of Object.keys(allDetectedTokens)) { + if (chainId === 'undefined' || chainId === undefined) { + delete allDetectedTokens[chainId]; + } + } + tokensControllerState.allDetectedTokens = mapKeys( allDetectedTokens, (_, chainId: string) => toHex(chainId), diff --git a/app/scripts/migrations/091.test.ts b/app/scripts/migrations/091.test.ts new file mode 100644 index 000000000..d4836f003 --- /dev/null +++ b/app/scripts/migrations/091.test.ts @@ -0,0 +1,150 @@ +import { cloneDeep } from 'lodash'; +import { migrate, version } from './091'; + +jest.mock('uuid', () => { + const actual = jest.requireActual('uuid'); + + return { + ...actual, + v4: jest.fn(), + }; +}); + +describe('migration #91', () => { + it('should update the version metadata', async () => { + const oldStorage = { + meta: { + version: 90, + }, + data: {}, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.meta).toStrictEqual({ + version, + }); + }); + + it('should return state unaltered if there is no network controller state', async () => { + const oldData = { + other: 'data', + }; + const oldStorage = { + meta: { + version: 90, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should return state unaltered if there is no network controller networkConfigurations state', async () => { + const oldData = { + other: 'data', + NetworkController: { + providerConfig: { + foo: 'bar', + }, + }, + }; + const oldStorage = { + meta: { + version: 90, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should return state unaltered if the networkConfigurations all have a chainId', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + chainId: '0x1', + }, + id2: { + fizz: 'buzz', + chainId: '0x2', + }, + }, + providerConfig: { + id: 'test', + }, + }, + }; + const oldStorage = { + meta: { + version: 90, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should delete networks that have an undefined or null chainId', async () => { + const oldData = { + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + chainId: '0x1', + }, + id2: { + fizz: 'buzz', + chainId: '0x2', + }, + id3: { + buzz: 'baz', + chainId: undefined, + }, + id4: { + foo: 'bar', + chainId: null, + }, + id5: { + fizz: 'buzz', + }, + }, + providerConfig: { + rpcUrl: 'http://foo.bar', + }, + }, + }; + const oldStorage = { + meta: { + version: 90, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual({ + other: 'data', + NetworkController: { + networkConfigurations: { + id1: { + foo: 'bar', + chainId: '0x1', + }, + id2: { + fizz: 'buzz', + chainId: '0x2', + }, + }, + providerConfig: { + rpcUrl: 'http://foo.bar', + }, + }, + }); + }); +}); diff --git a/app/scripts/migrations/091.ts b/app/scripts/migrations/091.ts new file mode 100644 index 000000000..c0661746a --- /dev/null +++ b/app/scripts/migrations/091.ts @@ -0,0 +1,55 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; + +export const version = 91; + +/** + * Delete network configurations if they do not have a chain id + * + * @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: { + meta: { version: number }; + data: Record; +}) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + versionedData.data = transformState(versionedData.data); + return versionedData; +} + +function transformState(state: Record) { + if ( + hasProperty(state, 'NetworkController') && + isObject(state.NetworkController) && + hasProperty(state.NetworkController, 'networkConfigurations') && + isObject(state.NetworkController.networkConfigurations) + ) { + const { networkConfigurations } = state.NetworkController; + + for (const [networkConfigurationId, networkConfiguration] of Object.entries( + networkConfigurations, + )) { + if (isObject(networkConfiguration)) { + if (!networkConfiguration.chainId) { + delete networkConfigurations[networkConfigurationId]; + } + } + } + + state.NetworkController = { + ...state.NetworkController, + networkConfigurations, + }; + + return { + ...state, + NetworkController: state.NetworkController, + }; + } + return state; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index adbe48252..a37648e78 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -94,6 +94,7 @@ import * as m087 from './087'; import * as m088 from './088'; import * as m089 from './089'; import * as m090 from './090'; +import * as m091 from './091'; const migrations = [ m002, @@ -185,6 +186,7 @@ const migrations = [ m088, m089, m090, + m091, ]; export default migrations; From fdc22247bebba4ee873f6e80e19281a56df27791 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Tue, 1 Aug 2023 20:37:16 -0230 Subject: [PATCH 23/31] Update CHANGELOG.md Co-authored-by: Mark Stacey --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97a336d0b..c3b398cc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Scroll to selected account when opening account list menu ([#20166](https://github.com/MetaMask/metamask-extension/pull/20166)) - Remove fallback phishing warning configuration ([#20327](https://github.com/MetaMask/metamask-extension/pull/20327)) - The phishing warning feature will no longer function if the wallet is unable to receive configuration updates. Previously a fallback config was used in this case, but we found that it was too outdated to be helpful and it caused many problems for users. +- Improved UI for downloading state logs on Chromium-based browsers ([#19872](https://github.com/MetaMask/metamask-extension/pull/19872)) + - We now use a file picker to let you select the download location, rather than saving the state logs in your downloads folder. ### Fixed - Fixed bug that could cause loss of network or token data for users upgrading from old versions ([#20276](https://github.com/MetaMask/metamask-extension/pull/20276)) From f526c170e28407356b1dbb3a7b9570805c4378e6 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Thu, 3 Aug 2023 13:41:16 -0230 Subject: [PATCH 24/31] v10.34.2 --- CHANGELOG.md | 9 ++++++++- package.json | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3b398cc8..989ee2e8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.34.3] +### Fixed +- Ensure users phishing warning list is properly updated ([#20381](https://github.com/MetaMask/metamask-extension/pull/20381)) +- Fix inaccurate info in swaps flow for zero-balance tokens ([#20388](https://github.com/MetaMask/metamask-extension/pull/20388)) +- Fix 'Global Menu Explorer / Account Details' What's New notification display ([#20371](https://github.com/MetaMask/metamask-extension/pull/20371)) + ## [10.34.2] ### Added - Add Address Details and View on Explorer to Global Menu ([#20013](https://github.com/MetaMask/metamask-extension/pull/20013)) @@ -3875,7 +3881,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Uncategorized - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.2...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.3...HEAD +[10.34.3]: https://github.com/MetaMask/metamask-extension/compare/v10.34.2...v10.34.3 [10.34.2]: https://github.com/MetaMask/metamask-extension/compare/v10.34.1...v10.34.2 [10.34.1]: https://github.com/MetaMask/metamask-extension/compare/v10.34.0...v10.34.1 [10.34.0]: https://github.com/MetaMask/metamask-extension/compare/v10.33.1...v10.34.0 diff --git a/package.json b/package.json index 93c082075..280eb8e0d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "10.34.2", + "version": "10.34.3", "private": true, "repository": { "type": "git", From 75dcb03069d607e6f99cf7d72ed587e6baf12cb4 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 3 Aug 2023 13:12:21 -0230 Subject: [PATCH 25/31] Force an update of the phishing warning configuration (#20381) The "last fetched" state for the `PhishingController` has been deleted to force an immediate full update of the phishing configuration state. We're doing this because the state was cleared in v10.34.2 because the format of that state had changed. This has been implemented in migration 92. The previous migration 92 has been renamed to 93 because it won't be included until a future release. We need the migrations to remain sequential, and this will save us from having to resolve a complex conflict when releasing this. --- app/scripts/migrations/092.test.ts | 78 ++++++++++++++++++++++++++++++ app/scripts/migrations/092.ts | 35 ++++++++++++++ app/scripts/migrations/index.js | 2 + 3 files changed, 115 insertions(+) create mode 100644 app/scripts/migrations/092.test.ts create mode 100644 app/scripts/migrations/092.ts diff --git a/app/scripts/migrations/092.test.ts b/app/scripts/migrations/092.test.ts new file mode 100644 index 000000000..b44c04602 --- /dev/null +++ b/app/scripts/migrations/092.test.ts @@ -0,0 +1,78 @@ +import { cloneDeep } from 'lodash'; +import { migrate, version } from './092'; + +const PREVIOUS_VERSION = version - 1; + +describe('migration #92', () => { + it('should update the version metadata', async () => { + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: {}, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.meta).toStrictEqual({ + version, + }); + }); + + it('should return state unaltered if there is no phishing controller state', async () => { + const oldData = { + other: 'data', + }; + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should return state unaltered if there is no phishing controller last fetched state', async () => { + const oldData = { + other: 'data', + PhishingController: { + whitelist: [], + }, + }; + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: oldData, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should remove both last fetched properties from phishing controller state', async () => { + const oldData = { + other: 'data', + PhishingController: { + whitelist: [], + hotlistLastFetched: 0, + stalelistLastFetched: 0, + }, + }; + const oldStorage = { + meta: { + version: PREVIOUS_VERSION, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual({ + other: 'data', + PhishingController: { + whitelist: [], + }, + }); + }); +}); diff --git a/app/scripts/migrations/092.ts b/app/scripts/migrations/092.ts new file mode 100644 index 000000000..bf5469614 --- /dev/null +++ b/app/scripts/migrations/092.ts @@ -0,0 +1,35 @@ +import { cloneDeep } from 'lodash'; +import { hasProperty, isObject } from '@metamask/utils'; + +export const version = 92; + +/** + * Delete `stalelistLastFetched` and `hotlistLastFetched` to force a phishing configuration refresh + * because the format has changed. + * + * @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: { + meta: { version: number }; + data: Record; +}) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + versionedData.data = transformState(versionedData.data); + return versionedData; +} + +function transformState(state: Record) { + if ( + hasProperty(state, 'PhishingController') && + isObject(state.PhishingController) + ) { + delete state.PhishingController.stalelistLastFetched; + delete state.PhishingController.hotlistLastFetched; + } + return state; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index a37648e78..dc4fcd4e0 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -95,6 +95,7 @@ import * as m088 from './088'; import * as m089 from './089'; import * as m090 from './090'; import * as m091 from './091'; +import * as m092 from './092'; const migrations = [ m002, @@ -187,6 +188,7 @@ const migrations = [ m089, m090, m091, + m092, ]; export default migrations; From eda24aab4fad9ae20bbddb02f00d68b5a0c26fb2 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Thu, 3 Aug 2023 09:16:01 -0500 Subject: [PATCH 26/31] Fix 'Global Menu Explorer / Account Details' What's New (#20371) --- shared/notifications/index.js | 1 - ui/components/app/whats-new-popup/whats-new-popup.js | 4 ++++ ui/selectors/selectors.js | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/shared/notifications/index.js b/shared/notifications/index.js index bc74e3222..f5d496b8b 100644 --- a/shared/notifications/index.js +++ b/shared/notifications/index.js @@ -119,7 +119,6 @@ export const UI_NOTIFICATIONS = { date: null, image: { src: 'images/global-menu-block-explorer.svg', - width: '100%', }, }, }; diff --git a/ui/components/app/whats-new-popup/whats-new-popup.js b/ui/components/app/whats-new-popup/whats-new-popup.js index 5743cc841..ba9675113 100644 --- a/ui/components/app/whats-new-popup/whats-new-popup.js +++ b/ui/components/app/whats-new-popup/whats-new-popup.js @@ -100,6 +100,9 @@ function getActionFunctionById(id, history) { updateViewedNotifications({ 21: true }); history.push(PREPARE_SWAP_ROUTE); }, + 22: () => { + updateViewedNotifications({ 22: true }); + }, }; return actionFunctions[id]; @@ -360,6 +363,7 @@ export default function WhatsNewPopup({ 18: renderFirstNotification, 19: renderFirstNotification, 21: renderFirstNotification, + 22: renderFirstNotification, }; return ( diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index a07c73788..2559c5340 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -995,6 +995,7 @@ function getAllowedAnnouncementIds(state) { 19: false, 20: currentKeyringIsLedger && isFirefox, 21: isSwapsChain, + 22: true, }; } From 87b3ac62f19e9a33e77feecdbe563a2140ee7f00 Mon Sep 17 00:00:00 2001 From: Daniel <80175477+dan437@users.noreply.github.com> Date: Thu, 3 Aug 2023 22:49:50 +0200 Subject: [PATCH 27/31] Improvements to Swaps quote auto-selection logic, fix and edge case with zero-balance tokens (#20388) * Add Token To into assets again (reverting commit 51f46eb65f48bdf4980f400a589bf1ac63a65222 ) * Update cleanup for an unswapped Token To from the Tokens list * Call "setLatestAddedTokenTo" conditionally * Update an E2E test for insufficient balance notification --- test/e2e/swaps/swaps-notifications.spec.js | 2 +- ui/ducks/swaps/swaps.js | 71 ++++++++++--------- ui/pages/swaps/build-quote/build-quote.js | 14 +++- .../swaps/build-quote/build-quote.test.js | 1 + ui/pages/swaps/index.js | 32 +++++++++ .../prepare-swap-page/prepare-swap-page.js | 14 +++- .../prepare-swap-page.test.js | 1 + 7 files changed, 99 insertions(+), 36 deletions(-) diff --git a/test/e2e/swaps/swaps-notifications.spec.js b/test/e2e/swaps/swaps-notifications.spec.js index 4d24bf5fe..42f17d5ec 100644 --- a/test/e2e/swaps/swaps-notifications.spec.js +++ b/test/e2e/swaps/swaps-notifications.spec.js @@ -111,7 +111,7 @@ describe('Swaps - notifications', function () { }); await checkNotification(driver, { title: 'Insufficient balance', - text: 'You need 50 more TESTETH to complete this swap', + text: 'You need 43.4467 more TESTETH to complete this swap', }); await reviewQuote(driver, { swapFrom: 'TESTETH', diff --git a/ui/ducks/swaps/swaps.js b/ui/ducks/swaps/swaps.js index 1c3501cb2..e44fe44fb 100644 --- a/ui/ducks/swaps/swaps.js +++ b/ui/ducks/swaps/swaps.js @@ -121,6 +121,7 @@ const initialState = { currentSmartTransactionsError: '', swapsSTXLoading: false, transactionSettingsOpened: false, + latestAddedTokenTo: '', }; const slice = createSlice({ @@ -150,6 +151,9 @@ const slice = createSlice({ setFetchingQuotes: (state, action) => { state.fetchingQuotes = action.payload; }, + setLatestAddedTokenTo: (state, action) => { + state.latestAddedTokenTo = action.payload; + }, setFromToken: (state, action) => { state.fromToken = action.payload; }, @@ -245,6 +249,8 @@ export const getToToken = (state) => state.swaps.toToken; export const getFetchingQuotes = (state) => state.swaps.fetchingQuotes; +export const getLatestAddedTokenTo = (state) => state.swaps.latestAddedTokenTo; + export const getQuotesFetchStartTime = (state) => state.swaps.quotesFetchStartTime; @@ -479,6 +485,7 @@ const { setAggregatorMetadata, setBalanceError, setFetchingQuotes, + setLatestAddedTokenTo, setFromToken, setFromTokenError, setFromTokenInputValue, @@ -502,6 +509,7 @@ export { setAggregatorMetadata, setBalanceError, setFetchingQuotes, + setLatestAddedTokenTo, setFromToken as setSwapsFromToken, setFromTokenError, setFromTokenInputValue, @@ -665,7 +673,12 @@ export const fetchQuotesAndSetQuoteState = ( iconUrl: fromTokenIconUrl, balance: fromTokenBalance, } = selectedFromToken; - const { address: toTokenAddress, symbol: toTokenSymbol } = selectedToToken; + const { + address: toTokenAddress, + symbol: toTokenSymbol, + decimals: toTokenDecimals, + iconUrl: toTokenIconUrl, + } = selectedToToken; // pageRedirectionDisabled is true if quotes prefetching is active (a user is on the Build Quote page). // In that case we just want to silently prefetch quotes without redirecting to the quotes loading page. if (!pageRedirectionDisabled) { @@ -676,6 +689,30 @@ export const fetchQuotesAndSetQuoteState = ( const contractExchangeRates = getTokenExchangeRates(state); + if ( + toTokenAddress && + toTokenSymbol !== swapsDefaultToken.symbol && + contractExchangeRates[toTokenAddress] === undefined && + !isTokenAlreadyAdded(toTokenAddress, getTokens(state)) + ) { + await dispatch( + addToken( + toTokenAddress, + toTokenSymbol, + toTokenDecimals, + toTokenIconUrl, + true, + ), + ); + await dispatch(setLatestAddedTokenTo(toTokenAddress)); + } else { + const latestAddedTokenTo = getLatestAddedTokenTo(state); + // Only reset the latest added Token To if it's a different token. + if (latestAddedTokenTo !== toTokenAddress) { + await dispatch(setLatestAddedTokenTo('')); + } + } + if ( fromTokenAddress && fromTokenSymbol !== swapsDefaultToken.symbol && @@ -831,36 +868,6 @@ export const fetchQuotesAndSetQuoteState = ( }; }; -const addTokenTo = (dispatch, state) => { - const fetchParams = getFetchParams(state); - const swapsDefaultToken = getSwapsDefaultToken(state); - const contractExchangeRates = getTokenExchangeRates(state); - const selectedToToken = - getToToken(state) || fetchParams?.metaData?.destinationTokenInfo || {}; - const { - address: toTokenAddress, - symbol: toTokenSymbol, - decimals: toTokenDecimals, - iconUrl: toTokenIconUrl, - } = selectedToToken; - if ( - toTokenAddress && - toTokenSymbol !== swapsDefaultToken.symbol && - contractExchangeRates[toTokenAddress] === undefined && - !isTokenAlreadyAdded(toTokenAddress, getTokens(state)) - ) { - dispatch( - addToken( - toTokenAddress, - toTokenSymbol, - toTokenDecimals, - toTokenIconUrl, - true, - ), - ); - } -}; - export const signAndSendSwapsSmartTransaction = ({ unsignedTransaction, trackEvent, @@ -960,7 +967,6 @@ export const signAndSendSwapsSmartTransaction = ({ dispatch(setCurrentSmartTransactionsError(StxErrorTypes.unavailable)); return; } - addTokenTo(dispatch, state); if (approveTxParams) { updatedApproveTxParams.gas = `0x${decimalToHex( fees.approvalTxFees?.gasLimit || 0, @@ -1204,7 +1210,6 @@ export const signAndSendTransactions = ( history.push(AWAITING_SIGNATURES_ROUTE); } - addTokenTo(dispatch, state); if (approveTxParams) { if (networkAndAccountSupports1559) { approveTxParams.maxFeePerGas = maxFeePerGas; diff --git a/ui/pages/swaps/build-quote/build-quote.js b/ui/pages/swaps/build-quote/build-quote.js index 7c2836703..9ed74343c 100644 --- a/ui/pages/swaps/build-quote/build-quote.js +++ b/ui/pages/swaps/build-quote/build-quote.js @@ -48,6 +48,7 @@ import { getIsFeatureFlagLoaded, getCurrentSmartTransactionsError, getSmartTransactionFees, + getLatestAddedTokenTo, } from '../../../ducks/swaps/swaps'; import { getSwapsDefaultToken, @@ -84,6 +85,7 @@ import { import { resetSwapsPostFetchState, + ignoreTokens, setBackgroundSwapRouteState, clearSwapsQuotes, stopPollingForQuotes, @@ -144,6 +146,7 @@ export default function BuildQuote({ const tokenList = useSelector(getTokenList, isEqual); const quotes = useSelector(getQuotes, isEqual); const areQuotesPresent = Object.keys(quotes).length > 0; + const latestAddedTokenTo = useSelector(getLatestAddedTokenTo, isEqual); const tokenConversionRates = useSelector(getTokenExchangeRates, isEqual); const conversionRate = useSelector(getConversionRate); @@ -347,12 +350,21 @@ export default function BuildQuote({ ? getURLHostName(blockExplorerTokenLink) : t('etherscan'); + const { address: toAddress } = toToken || {}; const onToSelect = useCallback( (token) => { + if (latestAddedTokenTo && token.address !== toAddress) { + dispatch( + ignoreTokens({ + tokensToIgnore: toAddress, + dontShowLoadingIndicator: true, + }), + ); + } dispatch(setSwapToToken(token)); setVerificationClicked(false); }, - [dispatch], + [dispatch, latestAddedTokenTo, toAddress], ); const hideDropdownItemIf = useCallback( diff --git a/ui/pages/swaps/build-quote/build-quote.test.js b/ui/pages/swaps/build-quote/build-quote.test.js index e09ec1149..ea8c2cc70 100644 --- a/ui/pages/swaps/build-quote/build-quote.test.js +++ b/ui/pages/swaps/build-quote/build-quote.test.js @@ -29,6 +29,7 @@ const createProps = (customProps = {}) => { setBackgroundConnection({ resetPostFetchState: jest.fn(), + ignoreTokens: jest.fn(), setBackgroundSwapRouteState: jest.fn(), clearSwapsQuotes: jest.fn(), stopPollingForQuotes: jest.fn(), diff --git a/ui/pages/swaps/index.js b/ui/pages/swaps/index.js index 351561f0b..1a37271b4 100644 --- a/ui/pages/swaps/index.js +++ b/ui/pages/swaps/index.js @@ -50,6 +50,7 @@ import { navigateBackToBuildQuote, getSwapRedesignEnabled, setTransactionSettingsOpened, + getLatestAddedTokenTo, } from '../../ducks/swaps/swaps'; import { checkNetworkAndAccountSupports1559, @@ -79,6 +80,7 @@ import { import { resetBackgroundSwapsState, setSwapsTokens, + ignoreTokens, setBackgroundSwapRouteState, setSwapsErrorKey, } from '../../store/actions'; @@ -134,6 +136,7 @@ export default function Swap() { const routeState = useSelector(getBackgroundSwapRouteState); const selectedAccount = useSelector(getSelectedAccount, shallowEqual); const quotes = useSelector(getQuotes, isEqual); + const latestAddedTokenTo = useSelector(getLatestAddedTokenTo, isEqual); const txList = useSelector(currentNetworkTxListSelector, shallowEqual); const tradeTxId = useSelector(getTradeTxId); const approveTxId = useSelector(getApproveTxId); @@ -209,6 +212,32 @@ export default function Swap() { swapsErrorKey = SWAP_FAILED_ERROR; } + const clearTemporaryTokenRef = useRef(); + useEffect(() => { + clearTemporaryTokenRef.current = () => { + if (latestAddedTokenTo && (!isAwaitingSwapRoute || conversionError)) { + dispatch( + ignoreTokens({ + tokensToIgnore: latestAddedTokenTo, + dontShowLoadingIndicator: true, + }), + ); + } + }; + }, [ + conversionError, + dispatch, + latestAddedTokenTo, + destinationTokenInfo, + fetchParams, + isAwaitingSwapRoute, + ]); + useEffect(() => { + return () => { + clearTemporaryTokenRef.current(); + }; + }, []); + // eslint-disable-next-line useEffect(() => { if (!isSwapsChain) { @@ -283,6 +312,7 @@ export default function Swap() { const beforeUnloadEventAddedRef = useRef(); useEffect(() => { const fn = () => { + clearTemporaryTokenRef.current(); if (isLoadingQuotesRoute) { dispatch(prepareToLeaveSwaps()); } @@ -349,6 +379,7 @@ export default function Swap() { } const redirectToDefaultRoute = async () => { + clearTemporaryTokenRef.current(); dispatch(clearSwapsState()); await dispatch(resetBackgroundSwapsState()); history.push(DEFAULT_ROUTE); @@ -400,6 +431,7 @@ export default function Swap() {
{ + clearTemporaryTokenRef.current(); dispatch(clearSwapsState()); await dispatch(resetBackgroundSwapsState()); history.push(DEFAULT_ROUTE); diff --git a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js index e1f0c5f0e..de7027c4c 100644 --- a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js +++ b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.js @@ -54,6 +54,7 @@ import { getAggregatorMetadata, getTransactionSettingsOpened, setTransactionSettingsOpened, + getLatestAddedTokenTo, } from '../../../ducks/swaps/swaps'; import { getSwapsDefaultToken, @@ -92,6 +93,7 @@ import { } from '../../../../shared/constants/swaps'; import { resetSwapsPostFetchState, + ignoreTokens, clearSwapsQuotes, stopPollingForQuotes, setSmartTransactionsOptInStatus, @@ -182,6 +184,7 @@ export default function PrepareSwapPage({ const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider, shallowEqual); const tokenList = useSelector(getTokenList, isEqual); const quotes = useSelector(getQuotes, isEqual); + const latestAddedTokenTo = useSelector(getLatestAddedTokenTo, isEqual); const numberOfQuotes = Object.keys(quotes).length; const areQuotesPresent = numberOfQuotes > 0; const swapsErrorKey = useSelector(getSwapsErrorKey); @@ -449,12 +452,21 @@ export default function PrepareSwapPage({ ? getURLHostName(blockExplorerTokenLink) : t('etherscan'); + const { address: toAddress } = toToken || {}; const onToSelect = useCallback( (token) => { + if (latestAddedTokenTo && token.address !== toAddress) { + dispatch( + ignoreTokens({ + tokensToIgnore: toAddress, + dontShowLoadingIndicator: true, + }), + ); + } dispatch(setSwapToToken(token)); setVerificationClicked(false); }, - [dispatch], + [dispatch, latestAddedTokenTo, toAddress], ); const tokensWithBalancesFromToken = tokensWithBalances.find((token) => diff --git a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.test.js b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.test.js index 61255621f..9f0a499f7 100644 --- a/ui/pages/swaps/prepare-swap-page/prepare-swap-page.test.js +++ b/ui/pages/swaps/prepare-swap-page/prepare-swap-page.test.js @@ -27,6 +27,7 @@ const createProps = (customProps = {}) => { setBackgroundConnection({ resetPostFetchState: jest.fn(), + ignoreTokens: jest.fn(), setBackgroundSwapRouteState: jest.fn(), clearSwapsQuotes: jest.fn(), stopPollingForQuotes: jest.fn(), From 15bf697538d59714933eaa85865b311c5b839e4e Mon Sep 17 00:00:00 2001 From: Dan Miller Date: Sun, 6 Aug 2023 20:42:30 -0230 Subject: [PATCH 28/31] Version v10.34.4 --- CHANGELOG.md | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 989ee2e8f..b2b42df2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.34.4] + ## [10.34.3] ### Fixed - Ensure users phishing warning list is properly updated ([#20381](https://github.com/MetaMask/metamask-extension/pull/20381)) @@ -3881,7 +3883,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Uncategorized - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.3...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.34.4...HEAD +[10.34.4]: https://github.com/MetaMask/metamask-extension/compare/v10.34.3...v10.34.4 [10.34.3]: https://github.com/MetaMask/metamask-extension/compare/v10.34.2...v10.34.3 [10.34.2]: https://github.com/MetaMask/metamask-extension/compare/v10.34.1...v10.34.2 [10.34.1]: https://github.com/MetaMask/metamask-extension/compare/v10.34.0...v10.34.1 diff --git a/package.json b/package.json index 280eb8e0d..297c63612 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "10.34.3", + "version": "10.34.4", "private": true, "repository": { "type": "git", From 9fdf2f0076a7fb22ea8153b77d81eba0d8f9da82 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 7 Aug 2023 13:46:05 +0200 Subject: [PATCH 29/31] [FLASK] `snaps@0.38.1-flask.1` (#20420) --- builds.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builds.yml b/builds.yml index 019da9378..26101ddac 100644 --- a/builds.yml +++ b/builds.yml @@ -52,7 +52,7 @@ buildTypes: - SEGMENT_FLASK_WRITE_KEY - ALLOW_LOCAL_SNAPS: true - REQUIRE_SNAPS_ALLOWLIST: false - - IFRAME_EXECUTION_ENVIRONMENT_URL: https://execution.metamask.io/0.35.2-flask.1/index.html + - IFRAME_EXECUTION_ENVIRONMENT_URL: https://execution.metamask.io/0.38.1-flask.1/index.html - SUPPORT_LINK: https://metamask-flask.zendesk.com/hc - SUPPORT_REQUEST_LINK: https://metamask-flask.zendesk.com/hc/en-us/requests/new - INFURA_ENV_KEY_REF: INFURA_FLASK_PROJECT_ID @@ -71,7 +71,7 @@ buildTypes: - SEGMENT_FLASK_WRITE_KEY - ALLOW_LOCAL_SNAPS: true - REQUIRE_SNAPS_ALLOWLIST: false - - IFRAME_EXECUTION_ENVIRONMENT_URL: https://execution.metamask.io/0.35.2-flask.1/index.html + - IFRAME_EXECUTION_ENVIRONMENT_URL: https://execution.metamask.io/0.38.1-flask.1/index.html - SUPPORT_LINK: https://metamask-flask.zendesk.com/hc - SUPPORT_REQUEST_LINK: https://metamask-flask.zendesk.com/hc/en-us/requests/new - INFURA_ENV_KEY_REF: INFURA_FLASK_PROJECT_ID From ce0fd8069e60b86ace8565b8302e605e945fc4b0 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Mon, 7 Aug 2023 09:34:56 -0230 Subject: [PATCH 30/31] Update changelog for v10.34.4 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2b42df2f..d50937b4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ## [10.34.4] +## Changed +- Updated snaps execution environment ([#20420](https://github.com/MetaMask/metamask-extension/pull/20420)) ## [10.34.3] ### Fixed From 0c8bd0ce4a0f4b23a0b212e8130c51b5c93a2999 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Mon, 7 Aug 2023 09:38:47 -0230 Subject: [PATCH 31/31] Update CHANGELOG.md Co-authored-by: Frederik Bolding --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d50937b4c..583701a69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ## [10.34.4] -## Changed +### Changed - Updated snaps execution environment ([#20420](https://github.com/MetaMask/metamask-extension/pull/20420)) ## [10.34.3]