From 39556ac3be5a118a977e879ea8ef9b519cc12a36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABtan=20Renaudeau?= Date: Fri, 6 Jul 2018 15:56:05 +0200 Subject: [PATCH] Better error precision on canBeSpent cases - error precision: in case there is an error but it's not a "NotEnoughBalance" error, we display it instead - canBeSpent(a,t)=>Promise was replaced by checkCanBeSpent(a,t)=>Promise which will throw if something is wrong (NotEnoughBalance or something else!) - cache eviction of bad error: in case such an error happen, we clear the cache to allow to always retry the call that fails - the cache make sure it is always up to date by taking a.blockHeight --- src/bridge/EthereumJSBridge.js | 6 +++- src/bridge/LibcoreBridge.js | 29 ++++++++++++++----- src/bridge/RippleJSBridge.js | 10 +++++-- src/bridge/UnsupportedBridge.js | 2 +- src/bridge/makeMockBridge.js | 4 +-- src/bridge/types.js | 2 +- src/components/RequestAmount/index.js | 9 ++---- .../modals/Send/fields/AmountField.js | 18 +++++++----- .../modals/Send/steps/01-step-amount.js | 4 ++- 9 files changed, 56 insertions(+), 28 deletions(-) diff --git a/src/bridge/EthereumJSBridge.js b/src/bridge/EthereumJSBridge.js index 2449aa2c..618b8c75 100644 --- a/src/bridge/EthereumJSBridge.js +++ b/src/bridge/EthereumJSBridge.js @@ -13,8 +13,11 @@ import { getDerivations } from 'helpers/derivations' import getAddressCommand from 'commands/getAddress' import signTransactionCommand from 'commands/signTransaction' import { getAccountPlaceholderName, getNewAccountPlaceholderName } from 'helpers/accountName' +import { createCustomErrorClass } from 'helpers/errors' import type { EditProps, WalletBridge } from './types' +const NotEnoughBalance = createCustomErrorClass('NotEnoughBalance') + // TODO in future it would be neat to support eip55 type Transaction = { @@ -363,7 +366,8 @@ const EthereumBridge: WalletBridge = { EditAdvancedOptions, - canBeSpent: (a, t) => Promise.resolve(t.amount <= a.balance), + checkCanBeSpent: (a, t) => + t.amount <= a.balance ? Promise.resolve() : Promise.reject(new NotEnoughBalance()), getTotalSpent: (a, t) => Promise.resolve(t.amount + t.gasPrice * t.gasLimit), getMaxAmount: (a, t) => Promise.resolve(a.balance - t.gasPrice * t.gasLimit), diff --git a/src/bridge/LibcoreBridge.js b/src/bridge/LibcoreBridge.js index fc622f9b..55f021ce 100644 --- a/src/bridge/LibcoreBridge.js +++ b/src/bridge/LibcoreBridge.js @@ -11,8 +11,12 @@ import libcoreSyncAccount from 'commands/libcoreSyncAccount' import libcoreSignAndBroadcast from 'commands/libcoreSignAndBroadcast' import libcoreGetFees from 'commands/libcoreGetFees' import libcoreValidAddress from 'commands/libcoreValidAddress' +import { createCustomErrorClass } from 'helpers/errors' import type { WalletBridge, EditProps } from './types' +const NOT_ENOUGH_FUNDS = 52 +const NotEnoughBalance = createCustomErrorClass('NotEnoughBalance') + const notImplemented = new Error('LibcoreBridge: not implemented') type Transaction = { @@ -65,10 +69,13 @@ const isRecipientValid = (currency, recipient) => { const feesLRU = LRU({ max: 100 }) +const getFeesKey = (a, t) => + `${a.id}_${a.blockHeight || 0}_${t.amount}_${t.recipient}_${t.feePerByte}` + const getFees = async (a, transaction) => { const isValid = await isRecipientValid(a.currency, transaction.recipient) if (!isValid) return null - const key = `${a.id}_${transaction.amount}_${transaction.recipient}_${transaction.feePerByte}` + const key = getFeesKey(a, transaction) let promise = feesLRU.get(key) if (promise) return promise promise = libcoreGetFees @@ -79,6 +86,19 @@ const getFees = async (a, transaction) => { return promise } +const checkCanBeSpent = (a, t) => + !t.amount + ? Promise.resolve() + : getFees(a, t) + .then(() => {}) + .catch(e => { + if (e.code === NOT_ENOUGH_FUNDS) { + throw new NotEnoughBalance() + } + feesLRU.del(getFeesKey(a, t)) + throw e + }) + const LibcoreBridge: WalletBridge = { scanAccountsOnDevice(currency, devicePath) { return libcoreScanAccounts @@ -173,12 +193,7 @@ const LibcoreBridge: WalletBridge = { isValidTransaction: (a, t) => (t.amount > 0 && t.recipient && true) || false, - canBeSpent: (a, t) => - !t.amount - ? Promise.resolve(true) - : getFees(a, t) - .then(() => true) - .catch(() => false), + checkCanBeSpent, getTotalSpent: (a, t) => !t.amount diff --git a/src/bridge/RippleJSBridge.js b/src/bridge/RippleJSBridge.js index 76a48587..a1664336 100644 --- a/src/bridge/RippleJSBridge.js +++ b/src/bridge/RippleJSBridge.js @@ -19,8 +19,11 @@ import { import FeesRippleKind from 'components/FeesField/RippleKind' import AdvancedOptionsRippleKind from 'components/AdvancedOptions/RippleKind' import { getAccountPlaceholderName, getNewAccountPlaceholderName } from 'helpers/accountName' +import { createCustomErrorClass } from 'helpers/errors' import type { WalletBridge, EditProps } from './types' +const NotEnoughBalance = createCustomErrorClass('NotEnoughBalance') + type Transaction = { amount: number, recipient: string, @@ -461,9 +464,12 @@ const RippleJSBridge: WalletBridge = { isValidTransaction: (a, t) => (t.amount > 0 && t.recipient && true) || false, - canBeSpent: async (a, t) => { + checkCanBeSpent: async (a, t) => { const r = await getServerInfo(a.endpointConfig) - return t.amount + t.fee + parseAPIValue(r.validatedLedger.reserveBaseXRP) <= a.balance + if (t.amount + t.fee + parseAPIValue(r.validatedLedger.reserveBaseXRP) <= a.balance) { + return + } + throw new NotEnoughBalance() }, getTotalSpent: (a, t) => Promise.resolve(t.amount + t.fee), diff --git a/src/bridge/UnsupportedBridge.js b/src/bridge/UnsupportedBridge.js index 4c3a548a..526624eb 100644 --- a/src/bridge/UnsupportedBridge.js +++ b/src/bridge/UnsupportedBridge.js @@ -31,7 +31,7 @@ const UnsupportedBridge: WalletBridge<*> = { getTransactionRecipient: () => '', - canBeSpent: () => Promise.resolve(false), + checkCanBeSpent: () => Promise.resolve(), getTotalSpent: () => Promise.resolve(0), diff --git a/src/bridge/makeMockBridge.js b/src/bridge/makeMockBridge.js index cbbe060f..e84a6beb 100644 --- a/src/bridge/makeMockBridge.js +++ b/src/bridge/makeMockBridge.js @@ -32,7 +32,7 @@ function makeMockBridge(opts?: Opts): WalletBridge<*> { extraInitialTransactionProps, getTotalSpent, getMaxAmount, - canBeSpent, + checkCanBeSpent, } = { ...defaultOpts, ...opts, @@ -145,7 +145,7 @@ function makeMockBridge(opts?: Opts): WalletBridge<*> { isValidTransaction: (a, t) => (t.amount > 0 && t.recipient && true) || false, - canBeSpent, + checkCanBeSpent, getTotalSpent, diff --git a/src/bridge/types.js b/src/bridge/types.js index cf58e57d..4b1a8d47 100644 --- a/src/bridge/types.js +++ b/src/bridge/types.js @@ -82,7 +82,7 @@ export interface WalletBridge { // render the whole advanced part of the form EditAdvancedOptions?: *; // React$ComponentType>; - canBeSpent(account: Account, transaction: Transaction): Promise; + checkCanBeSpent(account: Account, transaction: Transaction): Promise; getTotalSpent(account: Account, transaction: Transaction): Promise; diff --git a/src/components/RequestAmount/index.js b/src/components/RequestAmount/index.js index 792a5309..e0838d65 100644 --- a/src/components/RequestAmount/index.js +++ b/src/components/RequestAmount/index.js @@ -21,9 +21,6 @@ import InputCurrency from 'components/base/InputCurrency' import Button from 'components/base/Button' import Box from 'components/base/Box' import type { State } from 'reducers' -import { createCustomErrorClass } from 'helpers/errors' - -const NotEnoughBalance = createCustomErrorClass('NotEnoughBalance') const InputRight = styled(Box).attrs({ ff: 'Rubik', @@ -50,7 +47,7 @@ type OwnProps = { // left value (always the one which is returned) value: number, - canBeSpent: boolean, + canBeSpentError: ?Error, // max left value max: number, @@ -141,14 +138,14 @@ export class RequestAmount extends PureComponent { renderInputs(containerProps: Object) { // TODO move this inlined into render() for less spaghetti - const { value, account, rightCurrency, getCounterValue, canBeSpent } = this.props + const { value, account, rightCurrency, getCounterValue, canBeSpentError } = this.props const right = getCounterValue(value) || 0 const rightUnit = rightCurrency.units[0] // FIXME: no way InputCurrency pure can work here. inlined InputRight (should be static func?), inline containerProps object.. return ( { +class AmountField extends Component<*, { canBeSpentError: ?Error }> { state = { - canBeSpent: true, + canBeSpentError: null, } componentDidMount() { this.resync() @@ -26,9 +26,13 @@ class AmountField extends Component<*, { canBeSpent: boolean }> { async resync() { const { account, bridge, transaction } = this.props const syncId = ++this.syncId - const canBeSpent = await bridge.canBeSpent(account, transaction) - if (this.syncId !== syncId) return - this.setState({ canBeSpent }) + try { + await bridge.checkCanBeSpent(account, transaction) + if (this.syncId !== syncId) return + this.setState({ canBeSpentError: null }) + } catch (canBeSpentError) { + this.setState({ canBeSpentError }) + } } onChange = (amount: number) => { @@ -38,14 +42,14 @@ class AmountField extends Component<*, { canBeSpent: boolean }> { render() { const { bridge, account, transaction, t } = this.props - const { canBeSpent } = this.state + const { canBeSpentError } = this.state return ( diff --git a/src/components/modals/Send/steps/01-step-amount.js b/src/components/modals/Send/steps/01-step-amount.js index 3930522a..a84fe335 100644 --- a/src/components/modals/Send/steps/01-step-amount.js +++ b/src/components/modals/Send/steps/01-step-amount.js @@ -135,7 +135,9 @@ export class StepAmountFooter extends PureComponent< try { const totalSpent = await bridge.getTotalSpent(account, transaction) if (syncId !== this.syncId) return - const canBeSpent = await bridge.canBeSpent(account, transaction) + const canBeSpent = await bridge + .checkCanBeSpent(account, transaction) + .then(() => true, () => false) if (syncId !== this.syncId) return this.setState({ totalSpent, canBeSpent, isSyncing: false }) } catch (err) {