diff --git a/CHANGELOG.md b/CHANGELOG.md index f6d821b36..0add13fc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [9.5.3] +### Fixed +- [#11103](https://github.com/MetaMask/metamask-extension/pull/11103): Fixes bug that made MetaMask unusable and displayed 'Minified React error #130' on certain networks and accounts +- [#11015](https://github.com/MetaMask/metamask-extension/pull/11015): Prevent big number error when attempting to view transaction list + ## [9.5.2] ### Fixed - [#11071](https://github.com/MetaMask/metamask-extension/pull/11071): Fixing address entry error when sending a transaction on a custom network @@ -2230,7 +2235,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Uncategorized - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v9.5.2...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v9.5.3...HEAD +[9.5.3]: https://github.com/MetaMask/metamask-extension/compare/v9.5.2...v9.5.3 [9.5.2]: https://github.com/MetaMask/metamask-extension/compare/v9.5.1...v9.5.2 [9.5.1]: https://github.com/MetaMask/metamask-extension/compare/v9.5.0...v9.5.1 [9.5.0]: https://github.com/MetaMask/metamask-extension/compare/v9.4.0...v9.5.0 diff --git a/app/manifest/_base.json b/app/manifest/_base.json index 07664ccd7..bff810ebb 100644 --- a/app/manifest/_base.json +++ b/app/manifest/_base.json @@ -71,6 +71,6 @@ "notifications" ], "short_name": "__MSG_appName__", - "version": "9.5.2", + "version": "9.5.3", "web_accessible_resources": ["inpage.js", "phishing.html"] } diff --git a/app/scripts/controllers/ens/index.js b/app/scripts/controllers/ens/index.js index a7d4c696b..85477ef7e 100644 --- a/app/scripts/controllers/ens/index.js +++ b/app/scripts/controllers/ens/index.js @@ -1,8 +1,8 @@ import punycode from 'punycode/punycode'; -import { toChecksumAddress } from 'ethereumjs-util'; import { ObservableStore } from '@metamask/obs-store'; import log from 'loglevel'; import { CHAIN_ID_TO_NETWORK_ID_MAP } from '../../../../shared/constants/network'; +import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; import Ens from './ens'; const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; @@ -43,7 +43,7 @@ export default class EnsController { } reverseResolveAddress(address) { - return this._reverseResolveAddress(toChecksumAddress(address)); + return this._reverseResolveAddress(toChecksumHexAddress(address)); } async _reverseResolveAddress(address) { @@ -79,7 +79,7 @@ export default class EnsController { return undefined; } - if (toChecksumAddress(registeredAddress) !== address) { + if (toChecksumHexAddress(registeredAddress) !== address) { return undefined; } diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 9470df50c..95b0e1e4d 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -2,12 +2,12 @@ import { strict as assert } from 'assert'; import { ObservableStore } from '@metamask/obs-store'; import { ethErrors } from 'eth-rpc-errors'; import { normalize as normalizeAddress } from 'eth-sig-util'; -import { isValidAddress } from 'ethereumjs-util'; import ethers from 'ethers'; import log from 'loglevel'; import { LISTED_CONTRACT_ADDRESSES } from '../../../shared/constants/tokens'; import { NETWORK_TYPE_TO_ID_MAP } from '../../../shared/constants/network'; import { isPrefixedFormattedHexString } from '../../../shared/modules/network.utils'; +import { isValidHexAddress } from '../../../shared/modules/hexstring-utils'; import { NETWORK_EVENTS } from './network'; export default class PreferencesController { @@ -836,7 +836,7 @@ export default class PreferencesController { `Invalid decimals "${decimals}": must be 0 <= 36.`, ); } - if (!isValidAddress(address)) { + if (!isValidHexAddress(address, { allowNonPrefixed: false })) { throw ethErrors.rpc.invalidParams(`Invalid address "${address}".`); } } diff --git a/app/scripts/controllers/token-rates.js b/app/scripts/controllers/token-rates.js index b725b04c9..1bba3f389 100644 --- a/app/scripts/controllers/token-rates.js +++ b/app/scripts/controllers/token-rates.js @@ -1,8 +1,8 @@ import { ObservableStore } from '@metamask/obs-store'; import log from 'loglevel'; import { normalize as normalizeAddress } from 'eth-sig-util'; -import { toChecksumAddress } from 'ethereumjs-util'; import getFetchWithTimeout from '../../../shared/modules/fetch-with-timeout'; +import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils'; const fetchWithTimeout = getFetchWithTimeout(30000); @@ -45,7 +45,7 @@ export default class TokenRatesController { this._tokens.forEach((token) => { const price = prices[token.address.toLowerCase()] || - prices[toChecksumAddress(token.address)]; + prices[toChecksumHexAddress(token.address)]; contractExchangeRates[normalizeAddress(token.address)] = price ? price[nativeCurrency] : 0; diff --git a/app/scripts/controllers/transactions/lib/util.js b/app/scripts/controllers/transactions/lib/util.js index 70652a3c1..804f43ec2 100644 --- a/app/scripts/controllers/transactions/lib/util.js +++ b/app/scripts/controllers/transactions/lib/util.js @@ -1,7 +1,7 @@ -import { isValidAddress } from 'ethereumjs-util'; import { ethErrors } from 'eth-rpc-errors'; import { addHexPrefix } from '../../../lib/util'; import { TRANSACTION_STATUSES } from '../../../../../shared/constants/transaction'; +import { isValidHexAddress } from '../../../../../shared/modules/hexstring-utils'; const normalizers = { from: (from) => addHexPrefix(from), @@ -110,7 +110,7 @@ export function validateFrom(txParams) { `Invalid "from" address "${txParams.from}": not a string.`, ); } - if (!isValidAddress(txParams.from)) { + if (!isValidHexAddress(txParams.from, { allowNonPrefixed: false })) { throw ethErrors.rpc.invalidParams('Invalid "from" address.'); } } @@ -128,7 +128,10 @@ export function validateRecipient(txParams) { } else { throw ethErrors.rpc.invalidParams('Invalid "to" address.'); } - } else if (txParams.to !== undefined && !isValidAddress(txParams.to)) { + } else if ( + txParams.to !== undefined && + !isValidHexAddress(txParams.to, { allowNonPrefixed: false }) + ) { throw ethErrors.rpc.invalidParams('Invalid "to" address.'); } return txParams; diff --git a/app/scripts/controllers/transactions/tx-state-manager.js b/app/scripts/controllers/transactions/tx-state-manager.js index 9eaa260a1..eed1f6d79 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.js +++ b/app/scripts/controllers/transactions/tx-state-manager.js @@ -187,28 +187,43 @@ export default class TransactionStateManager extends EventEmitter { const transactions = this.getTransactions({ filterToCurrentNetwork: false, }); - const txCount = transactions.length; const { txHistoryLimit } = this; // checks if the length of the tx history is longer then desired persistence // limit and then if it is removes the oldest confirmed or rejected tx. // Pending or unapproved transactions will not be removed by this - // operation. + // operation. For safety of presenting a fully functional transaction UI + // representation, this function will not break apart transactions with the + // same nonce, per network. Not accounting for transactions of the same + // nonce and network combo can result in confusing or broken experiences + // in the UI. // // TODO: we are already limiting what we send to the UI, and in the future // we will send UI only collected groups of transactions *per page* so at // some point in the future, this persistence limit can be adjusted. When // we do that I think we should figure out a better storage solution for // transaction history entries. - if (txCount > txHistoryLimit - 1) { - const index = transactions.findIndex((metaTx) => { - return getFinalStates().includes(metaTx.status); - }); - if (index !== -1) { - this._deleteTransaction(transactions[index].id); - } - } + const nonceNetworkSet = new Set(); + const txsToDelete = transactions + .reverse() + .filter((tx) => { + const { nonce } = tx.txParams; + const { chainId, metamaskNetworkId, status } = tx; + const key = `${nonce}-${chainId ?? metamaskNetworkId}`; + if (nonceNetworkSet.has(key)) { + return false; + } else if ( + nonceNetworkSet.size < txHistoryLimit - 1 || + getFinalStates().includes(status) === false + ) { + nonceNetworkSet.add(key); + return false; + } + return true; + }) + .map((tx) => tx.id); + this._deleteTransactions(txsToDelete); this._addTransactionsToState([txMeta]); return txMeta; } @@ -612,4 +627,20 @@ export default class TransactionStateManager extends EventEmitter { transactions, }); } + + /** + * removes multiple transaction from state. This is not intended for external use. + * + * @private + * @param {number[]} targetTransactionIds - the transactions to delete + */ + _deleteTransactions(targetTransactionIds) { + const { transactions } = this.store.getState(); + targetTransactionIds.forEach((transactionId) => { + delete transactions[transactionId]; + }); + this.store.updateState({ + transactions, + }); + } } diff --git a/app/scripts/controllers/transactions/tx-state-manager.test.js b/app/scripts/controllers/transactions/tx-state-manager.test.js index 79c764643..895fb3626 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.test.js +++ b/app/scripts/controllers/transactions/tx-state-manager.test.js @@ -1,8 +1,13 @@ import { strict as assert } from 'assert'; import sinon from 'sinon'; -import { TRANSACTION_STATUSES } from '../../../../shared/constants/transaction'; +import { + TRANSACTION_STATUSES, + TRANSACTION_TYPES, +} from '../../../../shared/constants/transaction'; import { KOVAN_CHAIN_ID, + MAINNET_CHAIN_ID, + RINKEBY_CHAIN_ID, KOVAN_NETWORK_ID, } from '../../../../shared/constants/network'; import TxStateManager from './tx-state-manager'; @@ -10,6 +15,36 @@ import { snapshotFromTxMeta } from './lib/tx-state-history-helpers'; const VALID_ADDRESS = '0x0000000000000000000000000000000000000000'; const VALID_ADDRESS_TWO = '0x0000000000000000000000000000000000000001'; + +function generateTransactions( + numToGen, + { + chainId, + to, + from, + status, + type = TRANSACTION_TYPES.SENT_ETHER, + nonce = (i) => `${i}`, + }, +) { + const txs = []; + for (let i = 0; i < numToGen; i++) { + const tx = { + id: i, + time: new Date() * i, + status: typeof status === 'function' ? status(i) : status, + chainId: typeof chainId === 'function' ? chainId(i) : chainId, + txParams: { + nonce: nonce(i), + to, + from, + }, + type: typeof type === 'function' ? type(i) : type, + }; + txs.push(tx); + } + return txs; +} describe('TransactionStateManager', function () { let txStateManager; const currentNetworkId = KOVAN_NETWORK_ID; @@ -540,19 +575,13 @@ describe('TransactionStateManager', function () { it('cuts off early txs beyond a limit', function () { const limit = txStateManager.txHistoryLimit; - for (let i = 0; i < limit + 1; i++) { - const tx = { - id: i, - time: new Date(), - status: TRANSACTION_STATUSES.CONFIRMED, - metamaskNetworkId: currentNetworkId, - txParams: { - to: VALID_ADDRESS, - from: VALID_ADDRESS, - }, - }; - txStateManager.addTransaction(tx); - } + const txs = generateTransactions(limit + 1, { + chainId: currentChainId, + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + status: TRANSACTION_STATUSES.CONFIRMED, + }); + txs.forEach((tx) => txStateManager.addTransaction(tx)); const result = txStateManager.getTransactions(); assert.equal(result.length, limit, `limit of ${limit} txs enforced`); assert.equal(result[0].id, 1, 'early txs truncated'); @@ -560,52 +589,42 @@ describe('TransactionStateManager', function () { it('cuts off early txs beyond a limit whether or not it is confirmed or rejected', function () { const limit = txStateManager.txHistoryLimit; - for (let i = 0; i < limit + 1; i++) { - const tx = { - id: i, - time: new Date(), - status: TRANSACTION_STATUSES.REJECTED, - metamaskNetworkId: currentNetworkId, - txParams: { - to: VALID_ADDRESS, - from: VALID_ADDRESS, - }, - }; - txStateManager.addTransaction(tx); - } + const txs = generateTransactions(limit + 1, { + chainId: currentChainId, + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + status: TRANSACTION_STATUSES.REJECTED, + }); + txs.forEach((tx) => txStateManager.addTransaction(tx)); const result = txStateManager.getTransactions(); assert.equal(result.length, limit, `limit of ${limit} txs enforced`); assert.equal(result[0].id, 1, 'early txs truncated'); }); it('cuts off early txs beyond a limit but does not cut unapproved txs', function () { - const unconfirmedTx = { - id: 0, - time: new Date(), - status: TRANSACTION_STATUSES.UNAPPROVED, - metamaskNetworkId: currentNetworkId, - txParams: { - to: VALID_ADDRESS, - from: VALID_ADDRESS, - }, - }; - txStateManager.addTransaction(unconfirmedTx); const limit = txStateManager.txHistoryLimit; - for (let i = 1; i < limit + 1; i++) { - const tx = { - id: i, - time: new Date(), - status: TRANSACTION_STATUSES.CONFIRMED, - metamaskNetworkId: currentNetworkId, - txParams: { - to: VALID_ADDRESS, - from: VALID_ADDRESS, - }, - }; - txStateManager.addTransaction(tx); - } + const txs = generateTransactions( + // we add two transactions over limit here to first insert the must be always present + // unapproved tx, then another to force the original logic of adding + // one more beyond the first additional. + limit + 2, + { + chainId: currentChainId, + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + status: (i) => + i === 0 + ? TRANSACTION_STATUSES.UNAPPROVED + : TRANSACTION_STATUSES.CONFIRMED, + }, + ); + txs.forEach((tx) => txStateManager.addTransaction(tx)); const result = txStateManager.getTransactions(); - assert.equal(result.length, limit, `limit of ${limit} txs enforced`); + assert.equal( + result.length, + limit + 1, + `limit of ${limit} + 1 for the unapproved tx is enforced`, + ); assert.equal(result[0].id, 0, 'first tx should still be there'); assert.equal( result[0].status, @@ -614,6 +633,118 @@ describe('TransactionStateManager', function () { ); assert.equal(result[1].id, 2, 'early txs truncated'); }); + + it('cuts off entire groups of transactions by nonce when adding new transaction', function () { + const limit = txStateManager.txHistoryLimit; + // In this test case the earliest two transactions are a dropped attempted ether send and a + // following cancel transaction with the same nonce. these two transactions should be dropped + // together as soon as the 11th unique nonce is attempted to be added. We use limit + 2 to + // first get into the state where we are over the "limit" of transactions because of a set + // of transactions with a unique nonce/network combo, then add an additional new transaction + // to trigger the removal of one group of nonces. + const txs = generateTransactions(limit + 2, { + chainId: currentChainId, + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + nonce: (i) => (i === 1 ? `0` : `${i}`), + status: (i) => + i === 0 + ? TRANSACTION_STATUSES.DROPPED + : TRANSACTION_STATUSES.CONFIRMED, + type: (i) => + i === 1 ? TRANSACTION_TYPES.CANCEL : TRANSACTION_STATUSES.SENT_ETHER, + }); + txs.forEach((tx) => txStateManager.addTransaction(tx)); + const result = txStateManager.getTransactions(); + assert.equal(result.length, limit, `limit of ${limit} is enforced`); + assert.notEqual(result[0].id, 0, 'first tx should be removed'); + assert.equal( + result.some( + (tx) => + tx.status === TRANSACTION_STATUSES.DROPPED || + tx.status === TRANSACTION_TYPES.CANCEL, + ), + false, + 'the cancel and dropped transactions should not be present in the result', + ); + }); + + it('cuts off entire groups of transactions by nonce + network when adding new transaction', function () { + const limit = txStateManager.txHistoryLimit; + // In this test case the earliest two transactions are a dropped attempted ether send and a + // following cancel transaction with the same nonce. Then, a bit later the same scenario on a + // different network. The first two transactions should be dropped after adding even another + // single transaction but the other shouldn't be dropped until adding the fifth additional + // transaction + const txs = generateTransactions(limit + 5, { + chainId: (i) => { + if (i === 0 || i === 1) return MAINNET_CHAIN_ID; + else if (i === 4 || i === 5) return RINKEBY_CHAIN_ID; + return currentChainId; + }, + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + nonce: (i) => ([0, 1, 4, 5].includes(i) ? '0' : `${i}`), + status: (i) => + i === 0 || i === 4 + ? TRANSACTION_STATUSES.DROPPED + : TRANSACTION_STATUSES.CONFIRMED, + type: (i) => + i === 1 || i === 5 + ? TRANSACTION_TYPES.CANCEL + : TRANSACTION_STATUSES.SENT_ETHER, + }); + txs.forEach((tx) => txStateManager.addTransaction(tx)); + const result = txStateManager.getTransactions({ + filterToCurrentNetwork: false, + }); + + assert.equal( + result.length, + limit + 1, + `limit of ${limit} + 1 for the grouped transactions is enforced`, + ); + // The first group of transactions on mainnet should be removed + assert.equal( + result.some( + (tx) => + tx.chainId === MAINNET_CHAIN_ID && tx.txParams.nonce === '0x0', + ), + false, + 'the mainnet transactions with nonce 0x0 should not be present in the result', + ); + }); + + it('does not cut off entire groups of transactions when adding new transaction when under limit', function () { + // In this test case the earliest two transactions are a dropped attempted ether send and a + // following cancel transaction with the same nonce. Then, a bit later the same scenario on a + // different network. None of these should be dropped because we haven't yet reached the limit + const limit = txStateManager.txHistoryLimit; + const txs = generateTransactions(limit - 1, { + chainId: (i) => ([0, 1, 4, 5].includes(i) ? currentChainId : '0x1'), + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + nonce: (i) => { + if (i === 1) return '0'; + else if (i === 5) return '4'; + return `${i}`; + }, + status: (i) => + i === 0 || i === 4 + ? TRANSACTION_STATUSES.DROPPED + : TRANSACTION_STATUSES.CONFIRMED, + type: (i) => + i === 1 || i === 5 + ? TRANSACTION_TYPES.CANCEL + : TRANSACTION_STATUSES.SENT_ETHER, + }); + txs.forEach((tx) => txStateManager.addTransaction(tx)); + const result = txStateManager.getTransactions({ + filterToCurrentNetwork: false, + }); + assert.equal(result.length, 9, `all nine transactions should be present`); + assert.equal(result[0].id, 0, 'first tx should be present'); + }); }); describe('#updateTransaction', function () { diff --git a/app/scripts/lib/typed-message-manager.js b/app/scripts/lib/typed-message-manager.js index 28e7a2534..df0488df8 100644 --- a/app/scripts/lib/typed-message-manager.js +++ b/app/scripts/lib/typed-message-manager.js @@ -3,12 +3,12 @@ import assert from 'assert'; import { ObservableStore } from '@metamask/obs-store'; import { ethErrors } from 'eth-rpc-errors'; import { typedSignatureHash, TYPED_MESSAGE_SCHEMA } from 'eth-sig-util'; -import { isValidAddress } from 'ethereumjs-util'; import log from 'loglevel'; import jsonschema from 'jsonschema'; import { MESSAGE_TYPE } from '../../../shared/constants/app'; import { METAMASK_CONTROLLER_EVENTS } from '../metamask-controller'; import createId from '../../../shared/modules/random-id'; +import { isValidHexAddress } from '../../../shared/modules/hexstring-utils'; /** * Represents, and contains data about, an 'eth_signTypedData' type signature request. These are created when a @@ -160,7 +160,8 @@ export default class TypedMessageManager extends EventEmitter { assert.ok('data' in params, 'Params must include a "data" field.'); assert.ok('from' in params, 'Params must include a "from" field.'); assert.ok( - typeof params.from === 'string' && isValidAddress(params.from), + typeof params.from === 'string' && + isValidHexAddress(params.from, { allowNonPrefixed: false }), '"from" field must be a valid, lowercase, hexadecimal Ethereum address string.', ); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 00b3115c8..93faa0b48 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -10,7 +10,7 @@ import createSubscriptionManager from 'eth-json-rpc-filters/subscriptionManager' import providerAsMiddleware from 'eth-json-rpc-middleware/providerAsMiddleware'; import KeyringController from 'eth-keyring-controller'; import { Mutex } from 'await-semaphore'; -import { toChecksumAddress, stripHexPrefix } from 'ethereumjs-util'; +import { stripHexPrefix } from 'ethereumjs-util'; import log from 'loglevel'; import TrezorKeyring from 'eth-trezor-keyring'; import LedgerBridgeKeyring from '@metamask/eth-ledger-bridge-keyring'; @@ -27,6 +27,7 @@ import { import { TRANSACTION_STATUSES } from '../../shared/constants/transaction'; import { MAINNET_CHAIN_ID } from '../../shared/constants/network'; import { UI_NOTIFICATIONS } from '../../shared/notifications'; +import { toChecksumHexAddress } from '../../shared/modules/hexstring-utils'; import ComposableObservableStore from './lib/ComposableObservableStore'; import AccountTracker from './lib/account-tracker'; @@ -1107,14 +1108,14 @@ export default class MetamaskController extends EventEmitter { // Filter ERC20 tokens const filteredAccountTokens = {}; Object.keys(accountTokens).forEach((address) => { - const checksummedAddress = toChecksumAddress(address); + const checksummedAddress = toChecksumHexAddress(address); filteredAccountTokens[checksummedAddress] = {}; Object.keys(accountTokens[address]).forEach((chainId) => { filteredAccountTokens[checksummedAddress][chainId] = chainId === MAINNET_CHAIN_ID ? accountTokens[address][chainId].filter( ({ address: tokenAddress }) => { - const checksumAddress = toChecksumAddress(tokenAddress); + const checksumAddress = toChecksumHexAddress(tokenAddress); return contractMap[checksumAddress] ? contractMap[checksumAddress].erc20 : true; @@ -1151,10 +1152,10 @@ export default class MetamaskController extends EventEmitter { const accounts = { hd: hdAccounts .filter((item, pos) => hdAccounts.indexOf(item) === pos) - .map((address) => toChecksumAddress(address)), + .map((address) => toChecksumHexAddress(address)), simpleKeyPair: simpleKeyPairAccounts .filter((item, pos) => simpleKeyPairAccounts.indexOf(item) === pos) - .map((address) => toChecksumAddress(address)), + .map((address) => toChecksumHexAddress(address)), ledger: [], trezor: [], }; @@ -1164,7 +1165,7 @@ export default class MetamaskController extends EventEmitter { let { transactions } = this.txController.store.getState(); // delete tx for other accounts that we're not importing transactions = Object.values(transactions).filter((tx) => { - const checksummedTxFrom = toChecksumAddress(tx.txParams.from); + const checksummedTxFrom = toChecksumHexAddress(tx.txParams.from); return accounts.hd.includes(checksummedTxFrom); }); diff --git a/app/scripts/migrations/039.js b/app/scripts/migrations/039.js index 7dcf904de..aa288553a 100644 --- a/app/scripts/migrations/039.js +++ b/app/scripts/migrations/039.js @@ -1,5 +1,5 @@ import { cloneDeep } from 'lodash'; -import { toChecksumAddress } from 'ethereumjs-util'; +import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils'; const version = 39; @@ -12,7 +12,7 @@ function isOldDai(token = {}) { token && typeof token === 'object' && token.symbol === DAI_V1_TOKEN_SYMBOL && - toChecksumAddress(token.address) === DAI_V1_CONTRACT_ADDRESS + toChecksumHexAddress(token.address) === DAI_V1_CONTRACT_ADDRESS ); } diff --git a/app/scripts/migrations/059.js b/app/scripts/migrations/059.js new file mode 100644 index 000000000..bbec1b9b8 --- /dev/null +++ b/app/scripts/migrations/059.js @@ -0,0 +1,52 @@ +import { + cloneDeep, + concat, + groupBy, + keyBy, + pickBy, + isPlainObject, +} from 'lodash'; +import { TRANSACTION_TYPES } from '../../../shared/constants/transaction'; + +const version = 59; + +/** + * Removes orphaned cancel and retry transactions that no longer have the + * original transaction in state, which results in bugs. + */ +export default { + version, + async migrate(originalVersionedData) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + const state = versionedData.data; + versionedData.data = transformState(state); + return versionedData; + }, +}; + +function transformState(state) { + const transactions = state?.TransactionController?.transactions; + if (isPlainObject(transactions)) { + const nonceNetworkGroupedObject = groupBy( + Object.values(transactions), + (tx) => { + return `${tx.txParams?.nonce}-${tx.chainId ?? tx.metamaskNetworkId}`; + }, + ); + + const withoutOrphans = pickBy(nonceNetworkGroupedObject, (group) => { + return group.some( + (tx) => + tx.type !== TRANSACTION_TYPES.CANCEL && + tx.type !== TRANSACTION_TYPES.RETRY, + ); + }); + state.TransactionController.transactions = keyBy( + concat(...Object.values(withoutOrphans)), + (tx) => tx.id, + ); + } + + return state; +} diff --git a/app/scripts/migrations/059.test.js b/app/scripts/migrations/059.test.js new file mode 100644 index 000000000..bdf4263c2 --- /dev/null +++ b/app/scripts/migrations/059.test.js @@ -0,0 +1,385 @@ +import { strict as assert } from 'assert'; +import { cloneDeep } from 'lodash'; +import { + KOVAN_CHAIN_ID, + MAINNET_CHAIN_ID, + RINKEBY_CHAIN_ID, + GOERLI_CHAIN_ID, +} from '../../../shared/constants/network'; +import { + TRANSACTION_TYPES, + TRANSACTION_STATUSES, +} from '../../../shared/constants/transaction'; +import migration59 from './059'; + +const ERRONEOUS_TRANSACTION_STATE = { + 0: { + type: TRANSACTION_TYPES.CANCEL, + id: 0, + chainId: MAINNET_CHAIN_ID, + txParams: { + nonce: '0x0', + }, + }, + 1: { + type: TRANSACTION_TYPES.SENT_ETHER, + id: 1, + chainId: MAINNET_CHAIN_ID, + txParams: { + nonce: '0x1', + }, + }, + 2: { + type: TRANSACTION_TYPES.SENT_ETHER, + id: 2, + chainId: KOVAN_CHAIN_ID, + txParams: { + nonce: '0x2', + }, + }, + 3: { + type: TRANSACTION_TYPES.SENT_ETHER, + id: 3, + chainId: RINKEBY_CHAIN_ID, + txParams: { + nonce: '0x3', + }, + }, + 4: { + type: TRANSACTION_TYPES.SENT_ETHER, + id: 4, + chainId: RINKEBY_CHAIN_ID, + txParams: { + nonce: '0x4', + }, + }, + 5: { + type: TRANSACTION_TYPES.SENT_ETHER, + id: 5, + chainId: MAINNET_CHAIN_ID, + txParams: { + nonce: '0x5', + }, + }, + 6: { + type: TRANSACTION_TYPES.SENT_ETHER, + id: 6, + chainId: KOVAN_CHAIN_ID, + txParams: { + nonce: '0x6', + }, + }, + 7: { + type: TRANSACTION_TYPES.SENT_ETHER, + id: 7, + chainId: RINKEBY_CHAIN_ID, + txParams: { + nonce: '0x7', + }, + }, + 8: { + type: TRANSACTION_TYPES.SENT_ETHER, + id: 8, + chainId: RINKEBY_CHAIN_ID, + txParams: { + nonce: '0x8', + }, + }, + 9: { + type: TRANSACTION_TYPES.SENT_ETHER, + id: 9, + chainId: RINKEBY_CHAIN_ID, + status: TRANSACTION_STATUSES.UNAPPROVED, + }, +}; + +const ERRONEOUS_TRANSACTION_STATE_RETRY = { + ...ERRONEOUS_TRANSACTION_STATE, + 0: { + ...ERRONEOUS_TRANSACTION_STATE[0], + type: TRANSACTION_TYPES.RETRY, + }, +}; + +const ERRONEOUS_TRANSACTION_STATE_MIXED = { + ...ERRONEOUS_TRANSACTION_STATE, + 10: { + type: TRANSACTION_TYPES.RETRY, + id: 10, + chainId: MAINNET_CHAIN_ID, + txParams: { + nonce: '0xa', + }, + }, + 11: { + type: TRANSACTION_TYPES.RETRY, + id: 11, + chainId: MAINNET_CHAIN_ID, + txParams: { + nonce: '0xb', + }, + }, +}; + +describe('migration #59', function () { + it('should update the version metadata', async function () { + const oldStorage = { + meta: { + version: 58, + }, + data: {}, + }; + + const newStorage = await migration59.migrate(oldStorage); + assert.deepEqual(newStorage.meta, { + version: 59, + }); + }); + + it('should drop orphaned cancel transactions', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: ERRONEOUS_TRANSACTION_STATE, + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + const EXPECTED = cloneDeep(ERRONEOUS_TRANSACTION_STATE); + delete EXPECTED['0']; + assert.deepEqual(newStorage.data, { + TransactionController: { + transactions: EXPECTED, + }, + foo: 'bar', + }); + }); + + it('should drop orphaned cancel transactions even if a nonce exists on another network that is confirmed', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: { + ...ERRONEOUS_TRANSACTION_STATE, + 11: { + ...ERRONEOUS_TRANSACTION_STATE['0'], + id: 11, + chainId: GOERLI_CHAIN_ID, + type: TRANSACTION_TYPES.SENT_ETHER, + }, + }, + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + const EXPECTED = cloneDeep( + oldStorage.data.TransactionController.transactions, + ); + delete EXPECTED['0']; + assert.deepEqual(newStorage.data, { + TransactionController: { + transactions: EXPECTED, + }, + foo: 'bar', + }); + }); + + it('should not drop cancel transactions with matching non cancel or retry in same network and nonce', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: { + ...ERRONEOUS_TRANSACTION_STATE, + 11: { + ...ERRONEOUS_TRANSACTION_STATE['0'], + id: 11, + type: TRANSACTION_TYPES.SENT_ETHER, + }, + }, + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + assert.deepEqual(newStorage.data, { + TransactionController: { + transactions: oldStorage.data.TransactionController.transactions, + }, + foo: 'bar', + }); + }); + + it('should drop orphaned retry transactions', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: ERRONEOUS_TRANSACTION_STATE_RETRY, + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + const EXPECTED = cloneDeep(ERRONEOUS_TRANSACTION_STATE_RETRY); + delete EXPECTED['0']; + assert.deepEqual(newStorage.data, { + TransactionController: { + transactions: EXPECTED, + }, + foo: 'bar', + }); + }); + + it('should drop orphaned retry transactions even if a nonce exists on another network that is confirmed', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: { + ...ERRONEOUS_TRANSACTION_STATE_RETRY, + 11: { + ...ERRONEOUS_TRANSACTION_STATE_RETRY['0'], + id: 11, + chainId: GOERLI_CHAIN_ID, + type: TRANSACTION_TYPES.SENT_ETHER, + }, + }, + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + const EXPECTED = cloneDeep( + oldStorage.data.TransactionController.transactions, + ); + delete EXPECTED['0']; + assert.deepEqual(newStorage.data, { + TransactionController: { + transactions: EXPECTED, + }, + foo: 'bar', + }); + }); + + it('should not drop retry transactions with matching non cancel or retry in same network and nonce', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: { + ...ERRONEOUS_TRANSACTION_STATE_RETRY, + 11: { + ...ERRONEOUS_TRANSACTION_STATE_RETRY['0'], + id: 11, + type: TRANSACTION_TYPES.SENT_ETHER, + }, + }, + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + assert.deepEqual(newStorage.data, { + TransactionController: { + transactions: oldStorage.data.TransactionController.transactions, + }, + foo: 'bar', + }); + }); + + it('should drop all orphaned retry and cancel transactions', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: ERRONEOUS_TRANSACTION_STATE_MIXED, + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + // The following ERRONEOUS_TRANSACTION_STATE object only has one orphan in it + // so using it as the base for our expected output automatically removes a few + // transactions we expect to be missing. + const EXPECTED = cloneDeep(ERRONEOUS_TRANSACTION_STATE); + delete EXPECTED['0']; + assert.deepEqual(newStorage.data, { + TransactionController: { + transactions: EXPECTED, + }, + foo: 'bar', + }); + }); + + it('should do nothing if transactions state does not exist', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + bar: 'baz', + }, + IncomingTransactionsController: { + foo: 'baz', + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + assert.deepEqual(oldStorage.data, newStorage.data); + }); + + it('should do nothing if transactions state is empty', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: {}, + bar: 'baz', + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + assert.deepEqual(oldStorage.data, newStorage.data); + }); + + it('should do nothing if transactions state is not an object', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: [], + bar: 'baz', + }, + foo: 'bar', + }, + }; + + const newStorage = await migration59.migrate(oldStorage); + assert.deepEqual(oldStorage.data, newStorage.data); + }); + + it('should do nothing if state is empty', async function () { + const oldStorage = { + meta: {}, + data: {}, + }; + + const newStorage = await migration59.migrate(oldStorage); + assert.deepEqual(oldStorage.data, newStorage.data); + }); +}); diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index b0c1716f5..47925fbba 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -63,6 +63,7 @@ const migrations = [ require('./056').default, require('./057').default, require('./058').default, + require('./059').default, ]; export default migrations; diff --git a/shared/constants/transaction.js b/shared/constants/transaction.js index 44a777a94..5eedd6086 100644 --- a/shared/constants/transaction.js +++ b/shared/constants/transaction.js @@ -1,3 +1,5 @@ +import { MESSAGE_TYPE } from './app'; + /** * Transaction Type is a MetaMask construct used internally * @typedef {Object} TransactionTypes @@ -51,6 +53,11 @@ export const TRANSACTION_TYPES = { DEPLOY_CONTRACT: 'contractDeployment', SWAP: 'swap', SWAP_APPROVAL: 'swapApproval', + SIGN: MESSAGE_TYPE.ETH_SIGN, + SIGN_TYPED_DATA: MESSAGE_TYPE.ETH_SIGN_TYPED_DATA, + PERSONAL_SIGN: MESSAGE_TYPE.PERSONAL_SIGN, + ETH_DECRYPT: MESSAGE_TYPE.ETH_DECRYPT, + ETH_GET_ENCRYPTION_PUBLIC_KEY: MESSAGE_TYPE.ETH_GET_ENCRYPTION_PUBLIC_KEY, }; /** diff --git a/shared/modules/hexstring-utils.js b/shared/modules/hexstring-utils.js new file mode 100644 index 000000000..1f895a3c7 --- /dev/null +++ b/shared/modules/hexstring-utils.js @@ -0,0 +1,73 @@ +import { + isHexString, + isValidAddress, + isValidChecksumAddress, + addHexPrefix, + toChecksumAddress, +} from 'ethereumjs-util'; + +export const BURN_ADDRESS = '0x0000000000000000000000000000000000000000'; + +export function isBurnAddress(address) { + return address === BURN_ADDRESS; +} + +/** + * Validates that the input is a hex address. This utility method is a thin + * wrapper around ethereumjs-util.isValidAddress, with the exception that it + * does not throw an error when provided values that are not hex strings. In + * addition, and by default, this method will return true for hex strings that + * meet the length requirement of a hex address, but are not prefixed with `0x` + * Finally, if the mixedCaseUseChecksum flag is true and a mixed case string is + * provided this method will validate it has the proper checksum formatting. + * @param {string} possibleAddress - Input parameter to check against + * @param {Object} [options] - options bag + * @param {boolean} [options.allowNonPrefixed] - If true will first ensure '0x' + * is prepended to the string + * @param {boolean} [options.mixedCaseUseChecksum] - If true will treat mixed + * case addresses as checksum addresses and validate that proper checksum + * format is used + * @returns {boolean} whether or not the input is a valid hex address + */ +export function isValidHexAddress( + possibleAddress, + { allowNonPrefixed = true, mixedCaseUseChecksum = false } = {}, +) { + const addressToCheck = allowNonPrefixed + ? addHexPrefix(possibleAddress) + : possibleAddress; + if (!isHexString(addressToCheck)) { + return false; + } + + if (mixedCaseUseChecksum) { + const prefixRemoved = addressToCheck.slice(2); + const lower = prefixRemoved.toLowerCase(); + const upper = prefixRemoved.toUpperCase(); + const allOneCase = prefixRemoved === lower || prefixRemoved === upper; + if (!allOneCase) { + return isValidChecksumAddress(addressToCheck); + } + } + + return isValidAddress(addressToCheck); +} + +export function toChecksumHexAddress(address) { + if (!address) { + // our internal checksumAddress function that this method replaces would + // return an empty string for nullish input. If any direct usages of + // ethereumjs-util.toChecksumAddress were called with nullish input it + // would have resulted in an error on version 5.1. + return ''; + } + const hexPrefixed = addHexPrefix(address); + if (!isHexString(hexPrefixed)) { + // Version 5.1 of ethereumjs-utils would have returned '0xY' for input 'y' + // but we shouldn't waste effort trying to change case on a clearly invalid + // string. Instead just return the hex prefixed original string which most + // closely mimics the original behavior. + return hexPrefixed; + } + return toChecksumAddress(addHexPrefix(address)); +} diff --git a/shared/modules/hexstring-utils.test.js b/shared/modules/hexstring-utils.test.js new file mode 100644 index 000000000..3d292451e --- /dev/null +++ b/shared/modules/hexstring-utils.test.js @@ -0,0 +1,57 @@ +import { strict as assert } from 'assert'; +import { toChecksumAddress } from 'ethereumjs-util'; +import { isValidHexAddress } from './hexstring-utils'; + +describe('hexstring utils', function () { + describe('isValidHexAddress', function () { + it('should allow 40-char non-prefixed hex', function () { + const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b825'; + const result = isValidHexAddress(address); + assert.equal(result, true); + }); + + it('should allow 42-char prefixed hex', function () { + const address = '0xfdea65c8e26263f6d9a1b5de9555d2931a33b825'; + const result = isValidHexAddress(address); + assert.equal(result, true); + }); + + it('should NOT allow 40-char non-prefixed hex when allowNonPrefixed is false', function () { + const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b825'; + const result = isValidHexAddress(address, { allowNonPrefixed: false }); + assert.equal(result, false); + }); + + it('should NOT allow any length of non hex-prefixed string', function () { + const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b85'; + const result = isValidHexAddress(address); + assert.equal(result, false); + }); + + it('should NOT allow less than 42 character hex-prefixed string', function () { + const address = '0xfdea65ce26263f6d9a1b5de9555d2931a33b85'; + const result = isValidHexAddress(address); + assert.equal(result, false); + }); + + it('should recognize correct capitalized checksum', function () { + const address = '0xFDEa65C8e26263F6d9A1B5de9555D2931A33b825'; + const result = isValidHexAddress(address, { mixedCaseUseChecksum: true }); + assert.equal(result, true); + }); + + it('should recognize incorrect capitalized checksum', function () { + const address = '0xFDea65C8e26263F6d9A1B5de9555D2931A33b825'; + const result = isValidHexAddress(address, { mixedCaseUseChecksum: true }); + assert.equal(result, false); + }); + + it('should recognize this sample hashed address', function () { + const address = '0x5Fda30Bb72B8Dfe20e48A00dFc108d0915BE9Bb0'; + const result = isValidHexAddress(address, { mixedCaseUseChecksum: true }); + const hashed = toChecksumAddress(address.toLowerCase()); + assert.equal(hashed, address); + assert.equal(result, true); + }); + }); +}); diff --git a/ui/app/components/app/account-list-item/account-list-item-component.test.js b/ui/app/components/app/account-list-item/account-list-item-component.test.js index 11d5ba6eb..03c038cf4 100644 --- a/ui/app/components/app/account-list-item/account-list-item-component.test.js +++ b/ui/app/components/app/account-list-item/account-list-item-component.test.js @@ -1,18 +1,19 @@ import React from 'react'; import { shallow } from 'enzyme'; import sinon from 'sinon'; -import * as utils from '../../../helpers/utils/util'; import Identicon from '../../ui/identicon'; +import { toChecksumHexAddress } from '../../../../../shared/modules/hexstring-utils'; import AccountListItem from './account-list-item'; +jest.mock('../../../../../shared/modules/hexstring-utils', () => ({ + toChecksumHexAddress: jest.fn(() => 'mockCheckSumAddress'), +})); + describe('AccountListItem Component', () => { - let wrapper, propsMethodSpies, checksumAddressStub; + let wrapper, propsMethodSpies; describe('render', () => { beforeAll(() => { - checksumAddressStub = sinon - .stub(utils, 'checksumAddress') - .returns('mockCheckSumAddress'); propsMethodSpies = { handleClick: sinon.spy(), }; @@ -36,7 +37,6 @@ describe('AccountListItem Component', () => { afterEach(() => { propsMethodSpies.handleClick.resetHistory(); - checksumAddressStub.resetHistory(); }); afterAll(() => { @@ -126,9 +126,7 @@ describe('AccountListItem Component', () => { expect( wrapper.find('.account-list-item__account-address').text(), ).toStrictEqual('mockCheckSumAddress'); - expect(checksumAddressStub.getCall(0).args).toStrictEqual([ - 'mockAddress', - ]); + expect(toChecksumHexAddress).toHaveBeenCalledWith('mockAddress'); }); it('should not render the account address as a checksumAddress if displayAddress is false', () => { diff --git a/ui/app/components/app/account-list-item/account-list-item.js b/ui/app/components/app/account-list-item/account-list-item.js index 9b0c0e38f..fd1b80bc4 100644 --- a/ui/app/components/app/account-list-item/account-list-item.js +++ b/ui/app/components/app/account-list-item/account-list-item.js @@ -1,8 +1,8 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { checksumAddress } from '../../../helpers/utils/util'; import Identicon from '../../ui/identicon'; import AccountMismatchWarning from '../../ui/account-mismatch-warning/account-mismatch-warning.component'; +import { toChecksumHexAddress } from '../../../../../shared/modules/hexstring-utils'; export default function AccountListItem({ account, @@ -34,7 +34,7 @@ export default function AccountListItem({ {displayAddress && name && (
- {checksumAddress(address)} + {toChecksumHexAddress(address)}
)} diff --git a/ui/app/components/app/app-components.scss b/ui/app/components/app/app-components.scss index 4f407569d..8110d8341 100644 --- a/ui/app/components/app/app-components.scss +++ b/ui/app/components/app/app-components.scss @@ -31,6 +31,7 @@ @import 'token-cell/token-cell'; @import 'transaction-activity-log/index'; @import 'transaction-breakdown/index'; +@import 'transaction-icon/transaction-icon'; @import 'transaction-list-item-details/index'; @import 'transaction-list-item/index'; @import 'transaction-list/index'; diff --git a/ui/app/components/app/modals/export-private-key-modal/export-private-key-modal.component.js b/ui/app/components/app/modals/export-private-key-modal/export-private-key-modal.component.js index 6ac5219b3..4db92c4f5 100644 --- a/ui/app/components/app/modals/export-private-key-modal/export-private-key-modal.component.js +++ b/ui/app/components/app/modals/export-private-key-modal/export-private-key-modal.component.js @@ -4,10 +4,10 @@ import React, { Component } from 'react'; import { stripHexPrefix } from 'ethereumjs-util'; import copyToClipboard from 'copy-to-clipboard'; -import { checksumAddress } from '../../../../helpers/utils/util'; import ReadOnlyInput from '../../../ui/readonly-input'; import Button from '../../../ui/button'; import AccountModalContainer from '../account-modal-container'; +import { toChecksumHexAddress } from '../../../../../../shared/modules/hexstring-utils'; export default class ExportPrivateKeyModal extends Component { static contextTypes = { @@ -149,7 +149,7 @@ export default class ExportPrivateKeyModal extends Component { {name}
diff --git a/ui/app/components/app/selected-account/selected-account.component.js b/ui/app/components/app/selected-account/selected-account.component.js index f703678a1..40d8c2a4e 100644 --- a/ui/app/components/app/selected-account/selected-account.component.js +++ b/ui/app/components/app/selected-account/selected-account.component.js @@ -1,9 +1,10 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import copyToClipboard from 'copy-to-clipboard'; -import { shortenAddress, checksumAddress } from '../../../helpers/utils/util'; +import { shortenAddress } from '../../../helpers/utils/util'; import Tooltip from '../../ui/tooltip'; +import { toChecksumHexAddress } from '../../../../../shared/modules/hexstring-utils'; class SelectedAccount extends Component { state = { @@ -32,7 +33,7 @@ class SelectedAccount extends Component { render() { const { t } = this.context; const { selectedIdentity } = this.props; - const checksummedAddress = checksumAddress(selectedIdentity.address); + const checksummedAddress = toChecksumHexAddress(selectedIdentity.address); return (
diff --git a/ui/app/components/app/transaction-icon/transaction-icon.js b/ui/app/components/app/transaction-icon/transaction-icon.js index 5fd8a3cd7..1876e0535 100644 --- a/ui/app/components/app/transaction-icon/transaction-icon.js +++ b/ui/app/components/app/transaction-icon/transaction-icon.js @@ -1,5 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; +import { captureException } from '@sentry/browser'; import Approve from '../../ui/icon/approve-icon.component'; import Interaction from '../../ui/icon/interaction-icon.component'; import Receive from '../../ui/icon/receive-icon.component'; @@ -40,10 +41,35 @@ export default function TransactionIcon({ status, category }) { const Icon = ICON_MAP[category]; + if (!Icon) { + captureException( + Error( + `The category prop passed to TransactionIcon is not supported. The prop is: ${category}`, + ), + ); + + return
; + } + return ; } TransactionIcon.propTypes = { - status: PropTypes.string.isRequired, - category: PropTypes.string.isRequired, + status: PropTypes.oneOf([ + TRANSACTION_GROUP_CATEGORIES.APPROVAL, + TRANSACTION_GROUP_CATEGORIES.INTERACTION, + TRANSACTION_GROUP_CATEGORIES.SEND, + TRANSACTION_GROUP_CATEGORIES.SIGNATURE_REQUEST, + TRANSACTION_GROUP_CATEGORIES.RECEIVE, + TRANSACTION_GROUP_CATEGORIES.SWAP, + ]).isRequired, + category: PropTypes.oneOf([ + TRANSACTION_GROUP_STATUSES.PENDING, + TRANSACTION_STATUSES.UNAPPROVED, + TRANSACTION_STATUSES.APPROVED, + TRANSACTION_STATUSES.FAILED, + TRANSACTION_STATUSES.REJECTED, + TRANSACTION_GROUP_STATUSES.CANCELLED, + TRANSACTION_STATUSES.DROPPED, + ]).isRequired, }; diff --git a/ui/app/components/app/transaction-icon/transaction-icon.scss b/ui/app/components/app/transaction-icon/transaction-icon.scss new file mode 100644 index 000000000..743b66022 --- /dev/null +++ b/ui/app/components/app/transaction-icon/transaction-icon.scss @@ -0,0 +1,8 @@ +.transaction-icon { + &__grey-circle { + height: 28px; + width: 28px; + border-radius: 14px; + background: $Grey-100; + } +} diff --git a/ui/app/components/app/transaction-list-item-details/transaction-list-item-details.container.js b/ui/app/components/app/transaction-list-item-details/transaction-list-item-details.container.js index 4a96082c7..f1cbf6237 100644 --- a/ui/app/components/app/transaction-list-item-details/transaction-list-item-details.container.js +++ b/ui/app/components/app/transaction-list-item-details/transaction-list-item-details.container.js @@ -1,10 +1,10 @@ import { connect } from 'react-redux'; -import { checksumAddress } from '../../../helpers/utils/util'; import { tryReverseResolveAddress } from '../../../store/actions'; import { getAddressBook, getRpcPrefsForCurrentProvider, } from '../../../selectors'; +import { toChecksumHexAddress } from '../../../../../shared/modules/hexstring-utils'; import TransactionListItemDetails from './transaction-list-item-details.component'; const mapStateToProps = (state, ownProps) => { @@ -13,7 +13,7 @@ const mapStateToProps = (state, ownProps) => { const { recipientAddress, senderAddress } = ownProps; let recipientEns; if (recipientAddress) { - const address = checksumAddress(recipientAddress); + const address = toChecksumHexAddress(recipientAddress); recipientEns = ensResolutionsByAddress[address] || ''; } const addressBook = getAddressBook(state); diff --git a/ui/app/components/ui/identicon/identicon.component.js b/ui/app/components/ui/identicon/identicon.component.js index d52717d05..64a03ea01 100644 --- a/ui/app/components/ui/identicon/identicon.component.js +++ b/ui/app/components/ui/identicon/identicon.component.js @@ -2,8 +2,8 @@ import React, { PureComponent } from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import contractMap from '@metamask/contract-metadata'; +import { toChecksumHexAddress } from '../../../../../shared/modules/hexstring-utils'; -import { checksumAddress, isHex } from '../../../helpers/utils/util'; import Jazzicon from '../jazzicon'; import BlockieIdenticon from './blockieIdenticon'; @@ -85,12 +85,10 @@ export default class Identicon extends PureComponent { } if (address) { - if (isHex(address)) { - const checksummedAddress = checksumAddress(address); + const checksummedAddress = toChecksumHexAddress(address); - if (contractMap[checksummedAddress]?.logo) { - return this.renderJazzicon(); - } + if (checksummedAddress && contractMap[checksummedAddress]?.logo) { + return this.renderJazzicon(); } return ( diff --git a/ui/app/components/ui/identicon/identicon.component.test.js b/ui/app/components/ui/identicon/identicon.component.test.js index f78a47b6d..fb9702fe8 100644 --- a/ui/app/components/ui/identicon/identicon.component.test.js +++ b/ui/app/components/ui/identicon/identicon.component.test.js @@ -38,7 +38,11 @@ describe('Identicon', () => { it('renders div with address prop', () => { const wrapper = mount( - , + , ); expect(wrapper.find('div.test-address').prop('className')).toStrictEqual( diff --git a/ui/app/components/ui/qr-code/qr-code.js b/ui/app/components/ui/qr-code/qr-code.js index 6c00f33fc..ae43dc49b 100644 --- a/ui/app/components/ui/qr-code/qr-code.js +++ b/ui/app/components/ui/qr-code/qr-code.js @@ -4,7 +4,7 @@ import qrCode from 'qrcode-generator'; import { connect } from 'react-redux'; import { isHexPrefixed } from 'ethereumjs-util'; import ReadOnlyInput from '../readonly-input/readonly-input'; -import { checksumAddress } from '../../../helpers/utils/util'; +import { toChecksumHexAddress } from '../../../../../shared/modules/hexstring-utils'; export default connect(mapStateToProps)(QrCodeView); @@ -20,9 +20,9 @@ function mapStateToProps(state) { function QrCodeView(props) { const { Qr, warning } = props; const { message, data } = Qr; - const address = `${isHexPrefixed(data) ? 'ethereum:' : ''}${checksumAddress( - data, - )}`; + const address = `${ + isHexPrefixed(data) ? 'ethereum:' : '' + }${toChecksumHexAddress(data)}`; const qrImage = qrCode(4, 'M'); qrImage.addData(address); qrImage.make(); @@ -50,7 +50,7 @@ function QrCodeView(props) {
); diff --git a/ui/app/components/ui/sender-to-recipient/sender-to-recipient.component.js b/ui/app/components/ui/sender-to-recipient/sender-to-recipient.component.js index 00e099e24..69db31e1d 100644 --- a/ui/app/components/ui/sender-to-recipient/sender-to-recipient.component.js +++ b/ui/app/components/ui/sender-to-recipient/sender-to-recipient.component.js @@ -4,9 +4,10 @@ import classnames from 'classnames'; import copyToClipboard from 'copy-to-clipboard'; import Tooltip from '../tooltip'; import Identicon from '../identicon'; -import { checksumAddress, shortenAddress } from '../../../helpers/utils/util'; +import { shortenAddress } from '../../../helpers/utils/util'; import AccountMismatchWarning from '../account-mismatch-warning/account-mismatch-warning.component'; import { useI18nContext } from '../../../hooks/useI18nContext'; +import { toChecksumHexAddress } from '../../../../../shared/modules/hexstring-utils'; import { DEFAULT_VARIANT, CARDS_VARIANT, @@ -56,7 +57,10 @@ function SenderAddress({ > {!addressOnly && (
- +
)} diff --git a/ui/app/helpers/utils/util.js b/ui/app/helpers/utils/util.js index 64c11a135..512acaef8 100644 --- a/ui/app/helpers/utils/util.js +++ b/ui/app/helpers/utils/util.js @@ -12,6 +12,7 @@ import { RINKEBY_CHAIN_ID, ROPSTEN_CHAIN_ID, } from '../../../../shared/constants/network'; +import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; // formatData :: ( date: ) -> String export function formatDate(date, format = "M/d/y 'at' T") { @@ -67,7 +68,7 @@ export function addressSummary( if (!address) { return ''; } - let checked = checksumAddress(address); + let checked = toChecksumHexAddress(address); if (!includeHex) { checked = ethUtil.stripHexPrefix(checked); } @@ -78,20 +79,6 @@ export function addressSummary( : '...'; } -export function isValidAddress(address) { - if (!address || address === '0x0000000000000000000000000000000000000000') { - return false; - } - const prefixed = addHexPrefix(address); - if (!isHex(prefixed)) { - return false; - } - return ( - (isAllOneCase(prefixed.slice(2)) && ethUtil.isValidAddress(prefixed)) || - ethUtil.isValidChecksumAddress(prefixed) - ); -} - export function isValidDomainName(address) { const match = punycode .toASCII(address) @@ -112,15 +99,6 @@ export function isOriginContractAddress(to, sendTokenAddress) { return to.toLowerCase() === sendTokenAddress.toLowerCase(); } -export function isAllOneCase(address) { - if (!address) { - return true; - } - const lower = address.toLowerCase(); - const upper = address.toUpperCase(); - return address === lower || address === upper; -} - // Takes wei Hex, returns wei BN, even if input is null export function numericBalance(balance) { if (!balance) { @@ -182,10 +160,6 @@ export function formatBalance( return formatted; } -export function isHex(str) { - return Boolean(str.match(/^(0x)?[0-9a-fA-F]+$/u)); -} - export function getContractAtAddress(tokenAddress) { return global.eth.contract(abi).at(tokenAddress); } @@ -222,18 +196,6 @@ export function exportAsFile(filename, data, type = 'text/csv') { } } -/** - * Safely checksumms a potentially-null address - * - * @param {string} [address] - address to checksum - * @returns {string} checksummed address - * - */ -export function checksumAddress(address) { - const checksummed = address ? ethUtil.toChecksumAddress(address) : ''; - return checksummed; -} - /** * Shortens an Ethereum address for display, preserving the beginning and end. * Returns the given address if it is no longer than 10 characters. @@ -253,13 +215,6 @@ export function shortenAddress(address = '') { return `${address.slice(0, 6)}...${address.slice(-4)}`; } -export function isValidAddressHead(address) { - const addressLengthIsLessThanFull = address.length < 42; - const addressIsHex = isHex(address); - - return addressLengthIsLessThanFull && addressIsHex; -} - export function getAccountByAddress(accounts = [], targetAddress) { return accounts.find(({ address }) => address === targetAddress); } diff --git a/ui/app/helpers/utils/util.test.js b/ui/app/helpers/utils/util.test.js index 0dfa431f6..7038dd79e 100644 --- a/ui/app/helpers/utils/util.test.js +++ b/ui/app/helpers/utils/util.test.js @@ -1,4 +1,4 @@ -import { BN, toChecksumAddress } from 'ethereumjs-util'; +import { BN } from 'ethereumjs-util'; import * as util from './util'; describe('util', () => { @@ -47,52 +47,6 @@ describe('util', () => { }); }); - describe('#isValidAddress', () => { - it('should allow 40-char non-prefixed hex', () => { - const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b825'; - const result = util.isValidAddress(address); - expect(result).toStrictEqual(true); - }); - - it('should allow 42-char non-prefixed hex', () => { - const address = '0xfdea65c8e26263f6d9a1b5de9555d2931a33b825'; - const result = util.isValidAddress(address); - expect(result).toStrictEqual(true); - }); - - it('should not allow less non hex-prefixed', () => { - const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b85'; - const result = util.isValidAddress(address); - expect(result).toStrictEqual(false); - }); - - it('should not allow less hex-prefixed', () => { - const address = '0xfdea65ce26263f6d9a1b5de9555d2931a33b85'; - const result = util.isValidAddress(address); - expect(result).toStrictEqual(false); - }); - - it('should recognize correct capitalized checksum', () => { - const address = '0xFDEa65C8e26263F6d9A1B5de9555D2931A33b825'; - const result = util.isValidAddress(address); - expect(result).toStrictEqual(true); - }); - - it('should recognize incorrect capitalized checksum', () => { - const address = '0xFDea65C8e26263F6d9A1B5de9555D2931A33b825'; - const result = util.isValidAddress(address); - expect(result).toStrictEqual(false); - }); - - it('should recognize this sample hashed address', () => { - const address = '0x5Fda30Bb72B8Dfe20e48A00dFc108d0915BE9Bb0'; - const result = util.isValidAddress(address); - const hashed = toChecksumAddress(address.toLowerCase()); - expect(hashed).toStrictEqual(address); - expect(result).toStrictEqual(true); - }); - }); - describe('isValidDomainName', () => { it('should return true when given a valid domain name', () => { expect(util.isValidDomainName('foo.bar')).toStrictEqual(true); @@ -239,36 +193,6 @@ describe('util', () => { }); describe('normalizing values', function () { - describe('#isHex', function () { - it('should return true when given a hex string', function () { - const result = util.isHex( - 'c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2', - ); - expect(result).toStrictEqual(true); - }); - - it('should return false when given a non-hex string', () => { - const result = util.isHex( - 'c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714imnotreal', - ); - expect(result).toStrictEqual(false); - }); - - it('should return false when given a string containing a non letter/number character', () => { - const result = util.isHex( - 'c3ab8ff13720!8ad9047dd39466b3c%8974e592c2fa383d4a396071imnotreal', - ); - expect(result).toStrictEqual(false); - }); - - it('should return true when given a hex string with hex-prefix', () => { - const result = util.isHex( - '0xc3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2', - ); - expect(result).toStrictEqual(true); - }); - }); - describe('#getRandomFileName', () => { it('should only return a string containing alphanumeric characters', () => { const result = util.getRandomFileName(); diff --git a/ui/app/hooks/useCancelTransaction.js b/ui/app/hooks/useCancelTransaction.js index da4312d2a..2e71c184b 100644 --- a/ui/app/hooks/useCancelTransaction.js +++ b/ui/app/hooks/useCancelTransaction.js @@ -26,9 +26,12 @@ import { multiplyCurrencies } from '../helpers/utils/conversion-util'; */ export function useCancelTransaction(transactionGroup) { const { primaryTransaction } = transactionGroup; - const gasPrice = primaryTransaction.txParams?.gasPrice?.startsWith('-') - ? '0x0' - : primaryTransaction.txParams?.gasPrice; + + const transactionGasPrice = primaryTransaction.txParams?.gasPrice; + const gasPrice = + transactionGasPrice === undefined || transactionGasPrice?.startsWith('-') + ? '0x0' + : primaryTransaction.txParams?.gasPrice; const transaction = primaryTransaction; const dispatch = useDispatch(); const selectedAccount = useSelector(getSelectedAccount); diff --git a/ui/app/hooks/useTokensToSearch.js b/ui/app/hooks/useTokensToSearch.js index 4542882f0..e4bc6495e 100644 --- a/ui/app/hooks/useTokensToSearch.js +++ b/ui/app/hooks/useTokensToSearch.js @@ -3,7 +3,6 @@ import { useSelector } from 'react-redux'; import contractMap from '@metamask/contract-metadata'; import BigNumber from 'bignumber.js'; import { isEqual, shuffle } from 'lodash'; -import { checksumAddress } from '../helpers/utils/util'; import { getTokenFiatAmount } from '../helpers/utils/token-util'; import { getTokenExchangeRates, @@ -14,6 +13,7 @@ import { } from '../selectors'; import { getSwapsTokens } from '../ducks/swaps/swaps'; import { isSwapsDefaultTokenSymbol } from '../../../shared/modules/swaps.utils'; +import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils'; import { useEqualityCheck } from './useEqualityCheck'; const tokenList = shuffle( @@ -58,12 +58,12 @@ export function getRenderableTokenData( ) || ''; const usedIconUrl = iconUrl || - (contractMap[checksumAddress(address)] && - `images/contract/${contractMap[checksumAddress(address)].logo}`); + (contractMap[toChecksumHexAddress(address)] && + `images/contract/${contractMap[toChecksumHexAddress(address)].logo}`); return { ...token, primaryLabel: symbol, - secondaryLabel: name || contractMap[checksumAddress(address)]?.name, + secondaryLabel: name || contractMap[toChecksumHexAddress(address)]?.name, rightPrimaryLabel: string && `${new BigNumber(string).round(6).toString()} ${symbol}`, rightSecondaryLabel: formattedFiat, @@ -71,7 +71,7 @@ export function getRenderableTokenData( identiconAddress: usedIconUrl ? null : address, balance, decimals, - name: name || contractMap[checksumAddress(address)]?.name, + name: name || contractMap[toChecksumHexAddress(address)]?.name, rawFiat, }; } diff --git a/ui/app/hooks/useTransactionDisplayData.js b/ui/app/hooks/useTransactionDisplayData.js index ffdda4660..ea886205e 100644 --- a/ui/app/hooks/useTransactionDisplayData.js +++ b/ui/app/hooks/useTransactionDisplayData.js @@ -145,7 +145,17 @@ export function useTransactionDisplayData(transactionGroup) { // 6. Swap // 7. Swap Approval - if (type === null || type === undefined) { + const signatureTypes = [ + null, + undefined, + TRANSACTION_TYPES.SIGN, + TRANSACTION_TYPES.PERSONAL_SIGN, + TRANSACTION_TYPES.SIGN_TYPED_DATA, + TRANSACTION_TYPES.ETH_DECRYPT, + TRANSACTION_TYPES.ETH_GET_ENCRYPTION_PUBLIC_KEY, + ]; + + if (signatureTypes.includes(type)) { category = TRANSACTION_GROUP_CATEGORIES.SIGNATURE_REQUEST; title = t('signatureRequest'); subtitle = origin; @@ -210,6 +220,10 @@ export function useTransactionDisplayData(transactionGroup) { category = TRANSACTION_GROUP_CATEGORIES.SEND; title = t('send'); subtitle = t('toAddress', [shortenAddress(recipientAddress)]); + } else { + throw new Error( + `useTransactionDisplayData does not recognize transaction type. Type received is: ${type}`, + ); } const primaryCurrencyPreferences = useUserPreferencedCurrency(PRIMARY); diff --git a/ui/app/pages/add-token/add-token.component.js b/ui/app/pages/add-token/add-token.component.js index 664a368e6..cceeea624 100644 --- a/ui/app/pages/add-token/add-token.component.js +++ b/ui/app/pages/add-token/add-token.component.js @@ -1,15 +1,13 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; -import { - checkExistingAddresses, - isValidAddress, -} from '../../helpers/utils/util'; +import { checkExistingAddresses } from '../../helpers/utils/util'; import { tokenInfoGetter } from '../../helpers/utils/token-util'; import { CONFIRM_ADD_TOKEN_ROUTE } from '../../helpers/constants/routes'; import TextField from '../../components/ui/text-field'; import PageContainer from '../../components/ui/page-container'; import { Tabs, Tab } from '../../components/ui/tabs'; import { addHexPrefix } from '../../../../app/scripts/lib/util'; +import { isValidHexAddress } from '../../../../shared/modules/hexstring-utils'; import TokenList from './token-list'; import TokenSearch from './token-search'; @@ -167,7 +165,9 @@ class AddToken extends Component { autoFilled: false, }); - const addressIsValid = isValidAddress(customAddress); + const addressIsValid = isValidHexAddress(customAddress, { + allowNonPrefixed: false, + }); const standardAddress = addHexPrefix(customAddress).toLowerCase(); switch (true) { diff --git a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js index 7a761bd9b..48d4ff814 100644 --- a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -23,11 +23,7 @@ import { getHexGasTotal } from '../../helpers/utils/confirm-tx.util'; import { isBalanceSufficient, calcGasTotal } from '../send/send.utils'; import { conversionGreaterThan } from '../../helpers/utils/conversion-util'; import { MIN_GAS_LIMIT_DEC } from '../send/send.constants'; -import { - checksumAddress, - shortenAddress, - valuesFor, -} from '../../helpers/utils/util'; +import { shortenAddress, valuesFor } from '../../helpers/utils/util'; import { getAdvancedInlineGasShown, getCustomNonceValue, @@ -40,6 +36,7 @@ import { } from '../../selectors'; import { getMostRecentOverviewPage } from '../../ducks/history/history'; import { transactionMatchesNetwork } from '../../../../shared/modules/transaction.utils'; +import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; import ConfirmTransactionBase from './confirm-transaction-base.component'; const casedContractMap = Object.keys(contractMap).reduce((acc, base) => { @@ -104,9 +101,9 @@ const mapStateToProps = (state, ownProps) => { const toName = identities[toAddress]?.name || casedContractMap[toAddress]?.name || - shortenAddress(checksumAddress(toAddress)); + shortenAddress(toChecksumHexAddress(toAddress)); - const checksummedAddress = checksumAddress(toAddress); + const checksummedAddress = toChecksumHexAddress(toAddress); const addressBookObject = addressBook[checksummedAddress]; const toEns = ensResolutionsByAddress[checksummedAddress] || ''; const toNickname = addressBookObject ? addressBookObject.name : ''; diff --git a/ui/app/pages/send/send-content/add-recipient/add-recipient.component.js b/ui/app/pages/send/send-content/add-recipient/add-recipient.component.js index 82ccfa1d4..a7afd2141 100644 --- a/ui/app/pages/send/send-content/add-recipient/add-recipient.component.js +++ b/ui/app/pages/send/send-content/add-recipient/add-recipient.component.js @@ -2,13 +2,16 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import Fuse from 'fuse.js'; import Identicon from '../../../../components/ui/identicon'; -import { isValidAddress } from '../../../../helpers/utils/util'; import Dialog from '../../../../components/ui/dialog'; import ContactList from '../../../../components/app/contact-list'; import RecipientGroup from '../../../../components/app/contact-list/recipient-group/recipient-group.component'; import { ellipsify } from '../../send.utils'; import Button from '../../../../components/ui/button'; import Confusable from '../../../../components/ui/confusable'; +import { + isBurnAddress, + isValidHexAddress, +} from '../../../../../../shared/modules/hexstring-utils'; export default class AddRecipient extends Component { static propTypes = { @@ -101,7 +104,10 @@ export default class AddRecipient extends Component { let content; - if (isValidAddress(query)) { + if ( + !isBurnAddress(query) && + isValidHexAddress(query, { mixedCaseUseChecksum: true }) + ) { content = this.renderExplicitAddress(query); } else if (ensResolution) { content = this.renderExplicitAddress( diff --git a/ui/app/pages/send/send-content/add-recipient/add-recipient.js b/ui/app/pages/send/send-content/add-recipient/add-recipient.js index 3ba0077c7..3124ccdb5 100644 --- a/ui/app/pages/send/send-content/add-recipient/add-recipient.js +++ b/ui/app/pages/send/send-content/add-recipient/add-recipient.js @@ -1,4 +1,3 @@ -import { toChecksumAddress } from 'ethereumjs-util'; import contractMap from '@metamask/contract-metadata'; import { isConfusing } from 'unicode-confusables'; import { @@ -11,18 +10,26 @@ import { } from '../../send.constants'; import { - isValidAddress, checkExistingAddresses, isValidDomainName, isOriginContractAddress, isDefaultMetaMaskChain, } from '../../../../helpers/utils/util'; +import { + isBurnAddress, + isValidHexAddress, + toChecksumHexAddress, +} from '../../../../../../shared/modules/hexstring-utils'; export function getToErrorObject(to, sendTokenAddress, chainId) { let toError = null; if (!to) { toError = REQUIRED_ERROR; - } else if (!isValidAddress(to) && !isValidDomainName(to)) { + } else if ( + isBurnAddress(to) || + (!isValidHexAddress(to, { mixedCaseUseChecksum: true }) && + !isValidDomainName(to)) + ) { toError = isDefaultMetaMaskChain(chainId) ? INVALID_RECIPIENT_ADDRESS_ERROR : INVALID_RECIPIENT_ADDRESS_NOT_ETH_NETWORK_ERROR; @@ -37,7 +44,8 @@ export function getToWarningObject(to, tokens = [], sendToken = null) { let toWarning = null; if ( sendToken && - (toChecksumAddress(to) in contractMap || checkExistingAddresses(to, tokens)) + (toChecksumHexAddress(to) in contractMap || + checkExistingAddresses(to, tokens)) ) { toWarning = KNOWN_RECIPIENT_ADDRESS_ERROR; } else if (isValidDomainName(to) && isConfusing(to)) { diff --git a/ui/app/pages/send/send-content/add-recipient/add-recipient.utils.test.js b/ui/app/pages/send/send-content/add-recipient/add-recipient.utils.test.js index 6ed751664..33f70b876 100644 --- a/ui/app/pages/send/send-content/add-recipient/add-recipient.utils.test.js +++ b/ui/app/pages/send/send-content/add-recipient/add-recipient.utils.test.js @@ -11,7 +11,6 @@ jest.mock('../../../../../app/helpers/utils/util', () => ({ isDefaultMetaMaskChain: jest.fn().mockReturnValue(true), isEthNetwork: jest.fn().mockReturnValue(true), checkExistingAddresses: jest.fn().mockReturnValue(true), - isValidAddress: jest.fn((to) => Boolean(to.match(/^[0xabcdef123456798]+$/u))), isValidDomainName: jest.requireActual('../../../../../app/helpers/utils/util') .isValidDomainName, isOriginContractAddress: jest.requireActual( @@ -19,6 +18,14 @@ jest.mock('../../../../../app/helpers/utils/util', () => ({ ).isOriginContractAddress, })); +jest.mock('../../../../../../shared/modules/hexstring-utils', () => ({ + isValidHexAddress: jest.fn((to) => + Boolean(to.match(/^[0xabcdef123456798]+$/u)), + ), + isBurnAddress: jest.fn(() => false), + toChecksumHexAddress: jest.fn((input) => input), +})); + describe('add-recipient utils', () => { describe('getToErrorObject()', () => { it('should return a required error if "to" is falsy', () => { diff --git a/ui/app/pages/send/send-content/add-recipient/ens-input.component.js b/ui/app/pages/send/send-content/add-recipient/ens-input.component.js index 7d210080f..9012ee643 100644 --- a/ui/app/pages/send/send-content/add-recipient/ens-input.component.js +++ b/ui/app/pages/send/send-content/add-recipient/ens-input.component.js @@ -7,13 +7,14 @@ import copyToClipboard from 'copy-to-clipboard/index'; import ENS from 'ethjs-ens'; import networkMap from 'ethereum-ens-network-map'; import log from 'loglevel'; +import { isHexString } from 'ethereumjs-util'; import { ellipsify } from '../../send.utils'; -import { - isValidDomainName, - isValidAddress, - isValidAddressHead, -} from '../../../../helpers/utils/util'; +import { isValidDomainName } from '../../../../helpers/utils/util'; import { MAINNET_NETWORK_ID } from '../../../../../../shared/constants/network'; +import { + isBurnAddress, + isValidHexAddress, +} from '../../../../../../shared/modules/hexstring-utils'; // Local Constants const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; @@ -143,7 +144,10 @@ export default class EnsInput extends Component { onPaste = (event) => { event.clipboardData.items[0].getAsString((text) => { - if (isValidAddress(text)) { + if ( + !isBurnAddress(text) && + isValidHexAddress(text, { mixedCaseUseChecksum: true }) + ) { this.props.onPaste(text); } }); @@ -170,8 +174,11 @@ export default class EnsInput extends Component { if ( !networkHasEnsSupport && - !isValidAddress(input) && - !isValidAddressHead(input) + !( + isBurnAddress(input) === false && + isValidHexAddress(input, { mixedCaseUseChecksum: true }) + ) && + !isHexString(input) ) { updateEnsResolution(''); updateEnsResolutionError( @@ -182,7 +189,11 @@ export default class EnsInput extends Component { if (isValidDomainName(input)) { this.lookupEnsName(input); - } else if (onValidAddressTyped && isValidAddress(input)) { + } else if ( + onValidAddressTyped && + !isBurnAddress(input) && + isValidHexAddress(input, { mixedCaseUseChecksum: true }) + ) { onValidAddressTyped(input); } else { updateEnsResolution(''); diff --git a/ui/app/pages/send/send.component.js b/ui/app/pages/send/send.component.js index 56eafb1e7..24bd3dc7a 100644 --- a/ui/app/pages/send/send.component.js +++ b/ui/app/pages/send/send.component.js @@ -1,7 +1,7 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import { debounce } from 'lodash'; -import { isValidAddress } from '../../helpers/utils/util'; +import { isValidHexAddress } from '../../../../shared/modules/hexstring-utils'; import { getAmountErrorObject, getGasFeeErrorObject, @@ -171,7 +171,7 @@ export default class SendTransactionScreen extends Component { if (qrCodeData) { if (qrCodeData.type === 'address') { scannedAddress = qrCodeData.values.address.toLowerCase(); - if (isValidAddress(scannedAddress)) { + if (isValidHexAddress(scannedAddress, { allowNonPrefixed: false })) { const currentAddress = prevTo?.toLowerCase(); if (currentAddress !== scannedAddress) { updateSendTo(scannedAddress); diff --git a/ui/app/pages/settings/contact-list-tab/add-contact/add-contact.component.js b/ui/app/pages/settings/contact-list-tab/add-contact/add-contact.component.js index 896180517..8ddd29ac2 100644 --- a/ui/app/pages/settings/contact-list-tab/add-contact/add-contact.component.js +++ b/ui/app/pages/settings/contact-list-tab/add-contact/add-contact.component.js @@ -4,12 +4,13 @@ import { debounce } from 'lodash'; import Identicon from '../../../../components/ui/identicon'; import TextField from '../../../../components/ui/text-field'; import { CONTACT_LIST_ROUTE } from '../../../../helpers/constants/routes'; -import { - isValidAddress, - isValidDomainName, -} from '../../../../helpers/utils/util'; +import { isValidDomainName } from '../../../../helpers/utils/util'; import EnsInput from '../../../send/send-content/add-recipient/ens-input'; import PageContainerFooter from '../../../../components/ui/page-container/page-container-footer'; +import { + isBurnAddress, + isValidHexAddress, +} from '../../../../../../shared/modules/hexstring-utils'; export default class AddContact extends PureComponent { static contextTypes = { @@ -53,7 +54,9 @@ export default class AddContact extends PureComponent { } validate = (address) => { - const valid = isValidAddress(address); + const valid = + !isBurnAddress(address) && + isValidHexAddress(address, { mixedCaseUseChecksum: true }); const validEnsAddress = isValidDomainName(address); if (valid || validEnsAddress || address === '') { diff --git a/ui/app/pages/settings/contact-list-tab/edit-contact/edit-contact.component.js b/ui/app/pages/settings/contact-list-tab/edit-contact/edit-contact.component.js index 0ccc1f64a..00725f6ea 100644 --- a/ui/app/pages/settings/contact-list-tab/edit-contact/edit-contact.component.js +++ b/ui/app/pages/settings/contact-list-tab/edit-contact/edit-contact.component.js @@ -4,8 +4,11 @@ import { Redirect } from 'react-router-dom'; import Identicon from '../../../../components/ui/identicon'; import Button from '../../../../components/ui/button/button.component'; import TextField from '../../../../components/ui/text-field'; -import { isValidAddress } from '../../../../helpers/utils/util'; import PageContainerFooter from '../../../../components/ui/page-container/page-container-footer'; +import { + isBurnAddress, + isValidHexAddress, +} from '../../../../../../shared/modules/hexstring-utils'; export default class EditContact extends PureComponent { static contextTypes = { @@ -135,7 +138,12 @@ export default class EditContact extends PureComponent { this.state.newAddress !== address ) { // if the user makes a valid change to the address field, remove the original address - if (isValidAddress(this.state.newAddress)) { + if ( + !isBurnAddress(this.state.newAddress) && + isValidHexAddress(this.state.newAddress, { + mixedCaseUseChecksum: true, + }) + ) { await removeFromAddressBook(chainId, address); await addToAddressBook( this.state.newAddress, diff --git a/ui/app/pages/settings/contact-list-tab/view-contact/view-contact.container.js b/ui/app/pages/settings/contact-list-tab/view-contact/view-contact.container.js index 06a753446..2ce020ffa 100644 --- a/ui/app/pages/settings/contact-list-tab/view-contact/view-contact.container.js +++ b/ui/app/pages/settings/contact-list-tab/view-contact/view-contact.container.js @@ -2,11 +2,11 @@ import { compose } from 'redux'; import { connect } from 'react-redux'; import { withRouter } from 'react-router-dom'; import { getAddressBookEntry } from '../../../../selectors'; -import { checksumAddress } from '../../../../helpers/utils/util'; import { CONTACT_EDIT_ROUTE, CONTACT_LIST_ROUTE, } from '../../../../helpers/constants/routes'; +import { toChecksumHexAddress } from '../../../../../../shared/modules/hexstring-utils'; import ViewContact from './view-contact.component'; const mapStateToProps = (state, ownProps) => { @@ -25,7 +25,7 @@ const mapStateToProps = (state, ownProps) => { return { name, address: contact ? address : null, - checkSummedAddress: checksumAddress(address), + checkSummedAddress: toChecksumHexAddress(address), memo, editRoute: CONTACT_EDIT_ROUTE, listRoute: CONTACT_LIST_ROUTE, diff --git a/ui/app/pages/settings/settings.container.js b/ui/app/pages/settings/settings.container.js index 57fedadd2..1121c4755 100644 --- a/ui/app/pages/settings/settings.container.js +++ b/ui/app/pages/settings/settings.container.js @@ -2,10 +2,13 @@ import { compose } from 'redux'; import { connect } from 'react-redux'; import { withRouter } from 'react-router-dom'; import { getAddressBookEntryName } from '../../selectors'; -import { isValidAddress, isHex } from '../../helpers/utils/util'; import { ENVIRONMENT_TYPE_POPUP } from '../../../../shared/constants/app'; import { getEnvironmentType } from '../../../../app/scripts/lib/util'; import { getMostRecentOverviewPage } from '../../ducks/history/history'; +import { + isValidHexAddress, + isBurnAddress, +} from '../../../../shared/modules/hexstring-utils'; import { ABOUT_US_ROUTE, @@ -64,7 +67,10 @@ const mapStateToProps = (state, ownProps) => { const addressName = getAddressBookEntryName( state, - isHex(pathNameTail) && isValidAddress(pathNameTail) ? pathNameTail : '', + !isBurnAddress(pathNameTail) && + isValidHexAddress(pathNameTail, { mixedCaseUseChecksum: true }) + ? pathNameTail + : '', ); return { diff --git a/ui/app/pages/swaps/swaps.util.js b/ui/app/pages/swaps/swaps.util.js index c3a564146..968802bb3 100644 --- a/ui/app/pages/swaps/swaps.util.js +++ b/ui/app/pages/swaps/swaps.util.js @@ -1,7 +1,6 @@ import log from 'loglevel'; import BigNumber from 'bignumber.js'; import abi from 'human-standard-token-abi'; -import { isValidAddress } from 'ethereumjs-util'; import { SWAPS_CHAINID_DEFAULT_TOKEN_MAP, METASWAP_CHAINID_API_HOST_MAP, @@ -35,6 +34,7 @@ import { formatCurrency } from '../../helpers/utils/confirm-tx.util'; import fetchWithCache from '../../helpers/utils/fetch-with-cache'; import { calcGasTotal } from '../send/send.utils'; +import { isValidHexAddress } from '../../../../shared/modules/hexstring-utils'; const TOKEN_TRANSFER_LOG_TOPIC_HASH = '0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef'; @@ -74,8 +74,8 @@ const QUOTE_VALIDATORS = [ validator: (trade) => trade && validHex(trade.data) && - isValidAddress(trade.to) && - isValidAddress(trade.from) && + isValidHexAddress(trade.to, { allowNonPrefixed: false }) && + isValidHexAddress(trade.from, { allowNonPrefixed: false }) && truthyString(trade.value), }, { @@ -85,8 +85,8 @@ const QUOTE_VALIDATORS = [ approvalTx === null || (approvalTx && validHex(approvalTx.data) && - isValidAddress(approvalTx.to) && - isValidAddress(approvalTx.from)), + isValidHexAddress(approvalTx.to, { allowNonPrefixed: false }) && + isValidHexAddress(approvalTx.from, { allowNonPrefixed: false })), }, { property: 'sourceAmount', @@ -101,12 +101,12 @@ const QUOTE_VALIDATORS = [ { property: 'sourceToken', type: 'string', - validator: isValidAddress, + validator: (input) => isValidHexAddress(input, { allowNonPrefixed: false }), }, { property: 'destinationToken', type: 'string', - validator: isValidAddress, + validator: (input) => isValidHexAddress(input, { allowNonPrefixed: false }), }, { property: 'aggregator', @@ -146,7 +146,7 @@ const TOKEN_VALIDATORS = [ { property: 'address', type: 'string', - validator: isValidAddress, + validator: (input) => isValidHexAddress(input, { allowNonPrefixed: false }), }, { property: 'symbol', diff --git a/ui/app/selectors/selectors.js b/ui/app/selectors/selectors.js index 73cebf8db..eed91f858 100644 --- a/ui/app/selectors/selectors.js +++ b/ui/app/selectors/selectors.js @@ -13,11 +13,7 @@ import { ALLOWED_SWAPS_CHAIN_IDS, } from '../../../shared/constants/swaps'; -import { - shortenAddress, - checksumAddress, - getAccountByAddress, -} from '../helpers/utils/util'; +import { shortenAddress, getAccountByAddress } from '../helpers/utils/util'; import { getValueFromWeiHex, hexToDecimal, @@ -25,6 +21,7 @@ import { import { TEMPLATED_CONFIRMATION_MESSAGE_TYPES } from '../pages/confirmation/templates'; +import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils'; import { getNativeCurrency } from './send'; /** @@ -241,7 +238,7 @@ export function getAddressBook(state) { export function getAddressBookEntry(state, address) { const addressBook = getAddressBook(state); const entry = addressBook.find( - (contact) => contact.address === checksumAddress(address), + (contact) => contact.address === toChecksumHexAddress(address), ); return entry; } diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 2d404f52f..cbd90fe84 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -3,7 +3,6 @@ import pify from 'pify'; import log from 'loglevel'; import { capitalize } from 'lodash'; import getBuyEthUrl from '../../../app/scripts/lib/buy-eth-url'; -import { checksumAddress } from '../helpers/utils/util'; import { calcTokenBalance, estimateGasForSend } from '../pages/send/send.utils'; import { fetchLocale, @@ -27,6 +26,7 @@ import { import { switchedToUnconnectedAccount } from '../ducks/alerts/unconnected-account'; import { getUnconnectedAccountAlertEnabledness } from '../ducks/metamask/metamask'; import { LISTED_CONTRACT_ADDRESSES } from '../../../shared/constants/tokens'; +import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils'; import * as actionConstants from './actionConstants'; let background = null; @@ -1729,7 +1729,7 @@ export function addToAddressBook(recipient, nickname = '', memo = '') { let set; try { set = await promisifiedBackground.setAddressBook( - checksumAddress(recipient), + toChecksumHexAddress(recipient), nickname, chainId, memo, @@ -1755,7 +1755,7 @@ export function removeFromAddressBook(chainId, addressToRemove) { return async () => { await promisifiedBackground.removeFromAddressBook( chainId, - checksumAddress(addressToRemove), + toChecksumHexAddress(addressToRemove), ); }; } diff --git a/ui/lib/icon-factory.js b/ui/lib/icon-factory.js index 7e8a27aca..132f5330d 100644 --- a/ui/lib/icon-factory.js +++ b/ui/lib/icon-factory.js @@ -1,9 +1,8 @@ import contractMap from '@metamask/contract-metadata'; import { - isValidAddress, - checksumAddress, - isHex, -} from '../app/helpers/utils/util'; + isValidHexAddress, + toChecksumHexAddress, +} from '../../shared/modules/hexstring-utils'; let iconFactory; @@ -20,11 +19,7 @@ function IconFactory(jazzicon) { } IconFactory.prototype.iconForAddress = function (address, diameter) { - let addr = address; - - if (isHex(address)) { - addr = checksumAddress(address); - } + const addr = toChecksumHexAddress(address); if (iconExistsFor(addr)) { return imageElFor(addr); @@ -56,7 +51,9 @@ IconFactory.prototype.generateNewIdenticon = function (address, diameter) { function iconExistsFor(address) { return ( - contractMap[address] && isValidAddress(address) && contractMap[address].logo + contractMap[address] && + isValidHexAddress(address, { allowNonPrefixed: false }) && + contractMap[address].logo ); }