From 42be5a07d799bc7c82160dea10f95ffdae3d171a Mon Sep 17 00:00:00 2001 From: Vladimir Saric <92527393+VSaric@users.noreply.github.com> Date: Thu, 10 Nov 2022 11:28:34 +0100 Subject: [PATCH] Replaced addresses by the address component on SignTypedData v4 signatures (#16018) * Replaced addresses by the address component on SignTypedData v4 signatures * Fixing signature-request e2e tests * Modified scss file for signature-request message * Using address component for rendering the addresses and bold label where hex address is not valid * Modify the address component * Added proper BEM syntax for class names and used Box and Typography * FIxing e2e tests * Commited requested changes from George and added storybook * Review requested changes * Created new component for rendering data in signature-request-message.js * Fixing proper usage for getAccountName and getMetadataContractName selectors * Fixing e2e tests --- .../files-to-convert.json | 2 +- test/e2e/tests/signature-request.spec.js | 8 +- .../confirm-page-container.container.js | 6 +- .../app/signature-request/index.scss | 1 + .../signature-request-data/index.js | 1 + .../signature-request-data/index.scss | 26 +++++ .../signature-request-data.js | 79 ++++++++++++++ .../signature-request-message/index.js | 2 +- .../signature-request-message/index.scss | 59 +--------- .../signature-request-message.component.js | 103 ------------------ .../signature-request-message.js | 98 +++++++++++++++++ .../signature-request-message.stories.js | 58 ++++++++++ .../signature-request.container.test.js | 40 +++++++ .../decoding/address/address.component.js | 21 +++- .../app/transaction-decoding/index.scss | 9 ++ ...transaction-list-item-details.container.js | 9 +- ui/selectors/selectors.js | 9 +- 17 files changed, 357 insertions(+), 174 deletions(-) create mode 100644 ui/components/app/signature-request/signature-request-data/index.js create mode 100644 ui/components/app/signature-request/signature-request-data/index.scss create mode 100644 ui/components/app/signature-request/signature-request-data/signature-request-data.js delete mode 100644 ui/components/app/signature-request/signature-request-message/signature-request-message.component.js create mode 100644 ui/components/app/signature-request/signature-request-message/signature-request-message.js create mode 100644 ui/components/app/signature-request/signature-request-message/signature-request-message.stories.js diff --git a/development/ts-migration-dashboard/files-to-convert.json b/development/ts-migration-dashboard/files-to-convert.json index 6cce4cb55..bc94b3e44 100644 --- a/development/ts-migration-dashboard/files-to-convert.json +++ b/development/ts-migration-dashboard/files-to-convert.json @@ -698,7 +698,7 @@ "ui/components/app/signature-request/signature-request-header/signature-request-header.component.js", "ui/components/app/signature-request/signature-request-header/signature-request-header.stories.js", "ui/components/app/signature-request/signature-request-message/index.js", - "ui/components/app/signature-request/signature-request-message/signature-request-message.component.js", + "ui/components/app/signature-request/signature-request-message/signature-request-message.js", "ui/components/app/signature-request/signature-request.component.js", "ui/components/app/signature-request/signature-request.component.test.js", "ui/components/app/signature-request/signature-request.container.js", diff --git a/test/e2e/tests/signature-request.spec.js b/test/e2e/tests/signature-request.spec.js index 38141527b..9fa2a08d4 100644 --- a/test/e2e/tests/signature-request.spec.js +++ b/test/e2e/tests/signature-request.spec.js @@ -56,7 +56,7 @@ describe('Sign Typed Data V4 Signature Request', function () { const origin = content[0]; const address = content[1]; const message = await driver.findElement( - '.signature-request-message--node-value', + '.signature-request-data__node__value', ); assert.equal(await title.getText(), 'Signature request'); assert.equal(await name.getText(), 'Ether Mail'); @@ -140,7 +140,7 @@ describe('Sign Typed Data V3 Signature Request', function () { const origin = content[0]; const address = content[1]; const messages = await driver.findElements( - '.signature-request-message--node-value', + '.signature-request-data__node__value', ); assert.equal(await title.getText(), 'Signature request'); assert.equal(await name.getText(), 'Ether Mail'); @@ -154,6 +154,10 @@ describe('Sign Typed Data V3 Signature Request', function () { assert.equal(await messages[4].getText(), 'Hello, Bob!'); // Approve signing typed data + await driver.clickElement( + '[data-testid="signature-request-scroll-button"]', + ); + await driver.delay(regularDelayMs); await driver.clickElement({ text: 'Sign', tag: 'button' }); await driver.waitUntilXWindowHandles(2); windowHandles = await driver.getAllWindowHandles(); diff --git a/ui/components/app/confirm-page-container/confirm-page-container.container.js b/ui/components/app/confirm-page-container/confirm-page-container.container.js index 8d247971c..0ce2790fc 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container.container.js +++ b/ui/components/app/confirm-page-container/confirm-page-container.container.js @@ -7,6 +7,7 @@ import { getSwapsDefaultToken, getMetadataContractName, getAccountName, + getMetaMaskIdentities, } from '../../../selectors'; import ConfirmPageContainer from './confirm-page-container.component'; @@ -17,8 +18,9 @@ function mapStateToProps(state, ownProps) { const networkIdentifier = getNetworkIdentifier(state); const defaultToken = getSwapsDefaultToken(state); const accountBalance = defaultToken.string; - const toName = getAccountName(state, to); - const toMetadataName = getMetadataContractName(to); + const identities = getMetaMaskIdentities(state); + const toName = getAccountName(identities, to); + const toMetadataName = getMetadataContractName(state, to); return { isBuyableChain, diff --git a/ui/components/app/signature-request/index.scss b/ui/components/app/signature-request/index.scss index aacb6a2f9..f89b3c667 100644 --- a/ui/components/app/signature-request/index.scss +++ b/ui/components/app/signature-request/index.scss @@ -1,6 +1,7 @@ @import 'signature-request-footer/index'; @import 'signature-request-header/index'; @import 'signature-request-message/index'; +@import 'signature-request-data/index'; .signature-request { display: flex; diff --git a/ui/components/app/signature-request/signature-request-data/index.js b/ui/components/app/signature-request/signature-request-data/index.js new file mode 100644 index 000000000..64ed87d93 --- /dev/null +++ b/ui/components/app/signature-request/signature-request-data/index.js @@ -0,0 +1 @@ +export { default } from './signature-request-data'; diff --git a/ui/components/app/signature-request/signature-request-data/index.scss b/ui/components/app/signature-request/signature-request-data/index.scss new file mode 100644 index 000000000..6b81bd84d --- /dev/null +++ b/ui/components/app/signature-request/signature-request-data/index.scss @@ -0,0 +1,26 @@ +.signature-request-data { + &__node { + &__value { + white-space: pre-line; + overflow: hidden; + word-wrap: break-word; + + &__address { + [dir='rtl'] & { + /*rtl:ignore*/ + direction: ltr; + + /*rtl:ignore*/ + text-align: right; + + span { + display: block; + + /*rtl:ignore*/ + direction: rtl; + } + } + } + } + } +} diff --git a/ui/components/app/signature-request/signature-request-data/signature-request-data.js b/ui/components/app/signature-request/signature-request-data/signature-request-data.js new file mode 100644 index 000000000..6685f1c56 --- /dev/null +++ b/ui/components/app/signature-request/signature-request-data/signature-request-data.js @@ -0,0 +1,79 @@ +import React from 'react'; +import { useSelector } from 'react-redux'; +import PropTypes from 'prop-types'; +import { getMetaMaskIdentities, getAccountName } from '../../../../selectors'; +import Address from '../../transaction-decoding/components/decoding/address'; +import { + isValidHexAddress, + toChecksumHexAddress, +} from '../../../../../shared/modules/hexstring-utils'; +import Box from '../../../ui/box'; +import Typography from '../../../ui/typography'; +import { + DISPLAY, + COLORS, + FONT_WEIGHT, + TYPOGRAPHY, +} from '../../../../helpers/constants/design-system'; + +export default function SignatureRequestData({ data }) { + const identities = useSelector(getMetaMaskIdentities); + + return ( + + {Object.entries(data).map(([label, value], i) => ( + + + {label.charAt(0).toUpperCase() + label.slice(1)}:{' '} + + {typeof value === 'object' && value !== null ? ( + + ) : ( + + {isValidHexAddress(value, { + mixedCaseUseChecksum: true, + }) ? ( + +
+ + ) : ( + `${value}` + )} + + )} + + ))} + + ); +} + +SignatureRequestData.propTypes = { + data: PropTypes.oneOfType([PropTypes.object, PropTypes.array]).isRequired, +}; diff --git a/ui/components/app/signature-request/signature-request-message/index.js b/ui/components/app/signature-request/signature-request-message/index.js index 5fe8ddb1c..569576fdd 100644 --- a/ui/components/app/signature-request/signature-request-message/index.js +++ b/ui/components/app/signature-request/signature-request-message/index.js @@ -1 +1 @@ -export { default } from './signature-request-message.component'; +export { default } from './signature-request-message'; diff --git a/ui/components/app/signature-request/signature-request-message/index.scss b/ui/components/app/signature-request/signature-request-message/index.scss index b0b5c67ae..54fdb59d4 100644 --- a/ui/components/app/signature-request/signature-request-message/index.scss +++ b/ui/components/app/signature-request/signature-request-message/index.scss @@ -1,75 +1,24 @@ .signature-request-message { flex: 1 60%; - display: flex; max-height: 231px; - flex-direction: column; position: relative; - &__title { - @include H6; - - font-weight: 500; - color: var(--color-text-alternative); - margin-left: 12px; - } - - h2 { - @include H6; - - flex: 1 1 0; - text-align: left; - border-bottom: 1px solid var(--color-border-default); - padding: 0.5rem; - margin: 0; - color: var(--color-text-alternative); - } - - &--root { + &__root { flex: 1 100%; - background-color: var(--color-background-alternative); - padding-bottom: 0.5rem; overflow: auto; - padding-left: 12px; - padding-right: 12px; @include screen-sm-min { width: auto; } } - &--node, - &--node-leaf { - padding-left: 0.3rem; - - &-label { - color: var(--color-text-alternative); - margin-left: 0.5rem; - } - - &-value { - color: var(--color-text-default); - margin-left: 0.5rem; - white-space: pre-line; - overflow: hidden; - word-wrap: break-word; - } - } - - &--node-leaf { - display: flex; - } - &__scroll-button { - display: flex; - align-items: center; - justify-content: center; - border: 1px solid var(--color-border-default); - background: var(--color-background-alternative); - color: var(--color-icon-default); position: absolute; - right: 24px; + right: 28px; bottom: 12px; border-radius: 50%; + height: 24px; + width: 24px; cursor: pointer; } } diff --git a/ui/components/app/signature-request/signature-request-message/signature-request-message.component.js b/ui/components/app/signature-request/signature-request-message/signature-request-message.component.js deleted file mode 100644 index 641931919..000000000 --- a/ui/components/app/signature-request/signature-request-message/signature-request-message.component.js +++ /dev/null @@ -1,103 +0,0 @@ -import React, { PureComponent } from 'react'; -import PropTypes from 'prop-types'; -import { debounce } from 'lodash'; -import classnames from 'classnames'; - -export default class SignatureRequestMessage extends PureComponent { - static propTypes = { - data: PropTypes.object.isRequired, - onMessageScrolled: PropTypes.func, - setMessageRootRef: PropTypes.func, - messageRootRef: PropTypes.object, - messageIsScrollable: PropTypes.bool, - }; - - static contextTypes = { - t: PropTypes.func, - }; - - state = { - messageIsScrolled: false, - }; - - setMessageIsScrolled = () => { - if (!this.props.messageRootRef || this.state.messageIsScrolled) { - return; - } - - const { scrollTop, offsetHeight, scrollHeight } = this.props.messageRootRef; - const isAtBottom = Math.round(scrollTop) + offsetHeight >= scrollHeight; - - if (isAtBottom) { - this.setState({ messageIsScrolled: true }); - this.props.onMessageScrolled(); - } - }; - - onScroll = debounce(this.setMessageIsScrolled, 25); - - renderNode(data) { - return ( -
- {Object.entries(data).map(([label, value], i) => ( -
- - {label}:{' '} - - {typeof value === 'object' && value !== null ? ( - this.renderNode(value) - ) : ( - - {`${value}`} - - )} -
- ))} -
- ); - } - - renderScrollButton() { - return ( -
{ - this.setState({ messageIsScrolled: true }); - this.props.onMessageScrolled(); - this.props.messageRootRef.scrollTo( - 0, - this.props.messageRootRef.scrollHeight, - ); - }} - className="signature-request-message__scroll-button" - data-testid="signature-request-scroll-button" - > - -
- ); - } - - render() { - const { data, messageIsScrollable } = this.props; - - return ( -
- {messageIsScrollable ? this.renderScrollButton() : null} -
- {this.context.t('signatureRequest1')} -
-
- {this.renderNode(data)} -
-
- ); - } -} diff --git a/ui/components/app/signature-request/signature-request-message/signature-request-message.js b/ui/components/app/signature-request/signature-request-message/signature-request-message.js new file mode 100644 index 000000000..e86dfba3d --- /dev/null +++ b/ui/components/app/signature-request/signature-request-message/signature-request-message.js @@ -0,0 +1,98 @@ +import React, { useContext, useState } from 'react'; +import PropTypes from 'prop-types'; +import { debounce } from 'lodash'; +import { I18nContext } from '../../../../contexts/i18n'; +import Box from '../../../ui/box'; +import Typography from '../../../ui/typography'; +import { + DISPLAY, + ALIGN_ITEMS, + JUSTIFY_CONTENT, + COLORS, + FONT_WEIGHT, + FLEX_DIRECTION, + SIZES, +} from '../../../../helpers/constants/design-system'; +import SignatureRequestData from '../signature-request-data'; + +export default function SignatureRequestMessage({ + data, + onMessageScrolled, + setMessageRootRef, + messageRootRef, + messageIsScrollable, +}) { + const t = useContext(I18nContext); + const [messageIsScrolled, setMessageIsScrolled] = useState(false); + const setMessageIsScrolledAtBottom = () => { + if (!messageRootRef || messageIsScrolled) { + return; + } + + const { scrollTop, offsetHeight, scrollHeight } = messageRootRef; + const isAtBottom = Math.round(scrollTop) + offsetHeight >= scrollHeight; + + if (isAtBottom) { + setMessageIsScrolled(true); + onMessageScrolled(); + } + }; + + return ( + + {messageIsScrollable ? ( + { + setMessageIsScrolled(true); + onMessageScrolled(); + messageRootRef?.scrollTo(0, messageRootRef?.scrollHeight); + }} + className="signature-request-message__scroll-button" + data-testid="signature-request-scroll-button" + > + + + ) : null} + + + {t('signatureRequest1')} + + + + + ); +} + +SignatureRequestMessage.propTypes = { + data: PropTypes.object.isRequired, + onMessageScrolled: PropTypes.func, + setMessageRootRef: PropTypes.func, + messageRootRef: PropTypes.object, + messageIsScrollable: PropTypes.bool, +}; diff --git a/ui/components/app/signature-request/signature-request-message/signature-request-message.stories.js b/ui/components/app/signature-request/signature-request-message/signature-request-message.stories.js new file mode 100644 index 000000000..9d3743af0 --- /dev/null +++ b/ui/components/app/signature-request/signature-request-message/signature-request-message.stories.js @@ -0,0 +1,58 @@ +import React from 'react'; +import SignatureRequestMessage from './signature-request-message'; + +export default { + title: 'Components/App/SignatureRequestMessage', + id: __filename, + component: SignatureRequestMessage, + argTypes: { + data: { control: 'object' }, + onMessageScrolled: { action: 'onMessageScrolled' }, + setMessageRootRef: { action: 'setMessageRootRef' }, + messageRootRef: { control: 'object' }, + messageIsScrollable: { control: 'boolean' }, + }, +}; + +export const DefaultStory = (args) => { + return ; +}; + +DefaultStory.storyName = 'Default'; + +DefaultStory.args = { + data: JSON.parse( + JSON.stringify({ + domain: { + name: 'happydapp.website', + }, + message: { + string: 'haay wuurl', + number: 42, + }, + primaryType: 'Mail', + types: { + EIP712Domain: [ + { name: 'name', type: 'string' }, + { name: 'version', type: 'string' }, + { name: 'chainId', type: 'uint256' }, + { name: 'verifyingContract', type: 'address' }, + ], + Group: [ + { name: 'name', type: 'string' }, + { name: 'members', type: 'Person[]' }, + ], + Mail: [ + { name: 'from', type: 'Person' }, + { name: 'to', type: 'Person[]' }, + { name: 'contents', type: 'string' }, + ], + Person: [ + { name: 'name', type: 'string' }, + { name: 'wallets', type: 'address[]' }, + ], + }, + }), + ), + messageIsScrollable: true, +}; diff --git a/ui/components/app/signature-request/signature-request.container.test.js b/ui/components/app/signature-request/signature-request.container.test.js index 551e9645e..085b117d4 100644 --- a/ui/components/app/signature-request/signature-request.container.test.js +++ b/ui/components/app/signature-request/signature-request.container.test.js @@ -8,6 +8,46 @@ import SignatureRequest from './signature-request.container'; describe('Signature Request', () => { const mockStore = { metamask: { + tokenList: { + '0x514910771af9ca656af840dff83e8264ecf986ca': { + address: '0x514910771af9ca656af840dff83e8264ecf986ca', + symbol: 'LINK', + decimals: 18, + name: 'ChainLink Token', + iconUrl: + 'https://crypto.com/price/coin-data/icon/LINK/color_icon.png', + aggregators: [ + 'Aave', + 'Bancor', + 'CMC', + 'Crypto.com', + 'CoinGecko', + '1inch', + 'Paraswap', + 'PMM', + 'Zapper', + 'Zerion', + '0x', + ], + occurrences: 12, + unlisted: false, + }, + }, + identities: { + '0xb19ac54efa18cc3a14a5b821bfec73d284bf0c5e': { + name: 'Account 2', + address: '0xb19ac54efa18cc3a14a5b821bfec73d284bf0c5e', + }, + }, + addressBook: { + undefined: { + 0: { + address: '0x39a4e4Af7cCB654dB9500F258c64781c8FbD39F0', + name: '', + isEns: false, + }, + }, + }, provider: { type: 'rpc', }, diff --git a/ui/components/app/transaction-decoding/components/decoding/address/address.component.js b/ui/components/app/transaction-decoding/components/decoding/address/address.component.js index 7e6358634..ddb4d5cd7 100644 --- a/ui/components/app/transaction-decoding/components/decoding/address/address.component.js +++ b/ui/components/app/transaction-decoding/components/decoding/address/address.component.js @@ -5,7 +5,10 @@ import copyToClipboard from 'copy-to-clipboard'; import { shortenAddress } from '../../../../../../helpers/utils/util'; import Identicon from '../../../../../ui/identicon'; import { useI18nContext } from '../../../../../../hooks/useI18nContext'; -import { getAddressBook } from '../../../../../../selectors'; +import { + getMetadataContractName, + getAddressBook, +} from '../../../../../../selectors'; import NicknamePopovers from '../../../../modals/nickname-popovers'; const Address = ({ @@ -20,15 +23,25 @@ const Address = ({ const addressBook = useSelector(getAddressBook); const addressBookEntryObject = addressBook.find( - (entry) => entry.address === checksummedRecipientAddress, + (entry) => + entry.address.toLowerCase() === checksummedRecipientAddress.toLowerCase(), ); const recipientNickname = addressBookEntryObject?.name; + const recipientMetadataName = useSelector((state) => + getMetadataContractName(state, checksummedRecipientAddress), + ); const recipientToRender = addressOnly - ? recipientNickname || + ? recipientName || + recipientNickname || + recipientMetadataName || recipientEns || shortenAddress(checksummedRecipientAddress) - : recipientNickname || recipientEns || recipientName || t('newContract'); + : recipientName || + recipientNickname || + recipientMetadataName || + recipientEns || + t('newContract'); return (
{ recipientEns = getEnsResolutionByAddress(state, address); } const addressBook = getAddressBook(state); - const recipientName = getAccountName(state, recipientAddress); - const recipientMetadataName = getMetadataContractName(recipientAddress); + const identities = getMetaMaskIdentities(state); + const recipientName = getAccountName(identities, recipientAddress); + const recipientMetadataName = getMetadataContractName( + state, + recipientAddress, + ); const getNickName = (address) => { const entry = addressBook.find((contact) => { diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 7356a1dfc..a4ce54be8 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -412,15 +412,16 @@ export function getAddressBookEntryOrAccountName(state, address) { return entry && entry.name !== '' ? entry.name : address; } -export function getAccountName(state, address) { - const entry = Object.values(state.metamask.identities).find((identity) => +export function getAccountName(identities, address) { + const entry = Object.values(identities).find((identity) => isEqualCaseInsensitive(identity.address, toChecksumHexAddress(address)), ); return entry && entry.name !== '' ? entry.name : ''; } -export function getMetadataContractName(address) { - const entry = Object.values(STATIC_MAINNET_TOKEN_LIST).find((identity) => +export function getMetadataContractName(state, address) { + const tokenList = getTokenList(state); + const entry = Object.values(tokenList).find((identity) => isEqualCaseInsensitive(identity.address, toChecksumHexAddress(address)), ); return entry && entry.name !== '' ? entry.name : '';