Advanced Settings Input Validation (#872)

* add gas limit/price constants

* add gas limit/price validators & selectors

* apply new gas limit/price validation to components and sagas

* create/apply function to sanitize advanced fields input

* add types, update tests

* fix unrelated failing test

common/components/GasLimitField.tsx

@@ -3,6 +3,7 @@ import { GasLimitFieldFactory } from './GasLimitFieldFactory';
 import translate from 'translations';
 import { CSSTransition } from 'react-transition-group';
 import { Spinner } from 'components/ui';
+import { gasLimitValidator } from 'libs/validators';
 
 interface Props {
   includeLabel: boolean;
@@ -25,12 +26,12 @@ export const GasLimitField: React.SFC<Props> = ({ includeLabel, onlyIncludeLoade
     {includeLabel ? <label>{translate('TRANS_gas')} </label> : null}
 
     <GasLimitFieldFactory
-      withProps={({ gasLimit: { raw, value }, onChange, readOnly, gasEstimationPending }) => (
+      withProps={({ gasLimit: { raw }, onChange, readOnly, gasEstimationPending }) => (
         <>
           <GaslimitLoading gasEstimationPending={gasEstimationPending} />
           {onlyIncludeLoader ? null : (
             <input
-              className={`form-control ${!!value ? 'is-valid' : 'is-invalid'}`}
+              className={`form-control ${gasLimitValidator(raw) ? 'is-valid' : 'is-invalid'}`}
               type="number"
               placeholder="e.g. 21000"
               readOnly={!!readOnly}

common/components/GasLimitFieldFactory/GasLimitFieldFactory.tsx

@@ -4,6 +4,7 @@ import { GasLimitInput } from './GasLimitInputFactory';
 import { inputGasLimit, TInputGasLimit } from 'actions/transaction';
 import { connect } from 'react-redux';
 import { AppState } from 'reducers';
+import { sanitizeNumericalInput } from 'libs/values';
 
 const defaultGasLimit = '21000';
 
@@ -40,7 +41,7 @@ class GasLimitFieldClass extends Component<Props, {}> {
 
   private setGas = (ev: React.FormEvent<HTMLInputElement>) => {
     const { value } = ev.currentTarget;
-    this.props.inputGasLimit(value);
+    this.props.inputGasLimit(sanitizeNumericalInput(value));
   };
 }
 

common/components/GasSlider/components/AdvancedGas.tsx

@@ -9,6 +9,8 @@ import { TInputGasPrice } from 'actions/transaction';
 import { NonceField, GasLimitField, DataField } from 'components';
 import { connect } from 'react-redux';
 import { getAutoGasLimitEnabled } from 'selectors/config';
+import { isValidGasPrice } from 'selectors/transaction';
+import { sanitizeNumericalInput } from 'libs/values';
 
 interface OwnProps {
   inputGasPrice: TInputGasPrice;
@@ -17,6 +19,7 @@ interface OwnProps {
 
 interface StateProps {
   autoGasLimitEnabled: AppState['config']['autoGasLimit'];
+  validGasPrice: boolean;
 }
 
 interface DispatchProps {
@@ -27,7 +30,7 @@ type Props = OwnProps & StateProps & DispatchProps;
 
 class AdvancedGas extends React.Component<Props> {
   public render() {
-    const { autoGasLimitEnabled, gasPrice } = this.props;
+    const { autoGasLimitEnabled, gasPrice, validGasPrice } = this.props;
     return (
       <div className="AdvancedGas row form-group">
         <div className="col-md-12">
@@ -44,7 +47,7 @@ class AdvancedGas extends React.Component<Props> {
         <div className="col-md-4 col-sm-6 col-xs-12">
           <label>{translate('OFFLINE_Step2_Label_3')} (gwei)</label>
           <input
-            className={classnames('form-control', { 'is-invalid': !gasPrice.value })}
+            className={classnames('form-control', { 'is-invalid': !validGasPrice })}
             type="number"
             placeholder="e.g. 40"
             value={gasPrice.raw}
@@ -80,7 +83,8 @@ class AdvancedGas extends React.Component<Props> {
   }
 
   private handleGasPriceChange = (ev: React.FormEvent<HTMLInputElement>) => {
-    this.props.inputGasPrice(ev.currentTarget.value);
+    const { value } = ev.currentTarget;
+    this.props.inputGasPrice(sanitizeNumericalInput(value));
   };
 
   private handleToggleAutoGasLimit = (_: React.FormEvent<HTMLInputElement>) => {
@@ -89,6 +93,9 @@ class AdvancedGas extends React.Component<Props> {
 }
 
 export default connect(
-  (state: AppState) => ({ autoGasLimitEnabled: getAutoGasLimitEnabled(state) }),
+  (state: AppState) => ({
+    autoGasLimitEnabled: getAutoGasLimitEnabled(state),
+    validGasPrice: isValidGasPrice(state)
+  }),
   { toggleAutoGasLimit }
 )(AdvancedGas);

common/components/GenerateTransaction/GenerateTransaction.tsx

@@ -4,7 +4,13 @@ import EthTx from 'ethereumjs-tx';
 import React, { Component } from 'react';
 import { connect } from 'react-redux';
 import { AppState } from 'reducers';
-import { getTransaction, isNetworkRequestPending, isValidAmount } from 'selectors/transaction';
+import {
+  getTransaction,
+  isNetworkRequestPending,
+  isValidAmount,
+  isValidGasPrice,
+  isValidGasLimit
+} from 'selectors/transaction';
 import { getWalletType } from 'selectors/wallet';
 
 interface StateProps {
@@ -13,6 +19,8 @@ interface StateProps {
   isFullTransaction: boolean;
   isWeb3Wallet: boolean;
   validAmount: boolean;
+  validGasPrice: boolean;
+  validGasLimit: boolean;
 }
 
 class GenerateTransactionClass extends Component<StateProps> {
@@ -22,14 +30,23 @@ class GenerateTransactionClass extends Component<StateProps> {
       isWeb3Wallet,
       transaction,
       networkRequestPending,
-      validAmount
+      validAmount,
+      validGasPrice,
+      validGasLimit
     } = this.props;
+
+    const isButtonDisabled =
+      !isFullTransaction ||
+      networkRequestPending ||
+      !validAmount ||
+      !validGasPrice ||
+      !validGasLimit;
     return (
       <WithSigner
         isWeb3={isWeb3Wallet}
         withSigner={signer => (
           <button
-            disabled={!isFullTransaction || networkRequestPending || !validAmount}
+            disabled={isButtonDisabled}
             className="btn btn-info btn-block"
             onClick={signer(transaction)}
           >
@@ -45,5 +62,7 @@ export const GenerateTransaction = connect((state: AppState) => ({
   ...getTransaction(state),
   networkRequestPending: isNetworkRequestPending(state),
   isWeb3Wallet: getWalletType(state).isWeb3Wallet,
-  validAmount: isValidAmount(state)
+  validAmount: isValidAmount(state),
+  validGasPrice: isValidGasPrice(state),
+  validGasLimit: isValidGasLimit(state)
 }))(GenerateTransactionClass);

common/components/NonceFieldFactory/NonceFieldFactory.tsx

@@ -3,6 +3,7 @@ import { inputNonce, TInputNonce } from 'actions/transaction';
 import React, { Component } from 'react';
 import { connect } from 'react-redux';
 import { AppState } from 'reducers';
+import { sanitizeNumericalInput } from 'libs/values';
 
 export interface CallbackProps {
   nonce: AppState['transaction']['fields']['nonce'];
@@ -28,7 +29,7 @@ class NonceFieldClass extends Component<Props> {
 
   private setNonce = (ev: React.FormEvent<HTMLInputElement>) => {
     const { value } = ev.currentTarget;
-    this.props.inputNonce(value);
+    this.props.inputNonce(sanitizeNumericalInput(value));
   };
 }
 

common/config/constants.ts

@@ -0,0 +1,7 @@
+// Lower/upper ranges for gas limit
+export const GAS_LIMIT_LOWER_BOUND = 21000;
+export const GAS_LIMIT_UPPER_BOUND = 8000000;
+
+// Lower/upper ranges for gas price in gwei
+export const GAS_PRICE_GWEI_LOWER_BOUND = 1;
+export const GAS_PRICE_GWEI_UPPER_BOUND = 10000;

common/libs/validators.ts

@@ -5,6 +5,12 @@ import { normalise } from './ens';
 import { Validator } from 'jsonschema';
 import { JsonRpcResponse } from './nodes/rpc/types';
 import { isPositiveInteger } from 'utils/helpers';
+import {
+  GAS_LIMIT_LOWER_BOUND,
+  GAS_LIMIT_UPPER_BOUND,
+  GAS_PRICE_GWEI_LOWER_BOUND,
+  GAS_PRICE_GWEI_UPPER_BOUND
+} from 'config/constants';
 
 // FIXME we probably want to do checksum checks sideways
 export function isValidETHAddress(address: string): boolean {
@@ -127,8 +133,23 @@ export function isValidPath(dPath: string) {
 export const isValidValue = (value: string) =>
   !!(value && isFinite(parseFloat(value)) && parseFloat(value) >= 0);
 
-export const isValidGasPrice = (gasLimit: string) =>
-  !!(gasLimit && isFinite(parseFloat(gasLimit)) && parseFloat(gasLimit) > 0);
+export const gasLimitValidator = (gasLimit: number | string) => {
+  const gasLimitFloat = typeof gasLimit === 'string' ? parseFloat(gasLimit) : gasLimit;
+  return (
+    validNumber(gasLimitFloat) &&
+    gasLimitFloat >= GAS_LIMIT_LOWER_BOUND &&
+    gasLimitFloat <= GAS_LIMIT_UPPER_BOUND
+  );
+};
+
+export const gasPriceValidator = (gasPrice: number | string): boolean => {
+  const gasPriceFloat = typeof gasPrice === 'string' ? parseFloat(gasPrice) : gasPrice;
+  return (
+    validNumber(gasPriceFloat) &&
+    gasPriceFloat >= GAS_PRICE_GWEI_LOWER_BOUND &&
+    gasPriceFloat <= GAS_PRICE_GWEI_UPPER_BOUND
+  );
+};
 
 export const isValidByteCode = (byteCode: string) =>
   byteCode && byteCode.length > 0 && byteCode.length % 2 === 0;

common/libs/values.ts

@@ -58,3 +58,19 @@ export const buildEIP681TokenRequest = (
   }/transfer?address=${recipientAddr}&uint256=${toTokenBase(tokenValue.raw, decimal)}&gas=${
     gasLimit.raw
   }`;
+
+export const sanitizeNumericalInput = (input: string): string => {
+  const inputFloat = parseFloat(input);
+
+  if (!input || isNaN(inputFloat)) {
+    return input;
+  }
+
+  // limit input field decrement to 0
+  if (inputFloat === -1) {
+    return '0';
+  }
+
+  // convert negative values to positive
+  return Math.abs(inputFloat).toString();
+};

common/sagas/transaction/fields/fields.ts

@@ -14,7 +14,7 @@ import {
   InputNonceAction,
   TypeKeys
 } from 'actions/transaction';
-import { isValidHex, isValidNonce, validNumber } from 'libs/validators';
+import { isValidHex, isValidNonce, gasPriceValidator, gasLimitValidator } from 'libs/validators';
 import { Data, Wei, Nonce, gasPricetoBase } from 'libs/units';
 
 export function* handleDataInput({ payload }: InputDataAction): SagaIterator {
@@ -23,14 +23,13 @@ export function* handleDataInput({ payload }: InputDataAction): SagaIterator {
 }
 
 export function* handleGasLimitInput({ payload }: InputGasLimitAction): SagaIterator {
-  const validGasLimit =
-    validNumber(+payload) && isFinite(parseFloat(payload)) && parseFloat(payload);
+  const validGasLimit: boolean = yield call(gasLimitValidator, payload);
   yield put(setGasLimitField({ raw: payload, value: validGasLimit ? Wei(payload) : null }));
 }
 
 export function* handleGasPriceInput({ payload }: InputGasPriceAction): SagaIterator {
   const priceFloat = parseFloat(payload);
-  const validGasPrice = validNumber(priceFloat) && isFinite(priceFloat) && priceFloat > 0;
+  const validGasPrice: boolean = yield call(gasPriceValidator, priceFloat);
   yield put(
     setGasPriceField({
       raw: payload,

common/selectors/transaction/current.ts

@@ -2,7 +2,8 @@ import { getTo, getValue } from './fields';
 import { getUnit, getTokenTo, getTokenValue } from './meta';
 import { AppState } from 'reducers';
 import { isEtherUnit, TokenValue, Wei, Address } from 'libs/units';
-import { getDataExists, getValidGasCost } from 'selectors/transaction';
+import { gasPriceValidator, gasLimitValidator } from 'libs/validators';
+import { getDataExists, getValidGasCost, getGasPrice, getGasLimit } from 'selectors/transaction';
 import { getCurrentBalance } from 'selectors/wallet';
 import { getOffline } from 'selectors/config';
 
@@ -88,6 +89,10 @@ const isValidAmount = (state: AppState): boolean => {
   }
 };
 
+const isValidGasPrice = (state: AppState): boolean => gasPriceValidator(getGasPrice(state).raw);
+
+const isValidGasLimit = (state: AppState): boolean => gasLimitValidator(getGasLimit(state).raw);
+
 export {
   getCurrentValue,
   getCurrentTo,
@@ -95,5 +100,7 @@ export {
   ICurrentTo,
   isEtherTransaction,
   isValidCurrentTo,
-  isValidAmount
+  isValidAmount,
+  isValidGasPrice,
+  isValidGasLimit
 };

spec/sagas/transaction/fields/fields.spec.ts

@@ -1,13 +1,16 @@
+import BN from 'bn.js';
 import { call, put } from 'redux-saga/effects';
 import { setDataField, setGasLimitField, setNonceField } from 'actions/transaction/actionCreators';
-import { isValidHex, isValidNonce } from 'libs/validators';
-import { Data, Wei, Nonce } from 'libs/units';
+import { isValidHex, isValidNonce, gasPriceValidator, gasLimitValidator } from 'libs/validators';
+import { Data, Wei, Nonce, gasPricetoBase } from 'libs/units';
 import {
   handleDataInput,
   handleGasLimitInput,
-  handleNonceInput
+  handleNonceInput,
+  handleGasPriceInput
 } from 'sagas/transaction/fields/fields';
 import { cloneableGenerator } from 'redux-saga/utils';
+import { setGasPriceField } from 'actions/transaction';
 
 const itShouldBeDone = gen => {
   it('should be done', () => {
@@ -54,38 +57,86 @@ describe('handleDataInput*', () => {
 });
 
 describe('handleGasLimitInput*', () => {
-  const payload1 = 'invalidPayload';
-  const action1: any = { payload: payload1 };
-  const payload2 = '100.111';
-  const action2: any = { payload: payload2 };
+  const payload = '100.111';
+  const action: any = { payload };
 
-  const gen1 = handleGasLimitInput(action1);
-  const gen2 = handleGasLimitInput(action2);
+  const gens: any = {};
+  gens.gen = cloneableGenerator(handleGasLimitInput)(action);
 
-  it('should put setNonceField with null value when payload is invalid', () => {
-    expect(gen1.next().value).toEqual(
+  it('should call gasLimitValidator', () => {
+    expect(gens.gen.next().value).toEqual(call(gasLimitValidator, payload));
+  });
+
+  it('should put setGasLimitField with null value when payload is invalid', () => {
+    gens.gen.invalid = gens.gen.clone();
+    expect(gens.gen.invalid.next(false).value).toEqual(
       put(
         setGasLimitField({
-          raw: payload1,
+          raw: payload,
           value: null
         })
       )
     );
   });
 
-  it('should put setNonceField with Wei value', () => {
-    expect(gen2.next().value).toEqual(
+  it('should put setGasLimitField with Wei value', () => {
+    gens.gen.valid = gens.gen.clone();
+    expect(gens.gen.valid.next(true).value).toEqual(
       put(
         setGasLimitField({
-          raw: payload2,
-          value: Wei(payload2)
+          raw: payload,
+          value: Wei(payload)
         })
       )
     );
   });
 
-  itShouldBeDone(gen1);
-  itShouldBeDone(gen2);
+  it('should be done', () => {
+    expect(gens.gen.invalid.next().done).toEqual(true);
+    expect(gens.gen.valid.next().done).toEqual(true);
+  });
+});
+
+describe('handleGasPriceInput*', () => {
+  const payload = '100.111';
+  const action: any = { payload };
+  const priceFloat = parseFloat(payload);
+
+  const gens: any = {};
+  gens.gen = cloneableGenerator(handleGasPriceInput)(action);
+
+  it('should call gasPriceValidator', () => {
+    expect(gens.gen.next().value).toEqual(call(gasPriceValidator, priceFloat));
+  });
+
+  it('should put setGasPriceField with 0 value when payload is invalid', () => {
+    gens.gen.invalid = gens.gen.clone();
+    expect(gens.gen.invalid.next(false).value).toEqual(
+      put(
+        setGasPriceField({
+          raw: payload,
+          value: new BN(0)
+        })
+      )
+    );
+  });
+
+  it('should put setGasPriceField with base gas price value', () => {
+    gens.gen.valid = gens.gen.clone();
+    expect(gens.gen.valid.next(true).value).toEqual(
+      put(
+        setGasPriceField({
+          raw: payload,
+          value: gasPricetoBase(priceFloat)
+        })
+      )
+    );
+  });
+
+  it('should be done', () => {
+    expect(gens.gen.invalid.next().done).toEqual(true);
+    expect(gens.gen.valid.next().done).toEqual(true);
+  });
 });
 
 describe('handleNonceInput*', () => {