From 332d2407b6d1f27cdae68fea065d8b6b368cd140 Mon Sep 17 00:00:00 2001 From: meriadec Date: Wed, 8 Aug 2018 11:53:21 +0200 Subject: [PATCH] Display warning instead of rejecting ETH address without checksum Closes #1397 This commit introduce a new `warning` prop on Input component. `error` prop will always takes precedence over it if present, btw. We don't reject anymore recipient validity if address is either full lowercase OR full uppercase, to support legacy formats: having a warning will allow user to go to next step. Obviously the fix is available for ETC as well. Wording to be reviewed :) --- src/bridge/EthereumJSBridge.js | 28 +++++++++++++++-- src/bridge/LibcoreBridge.js | 1 + src/bridge/RippleJSBridge.js | 1 + src/bridge/UnsupportedBridge.js | 1 + src/bridge/makeMockBridge.js | 1 + src/bridge/types.js | 1 + src/components/base/Input/index.js | 19 +++++++++++- .../modals/Send/fields/RecipientField.js | 30 ++++++++++--------- src/config/errors.js | 1 + src/styles/theme.js | 1 + static/i18n/en/errors.json | 4 +++ 11 files changed, 70 insertions(+), 18 deletions(-) diff --git a/src/bridge/EthereumJSBridge.js b/src/bridge/EthereumJSBridge.js index ec1b6892..e68f70d2 100644 --- a/src/bridge/EthereumJSBridge.js +++ b/src/bridge/EthereumJSBridge.js @@ -16,11 +16,9 @@ import { getDerivations } from 'helpers/derivations' import getAddressCommand from 'commands/getAddress' import signTransactionCommand from 'commands/signTransaction' import { getAccountPlaceholderName, getNewAccountPlaceholderName } from 'helpers/accountName' -import { NotEnoughBalance } from 'config/errors' +import { NotEnoughBalance, ETHAddressNonEIP } from 'config/errors' import type { EditProps, WalletBridge } from './types' -// TODO in future it would be neat to support eip55 - type Transaction = { recipient: string, amount: BigNumber, @@ -99,6 +97,16 @@ const txToOps = (account: Account) => (tx: Tx): Operation[] => { } function isRecipientValid(currency, recipient) { + if (!recipient.match(/^0x[0-9a-fA-F]{40}$/)) return false + + // To handle non-eip55 addresses we stop validation here if we detect + // address is either full upper or full lower. + // see https://github.com/LedgerHQ/ledger-live-desktop/issues/1397 + const slice = recipient.substr(2) + const isFullUpper = slice === slice.toUpperCase() + const isFullLower = slice === slice.toLowerCase() + if (isFullUpper || isFullLower) return true + try { return eip55.verify(recipient) } catch (error) { @@ -106,6 +114,18 @@ function isRecipientValid(currency, recipient) { } } +// Returns a warning if we detect a non-eip address +function getRecipientWarning(currency, recipient) { + if (!recipient.match(/^0x[0-9a-fA-F]{40}$/)) return null + const slice = recipient.substr(2) + const isFullUpper = slice === slice.toUpperCase() + const isFullLower = slice === slice.toLowerCase() + if (isFullUpper || isFullLower) { + return new ETHAddressNonEIP() + } + return null +} + function mergeOps(existing: Operation[], newFetched: Operation[]) { const ids = newFetched.map(o => o.id) const all = newFetched.concat(existing.filter(o => !ids.includes(o.id))) @@ -376,6 +396,8 @@ const EthereumBridge: WalletBridge = { pullMoreOperations: () => Promise.resolve(a => a), // NOT IMPLEMENTED isRecipientValid: (currency, recipient) => Promise.resolve(isRecipientValid(currency, recipient)), + getRecipientWarning: (currency, recipient) => + Promise.resolve(getRecipientWarning(currency, recipient)), createTransaction: () => ({ amount: BigNumber(0), diff --git a/src/bridge/LibcoreBridge.js b/src/bridge/LibcoreBridge.js index d9638460..642788d1 100644 --- a/src/bridge/LibcoreBridge.js +++ b/src/bridge/LibcoreBridge.js @@ -165,6 +165,7 @@ const LibcoreBridge: WalletBridge = { pullMoreOperations: () => Promise.reject(notImplemented), isRecipientValid, + getRecipientWarning: () => Promise.resolve(null), createTransaction: () => ({ amount: BigNumber(0), diff --git a/src/bridge/RippleJSBridge.js b/src/bridge/RippleJSBridge.js index e12655a5..f872eeb6 100644 --- a/src/bridge/RippleJSBridge.js +++ b/src/bridge/RippleJSBridge.js @@ -447,6 +447,7 @@ const RippleJSBridge: WalletBridge = { pullMoreOperations: () => Promise.resolve(a => a), // FIXME not implemented isRecipientValid: (currency, recipient) => Promise.resolve(isRecipientValid(currency, recipient)), + getRecipientWarning: () => Promise.resolve(null), createTransaction: () => ({ amount: BigNumber(0), diff --git a/src/bridge/UnsupportedBridge.js b/src/bridge/UnsupportedBridge.js index 37b6edae..6f5d5c12 100644 --- a/src/bridge/UnsupportedBridge.js +++ b/src/bridge/UnsupportedBridge.js @@ -19,6 +19,7 @@ const UnsupportedBridge: WalletBridge<*> = { pullMoreOperations: () => Promise.reject(genericError), isRecipientValid: () => Promise.reject(genericError), + getRecipientWarning: () => Promise.reject(genericError), createTransaction: () => null, diff --git a/src/bridge/makeMockBridge.js b/src/bridge/makeMockBridge.js index 6d057a8e..909dfe91 100644 --- a/src/bridge/makeMockBridge.js +++ b/src/bridge/makeMockBridge.js @@ -129,6 +129,7 @@ function makeMockBridge(opts?: Opts): WalletBridge<*> { }, isRecipientValid: (currency, recipient) => Promise.resolve(recipient.length > 0), + getRecipientWarning: () => Promise.resolve(null), createTransaction: () => ({ amount: BigNumber(0), diff --git a/src/bridge/types.js b/src/bridge/types.js index 244a1251..53a28250 100644 --- a/src/bridge/types.js +++ b/src/bridge/types.js @@ -53,6 +53,7 @@ export interface WalletBridge { pullMoreOperations(initialAccount: Account, count: number): Promise<(Account) => Account>; isRecipientValid(currency: Currency, recipient: string): Promise; + getRecipientWarning(currency: Currency, recipient: string): Promise; // Related to send funds: diff --git a/src/components/base/Input/index.js b/src/components/base/Input/index.js index 7bc86011..f8f4aa67 100644 --- a/src/components/base/Input/index.js +++ b/src/components/base/Input/index.js @@ -18,7 +18,13 @@ const Container = styled(Box).attrs({ border-width: 1px; border-style: solid; border-color: ${p => - p.error ? p.theme.colors.pearl : p.isFocus ? p.theme.colors.wallet : p.theme.colors.fog}; + p.error + ? p.theme.colors.pearl + : p.warning + ? p.theme.colors.warning + : p.isFocus + ? p.theme.colors.wallet + : p.theme.colors.fog}; box-shadow: ${p => (p.isFocus ? `rgba(0, 0, 0, 0.05) 0 2px 2px` : 'none')}; height: ${p => (p.small ? '34' : '40')}px; position: relative; @@ -38,6 +44,10 @@ const ErrorDisplay = styled(Box)` color: ${p => p.theme.colors.pearl}; ` +const WarningDisplay = styled(ErrorDisplay)` + color: ${p => p.theme.colors.warning}; +` + const Base = styled.input.attrs({ ff: p => (p.ff || p.small ? 'Open Sans' : 'Open Sans|SemiBold'), fontSize: 4, @@ -89,6 +99,7 @@ type Props = { renderRight?: any, containerProps?: Object, error?: ?Error | boolean, + warning?: ?Error | boolean, small?: boolean, editInPlace?: boolean, } @@ -171,6 +182,7 @@ class Input extends PureComponent { editInPlace, small, error, + warning, ...props } = this.props @@ -182,6 +194,7 @@ class Input extends PureComponent { {...containerProps} small={small} error={error} + warning={warning} editInPlace={editInPlace} > {renderLeft} @@ -199,6 +212,10 @@ class Input extends PureComponent { + ) : warning ? ( + + + ) : null} {renderRight} diff --git a/src/components/modals/Send/fields/RecipientField.js b/src/components/modals/Send/fields/RecipientField.js index c7edf048..c037a150 100644 --- a/src/components/modals/Send/fields/RecipientField.js +++ b/src/components/modals/Send/fields/RecipientField.js @@ -20,9 +20,13 @@ type Props = { autoFocus?: boolean, } -class RecipientField extends Component, { isValid: boolean }> { +class RecipientField extends Component< + Props, + { isValid: boolean, warning: ?Error }, +> { state = { isValid: true, + warning: null, } componentDidMount() { this.resync() @@ -42,12 +46,11 @@ class RecipientField extends Component, { isVali async resync() { const { account, bridge, transaction } = this.props const syncId = ++this.syncId - const isValid = await bridge.isRecipientValid( - account.currency, - bridge.getTransactionRecipient(account, transaction), - ) + const recipient = bridge.getTransactionRecipient(account, transaction) + const isValid = await bridge.isRecipientValid(account.currency, recipient) + const warning = await bridge.getRecipientWarning(account.currency, recipient) if (syncId !== this.syncId) return - this.setState({ isValid }) + this.setState({ isValid, warning }) } onChange = (recipient: string, maybeExtra: ?Object) => { @@ -69,8 +72,12 @@ class RecipientField extends Component, { isVali } render() { const { bridge, account, transaction, t, autoFocus } = this.props - const { isValid } = this.state + const { isValid, warning } = this.state const value = bridge.getTransactionRecipient(account, transaction) + + const error = + !value || isValid ? null : new InvalidAddress(null, { currencyName: account.currency.name }) + return ( extends Component, { isVali diff --git a/src/config/errors.js b/src/config/errors.js index 069389b8..5afc3930 100644 --- a/src/config/errors.js +++ b/src/config/errors.js @@ -39,6 +39,7 @@ export const UserRefusedOnDevice = createCustomErrorClass('UserRefusedOnDevice') export const WebsocketConnectionError = createCustomErrorClass('WebsocketConnectionError') export const WebsocketConnectionFailed = createCustomErrorClass('WebsocketConnectionFailed') export const WrongDeviceForAccount = createCustomErrorClass('WrongDeviceForAccount') +export const ETHAddressNonEIP = createCustomErrorClass('ETHAddressNonEIP') // db stuff, no need to translate export const NoDBPathGiven = createCustomErrorClass('NoDBPathGiven') diff --git a/src/styles/theme.js b/src/styles/theme.js index 4914185f..ff165f59 100644 --- a/src/styles/theme.js +++ b/src/styles/theme.js @@ -67,6 +67,7 @@ export const colors = { // new colors alertRed: '#ea2e49', + warning: '#f57f17', black: '#000000', dark: '#142533', fog: '#d8d8d8', diff --git a/static/i18n/en/errors.json b/static/i18n/en/errors.json index 5425a74f..16bae1c4 100644 --- a/static/i18n/en/errors.json +++ b/static/i18n/en/errors.json @@ -161,5 +161,9 @@ "CantOpenDevice": { "title": "Oops, couldn’t connect to device", "description": "Device detected but connection failed. Please retry and get help if the problem persists." + }, + "ETHAddressNonEIP": { + "title": "Auto-verification not available: carefully verify the address", + "description": null } }