Browse Source

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 :)
master
meriadec 7 years ago
parent
commit
332d2407b6
No known key found for this signature in database GPG Key ID: 1D2FC2305E2CB399
  1. 28
      src/bridge/EthereumJSBridge.js
  2. 1
      src/bridge/LibcoreBridge.js
  3. 1
      src/bridge/RippleJSBridge.js
  4. 1
      src/bridge/UnsupportedBridge.js
  5. 1
      src/bridge/makeMockBridge.js
  6. 1
      src/bridge/types.js
  7. 19
      src/components/base/Input/index.js
  8. 30
      src/components/modals/Send/fields/RecipientField.js
  9. 1
      src/config/errors.js
  10. 1
      src/styles/theme.js
  11. 4
      static/i18n/en/errors.json

28
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<Transaction> = {
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),

1
src/bridge/LibcoreBridge.js

@ -165,6 +165,7 @@ const LibcoreBridge: WalletBridge<Transaction> = {
pullMoreOperations: () => Promise.reject(notImplemented),
isRecipientValid,
getRecipientWarning: () => Promise.resolve(null),
createTransaction: () => ({
amount: BigNumber(0),

1
src/bridge/RippleJSBridge.js

@ -447,6 +447,7 @@ const RippleJSBridge: WalletBridge<Transaction> = {
pullMoreOperations: () => Promise.resolve(a => a), // FIXME not implemented
isRecipientValid: (currency, recipient) => Promise.resolve(isRecipientValid(currency, recipient)),
getRecipientWarning: () => Promise.resolve(null),
createTransaction: () => ({
amount: BigNumber(0),

1
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,

1
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),

1
src/bridge/types.js

@ -53,6 +53,7 @@ export interface WalletBridge<Transaction> {
pullMoreOperations(initialAccount: Account, count: number): Promise<(Account) => Account>;
isRecipientValid(currency: Currency, recipient: string): Promise<boolean>;
getRecipientWarning(currency: Currency, recipient: string): Promise<?Error>;
// Related to send funds:

19
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<Props, State> {
editInPlace,
small,
error,
warning,
...props
} = this.props
@ -182,6 +194,7 @@ class Input extends PureComponent<Props, State> {
{...containerProps}
small={small}
error={error}
warning={warning}
editInPlace={editInPlace}
>
{renderLeft}
@ -199,6 +212,10 @@ class Input extends PureComponent<Props, State> {
<ErrorDisplay>
<TranslatedError error={error} />
</ErrorDisplay>
) : warning ? (
<WarningDisplay>
<TranslatedError error={warning} />
</WarningDisplay>
) : null}
</Box>
{renderRight}

30
src/components/modals/Send/fields/RecipientField.js

@ -20,9 +20,13 @@ type Props<Transaction> = {
autoFocus?: boolean,
}
class RecipientField<Transaction> extends Component<Props<Transaction>, { isValid: boolean }> {
class RecipientField<Transaction> extends Component<
Props<Transaction>,
{ isValid: boolean, warning: ?Error },
> {
state = {
isValid: true,
warning: null,
}
componentDidMount() {
this.resync()
@ -42,12 +46,11 @@ class RecipientField<Transaction> extends Component<Props<Transaction>, { 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<Transaction> extends Component<Props<Transaction>, { 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 (
<Box flow={1}>
<LabelWithExternalIcon
@ -80,13 +87,8 @@ class RecipientField<Transaction> extends Component<Props<Transaction>, { isVali
<RecipientAddress
autoFocus={autoFocus}
withQrCode={false}
error={
!value || isValid
? null
: new InvalidAddress(null, {
currencyName: account.currency.name,
})
}
error={error}
warning={warning}
value={value}
onChange={this.onChange}
/>

1
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')

1
src/styles/theme.js

@ -67,6 +67,7 @@ export const colors = {
// new colors
alertRed: '#ea2e49',
warning: '#f57f17',
black: '#000000',
dark: '#142533',
fog: '#d8d8d8',

4
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
}
}

Loading…
Cancel
Save