Fix sending more than max balance (fix #115)

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

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);
   };
 
-  handleMax = () => this.setState({ amount: this.state.maxAmount });
+  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 },
+      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 {
                   </fieldset>
                   <nav className='form-nav'>
                     <span data-tip={error || ''}>
-                      <button disabled={error} className='button'>
-                        Send
+                      <button disabled={error || estimating} className='button'>
+                        {estimating ? 'Checking...' : 'Send'}
                       </button>
                     </span>
                   </nav>

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
+  estimateGasForErc20 = memoize(
+    txForErc20 =>
+      this.contract.contractObject.instance.transfer
         .estimateGas(txForErc20.options, txForErc20.args)
         .then(this.setEstimated)
-      .catch(noop);
-  }, JSON.stringify);
+        .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
+  estimateGasForEth = memoize(
+    txForEth =>
+      parityStore.api.eth
         .estimateGas(txForEth)
         .then(this.setEstimated)
-      .catch(noop);
-  }, JSON.stringify);
+        .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

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));
   });
 });