From dafc5046ffe447ae2b56ccd6efd0a67f632b7a99 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 17 Jun 2020 00:40:56 -0300 Subject: [PATCH 01/11] Hide 'Expand view' button in fullscreen (#8826) The 'Expand view' button in the 'Account Options' menu was still being shown on the fullscreen UI. This button is not useful in fullscreen, as all it does is open the fullscreen UI. It is now hidden on the fullscreen UI. --- test/e2e/metamask-responsive-ui.spec.js | 2 +- .../app/menu-bar/account-options-menu.js | 28 ++++++++++++------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/test/e2e/metamask-responsive-ui.spec.js b/test/e2e/metamask-responsive-ui.spec.js index 0969af53f..7e98e4227 100644 --- a/test/e2e/metamask-responsive-ui.spec.js +++ b/test/e2e/metamask-responsive-ui.spec.js @@ -119,7 +119,7 @@ describe('MetaMask', function () { it('show account details dropdown menu', async function () { await driver.clickElement(By.css('[data-testid="account-options-menu-button"]')) const options = await driver.findElements(By.css('.account-options-menu .menu-item')) - assert.equal(options.length, 4) // HD Wallet type does not have to show the Remove Account option + assert.equal(options.length, 3) // HD Wallet type does not have to show the Remove Account option // click outside of menu to dismiss // account menu button chosen because the menu never covers it. await driver.clickPoint(By.css('.account-menu__icon'), 0, 0) diff --git a/ui/app/components/app/menu-bar/account-options-menu.js b/ui/app/components/app/menu-bar/account-options-menu.js index c87416cdb..9edf908d4 100644 --- a/ui/app/components/app/menu-bar/account-options-menu.js +++ b/ui/app/components/app/menu-bar/account-options-menu.js @@ -10,6 +10,8 @@ import genAccountLink from '../../../../lib/account-link' import { getCurrentKeyring, getCurrentNetwork, getRpcPrefsForCurrentProvider, getSelectedIdentity } from '../../../selectors' import { useI18nContext } from '../../../hooks/useI18nContext' import { useMetricEvent } from '../../../hooks/useMetricEvent' +import { getEnvironmentType } from '../../../../../app/scripts/lib/util' +import { ENVIRONMENT_TYPE_FULLSCREEN } from '../../../../../app/scripts/lib/enums' export default function AccountOptionsMenu ({ anchorElement, onClose }) { const t = useI18nContext() @@ -58,16 +60,22 @@ export default function AccountOptionsMenu ({ anchorElement, onClose }) { className="account-options-menu" onHide={onClose} > - { - openFullscreenEvent() - global.platform.openExtensionInBrowser() - onClose() - }} - iconClassName="fas fa-expand-alt" - > - { t('expandView') } - + { + getEnvironmentType() === ENVIRONMENT_TYPE_FULLSCREEN + ? null + : ( + { + openFullscreenEvent() + global.platform.openExtensionInBrowser() + onClose() + }} + iconClassName="fas fa-expand-alt" + > + { t('expandView') } + + ) + } { From c07bf62a7316035282937e8efb25b675eeeeaef4 Mon Sep 17 00:00:00 2001 From: Brad Decker Date: Wed, 17 Jun 2020 11:38:15 -0500 Subject: [PATCH 02/11] fix overflowing contract names and origins (#8823) * fix overflowing contract names and origins Moves heading and subtitle into divs with h3/h2 children so that the div can be display flex and still have ellipses overflow. Only the heading was display flex but I wanted the two to have similar structure. this allows subheading to be display flex in the future. Also uses stripHttpSchemes to remove that from origin in the subheading * rtl ellipses on domain * Update ui/app/components/app/transaction-list-item/index.scss Co-authored-by: Mark Stacey Co-authored-by: Mark Stacey --- .../app/asset-list-item/asset-list-item.js | 2 +- .../app/token-cell/token-cell.test.js | 2 +- .../app/transaction-list-item/index.scss | 23 ++++++++++++ .../transaction-list-item.component.js | 23 +++++++----- .../transaction-status.component.test.js | 10 +++--- .../transaction-status.component.js | 17 ++++----- ui/app/components/ui/list-item/index.scss | 12 +++++++ .../ui/list-item/list-item.component.js | 19 +++++----- .../tests/useTransactionDisplayData.test.js | 6 ++++ ui/app/hooks/useTransactionDisplayData.js | 35 +++++++++++-------- 10 files changed, 100 insertions(+), 49 deletions(-) diff --git a/ui/app/components/app/asset-list-item/asset-list-item.js b/ui/app/components/app/asset-list-item/asset-list-item.js index 8c0848abe..3737698d1 100644 --- a/ui/app/components/app/asset-list-item/asset-list-item.js +++ b/ui/app/components/app/asset-list-item/asset-list-item.js @@ -97,7 +97,7 @@ const AssetListItem = ({ data-testid={dataTestId} title={primary} titleIcon={titleIcon} - subtitle={secondary} + subtitle={

{secondary}

} onClick={onClick} icon={( h3 { + overflow: visible; + display: flex; + white-space: nowrap; + text-overflow: initial; + } + + .transaction-status:after { + content: "·"; + margin: 0 4px; + } + + &__origin, &__address { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + &__origin { + /*rtl:ignore*/ + direction: rtl; + } } diff --git a/ui/app/components/app/transaction-list-item/transaction-list-item.component.js b/ui/app/components/app/transaction-list-item/transaction-list-item.component.js index a92880c52..d056b0202 100644 --- a/ui/app/components/app/transaction-list-item/transaction-list-item.component.js +++ b/ui/app/components/app/transaction-list-item/transaction-list-item.component.js @@ -40,6 +40,7 @@ export default function TransactionListItem ({ transactionGroup, isEarliestNonce const { title, subtitle, + subtitleContainsOrigin, date, category, primaryCurrency, @@ -122,15 +123,19 @@ export default function TransactionListItem ({ transactionGroup, isEarliestNonce /> )} icon={} - subtitle={subtitle} - subtitleStatus={( - + subtitle={( +

+ + + {subtitle} + +

)} rightContent={!isSignatureReq && ( <> diff --git a/ui/app/components/app/transaction-status/tests/transaction-status.component.test.js b/ui/app/components/app/transaction-status/tests/transaction-status.component.test.js index 939090875..9cf9e59ad 100644 --- a/ui/app/components/app/transaction-status/tests/transaction-status.component.test.js +++ b/ui/app/components/app/transaction-status/tests/transaction-status.component.test.js @@ -20,7 +20,7 @@ describe('TransactionStatus Component', function () { ) assert.ok(wrapper) - assert.equal(wrapper.text(), 'June 1 · ') + assert.equal(wrapper.text(), 'June 1') }) it('should render PENDING properly when status is APPROVED', function () { @@ -33,7 +33,7 @@ describe('TransactionStatus Component', function () { ) assert.ok(wrapper) - assert.equal(wrapper.text(), 'PENDING · ') + assert.equal(wrapper.text(), 'PENDING') assert.equal(wrapper.find(Tooltip).props().title, 'test-title') }) @@ -47,7 +47,7 @@ describe('TransactionStatus Component', function () { ) assert.ok(wrapper) - assert.equal(wrapper.text(), 'PENDING · ') + assert.equal(wrapper.text(), 'PENDING') }) it('should render QUEUED properly', function () { @@ -59,7 +59,7 @@ describe('TransactionStatus Component', function () { assert.ok(wrapper) assert.ok(wrapper.find('.transaction-status--queued').length, 'queued className not found') - assert.equal(wrapper.text(), 'QUEUED · ') + assert.equal(wrapper.text(), 'QUEUED') }) it('should render UNAPPROVED properly', function () { @@ -71,7 +71,7 @@ describe('TransactionStatus Component', function () { assert.ok(wrapper) assert.ok(wrapper.find('.transaction-status--unapproved').length, 'unapproved className not found') - assert.equal(wrapper.text(), 'UNAPPROVED · ') + assert.equal(wrapper.text(), 'UNAPPROVED') }) after(function () { diff --git a/ui/app/components/app/transaction-status/transaction-status.component.js b/ui/app/components/app/transaction-status/transaction-status.component.js index c525f7484..2fd37efae 100644 --- a/ui/app/components/app/transaction-status/transaction-status.component.js +++ b/ui/app/components/app/transaction-status/transaction-status.component.js @@ -56,16 +56,13 @@ export default function TransactionStatus ({ status, date, error, isEarliestNonc const statusText = statusKey === CONFIRMED_STATUS ? date : t(statusKey) return ( - - - { statusText } - - {' · '} - + + { statusText } + ) } diff --git a/ui/app/components/ui/list-item/index.scss b/ui/app/components/ui/list-item/index.scss index b6991ea22..1aca82d2a 100644 --- a/ui/app/components/ui/list-item/index.scss +++ b/ui/app/components/ui/list-item/index.scss @@ -41,6 +41,11 @@ display: flex; align-items: center; + > h2 { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } &-wrap { display: inline-block; margin-left: 8px; @@ -53,6 +58,13 @@ line-height: 14px; color: $Grey-500; margin-top: 4px; + // all direct descendants should be truncated with ellipses + // allows flexibility in consuming components to use h3/other tag + > * { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } &:empty { display: none; } diff --git a/ui/app/components/ui/list-item/list-item.component.js b/ui/app/components/ui/list-item/list-item.component.js index 60d395ab3..7481d45db 100644 --- a/ui/app/components/ui/list-item/list-item.component.js +++ b/ui/app/components/ui/list-item/list-item.component.js @@ -6,7 +6,6 @@ export default function ListItem ({ title, subtitle, onClick, - subtitleStatus, children, titleIcon, icon, @@ -24,16 +23,19 @@ export default function ListItem ({ {icon} )} -

- { title } {titleIcon && ( +
+

{ title }

+ {titleIcon && (
{titleIcon}
)} -

-

- {subtitleStatus}{subtitle} -

+ + {subtitle && ( +
+ {subtitle} +
+ )} {children && (
{ children } @@ -56,8 +58,7 @@ export default function ListItem ({ ListItem.propTypes = { title: PropTypes.string.isRequired, titleIcon: PropTypes.node, - subtitle: PropTypes.string, - subtitleStatus: PropTypes.node, + subtitle: PropTypes.node, children: PropTypes.node, icon: PropTypes.node, rightContent: PropTypes.node, diff --git a/ui/app/hooks/tests/useTransactionDisplayData.test.js b/ui/app/hooks/tests/useTransactionDisplayData.test.js index e8281cc72..2cae72bbf 100644 --- a/ui/app/hooks/tests/useTransactionDisplayData.test.js +++ b/ui/app/hooks/tests/useTransactionDisplayData.test.js @@ -16,6 +16,7 @@ const expectedResults = [ { title: 'Send ETH', category: 'send', subtitle: 'To: 0xffe5...1a97', + subtitleContainsOrigin: false, date: 'May 12', primaryCurrency: '-1 ETH', senderAddress: '0x9eca64466f257793eaa52fcfff5066894b76a149', @@ -26,6 +27,7 @@ const expectedResults = [ { title: 'Send ETH', category: 'send', subtitle: 'To: 0x0ccc...8848', + subtitleContainsOrigin: false, date: 'May 12', primaryCurrency: '-2 ETH', senderAddress: '0x9eca64466f257793eaa52fcfff5066894b76a149', @@ -36,6 +38,7 @@ const expectedResults = [ { title: 'Send ETH', category: 'send', subtitle: 'To: 0xffe5...1a97', + subtitleContainsOrigin: false, date: 'May 12', primaryCurrency: '-2 ETH', senderAddress: '0x9eca64466f257793eaa52fcfff5066894b76a149', @@ -46,6 +49,7 @@ const expectedResults = [ { title: 'Receive', category: 'receive', subtitle: 'From: 0x31b9...4523', + subtitleContainsOrigin: false, date: 'May 12', primaryCurrency: '18.75 ETH', senderAddress: '0x31b98d14007bdee637298086988a0bbd31184523', @@ -56,6 +60,7 @@ const expectedResults = [ { title: 'Receive', category: 'receive', subtitle: 'From: 0x9eca...a149', + subtitleContainsOrigin: false, date: 'May 8', primaryCurrency: '0 ETH', senderAddress: '0x9eca64466f257793eaa52fcfff5066894b76a149', @@ -66,6 +71,7 @@ const expectedResults = [ { title: 'Receive', category: 'receive', subtitle: 'From: 0xee01...febb', + subtitleContainsOrigin: false, date: 'May 24', primaryCurrency: '1 ETH', senderAddress: '0xee014609ef9e09776ac5fe00bdbfef57bcdefebb', diff --git a/ui/app/hooks/useTransactionDisplayData.js b/ui/app/hooks/useTransactionDisplayData.js index a0509966f..f5e17f4b3 100644 --- a/ui/app/hooks/useTransactionDisplayData.js +++ b/ui/app/hooks/useTransactionDisplayData.js @@ -7,7 +7,7 @@ import { useTokenFiatAmount } from './useTokenFiatAmount' import { PRIMARY, SECONDARY } from '../helpers/constants/common' import { getTokenToAddress } from '../helpers/utils/token-util' import { useUserPreferencedCurrency } from './useUserPreferencedCurrency' -import { formatDateWithYearContext, shortenAddress } from '../helpers/utils/util' +import { formatDateWithYearContext, shortenAddress, stripHttpSchemes } from '../helpers/utils/util' import { CONTRACT_INTERACTION_KEY, DEPLOY_CONTRACT_ACTION_KEY, @@ -31,14 +31,15 @@ import { getTokens } from '../ducks/metamask/metamask' /** * @typedef {Object} TransactionDisplayData - * @property {string} title - primary description of the transaction - * @property {string} subtitle - supporting text describing the transaction - * @property {string} category - the transaction category - * @property {string} primaryCurrency - the currency string to display in the primary position - * @property {string} [secondaryCurrency] - the currency string to display in the secondary position - * @property {string} status - the status of the transaction - * @property {string} senderAddress - the Ethereum address of the sender - * @property {string} recipientAddress - the Ethereum address of the recipient + * @property {string} title - primary description of the transaction + * @property {string} subtitle - supporting text describing the transaction + * @property {bool} subtitleContainsOrigin - true if the subtitle includes the origin of the tx + * @property {string} category - the transaction category + * @property {string} primaryCurrency - the currency string to display in the primary position + * @property {string} [secondaryCurrency] - the currency string to display in the secondary position + * @property {string} status - the status of the transaction + * @property {string} senderAddress - the Ethereum address of the sender + * @property {string} recipientAddress - the Ethereum address of the recipient */ /** @@ -70,6 +71,7 @@ export function useTransactionDisplayData (transactionGroup) { let prefix = '-' const date = formatDateWithYearContext(initialTransaction.time || 0) let subtitle + let subtitleContainsOrigin = false let recipientAddress = to // This value is used to determine whether we should look inside txParams.data @@ -87,27 +89,31 @@ export function useTransactionDisplayData (transactionGroup) { const tokenDisplayValue = useTokenDisplayValue(initialTransaction?.txParams?.data, token, isTokenCategory) const tokenFiatAmount = useTokenFiatAmount(token?.address, tokenDisplayValue, token?.symbol) + const origin = stripHttpSchemes(initialTransaction.origin || initialTransaction.msgParams?.origin || '') + let category let title // There are four types of transaction entries that are currently differentiated in the design - // 1. (PENDING DESIGN) signature request + // 1. signature request // 2. Send (sendEth sendTokens) // 3. Deposit // 4. Site interaction // 5. Approval if (transactionCategory == null) { - const origin = initialTransaction.msgParams?.origin || initialTransaction.origin category = TRANSACTION_CATEGORY_SIGNATURE_REQUEST title = t('signatureRequest') - subtitle = origin || '' + subtitle = origin + subtitleContainsOrigin = true } else if (transactionCategory === TOKEN_METHOD_APPROVE) { category = TRANSACTION_CATEGORY_APPROVAL title = t('approve') - subtitle = initialTransaction.origin + subtitle = origin + subtitleContainsOrigin = true } else if (transactionCategory === DEPLOY_CONTRACT_ACTION_KEY || transactionCategory === CONTRACT_INTERACTION_KEY) { category = TRANSACTION_CATEGORY_INTERACTION title = (methodData?.name && camelCaseToCapitalize(methodData.name)) || (actionKey && t(actionKey)) || '' - subtitle = initialTransaction.origin + subtitle = origin + subtitleContainsOrigin = true } else if (transactionCategory === INCOMING_TRANSACTION) { category = TRANSACTION_CATEGORY_RECEIVE title = t('receive') @@ -146,6 +152,7 @@ export function useTransactionDisplayData (transactionGroup) { category, date, subtitle, + subtitleContainsOrigin, primaryCurrency, senderAddress, recipientAddress, From c8be5d07794aa9d851b3da068b38df66effd66f6 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 17 Jun 2020 15:12:41 -0300 Subject: [PATCH 03/11] Fix first time onboarding popup position (#8829) The connected status indicator had been moved left since this popup was first written. The position of the highlighted portion of the background has been updated reflect this. --- ui/app/pages/home/index.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/pages/home/index.scss b/ui/app/pages/home/index.scss index 95fc20746..2aad50ab4 100644 --- a/ui/app/pages/home/index.scss +++ b/ui/app/pages/home/index.scss @@ -93,7 +93,7 @@ border-radius: 34px; position: absolute; top: 82px; - left: 12px; + left: 8px; opacity: 1; box-shadow: 0 0 0 9999px rgba(0, 0, 0, 0.2); background: none; From 753a3eb4c9f57ca634325316eae99a9e8fbcd945 Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Wed, 17 Jun 2020 14:13:33 -0700 Subject: [PATCH 04/11] ParseInt nextworkNextNonce correction (#8827) * networkNextNonce toNumber * nonceBN for all getTransactionCount Co-authored-by: Mark Stacey --- .../controllers/transactions/pending-tx-tracker.js | 2 +- .../transactions/pending-tx-tracker-test.js | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index d56421fda..304246565 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.js @@ -211,7 +211,7 @@ export default class PendingTransactionTracker extends EventEmitter { const { hash: txHash, txParams: { nonce, from } } = txMeta const networkNextNonce = await this.query.getTransactionCount(from) - if (parseInt(nonce, 16) >= parseInt(networkNextNonce, 16)) { + if (parseInt(nonce, 16) >= networkNextNonce.toNumber()) { return false } diff --git a/test/unit/app/controllers/transactions/pending-tx-tracker-test.js b/test/unit/app/controllers/transactions/pending-tx-tracker-test.js index e2b68c775..facc0bb12 100644 --- a/test/unit/app/controllers/transactions/pending-tx-tracker-test.js +++ b/test/unit/app/controllers/transactions/pending-tx-tracker-test.js @@ -1,5 +1,6 @@ import sinon from 'sinon' import { strict as assert } from 'assert' +import BN from 'bn.js' import PendingTransactionTracker from '../../../../../app/scripts/controllers/transactions/pending-tx-tracker' describe('PendingTransactionTracker', function () { @@ -311,10 +312,11 @@ describe('PendingTransactionTracker', function () { describe('#_checkIfTxWasDropped', function () { it('should return true when the given nonce is lower than the network nonce', async function () { + const nonceBN = new BN(2) const pendingTxTracker = new PendingTransactionTracker({ query: { getTransactionReceipt: sinon.stub(), - getTransactionCount: sinon.stub().resolves('0x02'), + getTransactionCount: sinon.stub().resolves(nonceBN), }, nonceTracker: { getGlobalLock: sinon.stub().resolves({ @@ -343,10 +345,11 @@ describe('PendingTransactionTracker', function () { }) it('should return false when the given nonce is the network nonce', async function () { + const nonceBN = new BN(1) const pendingTxTracker = new PendingTransactionTracker({ query: { getTransactionReceipt: sinon.stub(), - getTransactionCount: sinon.stub().resolves('0x01'), + getTransactionCount: sinon.stub().resolves(nonceBN), }, nonceTracker: { getGlobalLock: sinon.stub().resolves({ @@ -487,10 +490,11 @@ describe('PendingTransactionTracker', function () { history: [{}], rawTx: '0xf86c808504a817c80082471d', } + const nonceBN = new BN(2) const pendingTxTracker = new PendingTransactionTracker({ query: { getTransactionReceipt: sinon.stub().rejects(), - getTransactionCount: sinon.stub().resolves('0x02'), + getTransactionCount: sinon.stub().resolves(nonceBN), }, nonceTracker: { getGlobalLock: sinon.stub().resolves({ @@ -647,6 +651,7 @@ describe('PendingTransactionTracker', function () { }) it("should emit 'tx:dropped' with the txMetas id only after the fourth call", async function () { + const nonceBN = new BN(2) const txMeta = { id: 1, hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', @@ -662,7 +667,7 @@ describe('PendingTransactionTracker', function () { const pendingTxTracker = new PendingTransactionTracker({ query: { getTransactionReceipt: sinon.stub().resolves(null), - getTransactionCount: sinon.stub().resolves('0x02'), + getTransactionCount: sinon.stub().resolves(nonceBN), }, nonceTracker: { getGlobalLock: sinon.stub().resolves({ From dc398191e004ae40cd47af64f778e40d0f0f8763 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Thu, 18 Jun 2020 12:10:01 -0230 Subject: [PATCH 05/11] Use @metamask/controllers@2.0.1 (#8832) --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 7901bbbaf..342c8db55 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,7 @@ "@formatjs/intl-relativetimeformat": "^5.2.6", "@fortawesome/fontawesome-free": "^5.13.0", "@material-ui/core": "1.0.0", - "@metamask/controllers": "^2.0.0", + "@metamask/controllers": "^2.0.1", "@metamask/eth-ledger-bridge-keyring": "^0.2.6", "@metamask/eth-token-tracker": "^2.0.0", "@metamask/etherscan-link": "^1.1.0", diff --git a/yarn.lock b/yarn.lock index dd57fc376..1c563cfeb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1629,10 +1629,10 @@ scroll "^2.0.3" warning "^3.0.0" -"@metamask/controllers@^2.0.0": - version "2.0.0" - resolved "https://registry.yarnpkg.com/@metamask/controllers/-/controllers-2.0.0.tgz#e57762c213d2722fd178d4c5c288ad98a2ee8bdf" - integrity sha512-J2DSoyyPRAilTdohl43O5tZdqOSMvB94Kx1ApRpkPZxQwtDUIzWHRpTXptyCEwkhaOdkNz9BvWMoaRQxjktfKw== +"@metamask/controllers@^2.0.1": + version "2.0.1" + resolved "https://registry.yarnpkg.com/@metamask/controllers/-/controllers-2.0.1.tgz#33b0e2826fb6d4aa582acaff6b347e6abe0f54f5" + integrity sha512-xioh4h+4D2pUSJ9H2CaffxKGmlg0kUK2bbRJ8c9GXPVTo8KhRHryvNKfkVCyoSt35FLROzzwTEdVJ4dmUFuELQ== dependencies: await-semaphore "^0.1.3" eth-contract-metadata "^1.11.0" From b090625dc15734dcea41e91d1f544a401a8865e3 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Sun, 21 Jun 2020 14:00:06 -0700 Subject: [PATCH 06/11] Update connected status popover content (#8834) * update connected status popover content * update highlight styling --- app/_locales/en/messages.json | 7 +++++-- ui/app/pages/home/home.component.js | 24 ++++++++++++++++------ ui/app/pages/home/index.scss | 31 ++++++++++++++++++++--------- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 60c283a29..76e3fafaa 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -909,10 +909,13 @@ "message": "Message" }, "metaMaskConnectStatusParagraphOne": { - "message": "This is the new MetaMask Connect status indicator. From here you can easily see and manage sites you’ve connected to with your MetaMask wallet." + "message": "You now have more control over your account connections in MetaMask." }, "metaMaskConnectStatusParagraphTwo": { - "message": "Click the Connect status to see your connected sites and their permissions." + "message": "The connection status button shows if the website you’re visiting is connected to your currently selected account." + }, + "metaMaskConnectStatusParagraphThree": { + "message": "Click it to manage your connected accounts." }, "metamaskDescription": { "message": "Connecting you to Ethereum and the Decentralized Web." diff --git a/ui/app/pages/home/home.component.js b/ui/app/pages/home/home.component.js index 4f470ebab..8f942de03 100644 --- a/ui/app/pages/home/home.component.js +++ b/ui/app/pages/home/home.component.js @@ -25,6 +25,8 @@ import { CONNECTED_ACCOUNTS_ROUTE, } from '../../helpers/constants/routes' +const LEARN_MORE_URL = 'https://metamask.zendesk.com/hc/en-us/articles/360045129011-Intro-to-MetaMask-v8-extension' + export default class Home extends PureComponent { static contextTypes = { t: PropTypes.func, @@ -187,17 +189,27 @@ export default class Home extends PureComponent { ) }} footer={( - + <> + + { t('learnMore') } + + + )} >
{ t('metaMaskConnectStatusParagraphOne') }
{ t('metaMaskConnectStatusParagraphTwo') }
+
{ t('metaMaskConnectStatusParagraphThree') }
) diff --git a/ui/app/pages/home/index.scss b/ui/app/pages/home/index.scss index 2aad50ab4..bb2ece13b 100644 --- a/ui/app/pages/home/index.scss +++ b/ui/app/pages/home/index.scss @@ -47,19 +47,24 @@ padding-right: 24px; div { + margin-bottom: 20px; + &:last-child { - margin-top: 26px; + margin-bottom: 0px; } } } &__connected-status-popover { width: 329px; - height: 295px; - margin-top: -12px; + margin-top: -15px; .popover-header { - padding-bottom: 4px; + padding-bottom: 20px; + + &__title { + padding-bottom: 0; + } } .popover-content { @@ -72,7 +77,11 @@ } .popover-footer { - justify-content: flex-end; + border-top: 0; + justify-content: space-between; + align-items: center; + padding-top: 20px; + font-size: 14px; & :only-child { margin: 0; @@ -84,16 +93,20 @@ border-radius: 39px; padding: 0; } + + a, a:hover { + color: $dodger-blue; + cursor: pointer; + } } } &__connected-status-popover-bg { - height: 34px; - width: 110px; + height: 55px; + width: 120px; border-radius: 34px; position: absolute; - top: 82px; - left: 8px; + top: 73px; opacity: 1; box-shadow: 0 0 0 9999px rgba(0, 0, 0, 0.2); background: none; From 4354e9eb9339759366e2d932a4353f943c091e8d Mon Sep 17 00:00:00 2001 From: Thomas Huang Date: Mon, 22 Jun 2020 21:30:45 -0700 Subject: [PATCH 07/11] Call getMethodDataAsync when knownMethodData[fourBytePrefix] object is empty (#8836) Fixes #8835 In cases where the registry failed to load, and the sig is set to `{}` on this line: https://github.com/MetaMask/metamask-extension/blob/e85b162651e887d79bfd15469289abc2c6753cbc/ui/app/helpers/utils/transactions.util.js#L78 this proceeds to set the method prefix to `{}` in knownMethodData. Additionally check if the method prefix object is empty to proceed call getMethodDataAsync again. I could only reproduce by intentionally failing the method registry lookup and found this solution. I could not find an instance where the registry consistently failed to lookup even on slow/throttled/high latency networks. --- ui/app/store/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index c99a566ba..b118b0227 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -2160,7 +2160,7 @@ export function getContractMethodData (data = '') { const prefixedData = ethUtil.addHexPrefix(data) const fourBytePrefix = prefixedData.slice(0, 10) const { knownMethodData } = getState().metamask - if (knownMethodData && knownMethodData[fourBytePrefix]) { + if (knownMethodData && knownMethodData[fourBytePrefix] && Object.keys(knownMethodData[fourBytePrefix]).length !== 0) { return Promise.resolve(knownMethodData[fourBytePrefix]) } From 2abbeadbfba1ee2474d930d32d71e6212556e0e7 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Tue, 23 Jun 2020 05:51:43 -0230 Subject: [PATCH 08/11] Use node-sass@4.14.1 (#8844) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change updates our `node-sass` dependency to the latest version, 4.14.1. This resolves two security advisories brought in by an outdated `yargs-parser` subdependency. See https://www.npmjs.com/advisories/1500 for more information. The `yarn audit` output: ``` ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ low │ Prototype Pollution │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ yargs-parser │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ node-sass │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ node-sass > sass-graph > yargs > yargs-parser │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://www.npmjs.com/advisories/1500 │ └───────────────┴──────────────────────────────────────────────────────────────┘ ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ low │ Prototype Pollution │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ yargs-parser │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=13.1.2 <14.0.0 || >=15.0.1 <16.0.0 || >=18.1.2 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ gulp-sass │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ gulp-sass > node-sass > sass-graph > yargs > yargs-parser │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://www.npmjs.com/advisories/1500 │ └───────────────┴──────────────────────────────────────────────────────────────┘ ``` --- package.json | 2 +- yarn.lock | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 342c8db55..2692d8af8 100644 --- a/package.json +++ b/package.json @@ -260,7 +260,7 @@ "mocha": "^7.2.0", "nock": "^9.0.14", "node-fetch": "^2.6.0", - "node-sass": "^4.12.0", + "node-sass": "^4.14.1", "nyc": "^15.0.0", "polyfill-crypto.getrandomvalues": "^1.0.0", "proxyquire": "^2.1.3", diff --git a/yarn.lock b/yarn.lock index 1c563cfeb..7d5dca909 100644 --- a/yarn.lock +++ b/yarn.lock @@ -19345,10 +19345,10 @@ node-releases@^1.1.46: dependencies: semver "^6.3.0" -node-sass@^4.12.0, node-sass@^4.8.3: - version "4.13.1" - resolved "https://registry.yarnpkg.com/node-sass/-/node-sass-4.13.1.tgz#9db5689696bb2eec2c32b98bfea4c7a2e992d0a3" - integrity sha512-TTWFx+ZhyDx1Biiez2nB0L3YrCZ/8oHagaDalbuBSlqXgUPsdkUSzJsVxeDO9LtPB49+Fh3WQl3slABo6AotNw== +node-sass@^4.14.1, node-sass@^4.8.3: + version "4.14.1" + resolved "https://registry.yarnpkg.com/node-sass/-/node-sass-4.14.1.tgz#99c87ec2efb7047ed638fb4c9db7f3a42e2217b5" + integrity sha512-sjCuOlvGyCJS40R8BscF5vhVlQjNN069NtQ1gSxyK1u9iqvn6tf7O1R4GNowVZfiZUCRt5MmMs1xd+4V/7Yr0g== dependencies: async-foreach "^0.1.3" chalk "^1.1.1" @@ -19364,7 +19364,7 @@ node-sass@^4.12.0, node-sass@^4.8.3: node-gyp "^3.8.0" npmlog "^4.0.0" request "^2.88.0" - sass-graph "^2.2.4" + sass-graph "2.2.5" stdout-stream "^1.4.0" "true-case-path" "^1.0.2" @@ -24044,15 +24044,15 @@ sanitize-filename@^1.6.1: dependencies: truncate-utf8-bytes "^1.0.0" -sass-graph@^2.2.4: - version "2.2.4" - resolved "https://registry.yarnpkg.com/sass-graph/-/sass-graph-2.2.4.tgz#13fbd63cd1caf0908b9fd93476ad43a51d1e0b49" - integrity sha1-E/vWPNHK8JCLn9k0dq1DpR0eC0k= +sass-graph@2.2.5: + version "2.2.5" + resolved "https://registry.yarnpkg.com/sass-graph/-/sass-graph-2.2.5.tgz#a981c87446b8319d96dce0671e487879bd24c2e8" + integrity sha512-VFWDAHOe6mRuT4mZRd4eKE+d8Uedrk6Xnh7Sh9b4NGufQLQjOrvf/MQoOdx+0s92L89FeyUUNfU597j/3uNpag== dependencies: glob "^7.0.0" lodash "^4.0.0" scss-tokenizer "^0.2.3" - yargs "^7.0.0" + yargs "^13.3.2" sass-loader@^7.0.1: version "7.0.1" @@ -28795,7 +28795,7 @@ yargs@13.2.4: y18n "^4.0.0" yargs-parser "^13.1.0" -yargs@13.3.2, yargs@^13.2.2, yargs@^13.2.4, yargs@^13.3.0: +yargs@13.3.2, yargs@^13.2.2, yargs@^13.2.4, yargs@^13.3.0, yargs@^13.3.2: version "13.3.2" resolved "https://registry.yarnpkg.com/yargs/-/yargs-13.3.2.tgz#ad7ffefec1aa59565ac915f82dccb38a9c31a2dd" integrity sha512-AX3Zw5iPruN5ie6xGRIDgqkT+ZhnRlZMLMHAs8tg7nRruy2Nb+i5o9bwghAogtM08q1dpr2LVoS8KSTMYpWXUw== @@ -28864,7 +28864,7 @@ yargs@^15.0.0, yargs@^15.0.2: y18n "^4.0.0" yargs-parser "^18.1.1" -yargs@^7.0.0, yargs@^7.1.0: +yargs@^7.1.0: version "7.1.0" resolved "https://registry.yarnpkg.com/yargs/-/yargs-7.1.0.tgz#6ba318eb16961727f5d284f8ea003e8d6154d0c8" integrity sha1-a6MY6xaWFyf10oT46gA+jWFU0Mg= From 3673d6981601460f7efe68aff17e4bf0168440b7 Mon Sep 17 00:00:00 2001 From: Whymarrh Whitby Date: Tue, 23 Jun 2020 05:51:55 -0230 Subject: [PATCH 09/11] Use gulp-cli@2.3.0 (#8845) --- yarn.lock | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/yarn.lock b/yarn.lock index 7d5dca909..f766b7a2c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13060,9 +13060,9 @@ gulp-babel@^8.0.0: vinyl-sourcemaps-apply "^0.2.0" gulp-cli@^2.2.0: - version "2.2.0" - resolved "https://registry.yarnpkg.com/gulp-cli/-/gulp-cli-2.2.0.tgz#5533126eeb7fe415a7e3e84a297d334d5cf70ebc" - integrity sha512-rGs3bVYHdyJpLqR0TUBnlcZ1O5O++Zs4bA0ajm+zr3WFCfiSLjGwoCBqFs18wzN+ZxahT9DkOK5nDf26iDsWjA== + version "2.3.0" + resolved "https://registry.yarnpkg.com/gulp-cli/-/gulp-cli-2.3.0.tgz#ec0d380e29e52aa45e47977f0d32e18fd161122f" + integrity sha512-zzGBl5fHo0EKSXsHzjspp3y5CONegCm8ErO5Qh0UzFzk2y4tMvzLWhoDokADbarfZRL2pGpRp7yt6gfJX4ph7A== dependencies: ansi-colors "^1.0.1" archy "^1.0.0" @@ -13072,7 +13072,7 @@ gulp-cli@^2.2.0: copy-props "^2.0.1" fancy-log "^1.3.2" gulplog "^1.0.0" - interpret "^1.1.0" + interpret "^1.4.0" isobject "^3.0.1" liftoff "^3.1.0" matchdep "^2.0.0" @@ -13080,7 +13080,7 @@ gulp-cli@^2.2.0: pretty-hrtime "^1.0.0" replace-homedir "^1.0.0" semver-greatest-satisfied-range "^1.1.0" - v8flags "^3.0.1" + v8flags "^3.2.0" yargs "^7.1.0" gulp-debug@^3.2.0: @@ -14269,10 +14269,10 @@ internal-slot@^1.0.2: has "^1.0.3" side-channel "^1.0.2" -interpret@^1.0.0, interpret@^1.1.0: - version "1.1.0" - resolved "https://registry.yarnpkg.com/interpret/-/interpret-1.1.0.tgz#7ed1b1410c6a0e0f78cf95d3b8440c63f78b8614" - integrity sha1-ftGxQQxqDg94z5XTuEQMY/eLhhQ= +interpret@^1.0.0, interpret@^1.1.0, interpret@^1.4.0: + version "1.4.0" + resolved "https://registry.yarnpkg.com/interpret/-/interpret-1.4.0.tgz#665ab8bc4da27a774a40584e812e3e0fa45b1a1e" + integrity sha512-agE4QfB2Lkp9uICn7BAqoscw4SZP9kTE2hxiFI3jBPmXJfdqiahTbUuKGsMoN2GtqL9AxhYioAcVvgsb1HvRbA== interpret@^2.0.0: version "2.0.0" @@ -27548,10 +27548,10 @@ uuid@^3.3.2, uuid@^3.3.3: resolved "https://registry.yarnpkg.com/uuid/-/uuid-3.3.3.tgz#4568f0216e78760ee1dbf3a4d2cf53e224112866" integrity sha512-pW0No1RGHgzlpHJO1nsVrHKpOEIxkGg1xB+v0ZmdNH5OAeAwzAVrCnI2/6Mtx+Uys6iaylxa+D3g4j63IKKjSQ== -v8flags@^3.0.1, v8flags@^3.1.1: - version "3.1.1" - resolved "https://registry.yarnpkg.com/v8flags/-/v8flags-3.1.1.tgz#42259a1461c08397e37fe1d4f1cfb59cad85a053" - integrity sha512-iw/1ViSEaff8NJ3HLyEjawk/8hjJib3E7pvG4pddVXfUg1983s3VGsiClDjhK64MQVDGqc1Q8r18S4VKQZS9EQ== +v8flags@^3.1.1, v8flags@^3.2.0: + version "3.2.0" + resolved "https://registry.yarnpkg.com/v8flags/-/v8flags-3.2.0.tgz#b243e3b4dfd731fa774e7492128109a0fe66d656" + integrity sha512-mH8etigqMfiGWdeXpaaqGfs6BndypxusHHcv2qSHyZkGEznCd/qAXCWWRzeowtL54147cktFOC4P5y+kl8d8Jg== dependencies: homedir-polyfill "^1.0.1" From 41c8e486af3c4cb2deb5332fb2bc0739156057c1 Mon Sep 17 00:00:00 2001 From: Brad Decker Date: Tue, 23 Jun 2020 09:26:33 -0500 Subject: [PATCH 10/11] replace icons with Checkbox component (#8830) in both permission flows the checkboxes were using the fa-check icon, and in the case of the connected accounts popover the color of the icon was wrong. It occurred to me while simply fixing that color would have been easier, we will be adding permissions at some point in the future that a user will be able to 'uncheck'. This PR replaces the usages of those icons with the Checkbox component that is equipped to handle the interactivity of checking/unchecking. --- .../connected-accounts-permissions.component.js | 4 +++- .../connected-accounts-permissions/index.scss | 10 ++++------ .../app/permission-page-container/index.scss | 10 +++++----- ...ermission-page-container-content.component.js | 16 +++++++++++----- .../ui/check-box/check-box.component.js | 4 +++- ui/app/components/ui/check-box/index.scss | 1 - 6 files changed, 26 insertions(+), 19 deletions(-) diff --git a/ui/app/components/app/connected-accounts-permissions/connected-accounts-permissions.component.js b/ui/app/components/app/connected-accounts-permissions/connected-accounts-permissions.component.js index 45f886912..6ccb5b16a 100644 --- a/ui/app/components/app/connected-accounts-permissions/connected-accounts-permissions.component.js +++ b/ui/app/components/app/connected-accounts-permissions/connected-accounts-permissions.component.js @@ -1,6 +1,7 @@ import classnames from 'classnames' import PropTypes from 'prop-types' import React, { PureComponent } from 'react' +import CheckBox from '../../ui/check-box' export default class ConnectedAccountsPermissions extends PureComponent { static contextTypes = { @@ -57,7 +58,8 @@ export default class ConnectedAccountsPermissions extends PureComponent {
    {permissions.map(({ key: permissionName }) => (
  • - {t(permissionName)} + +
    • ))}
diff --git a/ui/app/components/app/connected-accounts-permissions/index.scss b/ui/app/components/app/connected-accounts-permissions/index.scss index 838e727dc..712d0b431 100644 --- a/ui/app/components/app/connected-accounts-permissions/index.scss +++ b/ui/app/components/app/connected-accounts-permissions/index.scss @@ -41,13 +41,11 @@ &__list-item { display: flex; + } - i { - display: block; - padding-right: 8px; - font-size: 18px; - color: $Grey-800; - } + & &__checkbox { + margin: 0 8px 0 0; + font-size: 18px; } &__list-container { diff --git a/ui/app/components/app/permission-page-container/index.scss b/ui/app/components/app/permission-page-container/index.scss index 9a17655ec..576147aed 100644 --- a/ui/app/components/app/permission-page-container/index.scss +++ b/ui/app/components/app/permission-page-container/index.scss @@ -62,11 +62,6 @@ display: flex; align-items: center; - i { - font-size: 1.4rem; - color: $Grey-200; - } - label { font-size: 14px; margin-left: 16px; @@ -75,6 +70,11 @@ } } + & &__checkbox { + font-size: 1.4rem; + margin: 0; + } + &__content-container { display: flex; flex-direction: column; diff --git a/ui/app/components/app/permission-page-container/permission-page-container-content/permission-page-container-content.component.js b/ui/app/components/app/permission-page-container/permission-page-container-content/permission-page-container-content.component.js index c15e93a87..268c973bf 100644 --- a/ui/app/components/app/permission-page-container/permission-page-container-content/permission-page-container-content.component.js +++ b/ui/app/components/app/permission-page-container/permission-page-container-content/permission-page-container-content.component.js @@ -2,6 +2,7 @@ import PropTypes from 'prop-types' import React, { PureComponent } from 'react' import PermissionsConnectHeader from '../../permissions-connect-header' import Tooltip from '../../../ui/tooltip-v2' +import CheckBox from '../../../ui/check-box' export default class PermissionPageContainerContent extends PureComponent { @@ -33,6 +34,8 @@ export default class PermissionPageContainerContent extends PureComponent { const description = t(permissionName) // don't allow deselecting eth_accounts const isDisabled = permissionName === 'eth_accounts' + const isChecked = Boolean(selectedPermissions[permissionName]) + const title = isChecked ? t('permissionCheckedIconDescription') : t('permissionUncheckedIconDescription') return (
- { selectedPermissions[permissionName] - ? - : - } - + +
) }) diff --git a/ui/app/components/ui/check-box/check-box.component.js b/ui/app/components/ui/check-box/check-box.component.js index 5d062e0e8..b1c62433a 100644 --- a/ui/app/components/ui/check-box/check-box.component.js +++ b/ui/app/components/ui/check-box/check-box.component.js @@ -10,7 +10,7 @@ const CHECKBOX_STATE = { export const { CHECKED, INDETERMINATE, UNCHECKED } = CHECKBOX_STATE -const CheckBox = ({ className, disabled, id, onClick, checked }) => { +const CheckBox = ({ className, disabled, id, onClick, checked, title }) => { if (typeof checked === 'boolean') { checked = checked ? CHECKBOX_STATE.CHECKED @@ -41,6 +41,7 @@ const CheckBox = ({ className, disabled, id, onClick, checked }) => { } readOnly ref={ref} + title={title} type="checkbox" /> ) @@ -52,6 +53,7 @@ CheckBox.propTypes = { id: PropTypes.string, onClick: PropTypes.func, checked: PropTypes.oneOf([...Object.keys(CHECKBOX_STATE), true, false]).isRequired, + title: PropTypes.string, } CheckBox.defaultProps = { diff --git a/ui/app/components/ui/check-box/index.scss b/ui/app/components/ui/check-box/index.scss index 40273973d..446ce1683 100644 --- a/ui/app/components/ui/check-box/index.scss +++ b/ui/app/components/ui/check-box/index.scss @@ -18,6 +18,5 @@ &:disabled { color: $Grey-100; cursor: not-allowed; - opacity: 0.5; } } From 04de9a92c5349eb420ad787cc7fbde6fe5b27ded Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Tue, 23 Jun 2020 09:12:11 -0700 Subject: [PATCH 11/11] Fix signing method bugs (#8833) * update signTypedData validation * update tests for new eth-json-rpc-middleware * remove lowercasing of tx 'from' addresses --- app/scripts/controllers/permissions/enums.js | 1 + .../controllers/transactions/lib/util.js | 11 +-- .../lib/encryption-public-key-manager.js | 1 - app/scripts/lib/typed-message-manager.js | 38 ++++++----- package.json | 2 +- .../controllers/metamask-controller-test.js | 68 +++++++++++++------ yarn.lock | 29 +++++++- 7 files changed, 105 insertions(+), 45 deletions(-) diff --git a/app/scripts/controllers/permissions/enums.js b/app/scripts/controllers/permissions/enums.js index 12dc74a70..efd994c10 100644 --- a/app/scripts/controllers/permissions/enums.js +++ b/app/scripts/controllers/permissions/enums.js @@ -75,6 +75,7 @@ export const SAFE_METHODS = [ 'eth_sendTransaction', 'eth_sign', 'personal_sign', + 'personal_ecRecover', 'eth_signTypedData', 'eth_signTypedData_v1', 'eth_signTypedData_v3', diff --git a/app/scripts/controllers/transactions/lib/util.js b/app/scripts/controllers/transactions/lib/util.js index c77d70dd4..da58bf6f3 100644 --- a/app/scripts/controllers/transactions/lib/util.js +++ b/app/scripts/controllers/transactions/lib/util.js @@ -1,8 +1,8 @@ import { addHexPrefix, isValidAddress } from 'ethereumjs-util' const normalizers = { - from: (from, lowerCase = true) => (lowerCase ? addHexPrefix(from).toLowerCase() : addHexPrefix(from)), - to: (to, lowerCase = true) => (lowerCase ? addHexPrefix(to).toLowerCase() : addHexPrefix(to)), + from: (from) => addHexPrefix(from), + to: (to, lowerCase) => (lowerCase ? addHexPrefix(to).toLowerCase() : addHexPrefix(to)), nonce: (nonce) => addHexPrefix(nonce), value: (value) => addHexPrefix(value), data: (data) => addHexPrefix(data), @@ -12,11 +12,12 @@ const normalizers = { /** * Normalizes the given txParams - * @param {Object} txParams - the tx params - * @param {boolean} lowerCase - whether to return the addresses lower cased + * @param {Object} txParams - The transaction params + * @param {boolean} [lowerCase] - Whether to lowercase the 'to' address. + * Default: true * @returns {Object} the normalized tx params */ -export function normalizeTxParams (txParams, lowerCase) { +export function normalizeTxParams (txParams, lowerCase = true) { // apply only keys in the normalizers const normalizedTxParams = {} for (const key in normalizers) { diff --git a/app/scripts/lib/encryption-public-key-manager.js b/app/scripts/lib/encryption-public-key-manager.js index 0ae12442b..f568c62db 100644 --- a/app/scripts/lib/encryption-public-key-manager.js +++ b/app/scripts/lib/encryption-public-key-manager.js @@ -283,5 +283,4 @@ export default class EncryptionPublicKeyManager extends EventEmitter { this.memStore.updateState({ unapprovedEncryptionPublicKeyMsgs, unapprovedEncryptionPublicKeyMsgCount }) this.emit('updateBadge') } - } diff --git a/app/scripts/lib/typed-message-manager.js b/app/scripts/lib/typed-message-manager.js index 147180ffd..49e71e59f 100644 --- a/app/scripts/lib/typed-message-manager.js +++ b/app/scripts/lib/typed-message-manager.js @@ -4,9 +4,11 @@ import createId from './random-id' import assert from 'assert' import { ethErrors } from 'eth-json-rpc-errors' import sigUtil from 'eth-sig-util' +import { isValidAddress } from 'ethereumjs-util' import log from 'loglevel' import jsonschema from 'jsonschema' import { MESSAGE_TYPE } from './enums' + /** * Represents, and contains data about, an 'eth_signTypedData' type signature request. These are created when a * signature for an eth_signTypedData call is requested. @@ -102,14 +104,15 @@ export default class TypedMessageManager extends EventEmitter { * */ addUnapprovedMessage (msgParams, req, version) { + msgParams.version = version - this.validateParams(msgParams) - // add origin from request if (req) { msgParams.origin = req.origin } + this.validateParams(msgParams) log.debug(`TypedMessageManager addUnapprovedMessage: ${JSON.stringify(msgParams)}`) + // create txData obj with parameters and meta data const time = (new Date()).getTime() const msgId = createId() @@ -134,37 +137,38 @@ export default class TypedMessageManager extends EventEmitter { * */ validateParams (params) { + + assert.ok(params && typeof params === 'object', 'Params must be an object.') + 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), + '"from" field must be a valid, lowercase, hexadecimal Ethereum address string.' + ) + switch (params.version) { case 'V1': - assert.equal(typeof params, 'object', 'Params should ben an object.') - assert.ok('data' in params, 'Params must include a data field.') - assert.ok('from' in params, 'Params must include a from field.') - assert.ok(Array.isArray(params.data), 'Data should be an array.') - assert.equal(typeof params.from, 'string', 'From field must be a string.') + assert.ok(Array.isArray(params.data), '"params.data" must be an array.') assert.doesNotThrow(() => { sigUtil.typedSignatureHash(params.data) - }, 'Expected EIP712 typed data') + }, 'Signing data must be valid EIP-712 typed data.') break case 'V3': case 'V4': + assert.equal(typeof params.data, 'string', '"params.data" must be a string.') let data - assert.equal(typeof params, 'object', 'Params should be an object.') - assert.ok('data' in params, 'Params must include a data field.') - assert.ok('from' in params, 'Params must include a from field.') - assert.equal(typeof params.from, 'string', 'From field must be a string.') - assert.equal(typeof params.data, 'string', 'Data must be passed as a valid JSON string.') assert.doesNotThrow(() => { data = JSON.parse(params.data) - }, 'Data must be passed as a valid JSON string.') + }, '"data" must be a valid JSON string.') const validation = jsonschema.validate(data, sigUtil.TYPED_MESSAGE_SCHEMA) assert.ok(data.primaryType in data.types, `Primary type of "${data.primaryType}" has no type definition.`) - assert.equal(validation.errors.length, 0, 'Data must conform to EIP-712 schema. See https://git.io/fNtcx.') + assert.equal(validation.errors.length, 0, 'Signing data must conform to EIP-712 schema. See https://git.io/fNtcx.') const chainId = data.domain.chainId const activeChainId = parseInt(this.networkController.getNetworkState()) - chainId && assert.equal(chainId, activeChainId, `Provided chainId (${chainId}) must match the active chainId (${activeChainId})`) + chainId && assert.equal(chainId, activeChainId, `Provided chainId "${chainId}" must match the active chainId "${activeChainId}"`) break default: - assert.fail(`Unknown params.version ${params.version}`) + assert.fail(`Unknown typed data version "${params.version}"`) } } diff --git a/package.json b/package.json index 2692d8af8..1a68dfdb9 100644 --- a/package.json +++ b/package.json @@ -106,7 +106,7 @@ "eth-json-rpc-errors": "^2.0.2", "eth-json-rpc-filters": "^4.1.1", "eth-json-rpc-infura": "^4.0.2", - "eth-json-rpc-middleware": "^4.4.1", + "eth-json-rpc-middleware": "^5.0.0", "eth-keyring-controller": "^6.0.0", "eth-method-registry": "^1.2.0", "eth-phishing-detect": "^1.1.4", diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js index 262202419..4628aabc0 100644 --- a/test/unit/app/controllers/metamask-controller-test.js +++ b/test/unit/app/controllers/metamask-controller-test.js @@ -35,9 +35,33 @@ const ExtensionizerMock = { }, } +let loggerMiddlewareMock +const initializeMockMiddlewareLog = () => { + loggerMiddlewareMock = { + requests: [], + responses: [], + } +} +const tearDownMockMiddlewareLog = () => { + loggerMiddlewareMock = undefined +} + +const createLoggerMiddlewareMock = () => (req, res, next) => { + if (loggerMiddlewareMock) { + loggerMiddlewareMock.requests.push(req) + next((cb) => { + loggerMiddlewareMock.responses.push(res) + cb() + }) + } else { + next() + } +} + const MetaMaskController = proxyquire('../../../../app/scripts/metamask-controller', { './controllers/threebox': { default: ThreeBoxControllerMock }, 'extensionizer': ExtensionizerMock, + './lib/createLoggerMiddleware': { default: createLoggerMiddlewareMock }, }).default const currentNetworkId = 42 @@ -96,7 +120,6 @@ describe('MetaMaskController', function () { // add sinon method spies sandbox.spy(metamaskController.keyringController, 'createNewVaultAndKeychain') sandbox.spy(metamaskController.keyringController, 'createNewVaultAndRestore') - sandbox.spy(metamaskController.txController, 'newUnapprovedTransaction') }) afterEach(function () { @@ -776,6 +799,17 @@ describe('MetaMaskController', function () { }) describe('#setupUntrustedCommunication', function () { + + const mockTxParams = { from: TEST_ADDRESS } + + beforeEach(function () { + initializeMockMiddlewareLog() + }) + + after(function () { + tearDownMockMiddlewareLog() + }) + it('sets up phishing stream for untrusted communication', async function () { const phishingMessageSender = { url: 'http://myethereumwalletntw.com', @@ -815,7 +849,7 @@ describe('MetaMaskController', function () { const message = { id: 1999133338649204, jsonrpc: '2.0', - params: ['mock tx params'], + params: [{ ...mockTxParams }], method: 'eth_sendTransaction', } streamTest.write({ @@ -824,15 +858,12 @@ describe('MetaMaskController', function () { }, null, () => { setTimeout(() => { assert.deepStrictEqual( - metamaskController.txController.newUnapprovedTransaction.getCall(0).args, - [ - 'mock tx params', - { - ...message, - origin: 'http://mycrypto.com', - tabId: 456, - }, - ] + loggerMiddlewareMock.requests[0], + { + ...message, + origin: 'http://mycrypto.com', + tabId: 456, + }, ) done() }) @@ -856,7 +887,7 @@ describe('MetaMaskController', function () { const message = { id: 1999133338649204, jsonrpc: '2.0', - params: ['mock tx params'], + params: [{ ...mockTxParams }], method: 'eth_sendTransaction', } streamTest.write({ @@ -865,14 +896,11 @@ describe('MetaMaskController', function () { }, null, () => { setTimeout(() => { assert.deepStrictEqual( - metamaskController.txController.newUnapprovedTransaction.getCall(0).args, - [ - 'mock tx params', - { - ...message, - origin: 'http://mycrypto.com', - }, - ] + loggerMiddlewareMock.requests[0], + { + ...message, + origin: 'http://mycrypto.com', + }, ) done() }) diff --git a/yarn.lock b/yarn.lock index f766b7a2c..f5cb5825c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10174,7 +10174,7 @@ eth-json-rpc-middleware@^1.5.0: promise-to-callback "^1.0.0" tape "^4.6.3" -eth-json-rpc-middleware@^4.1.4, eth-json-rpc-middleware@^4.1.5, eth-json-rpc-middleware@^4.4.1: +eth-json-rpc-middleware@^4.1.4, eth-json-rpc-middleware@^4.1.5: version "4.4.1" resolved "https://registry.yarnpkg.com/eth-json-rpc-middleware/-/eth-json-rpc-middleware-4.4.1.tgz#07d3dd0724c24a8d31e4a172ee96271da71b4228" integrity sha512-yoSuRgEYYGFdVeZg3poWOwAlRI+MoBIltmOB86MtpoZjvLbou9EB/qWMOWSmH2ryCWLW97VYY6NWsmWm3OAA7A== @@ -10194,6 +10194,26 @@ eth-json-rpc-middleware@^4.1.4, eth-json-rpc-middleware@^4.1.5, eth-json-rpc-mid pify "^3.0.0" safe-event-emitter "^1.0.1" +eth-json-rpc-middleware@^5.0.0: + version "5.0.0" + resolved "https://registry.yarnpkg.com/eth-json-rpc-middleware/-/eth-json-rpc-middleware-5.0.0.tgz#9a202a41a2b1678c094cbffd0db0da619ec86f7b" + integrity sha512-31zLHvElrQV7opFZs47UqjMGFM4QCgMms7HJSQAZ3wjPuE3rFLxFUTeGHXwM3fAp9zh7n0M+B2gK7Ds4OUKfog== + dependencies: + btoa "^1.2.1" + clone "^2.1.1" + eth-query "^2.1.2" + eth-rpc-errors "^2.1.1" + eth-sig-util "^1.4.2" + ethereumjs-block "^1.6.0" + ethereumjs-tx "^1.3.7" + ethereumjs-util "^5.1.2" + ethereumjs-vm "^2.6.0" + fetch-ponyfill "^4.0.0" + json-rpc-engine "^5.1.3" + json-stable-stringify "^1.0.1" + pify "^3.0.0" + safe-event-emitter "^1.0.1" + eth-keyring-controller@^5.3.0, eth-keyring-controller@^5.6.1: version "5.6.1" resolved "https://registry.yarnpkg.com/eth-keyring-controller/-/eth-keyring-controller-5.6.1.tgz#7b7268400704c8f5ce98a055910341177dd207ca" @@ -10275,6 +10295,13 @@ 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-sig-util@2.3.0, eth-sig-util@^2.3.0: version "2.3.0" resolved "https://registry.yarnpkg.com/eth-sig-util/-/eth-sig-util-2.3.0.tgz#c54a6ac8e8796f7e25f59cf436982a930e645231"