diff --git a/.storybook/initial-states/approval-screens/add-suggested-token.js b/.storybook/initial-states/approval-screens/add-suggested-token.js index ee62778cc..301e6bfa1 100644 --- a/.storybook/initial-states/approval-screens/add-suggested-token.js +++ b/.storybook/initial-states/approval-screens/add-suggested-token.js @@ -1,83 +1,77 @@ -export const suggestedAssets = [ +import { ApprovalType } from '@metamask/controller-utils'; + +const suggestedAssets = [ { - asset: { - address: '0x6b175474e89094c44da98b954eedeac495271d0f', - symbol: 'ETH', - decimals: 18, - image: './images/eth_logo.png', - unlisted: false, - }, + address: '0x6b175474e89094c44da98b954eedeac495271d0f', + symbol: 'ETH', + decimals: 18, + image: './images/eth_logo.png', + unlisted: false, }, { - asset: { - address: '0xB8c77482e45F1F44dE1745F52C74426C631bDD52', - symbol: '0X', - decimals: 18, - image: '0x.svg', - unlisted: false, - }, + address: '0xB8c77482e45F1F44dE1745F52C74426C631bDD52', + symbol: '0X', + decimals: 18, + image: '0x.svg', + unlisted: false, }, { - asset: { - address: '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984', - symbol: 'AST', - decimals: 18, - image: 'ast.png', - unlisted: false, - }, + address: '0x1f9840a85d5af5bf1d1762f925bdaddc4201f984', + symbol: 'AST', + decimals: 18, + image: 'ast.png', + unlisted: false, }, { - asset: { - address: '0x9f8f72aa9304c8b593d555f12ef6589cc3a579a2', - symbol: 'BAT', - decimals: 18, - image: 'BAT_icon.svg', - unlisted: false, - }, + address: '0x9f8f72aa9304c8b593d555f12ef6589cc3a579a2', + symbol: 'BAT', + decimals: 18, + image: 'BAT_icon.svg', + unlisted: false, }, { - asset: { - address: '0xe83cccfabd4ed148903bf36d4283ee7c8b3494d1', - symbol: 'CVL', - decimals: 18, - image: 'CVL_token.svg', - unlisted: false, - }, + address: '0xe83cccfabd4ed148903bf36d4283ee7c8b3494d1', + symbol: 'CVL', + decimals: 18, + image: 'CVL_token.svg', + unlisted: false, }, { - asset: { - address: '0x0bc529c00C6401aEF6D220BE8C6Ea1667F6Ad93e', - symbol: 'GLA', - decimals: 18, - image: 'gladius.svg', - unlisted: false, - }, + address: '0x0bc529c00C6401aEF6D220BE8C6Ea1667F6Ad93e', + symbol: 'GLA', + decimals: 18, + image: 'gladius.svg', + unlisted: false, }, { - asset: { - address: '0x467Bccd9d29f223BcE8043b84E8C8B282827790F', - symbol: 'GNO', - decimals: 18, - image: 'gnosis.svg', - unlisted: false, - }, + address: '0x467Bccd9d29f223BcE8043b84E8C8B282827790F', + symbol: 'GNO', + decimals: 18, + image: 'gnosis.svg', + unlisted: false, }, { - asset: { - address: '0xff20817765cb7f73d4bde2e66e067e58d11095c2', - symbol: 'OMG', - decimals: 18, - image: 'omg.jpg', - unlisted: false, - }, + address: '0xff20817765cb7f73d4bde2e66e067e58d11095c2', + symbol: 'OMG', + decimals: 18, + image: 'omg.jpg', + unlisted: false, }, { - asset: { - address: '0x8e870d67f660d95d5be530380d0ec0bd388289e1', - symbol: 'WED', - decimals: 18, - image: 'wed.png', - unlisted: false, - }, + address: '0x8e870d67f660d95d5be530380d0ec0bd388289e1', + symbol: 'WED', + decimals: 18, + image: 'wed.png', + unlisted: false, }, ]; + +export const pendingAssetApprovals = suggestedAssets.map((asset, index) => { + return { + type: ApprovalType.WatchAsset, + requestData: { + id: index, + asset, + }, + }; +}); diff --git a/.storybook/test-data.js b/.storybook/test-data.js index 360e6e90d..f538ee255 100644 --- a/.storybook/test-data.js +++ b/.storybook/test-data.js @@ -1153,7 +1153,6 @@ const state = { '0xaD6D458402F60fD3Bd25163575031ACDce07538D': './sai.svg', }, hiddenTokens: [], - suggestedAssets: [], useNonceField: false, usePhishDetect: true, useTokenDetection: true, diff --git a/.yarn/patches/@metamask-assets-controllers-npm-7.0.0-9dec51787d.patch b/.yarn/patches/@metamask-assets-controllers-npm-7.0.0-9dec51787d.patch new file mode 100644 index 000000000..267786d5e --- /dev/null +++ b/.yarn/patches/@metamask-assets-controllers-npm-7.0.0-9dec51787d.patch @@ -0,0 +1,211 @@ +diff --git a/dist/TokensController.js b/dist/TokensController.js +index 0e03b88e8a46dce73a5cc87fb432b0b2431b3797..fc8893fa6bad76d65aa34fa1bfb0b233b1259ae6 100644 +--- a/dist/TokensController.js ++++ b/dist/TokensController.js +@@ -25,13 +25,6 @@ const base_controller_1 = require("@metamask/base-controller"); + const controller_utils_1 = require("@metamask/controller-utils"); + const assetsUtil_1 = require("./assetsUtil"); + const token_service_1 = require("./token-service"); +-var SuggestedAssetStatus; +-(function (SuggestedAssetStatus) { +- SuggestedAssetStatus["accepted"] = "accepted"; +- SuggestedAssetStatus["failed"] = "failed"; +- SuggestedAssetStatus["pending"] = "pending"; +- SuggestedAssetStatus["rejected"] = "rejected"; +-})(SuggestedAssetStatus || (SuggestedAssetStatus = {})); + /** + * The name of the {@link TokensController}. + */ +@@ -93,10 +86,6 @@ class TokensController extends base_controller_1.BaseController { + }); + }); + } +- failSuggestedAsset(suggestedAssetMeta, error) { +- const failedSuggestedAssetMeta = Object.assign(Object.assign({}, suggestedAssetMeta), { status: SuggestedAssetStatus.failed, error }); +- this.hub.emit(`${suggestedAssetMeta.id}:finished`, failedSuggestedAssetMeta); +- } + /** + * Fetch metadata for a token. + * +@@ -412,9 +401,10 @@ class TokensController extends base_controller_1.BaseController { + _generateRandomId() { + return (0, uuid_1.v1)(); + } ++ // THIS PATCHED METHOD HAS ALREADY BEEN RELEASED IN VERSION 8.0.0 of @metamask/assets-controllers + /** +- * Adds a new suggestedAsset to state. Parameters will be validated according to +- * asset type being watched. A `:pending` hub event will be emitted once added. ++ * Adds a new suggestedAsset to the list of watched assets. ++ * Parameters will be validated according to the asset type being watched. + * + * @param asset - The asset to be watched. For now only ERC20 tokens are accepted. + * @param type - The asset type. +@@ -423,103 +413,22 @@ class TokensController extends base_controller_1.BaseController { + */ + watchAsset(asset, type, interactingAddress) { + return __awaiter(this, void 0, void 0, function* () { ++ if (type !== controller_utils_1.ERC20) { ++ throw new Error(`Asset of type ${type} not supported`); ++ } + const { selectedAddress } = this.config; + const suggestedAssetMeta = { + asset, + id: this._generateRandomId(), +- status: SuggestedAssetStatus.pending, + time: Date.now(), + type, + interactingAddress: interactingAddress || selectedAddress, + }; +- try { +- switch (type) { +- case 'ERC20': +- (0, assetsUtil_1.validateTokenToWatch)(asset); +- break; +- default: +- throw new Error(`Asset of type ${type} not supported`); +- } +- } +- catch (error) { +- this.failSuggestedAsset(suggestedAssetMeta, error); +- return Promise.reject(error); +- } +- const result = new Promise((resolve, reject) => { +- this.hub.once(`${suggestedAssetMeta.id}:finished`, (meta) => { +- switch (meta.status) { +- case SuggestedAssetStatus.accepted: +- return resolve(meta.asset.address); +- case SuggestedAssetStatus.rejected: +- return reject(new Error('User rejected to watch the asset.')); +- case SuggestedAssetStatus.failed: +- return reject(new Error(meta.error.message)); +- /* istanbul ignore next */ +- default: +- return reject(new Error(`Unknown status: ${meta.status}`)); +- } +- }); +- }); +- const { suggestedAssets } = this.state; +- suggestedAssets.push(suggestedAssetMeta); +- this.update({ suggestedAssets: [...suggestedAssets] }); +- this._requestApproval(suggestedAssetMeta); +- return { result, suggestedAssetMeta }; +- }); +- } +- /** +- * Accepts to watch an asset and updates it's status and deletes the suggestedAsset from state, +- * adding the asset to corresponding asset state. In this case ERC20 tokens. +- * A `:finished` hub event is fired after accepted or failure. +- * +- * @param suggestedAssetID - The ID of the suggestedAsset to accept. +- */ +- acceptWatchAsset(suggestedAssetID) { +- return __awaiter(this, void 0, void 0, function* () { +- const { selectedAddress } = this.config; +- const { suggestedAssets } = this.state; +- const index = suggestedAssets.findIndex(({ id }) => suggestedAssetID === id); +- const suggestedAssetMeta = suggestedAssets[index]; +- try { +- switch (suggestedAssetMeta.type) { +- case 'ERC20': +- const { address, symbol, decimals, image } = suggestedAssetMeta.asset; +- yield this.addToken(address, symbol, decimals, image, (suggestedAssetMeta === null || suggestedAssetMeta === void 0 ? void 0 : suggestedAssetMeta.interactingAddress) || selectedAddress); +- this._acceptApproval(suggestedAssetID); +- const acceptedSuggestedAssetMeta = Object.assign(Object.assign({}, suggestedAssetMeta), { status: SuggestedAssetStatus.accepted }); +- this.hub.emit(`${suggestedAssetMeta.id}:finished`, acceptedSuggestedAssetMeta); +- break; +- default: +- throw new Error(`Asset of type ${suggestedAssetMeta.type} not supported`); +- } +- } +- catch (error) { +- this.failSuggestedAsset(suggestedAssetMeta, error); +- this._rejectApproval(suggestedAssetID); +- } +- const newSuggestedAssets = suggestedAssets.filter(({ id }) => id !== suggestedAssetID); +- this.update({ suggestedAssets: [...newSuggestedAssets] }); ++ (0, assetsUtil_1.validateTokenToWatch)(asset); ++ yield this._requestApproval(suggestedAssetMeta); ++ yield this.addToken(asset.address, asset.symbol, asset.decimals, asset.image, suggestedAssetMeta.interactingAddress); + }); + } +- /** +- * Rejects a watchAsset request based on its ID by setting its status to "rejected" +- * and emitting a `:finished` hub event. +- * +- * @param suggestedAssetID - The ID of the suggestedAsset to accept. +- */ +- rejectWatchAsset(suggestedAssetID) { +- const { suggestedAssets } = this.state; +- const index = suggestedAssets.findIndex(({ id }) => suggestedAssetID === id); +- const suggestedAssetMeta = suggestedAssets[index]; +- if (!suggestedAssetMeta) { +- return; +- } +- const rejectedSuggestedAssetMeta = Object.assign(Object.assign({}, suggestedAssetMeta), { status: SuggestedAssetStatus.rejected }); +- this.hub.emit(`${suggestedAssetMeta.id}:finished`, rejectedSuggestedAssetMeta); +- const newSuggestedAssets = suggestedAssets.filter(({ id }) => id !== suggestedAssetID); +- this.update({ suggestedAssets: [...newSuggestedAssets] }); +- this._rejectApproval(suggestedAssetID); +- } + /** + * Takes a new tokens and ignoredTokens array for the current network/account combination + * and returns new allTokens and allIgnoredTokens state to update to. +@@ -576,43 +485,26 @@ class TokensController extends base_controller_1.BaseController { + clearIgnoredTokens() { + this.update({ ignoredTokens: [], allIgnoredTokens: {} }); + } ++ // THIS PATCHED METHOD HAS ALREADY BEEN RELEASED IN VERSION 8.0.0 of @metamask/assets-controllers + _requestApproval(suggestedAssetMeta) { +- this.messagingSystem +- .call('ApprovalController:addRequest', { +- id: suggestedAssetMeta.id, +- origin: controller_utils_1.ORIGIN_METAMASK, +- type: controller_utils_1.ApprovalType.WatchAsset, +- requestData: { ++ return __awaiter(this, void 0, void 0, function* () { ++ return this.messagingSystem.call('ApprovalController:addRequest', { + id: suggestedAssetMeta.id, +- interactingAddress: suggestedAssetMeta.interactingAddress, +- asset: { +- address: suggestedAssetMeta.asset.address, +- decimals: suggestedAssetMeta.asset.decimals, +- symbol: suggestedAssetMeta.asset.symbol, +- image: suggestedAssetMeta.asset.image || null, ++ origin: controller_utils_1.ORIGIN_METAMASK, ++ type: controller_utils_1.ApprovalType.WatchAsset, ++ requestData: { ++ id: suggestedAssetMeta.id, ++ interactingAddress: suggestedAssetMeta.interactingAddress, ++ asset: { ++ address: suggestedAssetMeta.asset.address, ++ decimals: suggestedAssetMeta.asset.decimals, ++ symbol: suggestedAssetMeta.asset.symbol, ++ image: suggestedAssetMeta.asset.image || null, ++ }, + }, +- }, +- }, true) +- .catch(() => { +- // Intentionally ignored as promise not currently used ++ }, true); + }); + } +- _acceptApproval(approvalRequestId) { +- try { +- this.messagingSystem.call('ApprovalController:acceptRequest', approvalRequestId); +- } +- catch (error) { +- console.error('Failed to accept token watch approval request', error); +- } +- } +- _rejectApproval(approvalRequestId) { +- try { +- this.messagingSystem.call('ApprovalController:rejectRequest', approvalRequestId, new Error('Rejected')); +- } +- catch (messageCallError) { +- console.error('Failed to reject token watch approval request', messageCallError); +- } +- } + } + exports.TokensController = TokensController; + exports.default = TokensController; diff --git a/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js b/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js index 6935ef96c..58a3c22dd 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/watch-asset.js @@ -1,4 +1,3 @@ -import { ethErrors } from 'eth-rpc-errors'; import { MESSAGE_TYPE } from '../../../../../shared/constants/app'; const watchAsset = { @@ -37,14 +36,10 @@ async function watchAssetHandler( ) { try { const { options: asset, type } = req.params; - const handleWatchAssetResult = await handleWatchAssetRequest(asset, type); - await handleWatchAssetResult.result; + await handleWatchAssetRequest(asset, type); res.result = true; return end(); } catch (error) { - if (error.message === 'User rejected to watch the asset.') { - return end(ethErrors.provider.userRejectedRequest()); - } return end(error); } } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 93cd1af2f..0ff2eab4a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -2121,10 +2121,6 @@ export default class MetamaskController extends EventEmitter { preferencesController, ), addToken: tokensController.addToken.bind(tokensController), - rejectWatchAsset: - tokensController.rejectWatchAsset.bind(tokensController), - acceptWatchAsset: - tokensController.acceptWatchAsset.bind(tokensController), updateTokenType: tokensController.updateTokenType.bind(tokensController), setAccountLabel: preferencesController.setAccountLabel.bind( preferencesController, diff --git a/app/scripts/migrations/087.test.js b/app/scripts/migrations/087.test.js new file mode 100644 index 000000000..1d631e926 --- /dev/null +++ b/app/scripts/migrations/087.test.js @@ -0,0 +1,89 @@ +import { migrate, version } from './087'; + +describe('migration #87', () => { + it('should update the version metadata', async () => { + const oldStorage = { + meta: { + version: 86, + }, + data: {}, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.meta).toStrictEqual({ + version, + }); + }); + + it('should return state unaltered if there is no tokens controller state', async () => { + const oldData = { + other: 'data', + }; + const oldStorage = { + meta: { + version: 86, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should return state unaltered if there is no tokens controller suggested assets state', async () => { + const oldData = { + other: 'data', + TokensController: { + allDetectedTokens: {}, + allIgnoredTokens: {}, + allTokens: {}, + detectedTokens: [], + ignoredTokens: [], + tokens: [], + }, + }; + const oldStorage = { + meta: { + version: 86, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual(oldData); + }); + + it('should remove the suggested assets state', async () => { + const oldData = { + other: 'data', + TokensController: { + allDetectedTokens: {}, + allIgnoredTokens: {}, + allTokens: {}, + detectedTokens: [], + ignoredTokens: [], + suggestedAssets: [], + tokens: [], + }, + }; + const oldStorage = { + meta: { + version: 86, + }, + data: oldData, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual({ + other: 'data', + TokensController: { + allDetectedTokens: {}, + allIgnoredTokens: {}, + allTokens: {}, + detectedTokens: [], + ignoredTokens: [], + tokens: [], + }, + }); + }); +}); diff --git a/app/scripts/migrations/087.ts b/app/scripts/migrations/087.ts new file mode 100644 index 000000000..92093f967 --- /dev/null +++ b/app/scripts/migrations/087.ts @@ -0,0 +1,33 @@ +import { isObject } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; + +export const version = 87; + +/** + * Remove the now-obsolete tokens controller `suggestedAssets` state. + * + * @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 (!isObject(state.TokensController)) { + return state; + } + + delete state.TokensController.suggestedAssets; + + return state; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index acde4758b..d960fd658 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -90,6 +90,7 @@ import * as m083 from './083'; import * as m084 from './084'; import * as m085 from './085'; import * as m086 from './086'; +import * as m087 from './087'; const migrations = [ m002, @@ -177,6 +178,7 @@ const migrations = [ m084, m085, m086, + m087, ]; export default migrations; diff --git a/package.json b/package.json index edcdce0f9..f0ea30063 100644 --- a/package.json +++ b/package.json @@ -197,6 +197,7 @@ "request@^2.88.2": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch", "request@^2.85.0": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch", "@metamask/assets-controllers@^6.0.0": "patch:@metamask/assets-controllers@npm%3A6.0.0#./.yarn/patches/@metamask-assets-controllers-npm-6.0.0-0cb763bd07.patch", + "@metamask/assets-controllers@^7.0.0": "patch:@metamask/assets-controllers@npm%3A7.0.0#./.yarn/patches/@metamask-assets-controllers-npm-7.0.0-9dec51787d.patch", "@metamask/signature-controller@^2.0.0": "patch:@metamask/signature-controller@npm%3A2.0.0#./.yarn/patches/@metamask-signature-controller-npm-2.0.0-f441f2596e.patch" }, "dependencies": { diff --git a/test/e2e/fixture-builder.js b/test/e2e/fixture-builder.js index 41c3b12e8..e9a0bf672 100644 --- a/test/e2e/fixture-builder.js +++ b/test/e2e/fixture-builder.js @@ -290,7 +290,6 @@ function defaultFixture() { allTokens: {}, detectedTokens: [], ignoredTokens: [], - suggestedAssets: [], tokens: [], }, TransactionController: { @@ -394,7 +393,6 @@ function onboardingFixture() { allTokens: {}, detectedTokens: [], ignoredTokens: [], - suggestedAssets: [], tokens: [], }, config: {}, @@ -740,7 +738,6 @@ class FixtureBuilder { }, allIgnoredTokens: {}, allDetectedTokens: {}, - suggestedAssets: [], }); return this; } diff --git a/test/e2e/tests/add-hide-token.spec.js b/test/e2e/tests/add-hide-token.spec.js index f37508adf..9a6860547 100644 --- a/test/e2e/tests/add-hide-token.spec.js +++ b/test/e2e/tests/add-hide-token.spec.js @@ -123,3 +123,122 @@ describe('Add existing token using search', function () { ); }); }); + +describe('Add token using wallet_watchAsset', function () { + const ganacheOptions = { + accounts: [ + { + secretKey: + '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC', + balance: convertToHexValue(25000000000000000000), + }, + ], + }; + + it('opens a notification that adds a token when wallet_watchAsset is executed, then approves', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withPermissionControllerConnectedToTestDapp() + .build(), + ganacheOptions, + title: this.test.title, + }, + 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.executeScript(` + window.ethereum.request({ + method: 'wallet_watchAsset', + params: { + type: 'ERC20', + options: { + address: '0x86002be4cdd922de1ccb831582bf99284b99ac12', + symbol: 'TST', + decimals: 4 + }, + } + }) + `); + + const windowHandles = await driver.waitUntilXWindowHandles(3); + + await driver.switchToWindowWithTitle( + 'MetaMask Notification', + windowHandles, + ); + + await driver.clickElement({ + tag: 'button', + text: 'Add token', + }); + + await driver.switchToWindowWithTitle('MetaMask', windowHandles); + + await driver.waitForSelector({ + css: '[data-testid="multichain-token-list-item-value"]', + text: '0 TST', + }); + }, + ); + }); + + it('opens a notification that adds a token when wallet_watchAsset is executed, then rejects', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withPermissionControllerConnectedToTestDapp() + .build(), + ganacheOptions, + title: this.test.title, + }, + 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.executeScript(` + window.ethereum.request({ + method: 'wallet_watchAsset', + params: { + type: 'ERC20', + options: { + address: '0x86002be4cdd922de1ccb831582bf99284b99ac12', + symbol: 'TST', + decimals: 4 + }, + } + }) + `); + + const windowHandles = await driver.waitUntilXWindowHandles(3); + + await driver.switchToWindowWithTitle( + 'MetaMask Notification', + windowHandles, + ); + + await driver.clickElement({ + tag: 'button', + text: 'Cancel', + }); + + await driver.switchToWindowWithTitle('MetaMask', windowHandles); + + const assetListItems = await driver.findElements( + '.multichain-token-list-item', + ); + + assert.strictEqual(assetListItems.length, 1); + }, + ); + }); +}); diff --git a/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.js b/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.js index 1e8044af2..1a1c783d0 100644 --- a/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.js +++ b/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.js @@ -1,6 +1,8 @@ import React, { useCallback, useContext, useEffect, useMemo } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { useHistory } from 'react-router-dom'; +import { ethErrors, serializeError } from 'eth-rpc-errors'; +import { ApprovalType } from '@metamask/controller-utils'; import ActionableMessage from '../../components/ui/actionable-message/actionable-message'; import Button from '../../components/ui/button'; import Identicon from '../../components/ui/identicon'; @@ -12,8 +14,11 @@ import { getMostRecentOverviewPage } from '../../ducks/history/history'; import { getTokens } from '../../ducks/metamask/metamask'; import ZENDESK_URLS from '../../helpers/constants/zendesk-url'; import { isEqualCaseInsensitive } from '../../../shared/modules/string-utils'; -import { getSuggestedAssets } from '../../selectors'; -import { rejectWatchAsset, acceptWatchAsset } from '../../store/actions'; +import { getApprovalRequestsByType } from '../../selectors'; +import { + resolvePendingApproval, + rejectPendingApproval, +} from '../../store/actions'; import { MetaMetricsEventCategory, MetaMetricsEventName, @@ -72,7 +77,11 @@ const ConfirmAddSuggestedToken = () => { const history = useHistory(); const mostRecentOverviewPage = useSelector(getMostRecentOverviewPage); - const suggestedAssets = useSelector(getSuggestedAssets); + const suggestedAssets = useSelector((metamaskState) => + getApprovalRequestsByType(metamaskState, ApprovalType.WatchAsset).map( + ({ requestData }) => requestData, + ), + ); const tokens = useSelector(getTokens); const trackEvent = useContext(MetaMetricsContext); @@ -119,7 +128,7 @@ const ConfirmAddSuggestedToken = () => { const handleAddTokensClick = useCallback(async () => { await Promise.all( suggestedAssets.map(async ({ asset, id }) => { - await dispatch(acceptWatchAsset(id)); + await dispatch(resolvePendingApproval(id, null)); trackEvent({ event: MetaMetricsEventName.TokenAdded, @@ -136,10 +145,23 @@ const ConfirmAddSuggestedToken = () => { }); }), ); - history.push(mostRecentOverviewPage); }, [dispatch, history, trackEvent, mostRecentOverviewPage, suggestedAssets]); + const handleCancelClick = useCallback(async () => { + await Promise.all( + suggestedAssets.map(({ id }) => + dispatch( + rejectPendingApproval( + id, + serializeError(ethErrors.provider.userRejectedRequest()), + ), + ), + ), + ); + history.push(mostRecentOverviewPage); + }, [dispatch, history, mostRecentOverviewPage, suggestedAssets]); + const goBackIfNoSuggestedAssetsOnFirstRender = () => { if (!suggestedAssets.length) { history.push(mostRecentOverviewPage); @@ -201,12 +223,7 @@ const ConfirmAddSuggestedToken = () => { { - await Promise.all( - suggestedAssets.map(({ id }) => dispatch(rejectWatchAsset(id))), - ); - history.push(mostRecentOverviewPage); - }} + onCancel={handleCancelClick} onSubmit={handleAddTokensClick} disabled={suggestedAssets.length === 0} /> diff --git a/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.stories.js b/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.stories.js index ea1947804..f3f8c4a8d 100644 --- a/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.stories.js +++ b/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.stories.js @@ -1,7 +1,7 @@ /* eslint-disable react/prop-types */ import React from 'react'; import { Provider } from 'react-redux'; -import { suggestedAssets as mockSuggestedAssets } from '../../../.storybook/initial-states/approval-screens/add-suggested-token'; +import { pendingAssetApprovals as mockPendingAssetApprovals } from '../../../.storybook/initial-states/approval-screens/add-suggested-token'; import configureStore from '../../store/store'; @@ -12,7 +12,7 @@ import ConfirmAddSuggestedToken from '.'; const store = configureStore({ metamask: { ...mockState.metamask, - suggestedAssets: [...mockSuggestedAssets], + pendingApprovals: [...mockPendingAssetApprovals], tokens: [], }, }); @@ -29,10 +29,10 @@ export const WithDuplicateAddress = () => ; const WithDuplicateAddressStore = configureStore({ metamask: { ...mockState.metamask, - suggestedAssets: [...mockSuggestedAssets], + pendingApprovals: [...mockPendingAssetApprovals], tokens: [ { - ...mockSuggestedAssets[0].asset, + ...mockPendingAssetApprovals[0].requestData.asset, }, ], }, @@ -47,10 +47,10 @@ export const WithDuplicateSymbolAndDifferentAddress = () => ( const WithDuplicateSymbolAndDifferentAddressStore = configureStore({ metamask: { ...mockState.metamask, - suggestedAssets: [...mockSuggestedAssets], + pendingApprovals: [...mockPendingAssetApprovals], tokens: [ { - ...mockSuggestedAssets[0].asset, + ...mockPendingAssetApprovals[0].requestData.asset, address: '0xNonSuggestedAddress', }, ], diff --git a/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.test.js b/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.test.js index e48e96086..db96097d9 100644 --- a/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.test.js +++ b/ui/pages/confirm-add-suggested-token/confirm-add-suggested-token.test.js @@ -1,6 +1,11 @@ import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { ApprovalType } from '@metamask/controller-utils'; import { fireEvent, screen } from '@testing-library/react'; -import { acceptWatchAsset, rejectWatchAsset } from '../../store/actions'; +import { + resolvePendingApproval, + rejectPendingApproval, +} from '../../store/actions'; import configureStore from '../../store/store'; import { renderWithProvider } from '../../../test/jest/rendering'; import ConfirmAddSuggestedToken from '.'; @@ -28,6 +33,15 @@ const MOCK_SUGGESTED_ASSETS = [ }, ]; +const MOCK_PENDING_ASSET_APPROVALS = MOCK_SUGGESTED_ASSETS.map( + (requestData) => { + return { + type: ApprovalType.WatchAsset, + requestData, + }; + }, +); + const MOCK_TOKEN = { address: '0x108cf70c7d384c552f42c07c41c0e1e46d77ea0d', symbol: 'TEST', @@ -35,14 +49,14 @@ const MOCK_TOKEN = { }; jest.mock('../../store/actions', () => ({ - acceptWatchAsset: jest.fn().mockReturnValue({ type: 'test' }), - rejectWatchAsset: jest.fn().mockReturnValue({ type: 'test' }), + resolvePendingApproval: jest.fn().mockReturnValue({ type: 'test' }), + rejectPendingApproval: jest.fn().mockReturnValue({ type: 'test' }), })); const renderComponent = (tokens = []) => { const store = configureStore({ metamask: { - suggestedAssets: [...MOCK_SUGGESTED_ASSETS], + pendingApprovals: [...MOCK_PENDING_ASSET_APPROVALS], tokens, providerConfig: { chainId: '0x1' }, }, @@ -80,23 +94,45 @@ describe('ConfirmAddSuggestedToken Component', () => { ); }); - it('should dispatch acceptWatchAsset when clicking the "Add token" button', () => { + it('should dispatch resolvePendingApproval when clicking the "Add token" button', async () => { renderComponent(); const addTokenBtn = screen.getByRole('button', { name: 'Add token' }); - fireEvent.click(addTokenBtn); - expect(acceptWatchAsset).toHaveBeenCalled(); + await act(async () => { + fireEvent.click(addTokenBtn); + }); + + expect(resolvePendingApproval).toHaveBeenCalledTimes( + MOCK_SUGGESTED_ASSETS.length, + ); + + MOCK_SUGGESTED_ASSETS.forEach(({ id }) => { + expect(resolvePendingApproval).toHaveBeenCalledWith(id, null); + }); }); - it('should dispatch rejectWatchAsset when clicking the "Cancel" button', () => { + it('should dispatch rejectPendingApproval when clicking the "Cancel" button', async () => { renderComponent(); const cancelBtn = screen.getByRole('button', { name: 'Cancel' }); - expect(rejectWatchAsset).toHaveBeenCalledTimes(0); - fireEvent.click(cancelBtn); - expect(rejectWatchAsset).toHaveBeenCalledTimes( + await act(async () => { + fireEvent.click(cancelBtn); + }); + + expect(rejectPendingApproval).toHaveBeenCalledTimes( MOCK_SUGGESTED_ASSETS.length, ); + + MOCK_SUGGESTED_ASSETS.forEach(({ id }) => { + expect(rejectPendingApproval).toHaveBeenCalledWith( + id, + expect.objectContaining({ + code: 4001, + message: 'User rejected the request.', + stack: expect.any(String), + }), + ); + }); }); describe('when the suggested token address matches an existing token address', () => { diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 8843f9fb6..fdacbe144 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -502,9 +502,7 @@ export function getCurrentCurrency(state) { } export function getTotalUnapprovedCount(state) { - const { pendingApprovalCount = 0 } = state.metamask; - - return pendingApprovalCount + getSuggestedAssetCount(state); + return state.metamask.pendingApprovalCount ?? 0; } export function getTotalUnapprovedMessagesCount(state) { @@ -556,15 +554,6 @@ export function getUnapprovedTemplatedConfirmations(state) { ); } -function getSuggestedAssetCount(state) { - const { suggestedAssets = [] } = state.metamask; - return suggestedAssets.length; -} - -export function getSuggestedAssets(state) { - return state.metamask.suggestedAssets; -} - export function getIsMainnet(state) { const chainId = getCurrentChainId(state); return chainId === CHAIN_IDS.MAINNET; diff --git a/ui/store/actions.ts b/ui/store/actions.ts index 8eb1b67c6..89931d733 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -2298,44 +2298,6 @@ export function addTokens( }; } -export function rejectWatchAsset( - suggestedAssetID: string, -): ThunkAction { - return async (dispatch: MetaMaskReduxDispatch) => { - dispatch(showLoadingIndication()); - try { - await submitRequestToBackground('rejectWatchAsset', [suggestedAssetID]); - await forceUpdateMetamaskState(dispatch); - } catch (error) { - logErrorWithMessage(error); - dispatch(displayWarning(error)); - return; - } finally { - dispatch(hideLoadingIndication()); - } - dispatch(closeCurrentNotificationWindow()); - }; -} - -export function acceptWatchAsset( - suggestedAssetID: string, -): ThunkAction { - return async (dispatch: MetaMaskReduxDispatch) => { - dispatch(showLoadingIndication()); - try { - await submitRequestToBackground('acceptWatchAsset', [suggestedAssetID]); - await forceUpdateMetamaskState(dispatch); - } catch (error) { - logErrorWithMessage(error); - dispatch(displayWarning(error)); - return; - } finally { - dispatch(hideLoadingIndication()); - } - dispatch(closeCurrentNotificationWindow()); - }; -} - export function clearPendingTokens(): Action { return { type: actionConstants.CLEAR_PENDING_TOKENS, diff --git a/yarn.lock b/yarn.lock index aea5d8707..84241ba09 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3873,7 +3873,7 @@ __metadata: languageName: node linkType: hard -"@metamask/assets-controllers@npm:^7.0.0": +"@metamask/assets-controllers@npm:7.0.0": version: 7.0.0 resolution: "@metamask/assets-controllers@npm:7.0.0" dependencies: @@ -3907,6 +3907,40 @@ __metadata: languageName: node linkType: hard +"@metamask/assets-controllers@patch:@metamask/assets-controllers@npm%3A7.0.0#./.yarn/patches/@metamask-assets-controllers-npm-7.0.0-9dec51787d.patch::locator=metamask-crx%40workspace%3A.": + version: 7.0.0 + resolution: "@metamask/assets-controllers@patch:@metamask/assets-controllers@npm%3A7.0.0#./.yarn/patches/@metamask-assets-controllers-npm-7.0.0-9dec51787d.patch::version=7.0.0&hash=e60732&locator=metamask-crx%40workspace%3A." + dependencies: + "@ethersproject/bignumber": ^5.7.0 + "@ethersproject/contracts": ^5.7.0 + "@ethersproject/providers": ^5.7.0 + "@metamask/abi-utils": ^1.1.0 + "@metamask/approval-controller": ^2.1.1 + "@metamask/base-controller": ^2.0.0 + "@metamask/contract-metadata": ^2.3.1 + "@metamask/controller-utils": ^3.4.0 + "@metamask/metamask-eth-abis": 3.0.0 + "@metamask/network-controller": ^8.0.0 + "@metamask/preferences-controller": ^3.0.0 + "@metamask/utils": ^5.0.1 + "@types/uuid": ^8.3.0 + abort-controller: ^3.0.0 + async-mutex: ^0.2.6 + babel-runtime: ^6.26.0 + eth-query: ^2.1.2 + eth-rpc-errors: ^4.0.2 + ethereumjs-util: ^7.0.10 + immer: ^9.0.6 + multiformats: ^9.5.2 + single-call-balance-checker-abi: ^1.0.0 + uuid: ^8.3.2 + peerDependencies: + "@metamask/approval-controller": ^2.1.1 + "@metamask/network-controller": ^8.0.0 + checksum: 150461535d47ac8079f726a4b8b6c130043757d87bc715ae480ac10262613e3f06f930882fec0150717856ff9dfc8c1df3e6ff96a474dec0fe850aa70b2c51a8 + languageName: node + linkType: hard + "@metamask/auto-changelog@npm:^2.1.0": version: 2.6.1 resolution: "@metamask/auto-changelog@npm:2.6.1"