Merge pull request #1536 from gre/fees-failsafe

Add fail-safe for Fees to block the UI until loaded

src/bridge/EthereumJSBridge.js

@@ -16,20 +16,20 @@ import { getDerivations } from 'helpers/derivations'
 import getAddressCommand from 'commands/getAddress'
 import signTransactionCommand from 'commands/signTransaction'
 import { getAccountPlaceholderName, getNewAccountPlaceholderName } from 'helpers/accountName'
-import { NotEnoughBalance, ETHAddressNonEIP } from 'config/errors'
+import { NotEnoughBalance, FeeNotLoaded, ETHAddressNonEIP } from 'config/errors'
 import type { EditProps, WalletBridge } from './types'
 
 type Transaction = {
   recipient: string,
   amount: BigNumber,
-  gasPrice: BigNumber,
+  gasPrice: ?BigNumber,
   gasLimit: BigNumber,
 }
 
 const serializeTransaction = t => ({
   recipient: t.recipient,
   amount: `0x${BigNumber(t.amount).toString(16)}`,
-  gasPrice: `0x${BigNumber(t.gasPrice).toString(16)}`,
+  gasPrice: !t.gasPrice ? '0x00' : `0x${BigNumber(t.gasPrice).toString(16)}`,
   gasLimit: `0x${BigNumber(t.gasLimit).toString(16)}`,
 })
 
@@ -140,6 +140,8 @@ const signAndBroadcast = async ({
   onSigned,
   onOperationBroadcasted,
 }) => {
+  const { gasPrice, amount, gasLimit } = t
+  if (!gasPrice) throw new FeeNotLoaded()
   const api = apiForCurrency(a.currency)
 
   const nonce = await api.getAccountNonce(a.freshAddress)
@@ -162,8 +164,8 @@ const signAndBroadcast = async ({
       id: `${a.id}-${hash}-OUT`,
       hash,
       type: 'OUT',
-      value: t.amount,
-      fee: t.gasPrice.times(t.gasLimit),
+      value: amount,
+      fee: gasPrice.times(gasLimit),
       blockHeight: null,
       blockHash: null,
       accountId: a.id,
@@ -402,7 +404,7 @@ const EthereumBridge: WalletBridge<Transaction> = {
   createTransaction: () => ({
     amount: BigNumber(0),
     recipient: '',
-    gasPrice: BigNumber(0),
+    gasPrice: null,
     gasLimit: BigNumber(0x5208),
   }),
 
@@ -425,16 +427,22 @@ const EthereumBridge: WalletBridge<Transaction> = {
   EditAdvancedOptions,
 
   checkValidTransaction: (a, t) =>
-    t.amount.isLessThanOrEqualTo(a.balance)
+    !t.gasPrice
+      ? Promise.reject(new FeeNotLoaded())
+      : t.amount.isLessThanOrEqualTo(a.balance)
         ? Promise.resolve(true)
         : Promise.reject(new NotEnoughBalance()),
 
   getTotalSpent: (a, t) =>
-    t.amount.isGreaterThan(0) && t.gasPrice.isGreaterThan(0) && t.gasLimit.isGreaterThan(0)
+    t.amount.isGreaterThan(0) &&
+    t.gasPrice &&
+    t.gasPrice.isGreaterThan(0) &&
+    t.gasLimit.isGreaterThan(0)
       ? Promise.resolve(t.amount.plus(t.gasPrice.times(t.gasLimit)))
       : Promise.resolve(BigNumber(0)),
 
-  getMaxAmount: (a, t) => Promise.resolve(a.balance.minus(t.gasPrice.times(t.gasLimit))),
+  getMaxAmount: (a, t) =>
+    Promise.resolve(a.balance.minus((t.gasPrice || BigNumber(0)).times(t.gasLimit))),
 
   signAndBroadcast: (a, t, deviceId) =>
     Observable.create(o => {

src/bridge/LibcoreBridge.js

@@ -11,7 +11,7 @@ import libcoreSyncAccount from 'commands/libcoreSyncAccount'
 import libcoreSignAndBroadcast from 'commands/libcoreSignAndBroadcast'
 import libcoreGetFees, { extractGetFeesInputFromAccount } from 'commands/libcoreGetFees'
 import libcoreValidAddress from 'commands/libcoreValidAddress'
-import { NotEnoughBalance } from 'config/errors'
+import { NotEnoughBalance, FeeNotLoaded } from 'config/errors'
 import type { WalletBridge, EditProps } from './types'
 
 const NOT_ENOUGH_FUNDS = 52
@@ -20,15 +20,18 @@ const notImplemented = new Error('LibcoreBridge: not implemented')
 
 type Transaction = {
   amount: BigNumber,
-  feePerByte: BigNumber,
+  feePerByte: ?BigNumber,
   recipient: string,
 }
 
-const serializeTransaction = t => ({
+const serializeTransaction = t => {
+  const { feePerByte } = t
+  return {
     recipient: t.recipient,
     amount: t.amount.toString(),
-  feePerByte: t.feePerByte.toString(),
-})
+    feePerByte: (feePerByte && feePerByte.toString()) || '0',
+  }
+}
 
 const decodeOperation = (encodedAccount, rawOp) =>
   decodeAccount({ ...encodedAccount, operations: [rawOp] }).operations[0]
@@ -75,7 +78,9 @@ const isRecipientValid = (currency, recipient) => {
 const feesLRU = LRU({ max: 100 })
 
 const getFeesKey = (a, t) =>
-  `${a.id}_${a.blockHeight || 0}_${t.amount.toString()}_${t.recipient}_${t.feePerByte.toString()}`
+  `${a.id}_${a.blockHeight || 0}_${t.amount.toString()}_${t.recipient}_${
+    t.feePerByte ? t.feePerByte.toString() : ''
+  }`
 
 const getFees = async (a, transaction) => {
   const isValid = await isRecipientValid(a.currency, transaction.recipient)
@@ -83,6 +88,7 @@ const getFees = async (a, transaction) => {
   const key = getFeesKey(a, transaction)
   let promise = feesLRU.get(key)
   if (promise) return promise
+
   promise = libcoreGetFees
     .send({
       ...extractGetFeesInputFromAccount(a),
@@ -95,7 +101,9 @@ const getFees = async (a, transaction) => {
 }
 
 const checkValidTransaction = (a, t) =>
-  !t.amount
+  !t.feePerByte
+    ? Promise.reject(new FeeNotLoaded())
+    : !t.amount
       ? Promise.resolve(true)
       : getFees(a, t)
           .then(() => true)
@@ -169,7 +177,7 @@ const LibcoreBridge: WalletBridge<Transaction> = {
   createTransaction: () => ({
     amount: BigNumber(0),
     recipient: '',
-    feePerByte: BigNumber(0),
+    feePerByte: null,
     isRBF: false,
   }),
 

src/bridge/RippleJSBridge.js

@@ -20,13 +20,17 @@ import {
 import FeesRippleKind from 'components/FeesField/RippleKind'
 import AdvancedOptionsRippleKind from 'components/AdvancedOptions/RippleKind'
 import { getAccountPlaceholderName, getNewAccountPlaceholderName } from 'helpers/accountName'
-import { NotEnoughBalance, NotEnoughBalanceBecauseDestinationNotCreated } from 'config/errors'
+import {
+  NotEnoughBalance,
+  FeeNotLoaded,
+  NotEnoughBalanceBecauseDestinationNotCreated,
+} from 'config/errors'
 import type { WalletBridge, EditProps } from './types'
 
 type Transaction = {
   amount: BigNumber,
   recipient: string,
-  fee: BigNumber,
+  fee: ?BigNumber,
   tag: ?number,
 }
 
@@ -51,6 +55,8 @@ const EditAdvancedOptions = ({ onChange, value }: EditProps<Transaction>) => (
 
 async function signAndBroadcast({ a, t, deviceId, isCancelled, onSigned, onOperationBroadcasted }) {
   const api = apiForEndpointConfig(a.endpointConfig)
+  const { fee } = t
+  if (!fee) throw new FeeNotLoaded()
   try {
     await api.connect()
     const amount = formatAPICurrencyXRP(t.amount)
@@ -66,7 +72,7 @@ async function signAndBroadcast({ a, t, deviceId, isCancelled, onSigned, onOpera
       },
     }
     const instruction = {
-      fee: formatAPICurrencyXRP(t.fee).value,
+      fee: formatAPICurrencyXRP(fee).value,
       maxLedgerVersionOffset: 12,
     }
 
@@ -97,7 +103,7 @@ async function signAndBroadcast({ a, t, deviceId, isCancelled, onSigned, onOpera
         accountId: a.id,
         type: 'OUT',
         value: t.amount,
-        fee: t.fee,
+        fee,
         blockHash: null,
         blockHeight: null,
         senders: [a.freshAddress],
@@ -477,7 +483,7 @@ const RippleJSBridge: WalletBridge<Transaction> = {
   createTransaction: () => ({
     amount: BigNumber(0),
     recipient: '',
-    fee: BigNumber(0),
+    fee: null,
     tag: undefined,
   }),
 
@@ -520,6 +526,7 @@ const RippleJSBridge: WalletBridge<Transaction> = {
   getTransactionRecipient: (a, t) => t.recipient,
 
   checkValidTransaction: async (a, t) => {
+    if (!t.fee) throw new FeeNotLoaded()
     const r = await getServerInfo(a.endpointConfig)
     const reserveBaseXRP = parseAPIValue(r.validatedLedger.reserveBaseXRP)
     if (t.recipient) {
@@ -534,7 +541,7 @@ const RippleJSBridge: WalletBridge<Transaction> = {
     }
     if (
       t.amount
-        .plus(t.fee)
+        .plus(t.fee || 0)
         .plus(reserveBaseXRP)
         .isLessThanOrEqualTo(a.balance)
     ) {
@@ -543,9 +550,9 @@ const RippleJSBridge: WalletBridge<Transaction> = {
     throw new NotEnoughBalance()
   },
 
-  getTotalSpent: (a, t) => Promise.resolve(t.amount.plus(t.fee)),
+  getTotalSpent: (a, t) => Promise.resolve(t.amount.plus(t.fee || 0)),
 
-  getMaxAmount: (a, t) => Promise.resolve(a.balance.minus(t.fee)),
+  getMaxAmount: (a, t) => Promise.resolve(a.balance.minus(t.fee || 0)),
 
   signAndBroadcast: (a, t, deviceId) =>
     Observable.create(o => {

src/components/FeesField/BitcoinKind.js

@@ -8,6 +8,7 @@ import { translate } from 'react-i18next'
 
 import type { T } from 'types/common'
 
+import { FeeNotLoaded } from 'config/errors'
 import InputCurrency from 'components/base/InputCurrency'
 import Select from 'components/base/Select'
 import type { Fees } from 'api/Fees'
@@ -17,7 +18,7 @@ import Box from '../base/Box'
 
 type Props = {
   account: Account,
-  feePerByte: BigNumber,
+  feePerByte: ?BigNumber,
   onChange: BigNumber => void,
   t: T,
 }
@@ -50,6 +51,12 @@ const customItem = {
   blockCount: 0,
   feePerByte: BigNumber(0),
 }
+const notLoadedItem = {
+  label: 'Standard',
+  value: 'standard',
+  blockCount: 0,
+  feePerByte: BigNumber(0),
+}
 
 type State = { isFocused: boolean, items: FeeItem[], selectedItem: FeeItem }
 
@@ -57,13 +64,13 @@ type OwnProps = Props & { fees?: Fees, error?: Error }
 
 class FeesField extends Component<OwnProps, State> {
   state = {
-    items: [customItem],
-    selectedItem: customItem,
+    items: [notLoadedItem],
+    selectedItem: notLoadedItem,
     isFocused: false,
   }
 
   static getDerivedStateFromProps(nextProps, prevState) {
-    const { fees, feePerByte } = nextProps
+    const { fees, feePerByte, error } = nextProps
     let items: FeeItem[] = []
     if (fees) {
       for (const key of Object.keys(fees)) {
@@ -80,8 +87,9 @@ class FeesField extends Component<OwnProps, State> {
       }
       items = items.sort((a, b) => a.blockCount - b.blockCount)
     }
-    items.push(customItem)
-    const selectedItem = prevState.selectedItem.feePerByte.eq(feePerByte)
+    items.push(!feePerByte && !error ? notLoadedItem : customItem)
+    const selectedItem =
+      !feePerByte && prevState.selectedItem.feePerByte.eq(feePerByte)
         ? prevState.selectedItem
         : items.find(f => f.feePerByte.eq(feePerByte)) || items[items.length - 1]
     return { items, selectedItem }
@@ -90,7 +98,7 @@ class FeesField extends Component<OwnProps, State> {
   componentDidUpdate({ fees: prevFees }: OwnProps) {
     const { feePerByte, fees, onChange } = this.props
     const { items, isFocused } = this.state
-    if (fees && fees !== prevFees && feePerByte.isZero() && !isFocused) {
+    if (fees && fees !== prevFees && !feePerByte && !isFocused) {
       // initialize with the median
       const feePerByte = (items.find(item => item.blockCount === defaultBlockCount) || items[0])
         .feePerByte
@@ -127,7 +135,7 @@ class FeesField extends Component<OwnProps, State> {
     const satoshi = units[units.length - 1]
 
     return (
-      <GenericContainer error={error}>
+      <GenericContainer>
         <Select width={156} options={items} value={selectedItem} onChange={this.onSelectChange} />
         <InputCurrency
           ref={this.input}
@@ -137,6 +145,8 @@ class FeesField extends Component<OwnProps, State> {
           value={feePerByte}
           onChange={onChange}
           onChangeFocus={this.onChangeFocus}
+          loading={!feePerByte && !error}
+          error={!feePerByte && error ? new FeeNotLoaded() : null}
           renderRight={
             <InputRight>
               {t('app:send.steps.amount.unitPerByte', { unit: satoshi.code })}

src/components/FeesField/EthereumKind.js

@@ -4,6 +4,7 @@ import React, { Component } from 'react'
 import { BigNumber } from 'bignumber.js'
 import type { Account } from '@ledgerhq/live-common/lib/types'
 
+import { FeeNotLoaded } from 'config/errors'
 import InputCurrency from 'components/base/InputCurrency'
 import type { Fees } from 'api/Fees'
 import WithFeesAPI from '../WithFeesAPI'
@@ -11,7 +12,7 @@ import GenericContainer from './GenericContainer'
 
 type Props = {
   account: Account,
-  gasPrice: BigNumber,
+  gasPrice: ?BigNumber,
   onChange: BigNumber => void,
 }
 
@@ -22,7 +23,7 @@ class FeesField extends Component<Props & { fees?: Fees, error?: Error }, *> {
   componentDidUpdate() {
     const { gasPrice, fees, onChange } = this.props
     const { isFocused } = this.state
-    if (gasPrice.isZero() && fees && fees.gas_price && !isFocused) {
+    if (!gasPrice && fees && fees.gas_price && !isFocused) {
       onChange(BigNumber(fees.gas_price)) // we want to set the default to gas_price
     }
   }
@@ -33,12 +34,14 @@ class FeesField extends Component<Props & { fees?: Fees, error?: Error }, *> {
     const { account, gasPrice, error, onChange } = this.props
     const { units } = account.currency
     return (
-      <GenericContainer error={error}>
+      <GenericContainer>
         <InputCurrency
           defaultUnit={units.length > 1 ? units[1] : units[0]}
           units={units}
           containerProps={{ grow: true }}
           value={gasPrice}
+          loading={!error && !gasPrice}
+          error={!gasPrice && error ? new FeeNotLoaded() : null}
           onChange={onChange}
           onChangeFocus={this.onChangeFocus}
         />

src/components/FeesField/RippleKind.js

@@ -4,12 +4,13 @@ import React, { Component } from 'react'
 import type { BigNumber } from 'bignumber.js'
 import type { Account } from '@ledgerhq/live-common/lib/types'
 import { apiForEndpointConfig, parseAPIValue } from 'api/Ripple'
+import { FeeNotLoaded } from 'config/errors'
 import InputCurrency from 'components/base/InputCurrency'
 import GenericContainer from './GenericContainer'
 
 type Props = {
   account: Account,
-  fee: BigNumber,
+  fee: ?BigNumber,
   onChange: BigNumber => void,
 }
 
@@ -36,7 +37,7 @@ class FeesField extends Component<Props, State> {
       const info = await api.getServerInfo()
       if (syncId !== this.syncId) return
       const serverFee = parseAPIValue(info.validatedLedger.baseFeeXRP)
-      if (this.props.fee.isZero()) {
+      if (!this.props.fee) {
         this.props.onChange(serverFee)
       }
     } catch (error) {
@@ -50,11 +51,13 @@ class FeesField extends Component<Props, State> {
     const { error } = this.state
     const { units } = account.currency
     return (
-      <GenericContainer error={error}>
+      <GenericContainer>
         <InputCurrency
           defaultUnit={units[0]}
           units={units}
           containerProps={{ grow: true }}
+          loading={!error && !fee}
+          error={!fee && error ? new FeeNotLoaded() : null}
           value={fee}
           onChange={onChange}
         />

src/components/base/Input/index.js

@@ -4,9 +4,8 @@ import React, { PureComponent } from 'react'
 import styled from 'styled-components'
 import { fontSize, space } from 'styled-system'
 import noop from 'lodash/noop'
-
 import fontFamily from 'styles/styled/fontFamily'
-
+import Spinner from 'components/base/Spinner'
 import Box from 'components/base/Box'
 import TranslatedError from 'components/TranslatedError'
 
@@ -44,6 +43,19 @@ const ErrorDisplay = styled(Box)`
   color: ${p => p.theme.colors.pearl};
 `
 
+const LoadingDisplay = styled(Box)`
+  position: absolute;
+  left: 0px;
+  top: 0px;
+  bottom: 0px;
+  background: white;
+  pointer-events: none;
+  flex-direction: row;
+  align-items: center;
+  padding: 0 15px;
+  border-radius: 4px;
+`
+
 const WarningDisplay = styled(ErrorDisplay)`
   color: ${p => p.theme.colors.warning};
 `
@@ -98,6 +110,7 @@ type Props = {
   renderLeft?: any,
   renderRight?: any,
   containerProps?: Object,
+  loading?: boolean,
   error?: ?Error | boolean,
   warning?: ?Error | boolean,
   small?: boolean,
@@ -182,6 +195,7 @@ class Input extends PureComponent<Props, State> {
       editInPlace,
       small,
       error,
+      loading,
       warning,
       ...props
     } = this.props
@@ -217,6 +231,11 @@ class Input extends PureComponent<Props, State> {
               <TranslatedError error={warning} />
             </WarningDisplay>
           ) : null}
+          {loading && !isFocus ? (
+            <LoadingDisplay>
+              <Spinner size={16} />
+            </LoadingDisplay>
+          ) : null}
         </Box>
         {renderRight}
       </Container>

src/components/base/InputCurrency/index.js

@@ -81,7 +81,7 @@ type Props = {
   renderRight: any,
   unit: Unit,
   units: Unit[],
-  value: BigNumber,
+  value: ?BigNumber,
   showAllDigits?: boolean,
   subMagnitude: number,
   allowZero: boolean,
@@ -98,7 +98,7 @@ class InputCurrency extends PureComponent<Props, State> {
     onChange: noop,
     renderRight: null,
     units: [],
-    value: BigNumber(0),
+    value: null,
     showAllDigits: false,
     subMagnitude: 0,
     allowZero: false,
@@ -123,7 +123,8 @@ class InputCurrency extends PureComponent<Props, State> {
     if (needsToBeReformatted) {
       const { isFocused } = this.state
       this.setState({
-        displayValue: nextProps.value.isZero()
+        displayValue:
+          !nextProps.value || nextProps.value.isZero()
             ? ''
             : format(nextProps.unit, nextProps.value, {
                 isFocused,
@@ -138,7 +139,7 @@ class InputCurrency extends PureComponent<Props, State> {
     const { onChange, unit, value } = this.props
     const r = sanitizeValueString(unit, v)
     const satoshiValue = BigNumber(r.value)
-    if (!value.isEqualTo(satoshiValue)) {
+    if (!value || !value.isEqualTo(satoshiValue)) {
       onChange(satoshiValue, unit)
     }
     this.setState({ displayValue: r.display })
@@ -159,7 +160,7 @@ class InputCurrency extends PureComponent<Props, State> {
     this.setState({
       isFocused,
       displayValue:
-        (!value || value.isZero()) && !allowZero
+        !value || (value.isZero() && !allowZero)
           ? ''
           : format(unit, value, { isFocused, showAllDigits, subMagnitude }),
     })

src/components/modals/Send/fields/AmountField.js

@@ -4,6 +4,9 @@ import Box from 'components/base/Box'
 import Label from 'components/base/Label'
 import RequestAmount from 'components/RequestAmount'
 
+// list of errors that are handled somewhere else on UI, otherwise the field will catch every other errors.
+const blacklistErrorName = ['FeeNotLoaded', 'InvalidAddress']
+
 class AmountField extends Component<*, { validTransactionError: ?Error }> {
   state = {
     validTransactionError: null,
@@ -49,7 +52,11 @@ class AmountField extends Component<*, { validTransactionError: ?Error }> {
         <RequestAmount
           withMax={false}
           account={account}
-          validTransactionError={validTransactionError}
+          validTransactionError={
+            validTransactionError && blacklistErrorName.includes(validTransactionError.name)
+              ? null
+              : validTransactionError
+          }
           onChange={this.onChange}
           value={bridge.getTransactionAmount(account, transaction)}
         />

src/config/errors.js

@@ -45,6 +45,7 @@ export const WebsocketConnectionFailed = createCustomErrorClass('WebsocketConnec
 export const WrongDeviceForAccount = createCustomErrorClass('WrongDeviceForAccount')
 export const ETHAddressNonEIP = createCustomErrorClass('ETHAddressNonEIP')
 export const CantScanQRCode = createCustomErrorClass('CantScanQRCode')
+export const FeeNotLoaded = createCustomErrorClass('FeeNotLoaded')
 
 // db stuff, no need to translate
 export const NoDBPathGiven = createCustomErrorClass('NoDBPathGiven')

static/i18n/en/errors.json

@@ -175,5 +175,8 @@
   },
   "CantScanQRCode": {
     "title": "Couldn't scan this QR-code: auto-verification not supported by this address"
+  },
+  "FeeNotLoaded": {
+    "title": "Couldn’t load fee rates. Please set manual fees"
   }
 }