diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d8b93bdc..18c9eaab9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,37 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [10.35.0] ### Uncategorized - temp +## [10.34.4] +### Changed +- Updated snaps execution environment ([#20420](https://github.com/MetaMask/metamask-extension/pull/20420)) + +## [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)) + +## 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)) +- 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)) +- 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)) +- Fix ([#20080](https://github.com/MetaMask/metamask-extension/pull/20080)) ## [10.34.0] ### Added @@ -3859,6 +3890,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v10.35.0...HEAD [10.35.0]: https://github.com/MetaMask/metamask-extension/compare/v10.34.0...v10.35.0 +[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 [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/app/_locales/en/messages.json b/app/_locales/en/messages.json index 567e7ca3f..c267d5feb 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -2743,6 +2743,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/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 a383f8221..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({ @@ -352,6 +356,6 @@ function toMetamaskUrl(origUrl) { if (!filePath) { return origUrl; } - const metamaskUrl = `metamask${filePath}`; + const metamaskUrl = `/metamask${filePath}`; return metamaskUrl; } 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..2db150de3 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'; @@ -169,6 +177,18 @@ const firstTimeState = { }, }, }, + PhishingController: { + phishingLists: [ + { + allowlist: [], + blocklist: ['test.metamask-phishing.io'], + fuzzylist: [], + tolerance: 0, + version: 0, + name: 'MetaMask', + }, + ], + }, }; const noop = () => undefined; @@ -185,25 +205,36 @@ 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: ['test.metamask-phishing.io'], + name: ListNames.MetaMask, + }, + phishfort_hotlist: { + blocklist: [], + name: ListNames.Phishfort, + }, }), ) - .get('/api/v1/lists/hotlist.json') + .get(METAMASK_HOTLIST_DIFF_FILE) .reply( 200, JSON.stringify([ - { url: '127.0.0.1', targetList: 'blocklist', timestamp: 0 }, + { + url: 'test.metamask-phishing.io', + targetList: 'blocklist', + timestamp: 0, + }, ]), ); @@ -223,6 +254,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; @@ -931,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/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'], + }, + }, + }, + }, + }); + }); + }); + }); }); 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/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/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/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/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 813f5e799..dc4fcd4e0 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -92,6 +92,10 @@ 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'; +import * as m090 from './090'; +import * as m091 from './091'; +import * as m092 from './092'; const migrations = [ m002, @@ -181,6 +185,10 @@ const migrations = [ m086, m087, m088, + m089, + m090, + m091, + m092, ]; export default migrations; 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/builds.yml b/builds.yml index 4ea5e4eb7..81dbe9ef5 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.36.1-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.36.1-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 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/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index a23f68f9b..c767a46e5 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -887,8 +887,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, @@ -896,6 +896,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, @@ -1530,6 +1543,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1714,34 +1728,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 @@ -1772,6 +1764,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1914,6 +1907,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1949,6 +1943,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 4c80538b4..799ae791f 100644 --- a/lavamoat/browserify/desktop/policy.json +++ b/lavamoat/browserify/desktop/policy.json @@ -887,8 +887,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, @@ -896,6 +896,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, @@ -1277,6 +1290,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1294,6 +1308,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1656,6 +1671,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1847,34 +1863,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 @@ -1994,6 +1988,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2199,6 +2194,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2368,6 +2364,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2405,6 +2402,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2438,6 +2436,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 4c80538b4..799ae791f 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -887,8 +887,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, @@ -896,6 +896,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, @@ -1277,6 +1290,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1294,6 +1308,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1656,6 +1671,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1847,34 +1863,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 @@ -1994,6 +1988,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2199,6 +2194,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2368,6 +2364,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2405,6 +2402,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2438,6 +2436,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 a23f68f9b..c767a46e5 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -887,8 +887,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, @@ -896,6 +896,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, @@ -1530,6 +1543,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1714,34 +1728,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 @@ -1772,6 +1764,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1914,6 +1907,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1949,6 +1943,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 d198cb950..c56666bbd 100644 --- a/lavamoat/browserify/mmi/policy.json +++ b/lavamoat/browserify/mmi/policy.json @@ -1115,8 +1115,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, @@ -1124,6 +1124,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, @@ -1758,6 +1771,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -1942,34 +1956,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 @@ -2000,6 +1992,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2142,6 +2135,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, @@ -2177,6 +2171,7 @@ "TextEncoder": true }, "packages": { + "@metamask/key-tree>@noble/hashes": true, "browserify>buffer": true, "nock>debug": true, "semver": true, diff --git a/lavamoat/build-system/policy.json b/lavamoat/build-system/policy.json index d3aacfc15..fab32164f 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 @@ -8158,7 +8151,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 e5822e1a4..3dce2bbf4 100644 --- a/package.json +++ b/package.json @@ -229,7 +229,7 @@ "@metamask/announcement-controller": "^4.0.0", "@metamask/approval-controller": "^3.4.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", @@ -252,7 +252,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": "^6.0.0", "@metamask/post-message-stream": "^6.0.0", "@metamask/ppom-validator": "^0.0.1", "@metamask/providers": "^11.1.0", @@ -385,7 +385,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/shared/notifications/index.js b/shared/notifications/index.js index a5a689f3a..f5d496b8b 100644 --- a/shared/notifications/index.js +++ b/shared/notifications/index.js @@ -114,6 +114,13 @@ export const UI_NOTIFICATIONS = { width: '100%', }, }, + 22: { + id: 22, + date: null, + image: { + src: 'images/global-menu-block-explorer.svg', + }, + }, }; export const getTranslatedUINotifications = (t, locale) => { @@ -313,5 +320,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: { diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index 2e7b224e9..c96c8788f 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -499,85 +499,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'; @@ -787,8 +708,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 a3e48208d..997c040c2 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'); /** * Setup E2E network mocks. @@ -385,19 +373,7 @@ async function setupMocking(server, testSpecificMock, { chainId }) { }; }); - 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/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/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/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); + }, + ); + }); }); }); 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/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/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/components/app/whats-new-popup/whats-new-popup.js b/ui/components/app/whats-new-popup/whats-new-popup.js index 8b8be5b48..94631434e 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/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 d066dcc7f..90f070c45 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 { IconName, @@ -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,18 @@ 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/__snapshots__/account-list-item.test.js.snap b/ui/components/multichain/account-list-item/__snapshots__/account-list-item.test.js.snap index 248a913dc..c988cb28c 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

{ + if (selected) { + itemRef.current?.scrollIntoView?.(); + } + }, [itemRef, selected]); + const keyring = useSelector((state) => findKeyringForAddress(state, identity.address), ); const label = getLabel(keyring, t); - const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider); - const { blockExplorerUrl } = rpcPrefs; - const blockExplorerUrlSubTitle = getURLHostName(blockExplorerUrl); - const trackEvent = useContext(MetaMetricsContext); return ( @@ -107,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 @@ -202,7 +208,7 @@ export const AccountListItem = ({ /> ) : null} - {shortenAddress(identity.address)} + {shortenAddress(toChecksumHexAddress(identity.address))} setAccountOptionsMenuOpen(false)} isOpen={accountOptionsMenuOpen} diff --git a/ui/components/multichain/account-list-item/account-list-item.test.js b/ui/components/multichain/account-list-item/account-list-item.test.js index 3cd9cf3c3..354fdac0c 100644 --- a/ui/components/multichain/account-list-item/account-list-item.test.js +++ b/ui/components/multichain/account-list-item/account-list-item.test.js @@ -1,6 +1,7 @@ /* eslint-disable jest/require-top-level-describe */ import React from 'react'; import { screen, fireEvent } from '@testing-library/react'; +import { toChecksumHexAddress } from '@metamask/controller-utils'; import { renderWithProvider } from '../../../../test/jest'; import configureStore from '../../../store/store'; import mockState from '../../../../test/data/mock-state.json'; @@ -34,7 +35,7 @@ describe('AccountListItem', () => { 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/global-menu/global-menu.js b/ui/components/multichain/global-menu/global-menu.js index 350f15947..b70960227 100644 --- a/ui/components/multichain/global-menu/global-menu.js +++ b/ui/components/multichain/global-menu/global-menu.js @@ -42,6 +42,7 @@ import { ///: END:ONLY_INCLUDE_IN import { getMetaMetricsId, + getSelectedAddress, ///: BEGIN:ONLY_INCLUDE_IN(snaps) getUnreadNotificationsCount, ///: END:ONLY_INCLUDE_IN @@ -57,6 +58,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(); @@ -64,6 +68,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, @@ -86,6 +91,16 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { return ( + + { event: MetaMetricsEventName.NavConnectedSitesOpened, category: MetaMetricsEventCategory.Navigation, properties: { - location: 'Global Menu', + location: METRICS_LOCATION, }, }); closeMenu(); @@ -141,7 +156,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { event: MetaMetricsEventName.PortfolioLinkClicked, properties: { url: portfolioUrl, - location: 'Global Menu', + location: METRICS_LOCATION, }, }, { @@ -168,7 +183,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { event: MetaMetricsEventName.AppWindowExpanded, category: MetaMetricsEventCategory.Navigation, properties: { - location: 'Global Menu', + location: METRICS_LOCATION, }, }); closeMenu(); @@ -226,7 +241,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { event: MetaMetricsEventName.SupportLinkClicked, properties: { url: supportLink, - location: 'Global Menu', + location: METRICS_LOCATION, }, }, { @@ -250,7 +265,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => { category: MetaMetricsEventCategory.Navigation, event: MetaMetricsEventName.NavSettingsOpened, properties: { - location: 'Global Menu', + location: METRICS_LOCATION, }, }); closeMenu(); @@ -268,7 +283,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 36183e015..95a9b191b 100644 --- a/ui/components/multichain/index.js +++ b/ui/components/multichain/index.js @@ -17,3 +17,4 @@ export { AccountDetails } from './account-details'; export { CreateAccount } from './create-account'; export { ImportAccount } from './import-account'; export { ImportNftsModal } from './import-nfts-modal'; +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..60baab1a6 --- /dev/null +++ b/ui/components/multichain/menu-items/view-explorer-menu-item.js @@ -0,0 +1,102 @@ +import React, { useContext } from 'react'; +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'; +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, +} from '../../../selectors'; +import { getURLHostName } from '../../../helpers/utils/util'; +import { NETWORKS_ROUTE } from '../../../helpers/constants/routes'; + +export const ViewExplorerMenuItem = ({ + metricsLocation, + closeMenu, + textProps, + address, +}) => { + const t = useI18nContext(); + const trackEvent = useContext(MetaMetricsContext); + const history = useHistory(); + + const chainId = useSelector(getCurrentChainId); + const rpcPrefs = useSelector(getRpcPrefsForCurrentProvider); + const addressLink = getAccountLink( + toChecksumHexAddress(address), + 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, + 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 new file mode 100644 index 000000000..447635fd9 --- /dev/null +++ b/ui/components/multichain/menu-items/view-explorer-menu-item.test.js @@ -0,0 +1,30 @@ +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(); + }); +}); 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/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 cb3a701ee..a92451cf7 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, + ); } }); }} 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 654d2b766..f7c7f9fc6 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(), diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 4b8deb662..079c1f493 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -1008,6 +1008,7 @@ function getAllowedAnnouncementIds(state) { 19: false, 20: currentKeyringIsLedger && isFirefox, 21: isSwapsChain, + 22: true, }; } diff --git a/yarn.lock b/yarn.lock index b4ca89b19..6ab9f254b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3960,13 +3960,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 @@ -4027,20 +4027,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 @@ -4206,6 +4205,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" @@ -4531,16 +4540,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:^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.0.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: b0b9a86cba1928f0fd22a2aed196d75dc19a5e56547efe1b533d7ae06eaaf9432a6ee5004a8fd477f52310b50c2f3635a1e70ac83e3670f4cc6a1f488a674d73 + checksum: 13a85865cef1515f6d0ee1cd02da37e5e6b98c493676e3a80195294725b717aa17651a0c24d2e841f790bbd22ae16911cc16bab7846da8266f4ee03007a17f4e languageName: node linkType: hard @@ -5144,16 +5153,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 @@ -5735,18 +5745,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 @@ -24649,7 +24659,7 @@ __metadata: "@metamask/approval-controller": ^3.4.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 @@ -24678,7 +24688,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": ^6.0.0 "@metamask/phishing-warning": ^2.1.0 "@metamask/post-message-stream": ^6.0.0 "@metamask/ppom-validator": ^0.0.1 @@ -24705,7 +24715,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 @@ -26294,8 +26304,8 @@ __metadata: 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": - version: 2.6.11 - resolution: "node-fetch@npm:2.6.11" + version: 2.6.12 + resolution: "node-fetch@npm:2.6.12" dependencies: whatwg-url: ^5.0.0 peerDependencies: @@ -26303,7 +26313,7 @@ __metadata: peerDependenciesMeta: encoding: optional: true - checksum: 249d0666a9497553384d46b5ab296ba223521ac88fed4d8a17d6ee6c2efb0fc890f3e8091cafe7f9fba8151a5b8d925db2671543b3409a56c3cd522b468b47b3 + checksum: 3bc1655203d47ee8e313c0d96664b9673a3d4dd8002740318e9d27d14ef306693a4b2ef8d6525775056fd912a19e23f3ac0d7111ad8925877b7567b29a625592 languageName: node linkType: hard