From 9e6e209a1513400e8be634e2da4a6c73fbfdf366 Mon Sep 17 00:00:00 2001 From: Amaury Martiny Date: Tue, 3 Jul 2018 16:09:36 +0200 Subject: [PATCH] Fix sending more than max balance (fix #115) --- packages/fether-react/package.json | 2 +- .../fether-react/src/Send/TxForm/TxForm.js | 48 ++++++++++++------- packages/fether-react/src/stores/sendStore.js | 36 +++++++------- .../fether-react/src/stores/sendStore.spec.js | 11 +++-- 4 files changed, 60 insertions(+), 37 deletions(-) diff --git a/packages/fether-react/package.json b/packages/fether-react/package.json index 8cf42e8c..99370cc8 100644 --- a/packages/fether-react/package.json +++ b/packages/fether-react/package.json @@ -31,7 +31,7 @@ "start": "npm-run-all -p start-*", "start-css": "npm run build-css -- --watch --recursive", "start-js": "react-app-rewired start", - "test": "react-app-rewired test --env=jsdom" + "test": "react-app-rewired test --env=jsdom --coverage" }, "dependencies": { "@parity/api": "^2.1.22", diff --git a/packages/fether-react/src/Send/TxForm/TxForm.js b/packages/fether-react/src/Send/TxForm/TxForm.js index 14f62838..a93aca0b 100644 --- a/packages/fether-react/src/Send/TxForm/TxForm.js +++ b/packages/fether-react/src/Send/TxForm/TxForm.js @@ -29,6 +29,7 @@ class Send extends Component { amount: '', // In Ether or in token gasPrice: 4, // in Gwei to: '', + estimating: false, // Currently estimating gasPrice ...this.props.sendStore.tx }; @@ -51,29 +52,44 @@ class Send extends Component { }; } - componentDidUpdate () { - if (!this.hasError()) { - const { amount, gasPrice, to } = this.state; - this.props.sendStore.setTx({ amount, gasPrice, to }); - this.estimateGas(); - } + componentDidMount () { + this.handleEstimateGasPrice(); } - estimateGas = debounce(() => { - this.props.sendStore.estimateGas(); - }, 1000); + estimateGas = debounce( + () => + this.props.sendStore + .estimateGas() + .then(() => this.setState({ estimating: false })) + .catch(() => this.setState({ estimating: false })), + 1000 + ); handleChangeAmount = ({ target: { value } }) => - this.setState({ amount: value }); + this.setState({ amount: value }, this.handleEstimateGasPrice); handleChangeGasPrice = ({ target: { value } }) => - this.setState({ gasPrice: value }); + this.setState({ gasPrice: value }, this.handleEstimateGasPrice); handleChangeTo = ({ target: { value } }) => { - this.setState({ to: value }); + this.setState({ to: value }, this.handleEstimateGasPrice); + }; + + handleEstimateGasPrice = () => { + if (this.hasError()) { + return; + } + + const { amount, gasPrice, to } = this.state; + this.props.sendStore.setTx({ amount, gasPrice, to }); + this.setState({ estimating: true }, this.estimateGas); }; - handleMax = () => this.setState({ amount: this.state.maxAmount }); + handleMax = () => + this.setState( + { amount: this.state.maxAmount }, + this.handleEstimateGasPrice + ); handleSubmit = event => { event.preventDefault(); @@ -112,7 +128,7 @@ class Send extends Component { sendStore: { tokenAddress }, tokensStore } = this.props; - const { amount, gasPrice, maxAmount, to } = this.state; + const { amount, estimating, gasPrice, maxAmount, to } = this.state; const token = tokensStore.tokens[tokenAddress]; const error = this.hasError(); @@ -210,8 +226,8 @@ class Send extends Component { diff --git a/packages/fether-react/src/stores/sendStore.js b/packages/fether-react/src/stores/sendStore.js index 65ac2a07..2bfc036e 100644 --- a/packages/fether-react/src/stores/sendStore.js +++ b/packages/fether-react/src/stores/sendStore.js @@ -16,8 +16,8 @@ import parityStore from './parityStore'; import tokensStore from './tokensStore'; const debug = Debug('sendStore'); -const DEFAULT_GAS = new BigNumber(21000); // Default gas amount to show -const GAS_MULT_FACTOR = 1.33; // Since estimateGas is not always accurate, we add a 33% factor for buffer. +const GAS_MULT_FACTOR = 1.25; // Since estimateGas is not always accurate, we add a 33% factor for buffer. +const DEFAULT_GAS = new BigNumber(21000 * GAS_MULT_FACTOR); // Default gas amount export class SendStore { @observable blockNumber; // Current block number, used to calculate tx confirmations. @@ -71,7 +71,7 @@ export class SendStore { */ estimateGas = () => { if (!this.tx || !Object.keys(this.tx).length) { - return; + return Promise.reject(new Error('Tx not set in sendStore.')); } if (this.tokenAddress === 'ETH') { @@ -85,23 +85,27 @@ export class SendStore { * Estimate gas to transfer in ERC20 contract. Expensive function, so we * memoize it. */ - estimateGasForErc20 = memoize(txForErc20 => { - return this.contract.contractObject.instance.transfer - .estimateGas(txForErc20.options, txForErc20.args) - .then(this.setEstimated) - .catch(noop); - }, JSON.stringify); + estimateGasForErc20 = memoize( + txForErc20 => + this.contract.contractObject.instance.transfer + .estimateGas(txForErc20.options, txForErc20.args) + .then(this.setEstimated) + .catch(noop), + JSON.stringify + ); /** * Estimate gas to transfer to an ETH address. Expensive function, so we * memoize it. */ - estimateGasForEth = memoize(txForEth => { - return parityStore.api.eth - .estimateGas(txForEth) - .then(this.setEstimated) - .catch(noop); - }, JSON.stringify); + estimateGasForEth = memoize( + txForEth => + parityStore.api.eth + .estimateGas(txForEth) + .then(this.setEstimated) + .catch(noop), + JSON.stringify + ); /** * Create a transaction. @@ -174,7 +178,7 @@ export class SendStore { @action setEstimated = estimated => { this.estimated = estimated.mul(GAS_MULT_FACTOR); - debug('Estimated gas.', +estimated); + debug('Estimated gas,', +estimated, ', with buffer,', +this.estimated); }; @action diff --git a/packages/fether-react/src/stores/sendStore.spec.js b/packages/fether-react/src/stores/sendStore.spec.js index 5fcbd556..7f3c63db 100644 --- a/packages/fether-react/src/stores/sendStore.spec.js +++ b/packages/fether-react/src/stores/sendStore.spec.js @@ -127,10 +127,13 @@ describe('@computed contract', () => { }); describe('method estimateGas', () => { - test('should not estimate if no tx is set', () => { + test('should reject and not estimate if no tx is set', () => { sendStore.estimateGasForErc20 = jest.fn(); sendStore.estimateGasForEth = jest.fn(); - expect(sendStore.estimateGas()).toBe(undefined); + expect(sendStore.estimateGas()).rejects.toHaveProperty( + 'message', + 'Tx not set in sendStore.' + ); expect(sendStore.estimateGasForErc20).not.toHaveBeenCalled(); expect(sendStore.estimateGasForEth).not.toHaveBeenCalled(); }); @@ -223,9 +226,9 @@ describe('method send', () => { }); describe('setter setEstimated', () => { - test('should add a 1.33 factor', () => { + test('should add a 1.25 factor', () => { sendStore.setEstimated(new BigNumber(2)); - expect(sendStore.estimated).toEqual(new BigNumber(2 * 1.33)); + expect(sendStore.estimated).toEqual(new BigNumber(2 * 1.25)); }); }); -- GitLab