From 145079b8c7324a87e0cf7ffd29470384df931f9a Mon Sep 17 00:00:00 2001 From: David da Silva Rosa <712580+dasilvarosa@users.noreply.github.com> Date: Tue, 7 Aug 2018 16:55:51 +0200 Subject: [PATCH 1/3] Generalize app dependency error messages. App dependencies are not exclusive to the BTC app anymore, so we need to generalize the error messages. E.g., previously, you would see an error message that you need the Bitcoin app, when trying to install RSK that requires the ETH app. --- static/i18n/en/errors.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/static/i18n/en/errors.json b/static/i18n/en/errors.json index 154b6da5..5425a74f 100644 --- a/static/i18n/en/errors.json +++ b/static/i18n/en/errors.json @@ -72,8 +72,8 @@ "description": "Check your device to see which apps are already installed." }, "ManagerAppRelyOnBTC": { - "title": "Bitcoin app required", - "description": "Install the latest version of the Bitcoin app before installing this app." + "title": "Bitcoin or Ethereum app required", + "description": "Either install the latest Ethereum app (RSK/WAN/kUSD/POA), or the latest Bitcoin app." }, "ManagerDeviceLocked": { "title": "Please unlock your device", @@ -84,8 +84,8 @@ "description": "Uninstall some apps to increase available storage and try again." }, "ManagerUninstallBTCDep": { - "title": "Sorry, Bitcoin is required", - "description": "First uninstall apps that depend on Bitcoin." + "title": "Sorry, this app is required", + "description": "Uninstall the Bitcoin or Ethereum app last." }, "NetworkDown": { "title": "Oops, internet seems down", From b8cbabf7b496c3c404fc1a3a88de33d0a3fbf797 Mon Sep 17 00:00:00 2001 From: David da Silva Rosa <712580+dasilvarosa@users.noreply.github.com> Date: Wed, 8 Aug 2018 12:12:36 +0200 Subject: [PATCH 2/3] Update errors.json --- static/i18n/en/errors.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/i18n/en/errors.json b/static/i18n/en/errors.json index 5425a74f..227a9be7 100644 --- a/static/i18n/en/errors.json +++ b/static/i18n/en/errors.json @@ -73,7 +73,7 @@ }, "ManagerAppRelyOnBTC": { "title": "Bitcoin or Ethereum app required", - "description": "Either install the latest Ethereum app (RSK/WAN/kUSD/POA), or the latest Bitcoin app." + "description": "Either install the latest Ethereum app (for ETC/UBIQ/EXP/RSK/WAN/kUSD/POA), or the latest Bitcoin app." }, "ManagerDeviceLocked": { "title": "Please unlock your device", From 68585438cd47d0435f12e2618de3d45c35f27a29 Mon Sep 17 00:00:00 2001 From: meriadec Date: Wed, 8 Aug 2018 11:53:21 +0200 Subject: [PATCH 3/3] 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 | 27 +++++++++++++++-- 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(+), 17 deletions(-) diff --git a/src/bridge/EthereumJSBridge.js b/src/bridge/EthereumJSBridge.js index d07b3b62..65be4faa 100644 --- a/src/bridge/EthereumJSBridge.js +++ b/src/bridge/EthereumJSBridge.js @@ -17,12 +17,11 @@ import getAddressCommand from 'commands/getAddress' import signTransactionCommand from 'commands/signTransaction' import { getAccountPlaceholderName, getNewAccountPlaceholderName } from 'helpers/accountName' import { createCustomErrorClass } from 'helpers/errors' +import { ETHAddressNonEIP } from 'config/errors' import type { EditProps, WalletBridge } from './types' const NotEnoughBalance = createCustomErrorClass('NotEnoughBalance') -// TODO in future it would be neat to support eip55 - type Transaction = { recipient: string, amount: BigNumber, @@ -101,6 +100,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) { @@ -108,6 +117,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))) @@ -378,6 +399,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 fd11f02e..f2e3511e 100644 --- a/src/bridge/LibcoreBridge.js +++ b/src/bridge/LibcoreBridge.js @@ -166,6 +166,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 a201b621..d720c5aa 100644 --- a/src/bridge/RippleJSBridge.js +++ b/src/bridge/RippleJSBridge.js @@ -449,6 +449,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 872a6dd7..769e1483 100644 --- a/src/components/modals/Send/fields/RecipientField.js +++ b/src/components/modals/Send/fields/RecipientField.js @@ -22,9 +22,13 @@ type Props = { const InvalidAddress = createCustomErrorClass('InvalidAddress') -class RecipientField extends Component, { isValid: boolean }> { +class RecipientField extends Component< + Props, + { isValid: boolean, warning: ?Error }, +> { state = { isValid: true, + warning: null, } componentDidMount() { this.resync() @@ -44,12 +48,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) => { @@ -71,8 +74,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 d6140d6c..9f865d2e 100644 --- a/src/config/errors.js +++ b/src/config/errors.js @@ -13,6 +13,7 @@ export const WrongDeviceForAccount = createCustomErrorClass('WrongDeviceForAccou export const DeviceNotGenuineError = createCustomErrorClass('DeviceNotGenuine') export const DeviceGenuineSocketEarlyClose = createCustomErrorClass('DeviceGenuineSocketEarlyClose') export const TimeoutTagged = createCustomErrorClass('TimeoutTagged') +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 227a9be7..5c021f62 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 } }