diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 16e21905a..b9b2497be 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1605,6 +1605,9 @@ "message": "You need $1 more $2 to complete this swap", "description": "Tells the user how many more of a given token they need for a specific swap. $1 is an amount of tokens and $2 is the token symbol." }, + "swapBetterQuoteAvailable": { + "message": "A better quote is available" + }, "swapBuildQuotePlaceHolderText": { "message": "No tokens available matching $1", "description": "Tells the user that a given search string does not match any tokens in our token lists. $1 can be any string of text" @@ -1701,8 +1704,8 @@ "message": "We find the best price from the top liquidity sources, every time. A fee of $1% is automatically factored into each quote, which supports ongoing development to make MetaMask even better.", "description": "Provides information about the fee that metamask takes for swaps. $1 is a decimal number." }, - "swapNQuotesAvailable": { - "message": "$1 quotes available", + "swapNQuotes": { + "message": "$1 quotes", "description": "$1 is the number of quotes that the user can select from when opening the list of quotes on the 'view quote' screen" }, "swapNetworkFeeSummary": { @@ -1737,7 +1740,7 @@ "message": "Quote details" }, "swapQuoteDetailsSlippageInfo": { - "message": "If the price changes between the time your order is placed and confirmed it’s called \"slippage\". Your Swap will automatically cancel if slippage exceeds your \"max slippage\" setting." + "message": "If the price changes between the time your order is placed and confirmed it’s called \"slippage\". Your Swap will automatically cancel if slippage exceeds your \"slippage tolerance\" setting." }, "swapQuoteIncludesRate": { "message": "Quote includes a $1% MetaMask fee", @@ -1830,6 +1833,9 @@ "swapUnknown": { "message": "Unknown" }, + "swapUsingBestQuote": { + "message": "Using the best quote" + }, "swapVerifyTokenExplanation": { "message": "Multiple tokens can use the same name and symbol. Check Etherscan to verify this is the token you're looking for." }, @@ -1846,15 +1852,11 @@ "swapsAdvancedOptions": { "message": "Advanced Options" }, - "swapsBestQuote": { - "message": "Best quote" - }, - "swapsConvertToAbout": { - "message": "Convert $1 to about", - "description": "This message is part of a quote for a swap. The $1 is the amount being converted, and the amount it is being swapped for is below this message" + "swapsExcessiveSlippageWarning": { + "message": "Slippage amount is too high and will result in a bad rate. Please reduce your slippage tolerance to a value below 15%." }, "swapsMaxSlippage": { - "message": "Max slippage" + "message": "Slippage Tolerance" }, "swapsNotEnoughForTx": { "message": "Not enough $1 to complete this transaction", diff --git a/app/home.html b/app/home.html index 5c131bfc8..072eddea9 100644 --- a/app/home.html +++ b/app/home.html @@ -10,6 +10,10 @@
+ + + + diff --git a/app/images/down-arrow-grey.svg b/app/images/down-arrow-grey.svg new file mode 100644 index 000000000..fcdb33eec --- /dev/null +++ b/app/images/down-arrow-grey.svg @@ -0,0 +1,3 @@ + + + diff --git a/app/manifest/_base.json b/app/manifest/_base.json index 3192abeb4..b9b650071 100644 --- a/app/manifest/_base.json +++ b/app/manifest/_base.json @@ -2,6 +2,7 @@ "author": "https://metamask.io", "background": { "scripts": [ + "globalthis.js", "initSentry.js", "lockdown.cjs", "runLockdown.js", @@ -36,7 +37,12 @@ "content_scripts": [ { "matches": ["file://*/*", "http://*/*", "https://*/*"], - "js": ["lockdown.cjs", "runLockdown.js", "contentscript.js"], + "js": [ + "globalthis.js", + "lockdown.cjs", + "runLockdown.js", + "contentscript.js" + ], "run_at": "document_start", "all_frames": true }, diff --git a/app/notification.html b/app/notification.html index 3419acdca..4f424e3c0 100644 --- a/app/notification.html +++ b/app/notification.html @@ -33,6 +33,10 @@
+ + + + diff --git a/app/phishing.html b/app/phishing.html index 1c913db2e..59ea3ac71 100644 --- a/app/phishing.html +++ b/app/phishing.html @@ -2,6 +2,7 @@ Ethereum Phishing Detection - MetaMask + diff --git a/app/popup.html b/app/popup.html index 4d29f6153..e73f3e4d2 100644 --- a/app/popup.html +++ b/app/popup.html @@ -10,6 +10,7 @@
+ diff --git a/app/scripts/background.js b/app/scripts/background.js index fa1e48a10..92f55319a 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -19,6 +19,7 @@ import extension from 'extensionizer' import storeTransform from 'obs-store/lib/transform' import asStream from 'obs-store/lib/asStream' import PortStream from 'extension-port-stream' +import { captureException } from '@sentry/browser' import migrations from './migrations' import Migrator from './lib/migrator' import ExtensionPlatform from './platforms/extension' @@ -279,6 +280,7 @@ function setupController(initState, initLangCode) { await localStore.set(state) } catch (err) { // log error so we dont break the pipeline + captureException(err) log.error('error setting state in local store:', err) } } diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index 05d82c538..5f5f25f9d 100644 --- a/app/scripts/contentscript.js +++ b/app/scripts/contentscript.js @@ -16,16 +16,13 @@ const inpageContent = fs.readFileSync( const inpageSuffix = `//# sourceURL=${extension.runtime.getURL('inpage.js')}\n` const inpageBundle = inpageContent + inpageSuffix -// Eventually this streaming injection could be replaced with: -// https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.exportFunction -// -// But for now that is only Firefox -// If we create a FireFox-only code path using that API, -// MetaMask will be much faster loading and performant on Firefox. +const CONTENT_SCRIPT = 'metamask-contentscript' +const INPAGE = 'metamask-inpage' +const PROVIDER = 'metamask-provider' if (shouldInjectProvider()) { injectScript(inpageBundle) - start() + setupStreams() } /** @@ -46,15 +43,6 @@ function injectScript(content) { } } -/** - * Sets up the stream communication and submits site metadata - * - */ -async function start() { - await setupStreams() - await domIsReady() -} - /** * Sets up two-way communication streams between the * browser extension and local per-page browser context. @@ -63,10 +51,10 @@ async function start() { async function setupStreams() { // the transport-specific streams for communication between inpage and background const pageStream = new LocalMessageDuplexStream({ - name: 'contentscript', - target: 'inpage', + name: CONTENT_SCRIPT, + target: INPAGE, }) - const extensionPort = extension.runtime.connect({ name: 'contentscript' }) + const extensionPort = extension.runtime.connect({ name: CONTENT_SCRIPT }) const extensionStream = new PortStream(extensionPort) // create and connect channel muxers @@ -79,26 +67,26 @@ async function setupStreams() { pump(pageMux, pageStream, pageMux, (err) => logStreamDisconnectWarning('MetaMask Inpage Multiplex', err), ) - pump(extensionMux, extensionStream, extensionMux, (err) => - logStreamDisconnectWarning('MetaMask Background Multiplex', err), - ) + pump(extensionMux, extensionStream, extensionMux, (err) => { + logStreamDisconnectWarning('MetaMask Background Multiplex', err) + notifyInpageOfStreamFailure() + }) // forward communication across inpage-background for these channels only - forwardTrafficBetweenMuxers('provider', pageMux, extensionMux) - forwardTrafficBetweenMuxers('publicConfig', pageMux, extensionMux) + forwardTrafficBetweenMuxes(PROVIDER, pageMux, extensionMux) // connect "phishing" channel to warning system const phishingStream = extensionMux.createStream('phishing') phishingStream.once('data', redirectToPhishingWarning) } -function forwardTrafficBetweenMuxers(channelName, muxA, muxB) { +function forwardTrafficBetweenMuxes(channelName, muxA, muxB) { const channelA = muxA.createStream(channelName) const channelB = muxB.createStream(channelName) - pump(channelA, channelB, channelA, (err) => - logStreamDisconnectWarning( + pump(channelA, channelB, channelA, (error) => + console.debug( `MetaMask muxed traffic for channel "${channelName}" failed.`, - err, + error, ), ) } @@ -107,14 +95,35 @@ function forwardTrafficBetweenMuxers(channelName, muxA, muxB) { * Error handler for page to extension stream disconnections * * @param {string} remoteLabel - Remote stream name - * @param {Error} err - Stream connection error + * @param {Error} error - Stream connection error */ -function logStreamDisconnectWarning(remoteLabel, err) { - let warningMsg = `MetamaskContentscript - lost connection to ${remoteLabel}` - if (err) { - warningMsg += `\n${err.stack}` - } - console.warn(warningMsg) +function logStreamDisconnectWarning(remoteLabel, error) { + console.debug( + `MetaMask Contentscript: Lost connection to "${remoteLabel}".`, + error, + ) +} + +/** + * This function must ONLY be called in pump destruction/close callbacks. + * Notifies the inpage context that streams have failed, via window.postMessage. + * Relies on obj-multiplex and post-message-stream implementation details. + */ +function notifyInpageOfStreamFailure() { + window.postMessage( + { + target: INPAGE, // the post-message-stream "target" + data: { + // this object gets passed to obj-multiplex + name: PROVIDER, // the obj-multiplex channel name + data: { + jsonrpc: '2.0', + method: 'METAMASK_STREAM_FAILURE', + }, + }, + }, + window.location.origin, + ) } /** @@ -221,17 +230,3 @@ function redirectToPhishingWarning() { href: window.location.href, })}` } - -/** - * Returns a promise that resolves when the DOM is loaded (does not wait for images to load) - */ -async function domIsReady() { - // already loaded - if (['interactive', 'complete'].includes(document.readyState)) { - return undefined - } - // wait for load - return new Promise((resolve) => - window.addEventListener('DOMContentLoaded', resolve, { once: true }), - ) -} diff --git a/app/scripts/controllers/detect-tokens.js b/app/scripts/controllers/detect-tokens.js index 1fda6dccc..df821335c 100644 --- a/app/scripts/controllers/detect-tokens.js +++ b/app/scripts/controllers/detect-tokens.js @@ -47,7 +47,8 @@ export default class DetectTokensController { for (const contractAddress in contracts) { if ( contracts[contractAddress].erc20 && - !this.tokenAddresses.includes(contractAddress.toLowerCase()) + !this.tokenAddresses.includes(contractAddress.toLowerCase()) && + !this.hiddenTokens.includes(contractAddress.toLowerCase()) ) { tokensToDetect.push(contractAddress) } @@ -130,10 +131,12 @@ export default class DetectTokensController { this.tokenAddresses = currentTokens ? currentTokens.map((token) => token.address) : [] - preferences.store.subscribe(({ tokens = [] }) => { + this.hiddenTokens = preferences.store.getState().hiddenTokens + preferences.store.subscribe(({ tokens = [], hiddenTokens = [] }) => { this.tokenAddresses = tokens.map((token) => { return token.address }) + this.hiddenTokens = hiddenTokens }) preferences.store.subscribe(({ selectedAddress }) => { if (this.selectedAddress !== selectedAddress) { diff --git a/app/scripts/controllers/network/createInfuraClient.js b/app/scripts/controllers/network/createInfuraClient.js index 0d1514d61..2a26c9b75 100644 --- a/app/scripts/controllers/network/createInfuraClient.js +++ b/app/scripts/controllers/network/createInfuraClient.js @@ -7,7 +7,8 @@ import createBlockTrackerInspectorMiddleware from 'eth-json-rpc-middleware/block import providerFromMiddleware from 'eth-json-rpc-middleware/providerFromMiddleware' import createInfuraMiddleware from 'eth-json-rpc-infura' import BlockTracker from 'eth-block-tracker' -import * as networkEnums from './enums' + +import { NETWORK_TYPE_TO_ID_MAP } from './enums' export default function createInfuraClient({ network, projectId }) { const infuraMiddleware = createInfuraMiddleware({ @@ -32,36 +33,14 @@ export default function createInfuraClient({ network, projectId }) { } function createNetworkAndChainIdMiddleware({ network }) { - let chainId - let netId - - switch (network) { - case 'mainnet': - netId = networkEnums.MAINNET_NETWORK_ID - chainId = '0x01' - break - case 'ropsten': - netId = networkEnums.ROPSTEN_NETWORK_ID - chainId = '0x03' - break - case 'rinkeby': - netId = networkEnums.RINKEBY_NETWORK_ID - chainId = '0x04' - break - case 'kovan': - netId = networkEnums.KOVAN_NETWORK_ID - chainId = networkEnums.KOVAN_CHAIN_ID - break - case 'goerli': - netId = networkEnums.GOERLI_NETWORK_ID - chainId = '0x05' - break - default: - throw new Error(`createInfuraClient - unknown network "${network}"`) + if (!NETWORK_TYPE_TO_ID_MAP[network]) { + throw new Error(`createInfuraClient - unknown network "${network}"`) } + const { chainId, networkId } = NETWORK_TYPE_TO_ID_MAP[network] + return createScaffoldMiddleware({ eth_chainId: chainId, - net_version: netId, + net_version: networkId, }) } diff --git a/app/scripts/controllers/network/network.js b/app/scripts/controllers/network/network.js index 016331bce..9dc605cf2 100644 --- a/app/scripts/controllers/network/network.js +++ b/app/scripts/controllers/network/network.js @@ -19,6 +19,8 @@ import { MAINNET, INFURA_PROVIDER_TYPES, NETWORK_TYPE_TO_ID_MAP, + MAINNET_CHAIN_ID, + RINKEBY_CHAIN_ID, } from './enums' const env = process.env.METAMASK_ENV @@ -32,9 +34,9 @@ if (process.env.IN_TEST === 'true') { nickname: 'Localhost 8545', } } else if (process.env.METAMASK_DEBUG || env === 'test') { - defaultProviderConfigOpts = { type: RINKEBY } + defaultProviderConfigOpts = { type: RINKEBY, chainId: RINKEBY_CHAIN_ID } } else { - defaultProviderConfigOpts = { type: MAINNET } + defaultProviderConfigOpts = { type: MAINNET, chainId: MAINNET_CHAIN_ID } } const defaultProviderConfig = { diff --git a/app/scripts/controllers/permissions/enums.js b/app/scripts/controllers/permissions/enums.js index 56a87fdf5..d5ca52e30 100644 --- a/app/scripts/controllers/permissions/enums.js +++ b/app/scripts/controllers/permissions/enums.js @@ -19,10 +19,15 @@ export const CAVEAT_TYPES = { } export const NOTIFICATION_NAMES = { - accountsChanged: 'wallet_accountsChanged', + accountsChanged: 'metamask_accountsChanged', + unlockStateChanged: 'metamask_unlockStateChanged', + chainChanged: 'metamask_chainChanged', } -export const LOG_IGNORE_METHODS = ['wallet_sendDomainMetadata'] +export const LOG_IGNORE_METHODS = [ + 'wallet_registerOnboarding', + 'wallet_watchAsset', +] export const LOG_METHOD_TYPES = { restricted: 'restricted', @@ -82,8 +87,9 @@ export const SAFE_METHODS = [ 'eth_submitWork', 'eth_syncing', 'eth_uninstallFilter', - 'metamask_watchAsset', - 'wallet_watchAsset', 'eth_getEncryptionPublicKey', 'eth_decrypt', + 'metamask_watchAsset', + 'wallet_watchAsset', + 'metamask_getProviderState', ] diff --git a/app/scripts/controllers/permissions/index.js b/app/scripts/controllers/permissions/index.js index 2e3c12775..454bd8e56 100644 --- a/app/scripts/controllers/permissions/index.js +++ b/app/scripts/controllers/permissions/index.js @@ -28,6 +28,7 @@ export class PermissionsController { getKeyringAccounts, getRestrictedMethods, getUnlockPromise, + isUnlocked, notifyDomain, notifyAllDomains, preferences, @@ -47,6 +48,7 @@ export class PermissionsController { this._notifyDomain = notifyDomain this._notifyAllDomains = notifyAllDomains this._showPermissionRequest = showPermissionRequest + this._isUnlocked = isUnlocked this._restrictedMethods = getRestrictedMethods({ getKeyringAccounts: this.getKeyringAccounts.bind(this), @@ -463,21 +465,20 @@ export class PermissionsController { throw new Error('Invalid accounts', newAccounts) } - this._notifyDomain(origin, { - method: NOTIFICATION_NAMES.accountsChanged, - result: newAccounts, - }) - - // if the accounts changed from the perspective of the dapp, - // update "last seen" time for the origin and account(s) - // exception: no accounts -> no times to update - this.permissionsLog.updateAccountsHistory(origin, newAccounts) + // We do not share accounts when the extension is locked. + if (this._isUnlocked()) { + this._notifyDomain(origin, { + method: NOTIFICATION_NAMES.accountsChanged, + params: newAccounts, + }) + this.permissionsLog.updateAccountsHistory(origin, newAccounts) + } // NOTE: - // we don't check for accounts changing in the notifyAllDomains case, - // because the log only records when accounts were last seen, - // and the accounts only change for all domains at once when permissions - // are removed + // We don't check for accounts changing in the notifyAllDomains case, + // because the log only records when accounts were last seen, and the + // the accounts only change for all domains at once when permissions are + // removed. } /** @@ -508,9 +509,11 @@ export class PermissionsController { */ clearPermissions() { this.permissions.clearDomains() + // It's safe to notify that no accounts are available, regardless of + // extension lock state this._notifyAllDomains({ method: NOTIFICATION_NAMES.accountsChanged, - result: [], + params: [], }) } @@ -749,7 +752,3 @@ export class PermissionsController { ) } } - -export function addInternalMethodPrefix(method) { - return WALLET_PREFIX + method -} diff --git a/app/scripts/controllers/permissions/permissionsLog.js b/app/scripts/controllers/permissions/permissionsLog.js index 6bb142993..e1a37f0c8 100644 --- a/app/scripts/controllers/permissions/permissionsLog.js +++ b/app/scripts/controllers/permissions/permissionsLog.js @@ -58,6 +58,7 @@ export default class PermissionsLogController { /** * Updates the exposed account history for the given origin. * Sets the 'last seen' time to Date.now() for the given accounts. + * Returns if the accounts array is empty. * * @param {string} origin - The origin that the accounts are exposed to. * @param {Array} accounts - The accounts. diff --git a/app/scripts/controllers/permissions/permissionsMethodMiddleware.js b/app/scripts/controllers/permissions/permissionsMethodMiddleware.js index f2eec6cf4..de9009f13 100644 --- a/app/scripts/controllers/permissions/permissionsMethodMiddleware.js +++ b/app/scripts/controllers/permissions/permissionsMethodMiddleware.js @@ -73,7 +73,7 @@ export default function createPermissionsMethodMiddleware({ // custom method for getting metadata from the requesting domain, // sent automatically by the inpage provider when it's initialized - case 'wallet_sendDomainMetadata': { + case 'metamask_sendDomainMetadata': { if (typeof req.domainMetadata?.name === 'string') { addDomainMetadata(req.origin, req.domainMetadata) } diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index bfba2835b..12da861f6 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -34,8 +34,10 @@ export default class PreferencesController { const initState = { frequentRpcListDetail: [], accountTokens: {}, + accountHiddenTokens: {}, assetImages: {}, tokens: [], + hiddenTokens: [], suggestedTokens: {}, useBlockie: false, useNonceField: false, @@ -191,6 +193,7 @@ export default class PreferencesController { setAddresses(addresses) { const oldIdentities = this.store.getState().identities const oldAccountTokens = this.store.getState().accountTokens + const oldAccountHiddenTokens = this.store.getState().accountHiddenTokens const identities = addresses.reduce((ids, address, index) => { const oldId = oldIdentities[address] || {} @@ -202,7 +205,12 @@ export default class PreferencesController { tokens[address] = oldTokens return tokens }, {}) - this.store.updateState({ identities, accountTokens }) + const accountHiddenTokens = addresses.reduce((hiddenTokens, address) => { + const oldHiddenTokens = oldAccountHiddenTokens[address] || {} + hiddenTokens[address] = oldHiddenTokens + return hiddenTokens + }, {}) + this.store.updateState({ identities, accountTokens, accountHiddenTokens }) } /** @@ -212,14 +220,19 @@ export default class PreferencesController { * @returns {string} the address that was removed */ removeAddress(address) { - const { identities } = this.store.getState() - const { accountTokens } = this.store.getState() + const { + identities, + accountTokens, + accountHiddenTokens, + } = this.store.getState() + if (!identities[address]) { throw new Error(`${address} can't be deleted cause it was not found`) } delete identities[address] delete accountTokens[address] - this.store.updateState({ identities, accountTokens }) + delete accountHiddenTokens[address] + this.store.updateState({ identities, accountTokens, accountHiddenTokens }) // If the selected account is no longer valid, // select an arbitrary other account: @@ -237,7 +250,11 @@ export default class PreferencesController { * */ addAddresses(addresses) { - const { identities, accountTokens } = this.store.getState() + const { + identities, + accountTokens, + accountHiddenTokens, + } = this.store.getState() addresses.forEach((address) => { // skip if already exists if (identities[address]) { @@ -247,9 +264,10 @@ export default class PreferencesController { const identityCount = Object.keys(identities).length accountTokens[address] = {} + accountHiddenTokens[address] = {} identities[address] = { name: `Account ${identityCount + 1}`, address } }) - this.store.updateState({ identities, accountTokens }) + this.store.updateState({ identities, accountTokens, accountHiddenTokens }) } /** @@ -346,7 +364,7 @@ export default class PreferencesController { */ /** - * Adds a new token to the token array, or updates the token if passed an address that already exists. + * Adds a new token to the token array and removes it from the hiddenToken array, or updates the token if passed an address that already exists. * Modifies the existing tokens array from the store. All objects in the tokens array array AddedToken objects. * @see AddedToken {@link AddedToken} * @@ -359,8 +377,11 @@ export default class PreferencesController { async addToken(rawAddress, symbol, decimals, image) { const address = normalizeAddress(rawAddress) const newEntry = { address, symbol, decimals } - const { tokens } = this.store.getState() + const { tokens, hiddenTokens } = this.store.getState() const assetImages = this.getAssetImages() + const updatedHiddenTokens = hiddenTokens.filter( + (tokenAddress) => tokenAddress !== rawAddress.toLowerCase(), + ) const previousEntry = tokens.find((token) => { return token.address === address }) @@ -372,23 +393,24 @@ export default class PreferencesController { tokens.push(newEntry) } assetImages[address] = image - this._updateAccountTokens(tokens, assetImages) + this._updateAccountTokens(tokens, assetImages, updatedHiddenTokens) return Promise.resolve(tokens) } /** - * Removes a specified token from the tokens array. + * Removes a specified token from the tokens array and adds it to hiddenTokens array * * @param {string} rawAddress - Hex address of the token contract to remove. * @returns {Promise} The new array of AddedToken objects * */ removeToken(rawAddress) { - const { tokens } = this.store.getState() + const { tokens, hiddenTokens } = this.store.getState() const assetImages = this.getAssetImages() const updatedTokens = tokens.filter((token) => token.address !== rawAddress) + const updatedHiddenTokens = [...hiddenTokens, rawAddress.toLowerCase()] delete assetImages[rawAddress] - this._updateAccountTokens(updatedTokens, assetImages) + this._updateAccountTokens(updatedTokens, assetImages, updatedHiddenTokens) return Promise.resolve(updatedTokens) } @@ -643,47 +665,59 @@ export default class PreferencesController { */ _subscribeProviderType() { this.network.providerStore.subscribe(() => { - const { tokens } = this._getTokenRelatedStates() - this.store.updateState({ tokens }) + const { tokens, hiddenTokens } = this._getTokenRelatedStates() + this._updateAccountTokens(tokens, this.getAssetImages(), hiddenTokens) }) } /** - * Updates `accountTokens` and `tokens` of current account and network according to it. + * Updates `accountTokens`, `tokens`, `accountHiddenTokens` and `hiddenTokens` of current account and network according to it. * - * @param {Array} tokens - Array of tokens to be updated. + * @param {array} tokens - Array of tokens to be updated. + * @param {array} assetImages - Array of assets objects related to assets added + * @param {array} hiddenTokens - Array of tokens hidden by user * */ - _updateAccountTokens(tokens, assetImages) { + _updateAccountTokens(tokens, assetImages, hiddenTokens) { const { accountTokens, providerType, selectedAddress, + accountHiddenTokens, } = this._getTokenRelatedStates() accountTokens[selectedAddress][providerType] = tokens - this.store.updateState({ accountTokens, tokens, assetImages }) + accountHiddenTokens[selectedAddress][providerType] = hiddenTokens + this.store.updateState({ + accountTokens, + tokens, + assetImages, + accountHiddenTokens, + hiddenTokens, + }) } /** - * Updates `tokens` of current account and network. + * Updates `tokens` and `hiddenTokens` of current account and network. * * @param {string} selectedAddress - Account address to be updated with. * */ _updateTokens(selectedAddress) { - const { tokens } = this._getTokenRelatedStates(selectedAddress) - this.store.updateState({ tokens }) + const { tokens, hiddenTokens } = this._getTokenRelatedStates( + selectedAddress, + ) + this.store.updateState({ tokens, hiddenTokens }) } /** - * A getter for `tokens` and `accountTokens` related states. + * A getter for `tokens`, `accountTokens`, `hiddenTokens` and `accountHiddenTokens` related states. * * @param {string} [selectedAddress] - A new hex address for an account * @returns {Object.} States to interact with tokens in `accountTokens` * */ _getTokenRelatedStates(selectedAddress) { - const { accountTokens } = this.store.getState() + const { accountTokens, accountHiddenTokens } = this.store.getState() if (!selectedAddress) { // eslint-disable-next-line no-param-reassign selectedAddress = this.store.getState().selectedAddress @@ -692,11 +726,25 @@ export default class PreferencesController { if (!(selectedAddress in accountTokens)) { accountTokens[selectedAddress] = {} } + if (!(selectedAddress in accountHiddenTokens)) { + accountHiddenTokens[selectedAddress] = {} + } if (!(providerType in accountTokens[selectedAddress])) { accountTokens[selectedAddress][providerType] = [] } + if (!(providerType in accountHiddenTokens[selectedAddress])) { + accountHiddenTokens[selectedAddress][providerType] = [] + } const tokens = accountTokens[selectedAddress][providerType] - return { tokens, accountTokens, providerType, selectedAddress } + const hiddenTokens = accountHiddenTokens[selectedAddress][providerType] + return { + tokens, + accountTokens, + hiddenTokens, + accountHiddenTokens, + providerType, + selectedAddress, + } } /** diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 0c6f3c6ac..4a4488fc1 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -827,9 +827,9 @@ export default class TransactionController extends EventEmitter { ].find((methodName) => methodName === name && name.toLowerCase()) let result - if (txParams.data && tokenMethodName) { + if (data && tokenMethodName) { result = tokenMethodName - } else if (txParams.data && !to) { + } else if (data && !to) { result = TRANSACTION_CATEGORIES.DEPLOY_CONTRACT } diff --git a/app/scripts/controllers/transactions/lib/util.js b/app/scripts/controllers/transactions/lib/util.js index 758d70121..b33d3029f 100644 --- a/app/scripts/controllers/transactions/lib/util.js +++ b/app/scripts/controllers/transactions/lib/util.js @@ -1,4 +1,5 @@ import { isValidAddress } from 'ethereumjs-util' +import { ethErrors } from 'eth-json-rpc-errors' import { addHexPrefix } from '../../../lib/util' import { TRANSACTION_STATUSES } from '../../../../../shared/constants/transaction' @@ -37,19 +38,30 @@ export function normalizeTxParams(txParams, lowerCase = true) { * @throws {Error} if the tx params contains invalid fields */ export function validateTxParams(txParams) { + if (!txParams || typeof txParams !== 'object' || Array.isArray(txParams)) { + throw ethErrors.rpc.invalidParams( + 'Invalid transaction params: must be an object.', + ) + } + if (!txParams.to && !txParams.data) { + throw ethErrors.rpc.invalidParams( + 'Invalid transaction params: must specify "data" for contract deployments, or "to" (and optionally "data") for all other types of transactions.', + ) + } + validateFrom(txParams) validateRecipient(txParams) if ('value' in txParams) { const value = txParams.value.toString() if (value.includes('-')) { - throw new Error( - `Invalid transaction value of ${txParams.value} not a positive number.`, + throw ethErrors.rpc.invalidParams( + `Invalid transaction value "${txParams.value}": not a positive number.`, ) } if (value.includes('.')) { - throw new Error( - `Invalid transaction value of ${txParams.value} number must be in wei`, + throw ethErrors.rpc.invalidParams( + `Invalid transaction value of "${txParams.value}": number must be in wei.`, ) } } @@ -62,10 +74,12 @@ export function validateTxParams(txParams) { */ export function validateFrom(txParams) { if (!(typeof txParams.from === 'string')) { - throw new Error(`Invalid from address ${txParams.from} not a string`) + throw ethErrors.rpc.invalidParams( + `Invalid "from" address "${txParams.from}": not a string.`, + ) } if (!isValidAddress(txParams.from)) { - throw new Error('Invalid from address') + throw ethErrors.rpc.invalidParams('Invalid "from" address.') } } @@ -80,10 +94,10 @@ export function validateRecipient(txParams) { if (txParams.data) { delete txParams.to } else { - throw new Error('Invalid recipient address') + throw ethErrors.rpc.invalidParams('Invalid "to" address.') } } else if (txParams.to !== undefined && !isValidAddress(txParams.to)) { - throw new Error('Invalid recipient address') + throw ethErrors.rpc.invalidParams('Invalid "to" address.') } return txParams } diff --git a/app/scripts/inpage.js b/app/scripts/inpage.js index 8982a93f2..c7619f77e 100644 --- a/app/scripts/inpage.js +++ b/app/scripts/inpage.js @@ -33,11 +33,7 @@ cleanContextForImports() /* eslint-disable import/first */ import log from 'loglevel' import LocalMessageDuplexStream from 'post-message-stream' -import { initProvider } from '@metamask/inpage-provider' - -// TODO:deprecate:2020 -import setupWeb3 from './lib/setupWeb3' -/* eslint-enable import/first */ +import { initializeProvider } from '@metamask/inpage-provider' restoreContextAfterImports() @@ -49,24 +45,12 @@ log.setDefaultLevel(process.env.METAMASK_DEBUG ? 'debug' : 'warn') // setup background connection const metamaskStream = new LocalMessageDuplexStream({ - name: 'inpage', - target: 'contentscript', + name: 'metamask-inpage', + target: 'metamask-contentscript', }) -initProvider({ +initializeProvider({ connectionStream: metamaskStream, + logger: log, + shouldShimWeb3: true, }) - -// TODO:deprecate:2020 -// Setup web3 - -if (typeof window.web3 === 'undefined') { - // proxy web3, assign to window, and set up site auto reload - setupWeb3(log) -} else { - log.warn(`MetaMask detected another web3. - MetaMask will not work reliably with another web3 extension. - This usually happens if you have two MetaMasks installed, - or MetaMask and another web3 extension. Please remove one - and try again.`) -} diff --git a/app/scripts/lib/enums.js b/app/scripts/lib/enums.js index d93030f2c..a6b7a99de 100644 --- a/app/scripts/lib/enums.js +++ b/app/scripts/lib/enums.js @@ -23,7 +23,8 @@ const MESSAGE_TYPE = { ETH_GET_ENCRYPTION_PUBLIC_KEY: 'eth_getEncryptionPublicKey', ETH_SIGN: 'eth_sign', ETH_SIGN_TYPED_DATA: 'eth_signTypedData', - LOG_WEB3_USAGE: 'metamask_logInjectedWeb3Usage', + GET_PROVIDER_STATE: 'metamask_getProviderState', + LOG_WEB3_SHIM_USAGE: 'metamask_logWeb3ShimUsage', PERSONAL_SIGN: 'personal_sign', WATCH_ASSET: 'wallet_watchAsset', WATCH_ASSET_LEGACY: 'metamask_watchAsset', diff --git a/app/scripts/lib/rpc-method-middleware/createMethodMiddleware.js b/app/scripts/lib/rpc-method-middleware/createMethodMiddleware.js index b87047cd2..fdbc265e9 100644 --- a/app/scripts/lib/rpc-method-middleware/createMethodMiddleware.js +++ b/app/scripts/lib/rpc-method-middleware/createMethodMiddleware.js @@ -21,7 +21,6 @@ const handlerMap = handlers.reduce((map, handler) => { * Eventually, we'll want to extract this middleware into its own package. * * @param {Object} opts - The middleware options - * @param {string} opts.origin - The origin for the middleware stack * @param {Function} opts.sendMetrics - A function for sending a metrics event * @returns {(req: Object, res: Object, next: Function, end: Function) => void} */ diff --git a/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.js b/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.js new file mode 100644 index 000000000..32b3c18c6 --- /dev/null +++ b/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.js @@ -0,0 +1,46 @@ +import { MESSAGE_TYPE } from '../../enums' + +/** + * This RPC method gets background state relevant to the provider. + * The background sends RPC notifications on state changes, but the provider + * first requests state on initialization. + */ + +const getProviderState = { + methodNames: [MESSAGE_TYPE.GET_PROVIDER_STATE], + implementation: getProviderStateHandler, +} +export default getProviderState + +/** + * @typedef {Object} ProviderStateHandlerResult + * @property {string} chainId - The current chain ID. + * @property {boolean} isUnlocked - Whether the extension is unlocked or not. + * @property {string} networkVersion - The current network ID. + */ + +/** + * @typedef {Object} ProviderStateHandlerOptions + * @property {() => ProviderStateHandlerResult} getProviderState - A function that + * gets the current provider state. + */ + +/** + * @param {import('json-rpc-engine').JsonRpcRequest<[]>} req - The JSON-RPC request object. + * @param {import('json-rpc-engine').JsonRpcResponse} res - The JSON-RPC response object. + * @param {Function} _next - The json-rpc-engine 'next' callback. + * @param {Function} end - The json-rpc-engine 'end' callback. + * @param {ProviderStateHandlerOptions} options + */ +async function getProviderStateHandler( + req, + res, + _next, + end, + { getProviderState: _getProviderState }, +) { + res.result = { + ...(await _getProviderState(req.origin)), + } + return end() +} diff --git a/app/scripts/lib/rpc-method-middleware/handlers/index.js b/app/scripts/lib/rpc-method-middleware/handlers/index.js index 74e26b675..41011778a 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/index.js +++ b/app/scripts/lib/rpc-method-middleware/handlers/index.js @@ -1,5 +1,6 @@ -import logWeb3Usage from './log-web3-usage' +import getProviderState from './get-provider-state' +import logWeb3ShimUsage from './log-web3-shim-usage' import watchAsset from './watch-asset' -const handlers = [logWeb3Usage, watchAsset] +const handlers = [getProviderState, logWeb3ShimUsage, watchAsset] export default handlers diff --git a/app/scripts/lib/rpc-method-middleware/handlers/log-web3-shim-usage.js b/app/scripts/lib/rpc-method-middleware/handlers/log-web3-shim-usage.js new file mode 100644 index 000000000..142e46f44 --- /dev/null +++ b/app/scripts/lib/rpc-method-middleware/handlers/log-web3-shim-usage.js @@ -0,0 +1,49 @@ +import { MESSAGE_TYPE } from '../../enums' + +/** + * This RPC method is called by the inpage provider whenever it detects the + * accessing of a non-existent property on our window.web3 shim. + * We collect this data to understand which sites are breaking due to the + * removal of our window.web3. + */ + +const logWeb3ShimUsage = { + methodNames: [MESSAGE_TYPE.LOG_WEB3_SHIM_USAGE], + implementation: logWeb3ShimUsageHandler, +} +export default logWeb3ShimUsage + +const recordedWeb3ShimUsage = {} + +/** + * @typedef {Object} LogWeb3ShimUsageOptions + * @property {Function} sendMetrics - A function that registers a metrics event. + */ + +/** + * @param {import('json-rpc-engine').JsonRpcRequest} req - The JSON-RPC request object. + * @param {import('json-rpc-engine').JsonRpcResponse} res - The JSON-RPC response object. + * @param {Function} _next - The json-rpc-engine 'next' callback. + * @param {Function} end - The json-rpc-engine 'end' callback. + * @param {LogWeb3ShimUsageOptions} options + */ +function logWeb3ShimUsageHandler(req, res, _next, end, { sendMetrics }) { + const { origin } = req + if (!recordedWeb3ShimUsage[origin]) { + recordedWeb3ShimUsage[origin] = true + + sendMetrics({ + event: `Website Accessed window.web3 Shim`, + category: 'inpage_provider', + eventContext: { + referrer: { + url: origin, + }, + }, + excludeMetaMetricsId: true, + }) + } + + res.result = true + return end() +} diff --git a/app/scripts/lib/rpc-method-middleware/handlers/log-web3-usage.js b/app/scripts/lib/rpc-method-middleware/handlers/log-web3-usage.js deleted file mode 100644 index c80303223..000000000 --- a/app/scripts/lib/rpc-method-middleware/handlers/log-web3-usage.js +++ /dev/null @@ -1,63 +0,0 @@ -import { MESSAGE_TYPE } from '../../enums' - -/** - * This RPC method is called by our inpage web3 proxy whenever window.web3 is - * accessed. We're collecting data on window.web3 usage so that we can warn - * website maintainers, and possibly our users, before we remove window.web3 - * by November 16, 2020. - */ - -const logWeb3Usage = { - methodNames: [MESSAGE_TYPE.LOG_WEB3_USAGE], - implementation: logWeb3UsageHandler, -} -export default logWeb3Usage - -const recordedWeb3Usage = {} - -/** - * @typedef {Object} LogWeb3UsageOptions - * @property {string} origin - The origin of the request. - * @property {Function} sendMetrics - A function that registers a metrics event. - */ - -/** - * @typedef {Object} LogWeb3UsageParam - * @property {string} action - The action taken (get or set). - * @property {string} name - The window.web3 property name subject to the action. - */ - -/** - * @param {import('json-rpc-engine').JsonRpcRequest<[LogWeb3UsageParam]>} req - The JSON-RPC request object. - * @param {import('json-rpc-engine').JsonRpcResponse} res - The JSON-RPC response object. - * @param {Function} _next - The json-rpc-engine 'next' callback. - * @param {Function} end - The json-rpc-engine 'end' callback. - * @param {LogWeb3UsageOptions} options - */ -function logWeb3UsageHandler(req, res, _next, end, { origin, sendMetrics }) { - const { action, path } = req.params[0] - - if (!recordedWeb3Usage[origin]) { - recordedWeb3Usage[origin] = {} - } - if (!recordedWeb3Usage[origin][path]) { - recordedWeb3Usage[origin][path] = true - - sendMetrics( - { - event: `Website Used window.web3`, - category: 'inpage_provider', - properties: { action, web3Path: path }, - referrer: { - url: origin, - }, - }, - { - excludeMetaMetricsId: true, - }, - ) - } - - res.result = true - return end() -} diff --git a/app/scripts/lib/setupWeb3.js b/app/scripts/lib/setupWeb3.js deleted file mode 100644 index 9dc74ddb2..000000000 --- a/app/scripts/lib/setupWeb3.js +++ /dev/null @@ -1,251 +0,0 @@ -/*global Web3*/ - -// TODO:deprecate:2020 -// Delete this file - -import web3Entitites from './web3-entities.json' -import 'web3/dist/web3.min' - -const shouldLogUsage = ![ - 'docs.metamask.io', - 'metamask.github.io', - 'metamask.io', -].includes(window.location.hostname) - -/** - * To understand how we arrived at this implementation, please see: - * https://github.com/ethereum/web3.js/blob/0.20.7/DOCUMENTATION.md - */ -export default function setupWeb3(log) { - // export web3 as a global, checking for usage - let reloadInProgress = false - let lastTimeUsed - let lastSeenNetwork - let hasBeenWarned = false - - const web3 = new Web3(window.ethereum) - web3.setProvider = function () { - log.debug('MetaMask - overrode web3.setProvider') - } - Object.defineProperty(web3, '__isMetaMaskShim__', { - value: true, - enumerable: false, - configurable: false, - writable: false, - }) - - Object.defineProperty(window.ethereum, '_web3Ref', { - enumerable: false, - writable: true, - configurable: true, - value: web3.eth, - }) - - // Setup logging of nested property usage - if (shouldLogUsage) { - // web3 namespaces with common and uncommon dapp actions - const includedTopKeys = [ - 'eth', - 'db', - 'shh', - 'net', - 'personal', - 'bzz', - 'version', - ] - - // For each top-level property, create appropriate Proxy traps for all of - // their properties - includedTopKeys.forEach((topKey) => { - const applyTrapKeys = new Map() - const getTrapKeys = new Map() - - Object.keys(web3[topKey]).forEach((key) => { - const path = `web3.${topKey}.${key}` - - if (web3Entitites[path]) { - if (web3Entitites[path] === 'function') { - applyTrapKeys.set(key, path) - } else { - getTrapKeys.set(key, path) - } - } - }) - - // Create apply traps for function properties - for (const [key, path] of applyTrapKeys) { - web3[topKey][key] = new Proxy(web3[topKey][key], { - apply: (...params) => { - try { - window.ethereum.request({ - method: 'metamask_logInjectedWeb3Usage', - params: [ - { - action: 'apply', - path, - }, - ], - }) - } catch (error) { - log.debug('Failed to log web3 usage.', error) - } - - // Call function normally - return Reflect.apply(...params) - }, - }) - } - - // Create get trap for non-function properties - web3[topKey] = new Proxy(web3[topKey], { - get: (web3Prop, key, ...params) => { - const name = stringifyKey(key) - - if (getTrapKeys.has(name)) { - try { - window.ethereum.request({ - method: 'metamask_logInjectedWeb3Usage', - params: [ - { - action: 'get', - path: getTrapKeys.get(name), - }, - ], - }) - } catch (error) { - log.debug('Failed to log web3 usage.', error) - } - } - - // return value normally - return Reflect.get(web3Prop, key, ...params) - }, - }) - }) - - const topLevelFunctions = [ - 'isConnected', - 'setProvider', - 'reset', - 'sha3', - 'toHex', - 'toAscii', - 'fromAscii', - 'toDecimal', - 'fromDecimal', - 'fromWei', - 'toWei', - 'toBigNumber', - 'isAddress', - ] - - // apply-trap top-level functions - topLevelFunctions.forEach((key) => { - // This type check is probably redundant, but we've been burned before. - if (typeof web3[key] === 'function') { - web3[key] = new Proxy(web3[key], { - apply: (...params) => { - try { - window.ethereum.request({ - method: 'metamask_logInjectedWeb3Usage', - params: [ - { - action: 'apply', - path: `web3.${key}`, - }, - ], - }) - } catch (error) { - log.debug('Failed to log web3 usage.', error) - } - - // Call function normally - return Reflect.apply(...params) - }, - }) - } - }) - } - - const web3Proxy = new Proxy(web3, { - get: (...params) => { - // get the time of use - lastTimeUsed = Date.now() - - // show warning once on web3 access - if (!hasBeenWarned) { - console.warn( - `MetaMask: We will stop injecting web3 in Q4 2020.\nPlease see this article for more information: https://medium.com/metamask/no-longer-injecting-web3-js-4a899ad6e59e`, - ) - hasBeenWarned = true - } - - // return value normally - return Reflect.get(...params) - }, - }) - - Object.defineProperty(window, 'web3', { - enumerable: false, - writable: true, - configurable: true, - value: web3Proxy, - }) - log.debug('MetaMask - injected web3') - - window.ethereum._publicConfigStore.subscribe((state) => { - // if the auto refresh on network change is false do not - // do anything - if (!window.ethereum.autoRefreshOnNetworkChange) { - return - } - - // if reload in progress, no need to check reload logic - if (reloadInProgress) { - return - } - - const currentNetwork = state.networkVersion - - // set the initial network - if (!lastSeenNetwork) { - lastSeenNetwork = currentNetwork - return - } - - // skip reload logic if web3 not used - if (!lastTimeUsed) { - return - } - - // if network did not change, exit - if (currentNetwork === lastSeenNetwork) { - return - } - - // initiate page reload - reloadInProgress = true - const timeSinceUse = Date.now() - lastTimeUsed - // if web3 was recently used then delay the reloading of the page - if (timeSinceUse > 500) { - triggerReset() - } else { - setTimeout(triggerReset, 500) - } - }) -} - -// reload the page -function triggerReset() { - window.location.reload() -} - -/** - * Returns a "stringified" key. Keys that are already strings are returned - * unchanged, and any non-string values are returned as "typeof ". - * - * @param {any} key - The key to stringify - */ -function stringifyKey(key) { - return typeof key === 'string' ? key : `typeof ${typeof key}` -} diff --git a/app/scripts/lib/web3-entities.json b/app/scripts/lib/web3-entities.json deleted file mode 100644 index 0e9b435bc..000000000 --- a/app/scripts/lib/web3-entities.json +++ /dev/null @@ -1,101 +0,0 @@ -{ - "web3.bzz.blockNetworkRead": "function", - "web3.bzz.download": "function", - "web3.bzz.get": "function", - "web3.bzz.getHive": "function", - "web3.bzz.getInfo": "function", - "web3.bzz.hive": "TRAP", - "web3.bzz.info": "TRAP", - "web3.bzz.modify": "function", - "web3.bzz.put": "function", - "web3.bzz.retrieve": "function", - "web3.bzz.store": "function", - "web3.bzz.swapEnabled": "function", - "web3.bzz.syncEnabled": "function", - "web3.bzz.upload": "function", - "web3.db.getHex": "function", - "web3.db.getString": "function", - "web3.db.putHex": "function", - "web3.db.putString": "function", - "web3.eth.accounts": "object", - "web3.eth.blockNumber": "TRAP", - "web3.eth.call": "function", - "web3.eth.coinbase": "object", - "web3.eth.compile": "object", - "web3.eth.estimateGas": "function", - "web3.eth.gasPrice": "TRAP", - "web3.eth.getAccounts": "function", - "web3.eth.getBalance": "function", - "web3.eth.getBlock": "function", - "web3.eth.getBlockNumber": "function", - "web3.eth.getBlockTransactionCount": "function", - "web3.eth.getBlockUncleCount": "function", - "web3.eth.getCode": "function", - "web3.eth.getCoinbase": "function", - "web3.eth.getCompilers": "function", - "web3.eth.getGasPrice": "function", - "web3.eth.getHashrate": "function", - "web3.eth.getMining": "function", - "web3.eth.getProtocolVersion": "function", - "web3.eth.getStorageAt": "function", - "web3.eth.getSyncing": "function", - "web3.eth.getTransaction": "function", - "web3.eth.getTransactionCount": "function", - "web3.eth.getTransactionFromBlock": "function", - "web3.eth.getTransactionReceipt": "function", - "web3.eth.getUncle": "function", - "web3.eth.getWork": "function", - "web3.eth.hashrate": "TRAP", - "web3.eth.iban": "function", - "web3.eth.mining": "TRAP", - "web3.eth.protocolVersion": "TRAP", - "web3.eth.sendIBANTransaction": "function", - "web3.eth.sendRawTransaction": "function", - "web3.eth.sendTransaction": "function", - "web3.eth.sign": "function", - "web3.eth.signTransaction": "function", - "web3.eth.submitWork": "function", - "web3.eth.syncing": "TRAP", - "web3.net.getListening": "function", - "web3.net.getPeerCount": "function", - "web3.net.listening": "TRAP", - "web3.net.peerCount": "TRAP", - "web3.personal.ecRecover": "function", - "web3.personal.getListAccounts": "function", - "web3.personal.importRawKey": "function", - "web3.personal.listAccounts": "TRAP", - "web3.personal.lockAccount": "function", - "web3.personal.newAccount": "function", - "web3.personal.sendTransaction": "function", - "web3.personal.sign": "function", - "web3.personal.unlockAccount": "function", - "web3.providers.HttpProvider": "function", - "web3.providers.IpcProvider": "function", - "web3.shh.addPrivateKey": "function", - "web3.shh.addSymKey": "function", - "web3.shh.deleteKeyPair": "function", - "web3.shh.deleteSymKey": "function", - "web3.shh.generateSymKeyFromPassword": "function", - "web3.shh.getPrivateKey": "function", - "web3.shh.getPublicKey": "function", - "web3.shh.getSymKey": "function", - "web3.shh.hasKeyPair": "function", - "web3.shh.hasSymKey": "function", - "web3.shh.info": "function", - "web3.shh.markTrustedPeer": "function", - "web3.shh.newKeyPair": "function", - "web3.shh.newSymKey": "function", - "web3.shh.post": "function", - "web3.shh.setMaxMessageSize": "function", - "web3.shh.setMinPoW": "function", - "web3.shh.version": "function", - "web3.version.api": "string", - "web3.version.ethereum": "TRAP", - "web3.version.getEthereum": "function", - "web3.version.getNetwork": "function", - "web3.version.getNode": "function", - "web3.version.getWhisper": "function", - "web3.version.network": "string", - "web3.version.node": "TRAP", - "web3.version.whisper": "TRAP" -} diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a7517c5db..0e976a994 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1,9 +1,6 @@ import EventEmitter from 'events' - import pump from 'pump' import Dnode from 'dnode' -import ObservableStore from 'obs-store' -import asStream from 'obs-store/lib/asStream' import { JsonRpcEngine } from 'json-rpc-engine' import { debounce } from 'lodash' import createEngineStream from 'json-rpc-middleware-stream/engineStream' @@ -53,6 +50,7 @@ import TokenRatesController from './controllers/token-rates' import DetectTokensController from './controllers/detect-tokens' import SwapsController from './controllers/swaps' import { PermissionsController } from './controllers/permissions' +import { NOTIFICATION_NAMES } from './controllers/permissions/enums' import getRestrictedMethods from './controllers/permissions/restrictedMethods' import nodeify from './lib/nodeify' import accountImporter from './account-import-strategies' @@ -219,10 +217,11 @@ export default class MetamaskController extends EventEmitter { initState: initState.KeyringController, encryptor: opts.encryptor || undefined, }) - this.keyringController.memStore.subscribe((s) => - this._onKeyringControllerUpdate(s), + this.keyringController.memStore.subscribe((state) => + this._onKeyringControllerUpdate(state), ) this.keyringController.on('unlock', () => this.emit('unlock')) + this.keyringController.on('lock', () => this._onLock()) this.permissionsController = new PermissionsController( { @@ -233,6 +232,7 @@ export default class MetamaskController extends EventEmitter { getUnlockPromise: this.appStateController.getUnlockPromise.bind( this.appStateController, ), + isUnlocked: this.isUnlocked.bind(this), notifyDomain: this.notifyConnections.bind(this), notifyAllDomains: this.notifyAllConnections.bind(this), preferences: this.preferencesController.store, @@ -287,7 +287,9 @@ export default class MetamaskController extends EventEmitter { ), provider: this.provider, blockTracker: this.blockTracker, - trackMetaMetricsEvent: this.metaMetricsController.trackEvent, + trackMetaMetricsEvent: this.metaMetricsController.trackEvent.bind( + this.metaMetricsController, + ), getParticipateInMetrics: () => this.metaMetricsController.state.participateInMetaMetrics, }) @@ -346,6 +348,9 @@ export default class MetamaskController extends EventEmitter { tokenRatesStore: this.tokenRatesController.store, }) + // ensure isClientOpenAndUnlocked is updated when memState updates + this.on('update', (memState) => this._onStateUpdate(memState)) + this.store.updateStructure({ AppStateController: this.appStateController.store, TransactionController: this.txController.store, @@ -448,38 +453,37 @@ export default class MetamaskController extends EventEmitter { } /** - * Constructor helper: initialize a public config store. - * This store is used to make some config info available to Dapps synchronously. + * Gets relevant state for the provider of an external origin. + * + * @param {string} origin - The origin to get the provider state for. + * @returns {Promise<{ + * isUnlocked: boolean, + * networkVersion: string, + * chainId: string, + * accounts: string[], + * }>} An object with relevant state properties. */ - createPublicConfigStore() { - // subset of state for metamask inpage provider - const publicConfigStore = new ObservableStore() - const { networkController } = this - - // setup memStore subscription hooks - this.on('update', updatePublicConfigStore) - updatePublicConfigStore(this.getState()) - - publicConfigStore.destroy = () => { - this.removeEventListener && - this.removeEventListener('update', updatePublicConfigStore) + async getProviderState(origin) { + return { + isUnlocked: this.isUnlocked(), + ...this.getProviderNetworkState(), + accounts: await this.permissionsController.getAccounts(origin), } + } - function updatePublicConfigStore(memState) { - const chainId = networkController.getCurrentChainId() - if (memState.network !== 'loading') { - publicConfigStore.putState(selectPublicState(chainId, memState)) - } + /** + * Gets network state relevant for external providers. + * + * @param {Object} [memState] - The MetaMask memState. If not provided, + * this function will retrieve the most recent state. + * @returns {Object} An object with relevant network state properties. + */ + getProviderNetworkState(memState) { + const { network } = memState || this.getState() + return { + chainId: this.networkController.getCurrentChainId(), + networkVersion: network, } - - function selectPublicState(chainId, { isUnlocked, network }) { - return { - isUnlocked, - chainId, - networkVersion: network, - } - } - return publicConfigStore } //============================================================================= @@ -1811,8 +1815,7 @@ export default class MetamaskController extends EventEmitter { const mux = setupMultiplex(connectionStream) // messages between inpage and background - this.setupProviderConnection(mux.createStream('provider'), sender) - this.setupPublicConfig(mux.createStream('publicConfig')) + this.setupProviderConnection(mux.createStream('metamask-provider'), sender) } /** @@ -1969,7 +1972,10 @@ export default class MetamaskController extends EventEmitter { engine.push( createMethodMiddleware({ origin, - sendMetrics: this.metaMetricsController.trackEvent, + getProviderState: this.getProviderState.bind(this), + sendMetrics: this.metaMetricsController.trackEvent.bind( + this.metaMetricsController, + ), handleWatchAssetRequest: this.preferencesController.requestWatchAsset.bind( this.preferencesController, ), @@ -1989,29 +1995,6 @@ export default class MetamaskController extends EventEmitter { return engine } - /** - * A method for providing our public config info over a stream. - * This includes info we like to be synchronous if possible, like - * the current selected account, and network ID. - * - * Since synchronous methods have been deprecated in web3, - * this is a good candidate for deprecation. - * - * @param {*} outStream - The stream to provide public config over. - */ - setupPublicConfig(outStream) { - const configStore = this.createPublicConfigStore() - const configStream = asStream(configStore) - - pump(configStream, outStream, (err) => { - configStore.destroy() - configStream.destroy() - if (err) { - log.error(err) - } - }) - } - /** * Adds a reference to a connection by origin. Ignores the 'metamask' origin. * Caller must ensure that the returned id is stored such that the reference @@ -2062,37 +2045,51 @@ export default class MetamaskController extends EventEmitter { /** * Causes the RPC engines associated with the connections to the given origin * to emit a notification event with the given payload. - * Does nothing if the extension is locked or the origin is unknown. + * + * The caller is responsible for ensuring that only permitted notifications + * are sent. + * + * Ignores unknown origins. * * @param {string} origin - The connection's origin string. * @param {any} payload - The event payload. */ notifyConnections(origin, payload) { const connections = this.connections[origin] - if (!this.isUnlocked() || !connections) { - return - } - Object.values(connections).forEach((conn) => { - conn.engine && conn.engine.emit('notification', payload) - }) + if (connections) { + Object.values(connections).forEach((conn) => { + if (conn.engine) { + conn.engine.emit('notification', payload) + } + }) + } } /** * Causes the RPC engines associated with all connections to emit a * notification event with the given payload. - * Does nothing if the extension is locked. * - * @param {any} payload - The event payload. + * If the "payload" parameter is a function, the payload for each connection + * will be the return value of that function called with the connection's + * origin. + * + * The caller is responsible for ensuring that only permitted notifications + * are sent. + * + * @param {any} payload - The event payload, or payload getter function. */ notifyAllConnections(payload) { - if (!this.isUnlocked()) { - return - } + const getPayload = + typeof payload === 'function' + ? (origin) => payload(origin) + : () => payload Object.values(this.connections).forEach((origin) => { Object.values(origin).forEach((conn) => { - conn.engine && conn.engine.emit('notification', payload) + if (conn.engine) { + conn.engine.emit('notification', getPayload(origin)) + } }) }) } @@ -2121,6 +2118,51 @@ export default class MetamaskController extends EventEmitter { this.accountTracker.syncWithAddresses(addresses) } + /** + * Handle global unlock, triggered by KeyringController unlock. + * Notifies all connections that the extension is unlocked. + */ + _onUnlock() { + this.notifyAllConnections((origin) => { + return { + method: NOTIFICATION_NAMES.unlockStateChanged, + params: { + isUnlocked: true, + accounts: this.permissionsController.getAccounts(origin), + }, + } + }) + this.emit('unlock') + } + + /** + * Handle global lock, triggered by KeyringController lock. + * Notifies all connections that the extension is locked. + */ + _onLock() { + this.notifyAllConnections({ + method: NOTIFICATION_NAMES.unlockStateChanged, + params: { + isUnlocked: false, + }, + }) + this.emit('lock') + } + + /** + * Handle memory state updates. + * - Ensure isClientOpenAndUnlocked is updated + * - Notifies all connections with the new provider network state + * - The external providers handle diffing the state + */ + _onStateUpdate(newState) { + this.isClientOpenAndUnlocked = newState.isUnlocked && this._isClientOpen + this.notifyAllConnections({ + method: NOTIFICATION_NAMES.chainChanged, + params: this.getProviderNetworkState(newState), + }) + } + // misc /** diff --git a/app/scripts/runLockdown.js b/app/scripts/runLockdown.js index 00bc7658f..d8584c69c 100644 --- a/app/scripts/runLockdown.js +++ b/app/scripts/runLockdown.js @@ -1,6 +1,7 @@ // Freezes all intrinsics // eslint-disable-next-line no-undef,import/unambiguous lockdown({ + consoleTaming: 'unsafe', errorTaming: 'unsafe', mathTaming: 'unsafe', dateTaming: 'unsafe', diff --git a/development/build/static.js b/development/build/static.js index 142f290d1..1fe5dc959 100644 --- a/development/build/static.js +++ b/development/build/static.js @@ -44,6 +44,10 @@ const copyTargets = [ pattern: `*.html`, dest: ``, }, + { + src: `./node_modules/globalthis/dist/browser.js`, + dest: `globalthis.js`, + }, { src: `./node_modules/ses/dist/`, pattern: `lockdown.cjs`, diff --git a/package.json b/package.json index f11903fbb..519fae646 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ "@metamask/eth-ledger-bridge-keyring": "^0.2.6", "@metamask/eth-token-tracker": "^3.0.1", "@metamask/etherscan-link": "^1.4.0", - "@metamask/inpage-provider": "^6.1.0", + "@metamask/inpage-provider": "^8.0.1", "@metamask/jazzicon": "^2.0.0", "@metamask/logo": "^2.5.0", "@popperjs/core": "^2.4.0", @@ -128,6 +128,7 @@ "extensionizer": "^1.0.1", "fast-json-patch": "^2.0.4", "fuse.js": "^3.2.0", + "globalthis": "^1.0.1", "human-standard-token-abi": "^2.0.0", "json-rpc-engine": "^6.1.0", "json-rpc-middleware-stream": "^2.1.1", diff --git a/test/e2e/metrics.spec.js b/test/e2e/metrics.spec.js index 585a44e35..74c8f26d8 100644 --- a/test/e2e/metrics.spec.js +++ b/test/e2e/metrics.spec.js @@ -28,14 +28,16 @@ describe('Segment metrics', function () { mockSegment: true, }, async ({ driver, segmentStub }) => { - const threeSegmentEventsReceived = waitUntilCalled(segmentStub, null, 3) + const threeSegmentEventsReceived = waitUntilCalled(segmentStub, null, { + callCount: 3, + }) await driver.navigate() const passwordField = await driver.findElement(By.css('#password')) await passwordField.sendKeys('correct horse battery staple') await passwordField.sendKeys(Key.ENTER) - await threeSegmentEventsReceived + await threeSegmentEventsReceived() assert.ok(segmentStub.called, 'Segment should receive metrics') diff --git a/test/lib/wait-until-called.js b/test/lib/wait-until-called.js index 752ee7675..8252f6049 100644 --- a/test/lib/wait-until-called.js +++ b/test/lib/wait-until-called.js @@ -1,22 +1,41 @@ +const DEFAULT_TIMEOUT = 10000 + /** - * A function that wraps a sinon stubbed function and returns a Promise - * when this stub was called. + * A function that wraps a sinon stub and returns an asynchronous function + * that resolves if the stubbed function was called enough times, or throws + * if the timeout is exceeded. * * The stub that has been passed in will be setup to call the wrapped function - * directly, then trigger the returned Promise to resolve. + * directly. * * WARNING: Any existing `callsFake` behavior will be overwritten. * * @param {import('sinon').stub} stub - A sinon stub of a function - * @param {unknown} [wrappedThis] - The object the stubbed function was called on, if any (i.e. the `this` value) - * @param {number} [callCount] - The number of calls to wait for. Defaults to 1. - * @returns {Promise} A Promise that resolves when the stub has been called + * @param {unknown} [wrappedThis] - The object the stubbed function was called + * on, if any (i.e. the `this` value) + * @param {Object} [options] - Optional configuration + * @param {number} [options.callCount] - The number of calls to wait for. + * @param {number|null} [options.timeout] - The timeout, in milliseconds. Pass + * in `null` to disable the timeout. + * @returns {Function} An asynchronous function that resolves when the stub is + * called enough times, or throws if the timeout is reached. */ -function waitUntilCalled(stub, wrappedThis = null, callCount = 1) { +function waitUntilCalled( + stub, + wrappedThis = null, + { callCount = 1, timeout = DEFAULT_TIMEOUT } = {}, +) { let numCalls = 0 let resolve + let timeoutHandle const stubHasBeenCalled = new Promise((_resolve) => { resolve = _resolve + if (timeout !== null) { + timeoutHandle = setTimeout( + () => resolve(new Error('Timeout exceeded')), + timeout, + ) + } }) stub.callsFake((...args) => { try { @@ -27,12 +46,21 @@ function waitUntilCalled(stub, wrappedThis = null, callCount = 1) { if (numCalls < callCount) { numCalls += 1 if (numCalls === callCount) { + if (timeoutHandle) { + clearTimeout(timeoutHandle) + } resolve() } } } }) - return stubHasBeenCalled + + return async () => { + const error = await stubHasBeenCalled + if (error) { + throw error + } + } } module.exports = waitUntilCalled diff --git a/test/unit/app/controllers/detect-tokens-test.js b/test/unit/app/controllers/detect-tokens-test.js index 82be8808a..d54c55154 100644 --- a/test/unit/app/controllers/detect-tokens-test.js +++ b/test/unit/app/controllers/detect-tokens-test.js @@ -85,6 +85,53 @@ describe('DetectTokensController', function () { sandbox.assert.notCalled(stub) }) + it('should skip adding tokens listed in hiddenTokens array', async function () { + sandbox.useFakeTimers() + network.setProviderType(MAINNET) + const controller = new DetectTokensController({ + preferences, + network, + keyringMemStore, + }) + controller.isOpen = true + controller.isUnlocked = true + + const contractAddresses = Object.keys(contracts) + const erc20ContractAddresses = contractAddresses.filter( + (contractAddress) => contracts[contractAddress].erc20 === true, + ) + + const existingTokenAddress = erc20ContractAddresses[0] + const existingToken = contracts[existingTokenAddress] + await preferences.addToken( + existingTokenAddress, + existingToken.symbol, + existingToken.decimals, + ) + + const tokenAddressToSkip = erc20ContractAddresses[1] + + sandbox + .stub(controller, '_getTokenBalances') + .callsFake((tokensToDetect) => + tokensToDetect.map((token) => + token === tokenAddressToSkip ? new BigNumber(10) : 0, + ), + ) + + await preferences.removeToken(tokenAddressToSkip) + + await controller.detectNewTokens() + + assert.deepEqual(preferences.store.getState().tokens, [ + { + address: existingTokenAddress.toLowerCase(), + decimals: existingToken.decimals, + symbol: existingToken.symbol, + }, + ]) + }) + it('should check and add tokens while on main network', async function () { sandbox.useFakeTimers() network.setProviderType(MAINNET) diff --git a/test/unit/app/controllers/incoming-transactions-test.js b/test/unit/app/controllers/incoming-transactions-test.js index 8401d7f24..61768cee8 100644 --- a/test/unit/app/controllers/incoming-transactions-test.js +++ b/test/unit/app/controllers/incoming-transactions-test.js @@ -249,7 +249,7 @@ describe('IncomingTransactionsController', function () { ) incomingTransactionsController.start() - await updateStateCalled + await updateStateCalled() const actualState = incomingTransactionsController.store.getState() const generatedTxId = actualState?.incomingTransactions?.['0xfake']?.id @@ -345,8 +345,8 @@ describe('IncomingTransactionsController', function () { try { await Promise.race([ - updateStateCalled, - putStateCalled, + updateStateCalled(), + putStateCalled(), new Promise((_, reject) => { setTimeout(() => reject(new Error('TIMEOUT')), SET_STATE_TIMEOUT) }), @@ -412,8 +412,8 @@ describe('IncomingTransactionsController', function () { try { await Promise.race([ - updateStateCalled, - putStateCalled, + updateStateCalled(), + putStateCalled(), new Promise((_, reject) => { setTimeout(() => reject(new Error('TIMEOUT')), SET_STATE_TIMEOUT) }), @@ -475,8 +475,8 @@ describe('IncomingTransactionsController', function () { try { await Promise.race([ - updateStateCalled, - putStateCalled, + updateStateCalled(), + putStateCalled(), new Promise((_, reject) => { setTimeout(() => reject(new Error('TIMEOUT')), SET_STATE_TIMEOUT) }), @@ -540,8 +540,8 @@ describe('IncomingTransactionsController', function () { try { await Promise.race([ - updateStateCalled, - putStateCalled, + updateStateCalled(), + putStateCalled(), new Promise((_, reject) => { setTimeout(() => reject(new Error('TIMEOUT')), SET_STATE_TIMEOUT) }), @@ -592,7 +592,7 @@ describe('IncomingTransactionsController', function () { // TODO: stop skipping the first event await subscription({ selectedAddress: MOCK_SELECTED_ADDRESS }) await subscription({ selectedAddress: NEW_MOCK_SELECTED_ADDRESS }) - await updateStateCalled + await updateStateCalled() const actualState = incomingTransactionsController.store.getState() const generatedTxId = actualState?.incomingTransactions?.['0xfake']?.id @@ -696,8 +696,8 @@ describe('IncomingTransactionsController', function () { try { await Promise.race([ - updateStateCalled, - putStateCalled, + updateStateCalled(), + putStateCalled(), new Promise((_, reject) => { setTimeout(() => reject(new Error('TIMEOUT')), SET_STATE_TIMEOUT) }), @@ -746,7 +746,7 @@ describe('IncomingTransactionsController', function () { ROPSTEN_CHAIN_ID, ) await subscription(ROPSTEN) - await updateStateCalled + await updateStateCalled() const actualState = incomingTransactionsController.store.getState() const generatedTxId = actualState?.incomingTransactions?.['0xfake']?.id @@ -848,8 +848,8 @@ describe('IncomingTransactionsController', function () { try { await Promise.race([ - updateStateCalled, - putStateCalled, + updateStateCalled(), + putStateCalled(), new Promise((_, reject) => { setTimeout(() => reject(new Error('TIMEOUT')), SET_STATE_TIMEOUT) }), diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js index 99bd3f957..2e1c4021d 100644 --- a/test/unit/app/controllers/metamask-controller-test.js +++ b/test/unit/app/controllers/metamask-controller-test.js @@ -1022,7 +1022,7 @@ describe('MetaMaskController', function () { } streamTest.write( { - name: 'provider', + name: 'metamask-provider', data: message, }, null, @@ -1061,7 +1061,7 @@ describe('MetaMaskController', function () { } streamTest.write( { - name: 'provider', + name: 'metamask-provider', data: message, }, null, diff --git a/test/unit/app/controllers/metametrics-test.js b/test/unit/app/controllers/metametrics-test.js index d43f48f4f..4e66fec4a 100644 --- a/test/unit/app/controllers/metametrics-test.js +++ b/test/unit/app/controllers/metametrics-test.js @@ -400,7 +400,7 @@ describe('MetaMetricsController', function () { }, { flushImmediately: true }, ) - assert.doesNotReject(flushCalled) + assert.doesNotReject(flushCalled()) }) it('should throw if event or category not provided', function () { diff --git a/test/unit/app/controllers/permissions/mocks.js b/test/unit/app/controllers/permissions/mocks.js index 9040f6b12..d6b93dd68 100644 --- a/test/unit/app/controllers/permissions/mocks.js +++ b/test/unit/app/controllers/permissions/mocks.js @@ -32,8 +32,6 @@ const keyringAccounts = deepFreeze([ '0xcc74c7a59194e5d9268476955650d1e285be703c', ]) -const getKeyringAccounts = async () => [...keyringAccounts] - const getIdentities = () => { return keyringAccounts.reduce((identities, address, index) => { identities[address] = { address, name: `Account ${index}` } @@ -62,8 +60,6 @@ const getRestrictedMethods = (permController) => { } } -const getUnlockPromise = () => Promise.resolve() - /** * Gets default mock constructor options for a permissions controller. * @@ -71,10 +67,10 @@ const getUnlockPromise = () => Promise.resolve() */ export function getPermControllerOpts() { return { - showPermissionRequest: noop, - getKeyringAccounts, - getUnlockPromise, + getKeyringAccounts: async () => [...keyringAccounts], + getUnlockPromise: () => Promise.resolve(), getRestrictedMethods, + isUnlocked: () => true, notifyDomain: noop, notifyAllDomains: noop, preferences: { @@ -86,6 +82,7 @@ export function getPermControllerOpts() { }, subscribe: noop, }, + showPermissionRequest: noop, } } @@ -467,7 +464,7 @@ export const getters = deepFreeze({ removedAccounts: () => { return { method: NOTIFICATION_NAMES.accountsChanged, - result: [], + params: [], } }, @@ -480,7 +477,7 @@ export const getters = deepFreeze({ newAccounts: (accounts) => { return { method: NOTIFICATION_NAMES.accountsChanged, - result: accounts, + params: accounts, } }, }, @@ -586,17 +583,17 @@ export const getters = deepFreeze({ }, /** - * Gets a wallet_sendDomainMetadata RPC request object. + * Gets a metamask_sendDomainMetadata RPC request object. * * @param {string} origin - The origin of the request * @param {Object} name - The domainMetadata name * @param {Array} [args] - Any other data for the request's domainMetadata * @returns {Object} An RPC request object */ - wallet_sendDomainMetadata: (origin, name, ...args) => { + metamask_sendDomainMetadata: (origin, name, ...args) => { return { origin, - method: 'wallet_sendDomainMetadata', + method: 'metamask_sendDomainMetadata', domainMetadata: { ...args, name, diff --git a/test/unit/app/controllers/permissions/permissions-controller-test.js b/test/unit/app/controllers/permissions/permissions-controller-test.js index 5f79bb443..434ae6bd3 100644 --- a/test/unit/app/controllers/permissions/permissions-controller-test.js +++ b/test/unit/app/controllers/permissions/permissions-controller-test.js @@ -6,13 +6,9 @@ import sinon from 'sinon' import { METADATA_STORE_KEY, METADATA_CACHE_MAX_SIZE, - WALLET_PREFIX, } from '../../../../../app/scripts/controllers/permissions/enums' -import { - PermissionsController, - addInternalMethodPrefix, -} from '../../../../../app/scripts/controllers/permissions' +import { PermissionsController } from '../../../../../app/scripts/controllers/permissions' import { getRequestUserApprovalHelper, grantPermissions } from './helpers' @@ -187,7 +183,7 @@ describe('permissions controller', function () { assert.deepEqual( notifications[origin], [NOTIFICATIONS.removedAccounts()], - 'origin should have single wallet_accountsChanged:[] notification', + 'origin should have single metamask_accountsChanged:[] notification', ) }) @@ -1325,11 +1321,18 @@ describe('permissions controller', function () { }) it('notifyAccountsChanged records history and sends notification', async function () { + sinon.spy(permController, '_isUnlocked') + permController.notifyAccountsChanged( DOMAINS.a.origin, ACCOUNTS.a.permitted, ) + assert.ok( + permController._isUnlocked.calledOnce, + '_isUnlocked should have been called once', + ) + assert.ok( permController.permissionsLog.updateAccountsHistory.calledOnce, 'permissionsLog.updateAccountsHistory should have been called once', @@ -1342,6 +1345,25 @@ describe('permissions controller', function () { ) }) + it('notifyAccountsChanged does nothing if _isUnlocked returns false', async function () { + permController._isUnlocked = sinon.fake.returns(false) + + permController.notifyAccountsChanged( + DOMAINS.a.origin, + ACCOUNTS.a.permitted, + ) + + assert.ok( + permController._isUnlocked.calledOnce, + '_isUnlocked should have been called once', + ) + + assert.ok( + permController.permissionsLog.updateAccountsHistory.notCalled, + 'permissionsLog.updateAccountsHistory should not have been called', + ) + }) + it('notifyAccountsChanged throws on invalid origin', async function () { assert.throws( () => permController.notifyAccountsChanged(4, ACCOUNTS.a.permitted), @@ -1604,11 +1626,5 @@ describe('permissions controller', function () { 'pending approval origins should have expected item', ) }) - - it('addInternalMethodPrefix', function () { - const str = 'foo' - const res = addInternalMethodPrefix(str) - assert.equal(res, WALLET_PREFIX + str, 'should prefix correctly') - }) }) }) diff --git a/test/unit/app/controllers/permissions/permissions-log-controller-test.js b/test/unit/app/controllers/permissions/permissions-log-controller-test.js index ac498a0e0..101f898b9 100644 --- a/test/unit/app/controllers/permissions/permissions-log-controller-test.js +++ b/test/unit/app/controllers/permissions/permissions-log-controller-test.js @@ -286,7 +286,7 @@ describe('permissions log', function () { assert.equal(log.length, 0, 'log should be empty') const res = { foo: 'bar' } - const req1 = RPC_REQUESTS.wallet_sendDomainMetadata( + const req1 = RPC_REQUESTS.metamask_sendDomainMetadata( DOMAINS.c.origin, 'foobar', ) diff --git a/test/unit/app/controllers/permissions/permissions-middleware-test.js b/test/unit/app/controllers/permissions/permissions-middleware-test.js index 2888d3da5..ff4e832c9 100644 --- a/test/unit/app/controllers/permissions/permissions-middleware-test.js +++ b/test/unit/app/controllers/permissions/permissions-middleware-test.js @@ -808,7 +808,7 @@ describe('permissions middleware', function () { }) }) - describe('wallet_sendDomainMetadata', function () { + describe('metamask_sendDomainMetadata', function () { let permController, clock beforeEach(function () { @@ -828,7 +828,10 @@ describe('permissions middleware', function () { DOMAINS.c.origin, ) - const req = RPC_REQUESTS.wallet_sendDomainMetadata(DOMAINS.c.origin, name) + const req = RPC_REQUESTS.metamask_sendDomainMetadata( + DOMAINS.c.origin, + name, + ) const res = {} await assert.doesNotReject(cMiddleware(req, res), 'should not reject') @@ -861,7 +864,10 @@ describe('permissions middleware', function () { extensionId, ) - const req = RPC_REQUESTS.wallet_sendDomainMetadata(DOMAINS.c.origin, name) + const req = RPC_REQUESTS.metamask_sendDomainMetadata( + DOMAINS.c.origin, + name, + ) const res = {} await assert.doesNotReject(cMiddleware(req, res), 'should not reject') @@ -885,7 +891,10 @@ describe('permissions middleware', function () { DOMAINS.c.origin, ) - const req = RPC_REQUESTS.wallet_sendDomainMetadata(DOMAINS.c.origin, name) + const req = RPC_REQUESTS.metamask_sendDomainMetadata( + DOMAINS.c.origin, + name, + ) const res = {} await assert.doesNotReject(cMiddleware(req, res), 'should not reject') @@ -907,7 +916,7 @@ describe('permissions middleware', function () { DOMAINS.c.origin, ) - const req = RPC_REQUESTS.wallet_sendDomainMetadata(DOMAINS.c.origin) + const req = RPC_REQUESTS.metamask_sendDomainMetadata(DOMAINS.c.origin) delete req.domainMetadata const res = {} diff --git a/test/unit/app/controllers/transactions/tx-controller-test.js b/test/unit/app/controllers/transactions/tx-controller-test.js index f3426d5f9..0e39f791d 100644 --- a/test/unit/app/controllers/transactions/tx-controller-test.js +++ b/test/unit/app/controllers/transactions/tx-controller-test.js @@ -276,6 +276,7 @@ describe('Transaction Controller', function () { describe('#addUnapprovedTransaction', function () { const selectedAddress = '0x1678a085c290ebd122dc42cba69373b5953b831d' + const recipientAddress = '0xc42edfcc21ed14dda456aa0756c153f7985d8813' let getSelectedAddress, getPermittedAccounts beforeEach(function () { @@ -295,6 +296,7 @@ describe('Transaction Controller', function () { it('should add an unapproved transaction and return a valid txMeta', async function () { const txMeta = await txController.addUnapprovedTransaction({ from: selectedAddress, + to: recipientAddress, }) assert.ok('id' in txMeta, 'should have a id') assert.ok('time' in txMeta, 'should have a time stamp') @@ -321,7 +323,10 @@ describe('Transaction Controller', function () { done() }) txController - .addUnapprovedTransaction({ from: selectedAddress }) + .addUnapprovedTransaction({ + from: selectedAddress, + to: recipientAddress, + }) .catch(done) }) diff --git a/test/unit/app/controllers/transactions/tx-utils-test.js b/test/unit/app/controllers/transactions/tx-utils-test.js index 02c5171c7..77add3f62 100644 --- a/test/unit/app/controllers/transactions/tx-utils-test.js +++ b/test/unit/app/controllers/transactions/tx-utils-test.js @@ -6,21 +6,47 @@ describe('txUtils', function () { it('does not throw for positive values', function () { const sample = { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + to: '0xc42edfcc21ed14dda456aa0756c153f7985d8813', value: '0x01', } txUtils.validateTxParams(sample) }) - it('returns error for negative values', function () { + it('throws for invalid params value', function () { + assert.throws(() => txUtils.validateTxParams(), { + message: 'Invalid transaction params: must be an object.', + }) + assert.throws(() => txUtils.validateTxParams(null), { + message: 'Invalid transaction params: must be an object.', + }) + assert.throws(() => txUtils.validateTxParams(true), { + message: 'Invalid transaction params: must be an object.', + }) + assert.throws(() => txUtils.validateTxParams([]), { + message: 'Invalid transaction params: must be an object.', + }) + }) + + it('throws for missing "to" and "data"', function () { const sample = { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + value: '0x01', + } + assert.throws(() => txUtils.validateTxParams(sample), { + message: + 'Invalid transaction params: must specify "data" for contract deployments, or "to" (and optionally "data") for all other types of transactions.', + }) + }) + + it('throws for negative values', function () { + const sample = { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + to: '0xc42edfcc21ed14dda456aa0756c153f7985d8813', value: '-0x01', } - try { - txUtils.validateTxParams(sample) - } catch (err) { - assert.ok(err, 'error') - } + assert.throws(() => txUtils.validateTxParams(sample), { + message: 'Invalid transaction value "-0x01": not a positive number.', + }) }) }) diff --git a/ui/app/components/ui/url-icon/index.scss b/ui/app/components/ui/url-icon/index.scss index 482f327d3..14d5524cb 100644 --- a/ui/app/components/ui/url-icon/index.scss +++ b/ui/app/components/ui/url-icon/index.scss @@ -20,9 +20,9 @@ border-radius: 50%; background: #bbc0c5; flex: 0 1 auto; + display: flex; justify-content: center; align-items: center; - text-align: center; padding-top: 2px; } } diff --git a/ui/app/components/ui/url-icon/url-icon.js b/ui/app/components/ui/url-icon/url-icon.js index 6dcfcc06f..115eb5fe1 100644 --- a/ui/app/components/ui/url-icon/url-icon.js +++ b/ui/app/components/ui/url-icon/url-icon.js @@ -3,13 +3,13 @@ import PropTypes from 'prop-types' import classnames from 'classnames' import IconWithFallback from '../icon-with-fallback' -export default function UrlIcon({ url, className, name }) { +export default function UrlIcon({ url, className, name, fallbackClassName }) { return ( ) } @@ -18,4 +18,5 @@ UrlIcon.propTypes = { url: PropTypes.string, className: PropTypes.string, name: PropTypes.string, + fallbackClassName: PropTypes.string, } diff --git a/ui/app/ducks/swaps/swaps.js b/ui/app/ducks/swaps/swaps.js index 7f4c5df96..d09afd648 100644 --- a/ui/app/ducks/swaps/swaps.js +++ b/ui/app/ducks/swaps/swaps.js @@ -325,7 +325,6 @@ export { export const navigateBackToBuildQuote = (history) => { return async (dispatch) => { // TODO: Ensure any fetch in progress is cancelled - await dispatch(resetSwapsPostFetchState()) dispatch(navigatedBackToBuildQuote()) history.push(BUILD_QUOTE_ROUTE) diff --git a/ui/app/pages/swaps/build-quote/build-quote.js b/ui/app/pages/swaps/build-quote/build-quote.js index 3aabcf82e..4714f79ad 100644 --- a/ui/app/pages/swaps/build-quote/build-quote.js +++ b/ui/app/pages/swaps/build-quote/build-quote.js @@ -47,6 +47,8 @@ const fuseSearchKeys = [ { name: 'address', weight: 0.002 }, ] +const MAX_ALLOWED_SLIPPAGE = 15 + export default function BuildQuote({ inputValue, onInputChange, @@ -393,6 +395,7 @@ export default function BuildQuote({ onSelect={(newSlippage) => { setMaxSlippage(newSlippage) }} + maxAllowedSlippage={MAX_ALLOWED_SLIPPAGE} /> @@ -411,9 +414,11 @@ export default function BuildQuote({ disabled={ !Number(inputValue) || !selectedToToken?.address || - Number(maxSlippage) === 0 + Number(maxSlippage) === 0 || + Number(maxSlippage) > MAX_ALLOWED_SLIPPAGE } hideCancel + showTermsOfService /> ) diff --git a/ui/app/pages/swaps/exchange-rate-display/exchange-rate-display.js b/ui/app/pages/swaps/exchange-rate-display/exchange-rate-display.js index fbb268d65..e70ffd8e7 100644 --- a/ui/app/pages/swaps/exchange-rate-display/exchange-rate-display.js +++ b/ui/app/pages/swaps/exchange-rate-display/exchange-rate-display.js @@ -3,7 +3,7 @@ import PropTypes from 'prop-types' import BigNumber from 'bignumber.js' import classnames from 'classnames' import { calcTokenAmount } from '../../../helpers/utils/token-util' -import { toPrecisionWithoutTrailingZeros } from '../../../helpers/utils/util' +import { formatSwapsValueForDisplay } from '../swaps.util' export default function ExchangeRateDisplay({ primaryTokenValue, @@ -13,6 +13,7 @@ export default function ExchangeRateDisplay({ secondaryTokenDecimals = 18, secondaryTokenSymbol, arrowColor = 'black', + boldSymbols = true, className, }) { const [showPrimaryToSecondary, setShowPrimaryToSecondary] = useState(true) @@ -57,16 +58,24 @@ export default function ExchangeRateDisplay({ } else if (new BigNumber(rate, 10).lt('0.000001', 10)) { rateToDisplay = rate } else { - rateToDisplay = toPrecisionWithoutTrailingZeros(rate, 9) + rateToDisplay = formatSwapsValueForDisplay(rate) } return (
1 - {baseSymbol} + + {baseSymbol} + {comparisonSymbol} {rateToDisplay} - {ratiodSymbol} + + {ratiodSymbol} +
+
+
+ {bestQuoteText && ( +

{bestQuoteText}

+ )} +
+

+ {t('swapNQuotes', [numberOfQuotes])} +

+
+ +
+
+
+
@@ -83,26 +113,39 @@ export default function FeeCard({
{!hideTokenApprovalRow && ( -
+
{t('swapThisWillAllowApprove', [tokenApprovalTextComponent])}
-
onTokenApprovalClick()} - > - {t('swapEditLimit')} -
+
onTokenApprovalClick()} + > + {t('swapEditLimit')} +
)} +
+
+
+ {t('swapQuoteIncludesRate', [metaMaskFee])} +
+ +
+
) @@ -122,4 +165,9 @@ FeeCard.propTypes = { tokenApprovalTextComponent: PropTypes.node, tokenApprovalSourceTokenSymbol: PropTypes.string, onTokenApprovalClick: PropTypes.func, + metaMaskFee: PropTypes.string.isRequired, + isBestQuote: PropTypes.bool, + onQuotesClick: PropTypes.func.isRequired, + numberOfQuotes: PropTypes.number.isRequired, + tokenConversionRate: PropTypes.number, } diff --git a/ui/app/pages/swaps/fee-card/fee-card.stories.js b/ui/app/pages/swaps/fee-card/fee-card.stories.js index af57849f3..aaaf43d0f 100644 --- a/ui/app/pages/swaps/fee-card/fee-card.stories.js +++ b/ui/app/pages/swaps/fee-card/fee-card.stories.js @@ -1,6 +1,6 @@ import React from 'react' import { action } from '@storybook/addon-actions' -import { text } from '@storybook/addon-knobs' +import { text, boolean, number, object } from '@storybook/addon-knobs' import FeeCard from './fee-card' const tokenApprovalTextComponent = ( @@ -35,6 +35,13 @@ export const WithAllProps = () => { tokenApprovalSourceTokenSymbol="ABC" onTokenApprovalClick={action('Clicked third row link')} hideTokenApprovalRow={false} + metaMaskFee="0.875" + savings={object('savings 1', { total: '8.55' })} + onQuotesClick={action('Clicked quotes link')} + numberOfQuotes={number('numberOfQuotes', 6)} + isBestQuote={boolean('isBestQuote', true)} + conversionRate={300} + currentCurrency="usd" />
) @@ -55,6 +62,11 @@ export const WithoutThirdRow = () => { }} onFeeCardMaxRowClick={action('Clicked max fee row link')} hideTokenApprovalRow + onQuotesClick={action('Clicked quotes link')} + numberOfQuotes={number('numberOfQuotes', 1)} + isBestQuote={boolean('isBestQuote', true)} + savings={object('savings 1', { total: '8.55' })} + metaMaskFee="0.875" />
) @@ -70,6 +82,9 @@ export const WithOnlyRequiredProps = () => { }} onFeeCardMaxRowClick={action('Clicked max fee row link')} hideTokenApprovalRow + metaMaskFee="0.875" + onQuotesClick={action('Clicked quotes link')} + numberOfQuotes={2} /> ) diff --git a/ui/app/pages/swaps/fee-card/index.scss b/ui/app/pages/swaps/fee-card/index.scss index 0a86f6ab7..d1807fc0d 100644 --- a/ui/app/pages/swaps/fee-card/index.scss +++ b/ui/app/pages/swaps/fee-card/index.scss @@ -1,11 +1,69 @@ .fee-card { - border-radius: 8px; - border: 1px solid $Grey-100; width: 100%; @include H7; + &__savings-and-quotes-header { + display: flex; + position: relative; + align-items: center; + } + + &__savings-and-quotes-row { + display: flex; + justify-content: space-between; + align-items: center; + width: 100%; + left: 58px; + height: 39px; + background: $Blue-000; + border: 1px solid $Blue-500; + border-top-right-radius: 8px; + border-top-left-radius: 8px; + border-bottom: 0; + padding-left: 8px; + padding-right: 8px; + } + + &__savings-text { + @include H6; + + font-weight: bold; + color: $Blue-500; + } + + &__quote-link-container { + display: flex; + align-items: center; + cursor: pointer; + } + + &__quote-link-text { + @include H7; + + color: $Blue-500; + } + + &__caret-right { + color: $Blue-500; + width: 6px; + height: 6px; + display: flex; + justify-content: center; + align-items: center; + margin-left: 6px; + + i { + transform: rotate(90deg); + } + } + &__main { + border: 1px solid $Blue-500; + border-bottom-left-radius: 8px; + border-bottom-right-radius: 8px; + width: 100%; + max-width: 311px; padding: 16px 16px 12px 16px; } @@ -31,6 +89,10 @@ cursor: pointer; } + &__row-header-text--bold { + color: $Black-100; + } + &__row, &__top-bordered-row { display: flex; @@ -51,7 +113,6 @@ img { height: 10px; width: 10px; - margin-left: 4px; cursor: pointer; } } @@ -60,7 +121,12 @@ height: 10px; width: 10px; justify-content: center; - margin-top: 2px; + + div { + // Needed to override the style property added by the react-tippy library + display: flex !important; + height: 10px; + } } &__info-tooltip-paragraph { @@ -111,11 +177,14 @@ margin-right: 12px; } - &__row-header-primary, - &__row-header-primary--bold { + &__row-header-primary { color: $Grey-500; } + &__row-header-primary--bold { + color: $Black-100; + } + &__row-header-text--bold, &__row-header-secondary--bold, &__row-header-primary--bold { @@ -125,6 +194,11 @@ &__bold { font-weight: bold; } + + &__tilde { + font-family: Roboto, Helvetica, Arial, sans-serif; + margin-right: -3.5px; + } } .info-tooltip { diff --git a/ui/app/pages/swaps/fee-card/pig-icon.js b/ui/app/pages/swaps/fee-card/pig-icon.js new file mode 100644 index 000000000..ef7677ae4 --- /dev/null +++ b/ui/app/pages/swaps/fee-card/pig-icon.js @@ -0,0 +1,54 @@ +import React from 'react' + +export default function PigIcon() { + return ( + + + + + + + + + + ) +} diff --git a/ui/app/pages/swaps/index.scss b/ui/app/pages/swaps/index.scss index 1346eb364..cb08d457b 100644 --- a/ui/app/pages/swaps/index.scss +++ b/ui/app/pages/swaps/index.scss @@ -71,7 +71,7 @@ font-weight: bold; color: $Black-100; - margin-top: 3px; + margin-top: -5px; } &__header { diff --git a/ui/app/pages/swaps/main-quote-summary/index.scss b/ui/app/pages/swaps/main-quote-summary/index.scss index 7993ea724..dfdc882f1 100644 --- a/ui/app/pages/swaps/main-quote-summary/index.scss +++ b/ui/app/pages/swaps/main-quote-summary/index.scss @@ -1,28 +1,75 @@ .main-quote-summary { display: flex; flex-flow: column; + justify-content: center; align-items: center; position: relative; - height: 196px; + max-height: 196px; + min-height: 196px; width: 100%; - color: $white; + color: $Black-100; - &__quote-backdrop-with-top-tab, - &__quote-backdrop { - position: absolute; - box-shadow: 0 10px 39px rgba(3, 125, 214, 0.15); - border-radius: 8px; - background: #fafcff; + &__source-row, + &__destination-row { + width: 100%; + display: flex; + align-items: flex-start; + justify-content: center; + + @include H6; + + color: $Grey-500; } - &__quote-backdrop-with-top-tab { - width: 348px; - height: 215px; + &__source-row { + align-items: center; } - &__quote-backdrop { - width: 310px; - height: 164px; + &__source-row-value, + &__source-row-symbol { + // Each of these spans can be half their container width minus the space + // needed for the token icon and the span margins + max-width: calc(50% - 13px); + } + + + &__source-row-value { + margin-right: 5px; + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + } + + &__source-row-symbol { + margin-left: 5px; + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + } + + &__destination-row { + margin-top: 6px; + } + + &__destination-row-symbol { + margin-left: 5px; + color: $Black-100; + } + + &__icon, + &__icon-fallback { + height: 16px; + width: 16px; + } + + &__icon-fallback { + padding-top: 0; + font-size: 12px; + line-height: 16px; + } + + &__down-arrow { + margin-top: 5px; } &__details { @@ -33,62 +80,24 @@ position: relative; } - &__best-quote { - @include H7; - - font-weight: bold; - position: relative; - display: flex; - padding-top: 6px; - letter-spacing: 0.12px; - min-height: 16px; - - > span { - margin-left: 4px; - } - } - &__quote-details-top { - height: 94px; display: flex; flex-flow: column; justify-content: center; align-items: center; width: 100%; - padding: 12px; - padding-top: 2px; - margin-top: 4px; - } - - &__bold { - font-weight: 900; - } - - &__quote-small-white { - white-space: nowrap; - width: 100%; - text-align: center; - font-size: 14px; - margin-bottom: 8px; - margin-top: 6px; } &__quote-large { display: flex; - align-items: flex-end; + align-items: flex-start; + margin-top: 8px; + height: 50px; } &__quote-large-number { - font-size: 40px; - line-height: 32px; - margin-right: 6px; - } - - &__quote-large-symbol { - display: flex; - align-items: flex-end; - font-size: 32px; - line-height: 32px; + font-size: 60px; + line-height: 48px; } &__quote-large-white { @@ -104,7 +113,10 @@ justify-content: center; align-items: center; width: 287px; - border-top: 1px solid rgba(255, 255, 255, 0.2); - height: 42px; + margin-top: 14px; + } + + &__exchange-rate-display { + color: $Grey-500; } } diff --git a/ui/app/pages/swaps/main-quote-summary/main-quote-summary.js b/ui/app/pages/swaps/main-quote-summary/main-quote-summary.js index 1ae2e7d51..20aadf6fc 100644 --- a/ui/app/pages/swaps/main-quote-summary/main-quote-summary.js +++ b/ui/app/pages/swaps/main-quote-summary/main-quote-summary.js @@ -1,62 +1,33 @@ -import React, { useContext } from 'react' +import React from 'react' import PropTypes from 'prop-types' import BigNumber from 'bignumber.js' -import classnames from 'classnames' -import { I18nContext } from '../../../contexts/i18n' import { calcTokenAmount } from '../../../helpers/utils/token-util' import { toPrecisionWithoutTrailingZeros } from '../../../helpers/utils/util' import Tooltip from '../../../components/ui/tooltip' -import SunCheckIcon from '../../../components/ui/icon/sun-check-icon.component' +import UrlIcon from '../../../components/ui/url-icon' import ExchangeRateDisplay from '../exchange-rate-display' import { formatSwapsValueForDisplay } from '../swaps.util' -import QuoteBackdrop from './quote-backdrop' -function getFontSizes(fontSizeScore) { - if (fontSizeScore <= 11) { +function getFontSizesAndLineHeights(fontSizeScore) { + if (fontSizeScore <= 9) { + return [60, 48] + } + if (fontSizeScore <= 13) { return [40, 32] } - if (fontSizeScore <= 16) { - return [30, 24] - } - return [24, 14] -} - -function getLineHeight(fontSizeScore) { - if (fontSizeScore <= 11) { - return 32 - } - if (fontSizeScore <= 16) { - return 26 - } - return 18 -} - -// Returns a numerical value based on the length of the two passed strings: amount and symbol. -// The returned value equals the number of digits in the amount string plus a value calculated -// from the length of the symbol string. The returned number will be passed to the getFontSizes function -// to determine the font size to apply to the amount and symbol strings when rendered. The -// desired maximum digits and letters to show in the ultimately rendered string is 20, and in -// such cases there can also be ellipsis shown and a decimal, combinding for a rendered "string" -// length of ~22. As the symbol will always have a smaller font size than the amount, the -// additive value of the symbol length to the font size score is corrected based on the total -// number of alphanumeric characters in both strings and the desired rendered length of 22. -function getFontSizeScore(amount, symbol) { - const amountLength = amount.match(/\d+/gu).join('').length - const symbolModifier = Math.min((amountLength + symbol.length) / 22, 1) - return amountLength + symbol.length * symbolModifier + return [26, 15] } export default function MainQuoteSummary({ - isBestQuote, sourceValue, sourceSymbol, sourceDecimals, + sourceIconUrl, destinationValue, destinationSymbol, destinationDecimals, + destinationIconUrl, }) { - const t = useContext(I18nContext) - const sourceAmount = toPrecisionWithoutTrailingZeros( calcTokenAmount(sourceValue, sourceDecimals).toString(10), 12, @@ -67,43 +38,55 @@ export default function MainQuoteSummary({ ) const amountToDisplay = formatSwapsValueForDisplay(destinationAmount) - const fontSizeScore = getFontSizeScore(amountToDisplay, destinationSymbol) - const [numberFontSize, symbolFontSize] = getFontSizes(fontSizeScore) - const lineHeight = getLineHeight(fontSizeScore) - + const amountDigitLength = amountToDisplay.match(/\d+/gu).join('').length + const [numberFontSize, lineHeight] = getFontSizesAndLineHeights( + amountDigitLength, + ) let ellipsedAmountToDisplay = amountToDisplay - if (fontSizeScore > 20) { - ellipsedAmountToDisplay = `${amountToDisplay.slice( - 0, - amountToDisplay.length - (fontSizeScore - 20), - )}...` + + if (amountDigitLength > 20) { + ellipsedAmountToDisplay = `${amountToDisplay.slice(0, 20)}...` } return (
-
- -
-
- {isBestQuote && } - {isBestQuote && t('swapsBestQuote')} -
- - {t('swapsConvertToAbout', [ - - {`${sourceAmount} ${sourceSymbol}`} - , - ])} - +
+ + {formatSwapsValueForDisplay(sourceAmount)} + + + + {sourceSymbol} + +
+ +
+ + + {destinationSymbol} + +
- - {`${destinationSymbol}`} -
@@ -141,8 +115,9 @@ export default function MainQuoteSummary({ secondaryTokenValue={destinationValue} secondaryTokenDecimals={destinationDecimals} secondaryTokenSymbol={destinationSymbol} - className="exchange-rate-display--white" - arrowColor="white" + arrowColor="#037DD6" + boldSymbols={false} + className="main-quote-summary__exchange-rate-display" />
@@ -151,7 +126,6 @@ export default function MainQuoteSummary({ } MainQuoteSummary.propTypes = { - isBestQuote: PropTypes.bool, sourceValue: PropTypes.oneOfType([ PropTypes.string, PropTypes.instanceOf(BigNumber), @@ -167,4 +141,6 @@ MainQuoteSummary.propTypes = { PropTypes.number, ]), destinationSymbol: PropTypes.string.isRequired, + sourceIconUrl: PropTypes.string, + destinationIconUrl: PropTypes.string, } diff --git a/ui/app/pages/swaps/main-quote-summary/main-quote-summary.stories.js b/ui/app/pages/swaps/main-quote-summary/main-quote-summary.stories.js index ea267eb17..0f5ca998d 100644 --- a/ui/app/pages/swaps/main-quote-summary/main-quote-summary.stories.js +++ b/ui/app/pages/swaps/main-quote-summary/main-quote-summary.stories.js @@ -1,5 +1,5 @@ import React from 'react' -import { text, number, boolean } from '@storybook/addon-knobs' +import { text, number } from '@storybook/addon-knobs' import MainQuoteSummary from './main-quote-summary' export default { @@ -8,28 +8,24 @@ export default { export const BestQuote = () => { return ( - - ) -} - -export const NotBestQuote = () => { - return ( - +
+ +
) } diff --git a/ui/app/pages/swaps/slippage-buttons/slippage-buttons.js b/ui/app/pages/swaps/slippage-buttons/slippage-buttons.js index fe5daaa63..293422f0f 100644 --- a/ui/app/pages/swaps/slippage-buttons/slippage-buttons.js +++ b/ui/app/pages/swaps/slippage-buttons/slippage-buttons.js @@ -6,7 +6,7 @@ import ButtonGroup from '../../../components/ui/button-group' import Button from '../../../components/ui/button' import InfoTooltip from '../../../components/ui/info-tooltip' -export default function SlippageButtons({ onSelect }) { +export default function SlippageButtons({ onSelect, maxAllowedSlippage }) { const t = useContext(I18nContext) const [open, setOpen] = useState(false) const [customValue, setCustomValue] = useState('') @@ -15,12 +15,19 @@ export default function SlippageButtons({ onSelect }) { const [inputRef, setInputRef] = useState(null) let errorText = '' - if (customValue && Number(customValue) <= 0) { - errorText = t('swapSlippageTooLow') - } else if (customValue && Number(customValue) < 0.5) { - errorText = t('swapLowSlippageError') - } else if (customValue && Number(customValue) > 5) { - errorText = t('swapHighSlippageWarning') + if (customValue) { + if (Number(customValue) <= 0) { + errorText = t('swapSlippageTooLow') + } else if (Number(customValue) < 0.5) { + errorText = t('swapLowSlippageError') + } else if ( + Number(customValue) >= 5 && + Number(customValue) <= maxAllowedSlippage + ) { + errorText = t('swapHighSlippageWarning') + } else if (Number(customValue) > maxAllowedSlippage) { + errorText = t('swapsExcessiveSlippageWarning') + } } const customValueText = customValue || t('swapCustom') @@ -151,4 +158,5 @@ export default function SlippageButtons({ onSelect }) { SlippageButtons.propTypes = { onSelect: PropTypes.func.isRequired, + maxAllowedSlippage: PropTypes.number.isRequired, } diff --git a/ui/app/pages/swaps/swaps-footer/swaps-footer.js b/ui/app/pages/swaps/swaps-footer/swaps-footer.js index 870ce028b..62a3e2a70 100644 --- a/ui/app/pages/swaps/swaps-footer/swaps-footer.js +++ b/ui/app/pages/swaps/swaps-footer/swaps-footer.js @@ -31,7 +31,10 @@ export default function SwapsFooter({ onSubmit={onSubmit} submitText={submitText} submitButtonType="confirm" - footerClassName="swaps-footer__custom-page-container-footer-class" + footerClassName={classnames( + 'swaps-footer__custom-page-container-footer-class', + className, + )} footerButtonClassName={classnames( 'swaps-footer__custom-page-container-footer-button-class', { diff --git a/ui/app/pages/swaps/swaps-gas-customization-modal/swaps-gas-customization-modal.component.js b/ui/app/pages/swaps/swaps-gas-customization-modal/swaps-gas-customization-modal.component.js index 53b4583dd..1347529dc 100644 --- a/ui/app/pages/swaps/swaps-gas-customization-modal/swaps-gas-customization-modal.component.js +++ b/ui/app/pages/swaps/swaps-gas-customization-modal/swaps-gas-customization-modal.component.js @@ -35,7 +35,7 @@ export default class GasModalPageContainer extends Component { disableSave: PropTypes.bool, customGasLimitMessage: PropTypes.string, customTotalSupplement: PropTypes.string, - usdConversionRate: PropTypes.string, + usdConversionRate: PropTypes.number, customGasPrice: PropTypes.string, customGasLimit: PropTypes.string, setSwapsCustomizationModalPrice: PropTypes.func, diff --git a/ui/app/pages/swaps/swaps.util.js b/ui/app/pages/swaps/swaps.util.js index d3189e5b7..189ff877b 100644 --- a/ui/app/pages/swaps/swaps.util.js +++ b/ui/app/pages/swaps/swaps.util.js @@ -507,6 +507,7 @@ export function quotesToRenderableData( destinationTokenDecimals: destinationTokenInfo.decimals, destinationTokenSymbol: destinationTokenInfo.symbol, destinationTokenValue: formatSwapsValueForDisplay(destinationValue), + destinationIconUrl: destinationTokenInfo.iconUrl, isBestQuote: quote.isBestQuote, liquiditySourceKey, feeInEth, @@ -518,6 +519,7 @@ export function quotesToRenderableData( sourceTokenDecimals: sourceTokenInfo.decimals, sourceTokenSymbol: sourceTokenInfo.symbol, sourceTokenValue: sourceValue, + sourceTokenIconUrl: sourceTokenInfo.iconUrl, ethValueOfTrade, minimumAmountReceived, metaMaskFee: fee, diff --git a/ui/app/pages/swaps/view-quote/index.scss b/ui/app/pages/swaps/view-quote/index.scss index 474eced60..3382befff 100644 --- a/ui/app/pages/swaps/view-quote/index.scss +++ b/ui/app/pages/swaps/view-quote/index.scss @@ -44,13 +44,13 @@ display: flex; align-items: center; justify-content: center; + min-height: 46px; } &__view-other-button, &__view-other-button-fade { display: flex; align-items: center; - margin-bottom: 16px; position: absolute; @include H7; @@ -133,6 +133,9 @@ width: 100%; align-items: center; justify-content: center; + width: intrinsic; /* Safari/WebKit uses a non-standard name */ + width: max-content; + max-width: 340px; margin-top: 8px; @media screen and (min-width: 576px) { @@ -149,57 +152,29 @@ } &__countdown-timer-container { - @media screen and (max-width: 576px) { - margin-top: 12px; - margin-bottom: 16px; - - &--thin { - margin-top: 8px; - margin-bottom: 8px; - - > div { - margin-top: 0; - margin-bottom: 0; - } - } - } - - @media screen and (min-width: 576px) { - &--thin { - margin-top: 6px; - } - } + width: 152px; + min-height: 32px; + display: flex; + justify-content: center; + border-radius: 42px; + background: #f2f3f4; + margin-top: 8px; } &__fee-card-container { + display: flex; + align-items: center; width: 100%; - margin-top: 8px; + max-width: 311px; margin-bottom: 8px; @media screen and (min-width: 576px) { margin-bottom: 0; - - &--three-rows { - margin-bottom: -16px; - } - } - } - - &__main-quote-summary-container { - margin-top: 24px; - - @media screen and (max-width: 576px) { - margin-top: 0; - } - - &--thin { - margin-top: 8px; } } &__metamask-rate { display: flex; - margin-top: 8%; } &__metamask-rate-text { @@ -214,5 +189,9 @@ &__thin-swaps-footer { max-height: 82px; + + @media screen and (min-width: 576px) { + height: 72px; + } } } diff --git a/ui/app/pages/swaps/view-quote/view-quote.js b/ui/app/pages/swaps/view-quote/view-quote.js index 4db97824f..1f5769f0c 100644 --- a/ui/app/pages/swaps/view-quote/view-quote.js +++ b/ui/app/pages/swaps/view-quote/view-quote.js @@ -73,7 +73,6 @@ import { useTokenTracker } from '../../../hooks/useTokenTracker' import { QUOTES_EXPIRED_ERROR } from '../../../helpers/constants/swaps' import CountdownTimer from '../countdown-timer' import SwapsFooter from '../swaps-footer' -import InfoTooltip from '../../../components/ui/info-tooltip' import ViewQuotePriceDifference from './view-quote-price-difference' export default function ViewQuote() { @@ -117,6 +116,7 @@ export default function ViewQuote() { const tradeValue = usedQuote?.trade?.value ?? '0x0' const { isBestQuote } = usedQuote + const fetchParamsSourceToken = fetchParams?.sourceToken const usedGasLimit = @@ -191,9 +191,11 @@ export default function ViewQuote() { destinationTokenDecimals, destinationTokenSymbol, destinationTokenValue, + destinationIconUrl, sourceTokenDecimals, sourceTokenSymbol, sourceTokenValue, + sourceTokenIconUrl, } = renderableDataForUsedQuote const { feeInFiat, feeInEth } = getRenderableNetworkFeesForQuote( @@ -499,11 +501,7 @@ export default function ViewQuote() { /> )}
-
+
-
- -
-
-
- {t('swapNQuotesAvailable', [Object.values(quotes).length])} - -
-
{ - allAvailableQuotesOpened() - setSelectQuotePopoverShown(true) - }} - > - {t('swapNQuotesAvailable', [Object.values(quotes).length])} - -
-
-
-

- {t('swapQuoteIncludesRate', [metaMaskFee])} -

- -
+
{ + allAvailableQuotesOpened() + setSelectQuotePopoverShown(true) + }} + tokenConversionRate={ + destinationTokenSymbol === 'ETH' + ? 1 + : memoizedTokenConversionRates[destinationToken.address] + } />
@@ -595,7 +573,6 @@ export default function ViewQuote() { onCancel={async () => await dispatch(navigateBackToBuildQuote(history))} disabled={balanceError || gasPrice === null || gasPrice === undefined} className={isShowingWarning && 'view-quote__thin-swaps-footer'} - showTermsOfService showTopBorder /> diff --git a/yarn.lock b/yarn.lock index f328d7c05..da2e567eb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2115,20 +2115,19 @@ resolved "https://registry.yarnpkg.com/@metamask/forwarder/-/forwarder-1.1.0.tgz#13829d8244bbf19ea658c0b20d21a77b67de0bdd" integrity sha512-Hggj4y0QIjDzKGTXzarhEPIQyFSB2bi2y6YLJNwaT4JmP30UB5Cj6gqoY0M4pj3QT57fzp0BUuGp7F/AUe28tw== -"@metamask/inpage-provider@^6.1.0": - version "6.3.0" - resolved "https://registry.yarnpkg.com/@metamask/inpage-provider/-/inpage-provider-6.3.0.tgz#92d965e20912c24adbf973efbd07dbf339547741" - integrity sha512-n7E06+8hWdYKmgJo84WFvgX6/BSqaOQEOMIrcbrP48LdkkZNEAChx6D8oUb2lYDQiWgahR+f20jsJoN4WmOjxw== +"@metamask/inpage-provider@^8.0.1": + version "8.0.1" + resolved "https://registry.yarnpkg.com/@metamask/inpage-provider/-/inpage-provider-8.0.1.tgz#67b1f0733ae7c0e0e429dc5c067ba9d2dd6d66da" + integrity sha512-dN3IpiJtaHeiPzF01UXnrQ6TxXbXbkU54kiOHuIUe9e8s7vyPzgDgN2nj84xjmIkqxL0MKY90Wcp0obFKnNj+Q== dependencies: - eth-rpc-errors "^2.1.1" + "@metamask/safe-event-emitter" "^2.0.0" + eth-rpc-errors "^4.0.2" fast-deep-equal "^2.0.1" is-stream "^2.0.0" - json-rpc-engine "^5.2.0" + json-rpc-engine "^6.1.0" json-rpc-middleware-stream "^2.1.1" obj-multiplex "^1.0.0" - obs-store "^4.0.3" pump "^3.0.0" - safe-event-emitter "^1.0.1" "@metamask/jazzicon@^2.0.0": version "2.0.0" @@ -9889,13 +9888,6 @@ eth-query@^2.0.2, eth-query@^2.1.0, eth-query@^2.1.2: json-rpc-random-id "^1.0.0" xtend "^4.0.1" -eth-rpc-errors@^2.1.1: - version "2.1.1" - resolved "https://registry.yarnpkg.com/eth-rpc-errors/-/eth-rpc-errors-2.1.1.tgz#00a7d6c8a9c864a8ab7d0356be20964e5bee4b13" - integrity sha512-MY3zAa5ZF8hvgQu1HOF9agaK5GgigBRGpTJ8H0oVlE0NqMu13CW6syyjLXdeIDCGQTbUeHliU1z9dVmvMKx1Tg== - dependencies: - fast-safe-stringify "^2.0.6" - eth-rpc-errors@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/eth-rpc-errors/-/eth-rpc-errors-3.0.0.tgz#d7b22653c70dbf9defd4ef490fd08fe70608ca10" @@ -12935,9 +12927,9 @@ hi-base32@~0.5.0: integrity sha512-DDRmxSyoYuvjUb9EnXdoiMChBZ7ZcUVJsK5Frd3kqMhuBxvmZdnBeynAVfj7/ECbn++CekcoprvC/rprHPAtow== highlight.js@^10.1.1, highlight.js@~10.4.0: - version "10.4.0" - resolved "https://registry.yarnpkg.com/highlight.js/-/highlight.js-10.4.0.tgz#ef3ce475e5dfa7a48484260b49ea242ddab823a0" - integrity sha512-EfrUGcQ63oLJbj0J0RI9ebX6TAITbsDBLbsjr881L/X5fMO9+oadKzEF21C7R3ULKG6Gv3uoab2HiqVJa/4+oA== + version "10.4.1" + resolved "https://registry.yarnpkg.com/highlight.js/-/highlight.js-10.4.1.tgz#d48fbcf4a9971c4361b3f95f302747afe19dbad0" + integrity sha512-yR5lWvNz7c85OhVAEAeFhVCc/GV4C30Fjzc/rCP0aCWzc1UUOPUk55dK/qdwTZHBvMZo+eZ2jpk62ndX/xMFlg== history@^4.9.0: version "4.10.1" @@ -15221,7 +15213,7 @@ json-rpc-engine@^3.4.0, json-rpc-engine@^3.6.0: promise-to-callback "^1.0.0" safe-event-emitter "^1.0.1" -json-rpc-engine@^5.2.0, json-rpc-engine@^5.3.0: +json-rpc-engine@^5.3.0: version "5.3.0" resolved "https://registry.yarnpkg.com/json-rpc-engine/-/json-rpc-engine-5.3.0.tgz#7dc7291766b28766ebda33eb6d3f4c6301c44ff4" integrity sha512-+diJ9s8rxB+fbJhT7ZEf8r8spaLRignLd8jTgQ/h5JSGppAHGtNMZtCoabipCaleR1B3GTGxbXBOqhaJSGmPGQ==